We should go back and address #1113904: Use drupal_static() instead of plain ol' static, but this patch isn't a replacement, this is a bug fix.

CommentFileSizeAuthor
#6 1395034-flag-reset-cache-6.patch1.64 KBamitaibu
flag-reset-static.patch1.62 KBamitaibu

Comments

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

btw, You can see this bug if you try to use flag in simpleTest.

joachim’s picture

joachim commented

I don't entirely understand your first comment -- does this patch depend on the bugfix at #1113904: Use drupal_static() instead of plain ol' static?

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

No it doesn't. I didn't want to change the cache to drupal_static() in this patch.
The patch simply adds a missing reset.

joachim’s picture

joachim commented

OK, so I can commit this fix first, without the patch over at that other issue?

joachim’s picture

joachim commented
Status: Needs review » Needs work 
+++ b/flag.module
@@ -1699,17 +1699,17 @@ function flag_get_user_flags($content_type, $content_id = NULL, $uid = NULL, $si
-function flag_get_content_flags($content_type, $content_id, $flag_name = NULL, $reset = FALSE) {
-  static $content_flags;
+function flag_get_content_flags($content_type, $content_id, $flag_name = NULL) {
+  $content_flags = &drupal_static(__FUNCTION__, array());

We've lost a parameter here.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Needs work » Needs review
StatusFileSize
new 1395034-flag-reset-cache-6.patch1.64 KB

Rerolled.

> We've lost a parameter here.

Indeed, we don't need the $reset anymore, as we can/ should use drupal_static_reset()

joachim’s picture

joachim commented
Status: Needs review » Fixed

Committed to both 7 branches.

Thanks!

Issue #1395034 by Amitaibu: Fixed flag_get_content_flags() static cache not cleared on flag.

Status: Fixed » Closed (fixed)

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