Bug 1197696 - group_members report doesn't display nested inherited groups

5 files changed, 72 insertions, 49 deletions

diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm
index 732de348f..bb84ca229 100644
--- a/Bugzilla/Group.pm
+++ b/Bugzilla/Group.pm
@@ -107,6 +107,27 @@ sub members_non_inherited {
     return $self->{members_non_inherited};
 }
 
+# returns all possible members of groups, keyed by the group name or _direct
+# a user present in multiple groups will be returned multiple times
+sub members_complete {
+    my ($self) = @_;
+    my $dbh = Bugzilla->dbh;
+    require Bugzilla::User;
+
+    my $sth = $dbh->prepare(
+        "SELECT DISTINCT user_id FROM user_group_map WHERE isbless = 0 AND group_id = ?"
+    );
+
+    my $result = { _direct => $self->members_direct() };
+    foreach my $group_id (@{ $self->flatten_group_membership($self->id) }) {
+        next if $group_id == $self->id;
+        my $group_name = Bugzilla::Group->new({ id => $group_id, cache => 1 })->name;
+        my $user_ids = $dbh->selectcol_arrayref($sth, undef, $group_id);
+        $result->{$group_name} = Bugzilla::User->new_from_list($user_ids);
+    }
+    return $result;
+}
+
 # A helper for members_direct and members_non_inherited
 sub _get_members {
     my ($self, $grant_type) = @_;
diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm
index 63f53bc06..cc439eeed 100644
--- a/extensions/BMO/Extension.pm
+++ b/extensions/BMO/Extension.pm
@@ -196,7 +196,7 @@ sub page_before_template {
     }
     elsif ($page eq 'group_members.html' or $page eq 'group_members.json') {
         require Bugzilla::Extension::BMO::Reports::Groups;
-        Bugzilla::Extension::BMO::Reports::Groups::members_report($vars);
+        Bugzilla::Extension::BMO::Reports::Groups::members_report($page, $vars);
     }
     elsif ($page eq 'recruiting_dashboard.html') {
         require Bugzilla::Extension::BMO::Reports::Recruiting;
diff --git a/extensions/BMO/lib/Reports/Groups.pm b/extensions/BMO/lib/Reports/Groups.pm
index 7fa86b243..4a831fab3 100644
--- a/extensions/BMO/lib/Reports/Groups.pm
+++ b/extensions/BMO/lib/Reports/Groups.pm
@@ -14,6 +14,7 @@ use Bugzilla::Error;
 use Bugzilla::Group;
 use Bugzilla::User;
 use Bugzilla::Util qw(trim);
+use JSON qw(encode_json);
 
 sub admins_report {
     my ($vars) = @_;
@@ -163,7 +164,7 @@ sub membership_report {
 }
 
 sub members_report {
-    my ($vars) = @_;
+    my ($page, $vars) = @_;
     my $dbh = Bugzilla->dbh;
     my $user = Bugzilla->user;
     my $cgi = Bugzilla->cgi;
@@ -196,20 +197,13 @@ sub members_report {
     my $group_obj = Bugzilla::Group->new({ name => $group });
     $vars->{'group'} = $group;
 
-    # direct members
-    my @types = (
-        {
-            name    => 'direct',
-            members => _filter_userlist($group_obj->members_direct, $include_disabled),
-        },
-    );
-
-    # indirect members, by group
-    foreach my $member_group (sort @{ $group_obj->grant_direct(GROUP_MEMBERSHIP) }) {
+    my @types;
+    my $members = $group_obj->members_complete();
+    foreach my $name (sort keys %$members) {
         push @types, {
-            name    => $member_group->name,
-            members => _filter_userlist($member_group->members_direct, $include_disabled),
-        },
+            name    => ($name eq '_direct' ? 'direct' : $name),
+            members => _filter_userlist($members->{$name}),
+        }
     }
 
     # make it easy for the template to detect an empty group
@@ -243,7 +237,38 @@ sub members_report {
         }
     }
 
-    $vars->{'types'} = \@types;
+    if ($page eq 'group_members.json') {
+        my %users;
+        foreach my $rh (@types) {
+            my $group_name = $rh->{name} eq '_direct' ? 'direct' : $rh->{name};
+            foreach my $member (@{ $rh->{members} }) {
+                my $login = $member->login;
+                if (exists $users{$login}) {
+                    push @{ $users{$login}->{groups} }, $group_name;
+                }
+                else {
+                    $users{$login} = {
+                        login      => $login,
+                        membership => $rh->{name} eq '_direct' ? 'direct' : 'indirect',
+                        group      => $group_name,
+                        groups     => [ $group_name ],
+                        lastseen   => $member->{lastseen},
+                    };
+                }
+            }
+        }
+        $vars->{types_json} = JSON->new->pretty->canonical->utf8->encode([ values %users ]);
+    }
+    else {
+        my %users;
+        foreach my $rh (@types) {
+            foreach my $member (@{ $rh->{members} }) {
+                $users{$member->login} = 1 unless exists $users{$member->login};
+            }
+        }
+        $vars->{types} = \@types;
+        $vars->{count} = scalar(keys %users);
+    }
 }
 
 sub _filter_userlist {
diff --git a/extensions/BMO/template/en/default/pages/group_members.html.tmpl b/extensions/BMO/template/en/default/pages/group_members.html.tmpl
index 67db8ea2e..bd27b8be2 100644
--- a/extensions/BMO/template/en/default/pages/group_members.html.tmpl
+++ b/extensions/BMO/template/en/default/pages/group_members.html.tmpl
@@ -41,7 +41,7 @@
 [% IF group != '' %]
 
   <p>
-    Members of the <b>[% group FILTER html %]</b> group:
+    [% count FILTER none %] member[% count == 1 ? '' : 's' %] of the <b>[% group FILTER html %]</b> group:
   </p>
 
   [% IF types.size > 0 %]
@@ -58,14 +58,18 @@
       [% FOREACH type = types %]
         [% count = loop.count() %]
         <tr class="report_item [% count % 2 == 1 ? "report_row_odd" : "report_row_even" %]">
-          <td valign="top">
+          <td valign="top" nowrap>
             [% IF type.name == 'direct' %]
               direct
             [% ELSE %]
-              via&nbsp;
+              via
               [% IF privileged %]
-                [% type.name FILTER html %]
-              [% ELSE %]another&nbsp;group[% END %]
+                <a href="page.cgi?id=group_members.html&group=[% type.name FILTER uri %]">
+                  [% type.name FILTER html %]
+                </a>
+              [% ELSE %]
+                another group
+              [% END %]
             [% END %]
           </td>
           <td valign="top" align="right">
diff --git a/extensions/BMO/template/en/default/pages/group_members.json.tmpl b/extensions/BMO/template/en/default/pages/group_members.json.tmpl
index e982731f7..30b969e2f 100644
--- a/extensions/BMO/template/en/default/pages/group_members.json.tmpl
+++ b/extensions/BMO/template/en/default/pages/group_members.json.tmpl
@@ -6,31 +6,4 @@
   # defined by the Mozilla Public License, v. 2.0.
   #%]
 
-[
-  [% SET count = 0 %]
-  [% FOREACH type = types %]
-    [% SET count = count + type.members.size %]
-  [% END %]
-  [% SET i = 0 %]
-  [% FOREACH type = types %]
-    [% FOREACH member = type.members %]
-      [% SET i = i + 1 %]
-      { "login": "[% member.login FILTER email FILTER js %]",
-        [% IF type.name == "direct" %]
-          "membership": "direct",
-        [% ELSE %]
-          "membership": "indirect",
-          [% IF user.in_group('editusers') || user.in_group('infrasec') %]
-            "group": "[% type.name FILTER js %]",
-          [% END %]
-        [% END %]
-        [% IF include_disabled %]
-          "disabled": "[% member.is_enabled ? "false" : "true" %]",
-        [% END %]
-        [% IF user.in_group('editusers') || user.in_group('infrasec') %]
-          "lastseen": "[% member.lastseen FILTER js %]"
-        [% END %]
-      }[% "," UNLESS i == count %]
-    [% END %]
-  [% END %]
-]
+[% types_json FILTER none %]