428490 - Group icons should be displayed for indirect memberships, too

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[Marc Schumann [:Wurblzap]]

Attachment: Patch

Since bug 332149 has landed, there may be icons displayed with comments, indicating some direct group membership. This should be extended to *all* group memberships, not only direct ones. If somebody is an editbugs member, it should show, regardless of whether this is because the membership was given explicitly or because the person is a member of, say, the admin group.

Comment 1

[Frédéric Buclin]

It was intentional to implement it based on direct membership only. I discussed this with justdave at the time I implemented it, and he said he wouldn't want to inherit all icons from all groups only because he is an admin. Assuming all groups have a icon set and that an installation has 30 groups or so, admins would inherit membership for all these 30 groups and would have 30 icons displayed, which is bad. We came to the conclusion that it made much more sense to only display direct membership. So this is not a bug, but is per design.

Asking justdave if he changed his mind meanwhile or not.

Comment 2

[Max Kanat-Alexander]

I think what we should do is probably make an exception for people in the admin group.

Comment 3

[Max Kanat-Alexander]

If we make such an exception, though, it has to be noted clearly somewhere in the UI, not just in the docs, or admins will be confused.

Comment 4

[Marc Schumann [:Wurblzap]]

I agree that we need to care for installations which have many icons. We shouldn't lose sight of small and simple installations either, though. These will want their <some_group> people marked up no matter what, even if it's because the person in question happens to be an admin and is in <some_group> because of this.

So I agree that maybe special-casing the admin group in conjunction with good documentation is really the best compromise.

Comment 5

[Dave Miller [:justdave]]

I don't recall that conversation off-hand (doesn't mean it didn't happen, I'm forgetful occasionally), but my opinion at this moment is that if a person is a member of a group, the icon should show, no matter how they got that membership.  The "too many icons" thing is something for the installation to deal with -- i.e. not all groups should have icons, and you should pick and choose which ones are important to have icons for.  If I recall correctly, what I was thinking is that admins would be the EXCEPTION to this, that an admin might not want all of the groups hes a member of to have their icons shown, because some of them he might not be a participating member of but only have it because he's an admin and inherited it.  Perhaps require an explicit membership for an admin to have an icon shown, but just show them for everyone else?  Or maybe that's what you all meant already.  Obviously it needs to be clear in the UI no matter what's done.

Comment 6

[Frédéric Buclin]

So what do we decide to do here? I'm fine with inheritance, but do we want to make a special distinction for the admin group? Max?

Comment 7

[Frédéric Buclin]

Comment on attachment 315083 [details] [diff] [review]
Patch

>Index: Bugzilla/User.pm

>+sub group_membership {
>+    my $dbh = Bugzilla->dbh;

$dbh is never used. Remove it.


> sub direct_group_membership {

I implemented this method for this unique purpose in bug 332149. As we no longer use it, please delete it entirely, including its POD.

Otherwise looks good. The admin thing can probably be fixed in a separate bug.

Comment 8

[Max Kanat-Alexander]

(In reply to comment #6)
> So what do we decide to do here? I'm fine with inheritance, but do we want to
> make a special distinction for the admin group? Max?

  Yeah, I think we should make a special distinction for the admin group.

Comment 9

[timeless]

no, not exception. it should be an option per group.

fwiw, this came up *today* on bugs.maemo.org. They're using a custom hack which implements something like bug 332149.

Sadly, I was listed as a member of the Nokia group and someone abused me because of that. Karsten was kind enough to remove me from the Nokia group which fixed the badging problem. However if they used Bugzilla's badges and this bug were fixed, then I'd again have the Nokia badge because I'm an admin.

Comment 10

[Frédéric Buclin]

Bugzilla 3.2 and 3.4 are closed for enhancement requests.

Comment 11

[Marc Schumann [:Wurblzap]]

Attachment: Patch 1.1

This has become rather easy. The patch doesn't special-case the admin as of now; maybe this can be done in a separate bug as comment 7 suggests.

Comment 12

[Frédéric Buclin]

Comment on attachment 558479 [details] [diff] [review]
Patch 1.1

r=LpSolit

Comment 13

[Marc Schumann [:Wurblzap]]

Thanks for the review.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/User.pm
modified template/en/default/bug/comments.html.tmpl
Committed revision 8014.

Comment 14

[Marc Schumann [:Wurblzap]]

Why did the milestone get reset??