Download & Extend
Views Bookmark » Issues

Bookmark Ops field in Views has arbitrary value

Posted by moshe weitzman on January 23, 2008 at 10:03pm
Project:Views Bookmark
Version:5.x-1.x-dev
Component:Views integration
Category:bug report
Priority:critical
Assigned:mooffie
Status:closed (fixed)

Issue Summary

Sadly, I had to disable this module from http://boston2008.drupalcon.org/sessions because of this bug.

This Bookmark: Ops field in Views (see views_bookmark_nodes table definition) shows an arbitrary value because its value is never filtered for the current user. I think the current user should be added to the JOIN with the views_bookmark_nodes table. This should probably happen in the query handler for this field.

I tried to fix this but a bit lost in the query handler. Help moofie!

Login or register to post comments

Comments

#1

Posted by moshe weitzman on January 24, 2008 at 12:45am

Actually, we should probably move the 'Bookmark: Ops for @s' field to a new pseudotable that has an 'extra' clause that adds the JOIN on uid = ***CURRENT_USER***. this will cause the least interference with other filters and arguments.

#2

Posted by mooffie on January 27, 2008 at 1:44pm
Assigned to:Anonymous» mooffie

I'll work on this today.

#3

Posted by mooffie on January 28, 2008 at 12:22am
Status:active» needs review

http://boston2008.drupalcon.org/sessions

Oh, nice! Views_bookmark will be perfect for this! :-)

Ops field [...] shows an arbitrary value because its value is never filtered for the current user.

That's correct.

I'm attaching a patch. (You should apply it to the latest CVS. But, you know, don't do this on a live site --I did test it numerous times, but maybe it won't be the final patch.)

I decided to add a new [pseudo] table. I did a lot of thinking about this, and finally came to the conclusion that it's the better and clearer route.

we should probably move the 'Bookmark: Ops for @s' field to a new pseudotable that has an 'extra' clause that adds the JOIN on uid = ***CURRENT_USER***.

Yes, that's what I did.

this will cause the least interference with other filters and arguments.

That's a by-product, but it actually wasn't my prime rationale.

Notes:

  • Update path: I didn't want to have to maintain two pieces of code, so I moved the code from 5100 to 5101.
  • I cleaned up the handlers for the Ops field. So we end up with less code, I think. (But I added some comments.)
  • I commented-out an unused table, "views_bookmarks_$bookmark->vbid". (You know it's unused because it has a 'table' => 'views_bookmark_nodes_$bookmark->vbid' line --note the *single* quotes.)
AttachmentSize
vb_ops.diff 11.58 KB

#4

Posted by moshe weitzman on January 28, 2008 at 2:46pm
Status:needs review» reviewed & tested by the community

tested and verified to work. i didn't test upgrade path but i think that can be fixed (or ignored - re-adding a field is easy) later. thanks much.

i think moofie deserves commit rights on this module if he/she wants them.

#5

Posted by mooffie on January 28, 2008 at 7:50pm

Oh, thanks for the compliment :-)
But I want to prove myself some more before I ask to be a co-maintainer.

BTW, I've just posted a How to setup a "Who's boorkmarked this" tab recipe.

#6

Posted by quicksketch on January 28, 2008 at 10:40pm

I'm alright with this patch also, just a small concern. Will this adversely affect users that already ran update_5100 to run update_5101?

#7

Posted by mooffie on January 30, 2008 at 9:16am

Will this adversely affect users that already ran update_5100 to run update_5101?

It's safe to run 5101 multiple times, so it won't adversely affect users who already run 5100.

#8

Posted by quicksketch on January 30, 2008 at 7:36pm
Status:reviewed & tested by the community» fixed

Fantastic. Committed. Thanks again mooffie!

#9

Posted by Anonymous (not verified) on February 13, 2008 at 7:44pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

nobody click here