Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.There is a mini-refactoring of all things flaggings...
Grouped by area, and in rough order in which they should be tackled:
FlagService
#2468275: Create a unified and flexible method of flagging an entity.
Flagging entity
#2087797: Record uid for global flags
#2488564: [regression] load all flagging data on an entity when viewing it
#2488458: remove calls to drupal_static()
#2629426: Deleting a flag should clear entity caches
Misc.
#2475223: flag/unflag links: Use attribute class not id.
#2491219: Flag ESLint errors
#2613150: AJAX link broken if no JS
#2628494: user flags show in the user picture on nodes
Please feel free to add to the list or introduce subcategories like high/priority
Comments
Comment #1
joachim CreditAttribution: joachim commentedYou know, I was pondering creating an issue exactly like this... thanks for beating me to it! :)
I've re-grouped into what I think is the logical order to tackle these in, and split them up into the different areas.
Comment #2
martin107 CreditAttribution: martin107 commentedadded two more issues to the list and moved one to complete.
Comment #3
martin107 CreditAttribution: martin107 commentedCool, two fixed.
Comment #4
martin107 CreditAttribution: martin107 commentedAdding 3 flag count related issues.
Comment #5
martin107 CreditAttribution: martin107 commentedadded extra issue
Comment #6
joachim CreditAttribution: joachim commentedWhich issue was it where there was a side-discussion about moving flag() and unflag() from the service and into the flag entity? Can we file a new task for that? I know socketwench is against the idea, so it may well end up as a wontfix, but it would be nice to have that in its own issue so we can discuss the pros and cons and have it for future reference.
Comment #7
martin107 CreditAttribution: martin107 commentedHere is a comment about the objection.
https://www.drupal.org/node/2465865#comment-9795351
[ Issue summary changes because in #5 I incorrectly added the issue to the Flag Count section not FlagService. ]
PS I hope to continue munching through this meta soon... but not tonight.
Comment #8
joachim CreditAttribution: joachim commentedCool, thanks. I tracked down the earlier issue I was referring to in that issue you linked to.
Filed #2473253: Consider moving flag()/unflag() methods from FlagService to Flag and attempted to summarize the different positions.
Comment #9
martin107 CreditAttribution: martin107 commentedJust making a note in the issue summary of the functions in flag,module marked deprecated move into flag count service.
Comment #10
martin107 CreditAttribution: martin107 commentedMoving one item to complete section.
Comment #11
martin107 CreditAttribution: martin107 commentedadded 3 new issues,
Comment #12
martin107 CreditAttribution: martin107 commentedRemoved mention of FlagCountService functions needing an issue.
Comment #13
martin107 CreditAttribution: martin107 commentedadded issue to flagging entity.
Comment #14
martin107 CreditAttribution: martin107 commentedAdded more issues to the list. Created misc section.
Comment #15
martin107 CreditAttribution: martin107 commentedMoved one to complete.
Comment #16
joachim CreditAttribution: joachim commentedComment #17
martin107 CreditAttribution: martin107 commented2 complete.. 1 new
Comment #18
martin107 CreditAttribution: martin107 commentedadded #2478565: flag_reset_flag() is broken to the pile ( under misc. )
Comment #19
martin107 CreditAttribution: martin107 commentedadded one to the flag count list.
Comment #20
martin107 CreditAttribution: martin107 commentedAdded #2473253: Consider moving flag()/unflag() methods from FlagService to Flag to the flag service list
Comment #21
martin107 CreditAttribution: martin107 commented2 complete, 3 added.
Comment #22
martin107 CreditAttribution: martin107 commented3 added.
Comment #23
martin107 CreditAttribution: martin107 commented$flag_service_list++
Comment #24
martin107 CreditAttribution: martin107 commentedAnd the scores on the doors.
+1-5
Comment #25
socketwench CreditAttribution: socketwench commentedComment #26
martin107 CreditAttribution: martin107 commented1 added.
Comment #27
martin107 CreditAttribution: martin107 commentedRemoving rules related issues from flag count section as they are already adequately covered by #2424465: [Meta] Implement Rules 8.x Support
Comment #28
martin107 CreditAttribution: martin107 commentedtwo complete.
Comment #29
martin107 CreditAttribution: martin107 commented1 off, 2 on
Comment #30
martin107 CreditAttribution: martin107 commented2 complete
Comment #31
martin107 CreditAttribution: martin107 commented2 fixes
Comment #32
martin107 CreditAttribution: martin107 commentedAdded 5 issues. Flag Entity Section
Comment #33
martin107 CreditAttribution: martin107 commentedThis is a 8.x meta - so move one off because its become a 7.x issue
1 complete.
Comment #34
martin107 CreditAttribution: martin107 commentedadded one more
Comment #35
martin107 CreditAttribution: martin107 commentedAdded one to misc.
Comment #36
martin107 CreditAttribution: martin107 commented+2 -1
Streamlined issue summary.
Comment #37
martin107 CreditAttribution: martin107 commentedComment #38
martin107 CreditAttribution: martin107 commentedtriggering update
Comment #39
martin107 CreditAttribution: martin107 commentedone fix, one added.
Comment #40
BerdirHint: It's helpful for people following the comments/notifications to actually list the new/resolved issues in the comment when updating. Otherwise I'd have to go to the revisions tab and diff the revisions to figure that out.
Comment #41
martin107 CreditAttribution: martin107 commented#2466519: assumptions in code that more that one flagging can exist for a flag-entity-account triple is now complete.
Comment #42
martin107 CreditAttribution: martin107 commented#2461411: add comment explaining why we have seemingly redundant entity_type field on Flaggings is now fixed.
Comment #43
martin107 CreditAttribution: martin107 commentedAlso fixed
#2416321: Flag service API (un)flagging methods don't check for access
#2466423: Flag counts lacks test coverage
#2488572: add global field to Flagging entity
#2488782: Define a FLAG_RESET event and kill hook_flag_reset().
Comment #44
martin107 CreditAttribution: martin107 commentedJust a little housekeeping, removing fixed issues.
From Flag Count
#2472695: Test method doTestFlagCounts() is not executed.
#2481107: rethink the names of the flag count API methods
From Misc.
#2477783: is FlagDeleteEvent necessary?
#2478565: flag_reset_flag() is broken
Comment #45
martin107 CreditAttribution: martin107 commentedJust a little housekeeping, removing fixed issues.
From Flag Count
#2472695: Test method doTestFlagCounts() is not executed.
#2481107: rethink the names of the flag count API methods
From Misc.
#2477783: is FlagDeleteEvent necessary?
#2478565: flag_reset_flag() is broken
Comment #46
martin107 CreditAttribution: martin107 commentedmarking
#2489072: Deleting a Flag doesn't delete flaggings
as fixed.
Comment #47
martin107 CreditAttribution: martin107 commentedOne the dust has settled... the issue has been fixed.
#2465095: classes in @param and @return must be fully-namespaced
Comment #48
martin107 CreditAttribution: martin107 commentedI am just trying to order my thoughts ... so I can prioritize and not loose track
removing count sub-section.
to Misc I am adding
#2613150: AJAX link broken if no JS
#2634572: Flag assumes nodes and comments are stored in a database.
#2628494: user flags show in the user picture on nodes
to Flagging entity I am adding
#2501575: Avoid fatal error when flagged entity is missing in flag_user_account_removal()
#2629426: Deleting a flag should clear entity caches
Comment #49
martin107 CreditAttribution: martin107 commentedRemoving closed issue
#2473253: Consider moving flag()/unflag() methods from FlagService to Flag
#2496345: Flagging is missing some owner methods.
Comment #50
martin107 CreditAttribution: martin107 commentedSing along with me now...
"If there are 10 green bottles sitting on a wall..."
and if
#2482577: Give the FlagServiceInterface documentation some love AND #2634572: Flag assumes nodes and comments are stored in a database. got resolved there would be 8 green bottles sitting on a wall.
Comment #51
martin107 CreditAttribution: martin107 commentedRemoving fixed issue
#2501575: Avoid fatal error when flagged entity is missing in flag_user_account_removal()
Comment #52
socketwench CreditAttribution: socketwench at FFW commentedI'd like to close this issue and transition us to a more release focused mindset. Please post outstanding issues in the linked issue.