Bugzilla::User::flatten_group_membership should be using the fast code from User::groups

RESOLVED INVALID

Status

()

Bugzilla
User Accounts
--
enhancement
RESOLVED INVALID
10 years ago
10 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
For some reason flatten_group_membership has its own code, when really it should just be using the code that's already in User::groups.
(Assignee)

Comment 1

10 years ago
Created attachment 326921 [details] [diff] [review]
v1

Here we go, easy as pie. :-)
Assignee: user-accounts → mkanat
Status: NEW → ASSIGNED
Attachment #326921 - Flags: review?(LpSolit)

Comment 2

10 years ago
Comment on attachment 326921 [details] [diff] [review]
v1

Wow, this change is completely wrong. You misunderstood what flatten_group_membership() does. groups() returns all groups you belong to thanks to your direct membership to some given groups and then letting group inheritence giving you indirect membership. flatten_group_membership() does exactly the opposite! Given a group, it tells you in which groups you must belong to in order to indirectly belong to the given group thanks to group inheritence. To make this clear:

Assume all members of the admin group are automatically in the editbugs group, and that members of the editbugs group are automatically in the canconfirm group. If you explicitly belong to the editbugs group only, then $user->groups() will return both editbugs and canconfirm, because you indirectly belong to the canconfirm group thanks to group inheritence. Now, flatten_group_membership($editbugs_gid) will return both admin and editbugs groups, because those are the groups you must belong to in order to be (directly or indirectly) in the editbugs group.

It's the same distinction as in bugs blocking and bugs depending on another bug.
Attachment #326921 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 3

10 years ago
Ah, okay. That subroutine should clearly be in Bugzilla::Group then, and not Bugzilla::User.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → INVALID

Comment 4

10 years ago
(In reply to comment #3)
> That subroutine should clearly be in Bugzilla::Group then, and not
> Bugzilla::User.

Exactly, I agree!
Target Milestone: Bugzilla 4.0 → ---
You need to log in before you can comment on or make changes to this bug.