Unflagged Content stays shown as flagged

Could you provide steps to reproduce this problem? It's not immediately apparent that this bug exists in my testing.

I'm not sure, but perhaps this is the same problem: http://drupal.org/node/640698

Some kind of the same problem here; [edit: for version 6.x-1.1]

1. create a new Flag called 'Venue' (global flag, display checkbox on node edit form, authenticated user may use this flag, flag may be used on Node Type 'Venue')
2. flag a node of type 'Venue' as 'Venue'
3. create a view to display nodes flagged 'Venue', which shows the flagged node all right.
4. Then edit the Flag (flag may be used on Node Type 'Place', instead of Node type 'Venue')

After saving the flag, the view still was showing the particular node, so i figured I should have first disabled the flag on the node, so I reverted step 4, then unchecked the 'Venue' checkbox on the particular node, and saved the node.
After doing this, the checkbox is still checked when opening the node for editing again. And it still shows in the created view.
So, can't seem to unflag the node any more.

Hopefully these reproduced steps are helpfull (and clear?).

Thanks for the excellent report erik. I'll try that out and see if I can reproduce it.

Note that I'm not sure that Flag should erase data if the node type becomes disabled. It seems like you would simply want to filter your View to only show the types that are flaggable.

Same problem with beta2. When deleting the row from flag_content the item disappears from the flagged list.

We have the same problem as described above. I haven't been able to figure out how to reproduce it, though. But I will try what erik described above and see if it is possible to solve in some way.

I wasn't able to reproduce the problme following the steps from #5.

@erik: Can you still reproduce the problem with the latest version of flags 2.x?

Hi,

we've got the same problem here. We use a flag for users called follow. We have a user which was flagged and it's now impossible to unflag it. The link under the user stays to Unfollow, where it should show Follow.

More strangely we use a view to see the people that the current user follows, and this user doesn't appear, meaning that he is not flagged anymore in, the DB. It's like if the link is blocked on unfollow.

Thanks.

I looked into this issue a little, and it seems like everything flips out if you for instance have:

* A flag for which there's an entry in flag_content that IS NOT 0 and the flag IS set to global. This could happen if you first set the flag as global and a user flagged content, and then changes the flag to not global.

OR

* A flag for which there's an entry in flag_content that IS 0 and the flag IS NOT set to global. This could happen if you set the flag as not global and have users flagging content with their uids and then set the flag to global.

I'm not really an expert at the flag code and the inner workings of the module (looked at the code for the first time today =) but to me it seems it could have to do with this line of code in flag_get_user_flags function, line 1480, flag.module:

$result = db_query("SELECT * FROM {flag_content} WHERE content_type = '%s' AND content_id = %d AND (uid = %d OR uid = 0) AND sid = %s", $content_type, $content_id, $uid, $sid);

and line 1492 in flag.module

$result = db_query("SELECT * FROM {flag_content} WHERE content_type = '%s' AND (uid = %d OR uid = 0) AND sid = %s", $content_type, $uid, $sid);

as you can see, it returns true if the uid matches OR the uid is 0, which means that it will return true regardless of the flags status, (global or not global).

Couldn't this safely be changed to:

$result = db_query("SELECT * FROM {flag_content} WHERE content_type = '%s' AND content_id = %d AND (uid = %d) AND sid = %s", $content_type, $content_id, $uid, $sid);

and:

$result = db_query("SELECT * FROM {flag_content} WHERE content_type = '%s' AND (uid = %d) AND sid = %s", $content_type, $uid, $sid);

I tried this and it seems to remedy the problems that were occuring, but I don't know if there's some other part of the flag module that uses the condition that was in the query before.

Not sure if this is quite the same issue, but just in case...

I have Flag set up for use on a single content type for both authenticated and anonymous users (not global). After a few flags have been submitted on the site, the link says a user has flagged it when they haven't. This happens for both anonymous and authenticated users. If you unflag it, the JavaScript toggle changes to unflagged, but on page refresh it goes back to being flagged again.

I verified that the database does not have a record of the flag for the authenticated users - it is just the link that is incorrect.

Looking at the database, it appears that the flag_content table for the nodes that have this problem have a flag with a uid of 0 (anonymous) and an sid of 0 (no sesssion id). If I add an sid to these records, the problem goes away.

After reading through issue #271582: Allow anonymous users to flag content I see that uid of 0 + sid of 0 = global flag, which would probably be why these links weren't displaying properly.

No idea why there was an sid of 0 for these anonymous flags. I had some memory issues around the same time these nodes were flagged, so possibly the sid did not get added because of an incomplete post to the database. Not sure what else would cause an anonymous user to have no session id (cookies blocked?)

Not sure what else would cause an anonymous user to have no session id (cookies blocked?)

A good question... It may be that Session module is failing us in some way. Please continue your most helpful research. The sooner we know what causes this problem the quicker we can fix it.

I can report having this same issue today with a "promoted" flag we were using to bring items into a views block on the front of the website. Items that were unflagged kept showing up in the view.
I had to resort to removing Flag and creating a cck checkbox for promoting nodes to the view.

My next best guess is that the anonymous flags with an sid of 0 are bots. Over the weekend I had over a 100 anonymous flags with a uid + sid of 0 within the time span of an hour. The site typically gets only about 5 flags in a 24 hour period. I have all kinds of issues with wack bots on this site.

Having the same issue as metastate describes

I found the following way of reproducing the cannot-unflag bug:

1. Activate Flag 2
2. Create a story node
3. Change the bookmark flag to make it global
4. Flag the story
5. Change the flag to non-global again

=> You cannot unflag the story.

Reproduced multiple times on Drupal 6.16, Flag 2.0-beta2

Hope this may shed some light on the issue.
//Johan Falk, NodeOne

The problem is that global flags with an entry in {flag_content} gets a uid that isn't 0. This bug is possible to reproduce in two ways. It's two completely different scenarios that produces the same problem in the database.

The first way to reproduce the problem is to follow Itangalo's steps in #19.

The other way is this:

1. Use Pressflow (that has made major modifications to how anonymous sessions are handled)
2. Create one global flag that only authenticated users can flag with
3. Create one non global flag that anonymous users has permission to flag with
4. Flag some content with the global flag
5. Let anonymous users (without prior session cookies from the site) flag some content on the site (doesn't matter what content they flag)
6. Login on the site (doesn't matter who logs in)

=> All the content that was flagged with the global flag is now impossible to unflag.

What happens in the last case is that all entries flagged by anonymous users is recorded with a sessions id of 0. This is because the Session API is unable to retrieve the session id from Pressflow (due to it's modifications to how anonymous sessions are handled). So when someone logs in on the site the Flag module has this smart feature that tries to migrate all the flag entries made by the anonymous user to it's registered account. And because the user logs in from a session id that is 0 the Flag module tries to migrate all flag entries with a session id of 0 to it's user account. This will migrate all entries made from authenticated users to the user who tries to log in because flag entries made by authenticated users are always recorded with a session id of 0. This will also update the uid column with the user id from the user who logs in on all entries, even if the flag is global. And this is what makes the flag impossible to unflag.

So we have a couple of different problems here:

1. The Session API is unable to retrieve a session id from Pressflow.
2. The Flag module accepts a session id of 0 even if the user is anonymous.
3. The Flag module doesn't respect global flags if the session id it migrates entries from is 0.

The way Pressflow handles anonymous sessions is made by design for performance reasons. I don't expect Flag nor Session API to fix that. But what we could consider is:

1. Make the Flag module reject flag entries from anonymous users with a session id of 0.
2. Re-evaluate if the "migration" feature necessary at all. It will cause problems when the Session API for unknown reason is unable to retrieve a session id.

Attached is a start on both of the ideas. The first removes the "migration" feature and the second patch rejects anonymous sessions with id 0. The patches aren't compatible with each other.

Marking this as "needs review" to get some eyes on it.

I think your second patch makes the most sense (fixing the problem rather than removing it). Removing the migration path for logging-in users would only make it a feature request again immediately. All your points make sense. This patch should essentially fix problems 2 and 3 that you've described, the problem with PressFlow is mostly a configuration issue and not something either Flag or Session API should probably address.

Yeah, I agree that the second approach makes most sense here. I rerolled the patch and fixed a comment type. All tests I've done proves that this patch solves the problem. When a user is trying to flag something with a non-global flag (on Pressflow) the user get's the standard message saying something like "You're not allowed to flag this content...". I didn't find it necessary to use a custom message for this edge case.

Attached is the working patch.

// dixon_, NodeOne

Applied to DRUPAL-6--2 without problem, seems to solve the problem. Looks good to me!

I merged together dixon's conditional statements to make for less nesting in this patch. Logic is exactly the same though. Thanks ximo and a huger thanks to dixon_ to sleuthing this problem down, explaining it clearly, and then fixing it! Committed to 2.x branch.

Oh dear, this patch had a typo which prevents any content from being flagged. :D

Here's a followup that makes things work again. Committed.