Service method to get all flags for a user

I am working on it.

Thanks I just caught up the code, it was failing on some cleaned up namespace use statements. We are using this and it's very helpful, thank you @malcomio. I manually reviewed the code, and the abstraction is nice clean-up for the unflagAllByUser()

Please fix https://git.drupalcode.org/issue/flag-3381654/-/jobs/4104065

Looks like the issues were corrected, thanks Ramil g. The remaining are unrelated phpstan for other tests, not appropriate to fix here.

Good work, but needs tests

Tests added, back to RTBC.

Looks good for me. Waiting for @Berdir opinion.

Sorry, already fixed here #2769303: add a service method to get all of a user's flagged entities for a given flag. Credits will be added

@ivnish, I think this one’s actually a different use case, so reopening as RTBC 🙂
The diff:

• getFlagUserFlaggings($flag, $user): Only returns the user’s flaggings for a specific flag.
• getAllFlaggingByUser(?$account, $flag = NULL, $session_id = NULL): Returns all flaggings for a user, with extra handling for anonymous users via session ID.

Since getFlagUserFlaggings() already covers the per-flag case, maybe we could simplify getAllFlaggingByUser() to just handle “all by user” and drop the “optionally narrowed by flag” bit? That way each method has a clear, distinct role.

Looks like you’re targeting 5.x, so I’ve moved it over there.

so reopening as RTBC

Ah, sorry)

Since getFlagUserFlaggings() already covers the per-flag case, maybe we could simplify getAllFlaggingByUser() to just handle “all by user” and drop the “optionally narrowed by flag” bit? That way each method has a clear, distinct role.

Could you update the MR ?

Or... I can revert my commit and use yours, because it more complex

@ivnish woah, you’re quick! I was juggling branches since I couldn’t retarget the MR to 5.x, so I created a new one and moved the code over.

Totally up to you, but my thought is to keep the signature specific to “all flags for user,” so the distinction between the two methods stays clear.

Thanks for retargetting the code in MR 49 to 5.x branch. I will put my proposal in MR 156 since they are now both on 5.x

I added populateFlaggerDefaults() because I think we were missing that, and removed the $flag because that use-case is covered by the other issue. Moving to Needs Review, maybe you can have a look @ivnish?

LGTM

if no more code updates are planned, I think I can commit this

@ivnish I am good with committing this as-is. We could test session too but there are probably other tests already checking for that condition as we copied from other service methods. So 🚀

Thanks!

Awesome, thanks for giving the feature a second thought!