Bug 1196618 - add support for group owners

11 files changed, 119 insertions, 15 deletions

diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm
index bb84ca229..37d4acf90 100644
--- a/Bugzilla/Group.pm
+++ b/Bugzilla/Group.pm
@@ -33,6 +33,8 @@ use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::Config qw(:admin);
 
+use Scalar::Util qw(blessed);
+
 ###############################
 ##### Module Initialization ###
 ###############################
@@ -47,6 +49,7 @@ use constant DB_COLUMNS => qw(
     groups.userregexp
     groups.isactive
     groups.icon_url
+    groups.owner_user_id
 );
 
 use constant DB_TABLE => 'groups';
@@ -60,6 +63,7 @@ use constant VALIDATORS => {
     isactive    => \&_check_is_active,
     isbuggroup  => \&_check_is_bug_group,
     icon_url    => \&_check_icon_url,
+    owner_user_id => \&_check_owner,
 };
 
 use constant UPDATE_COLUMNS => qw(
@@ -68,6 +72,7 @@ use constant UPDATE_COLUMNS => qw(
     userregexp
     isactive
     icon_url
+    owner_user_id
 );
 
 # Parameters that are lists of groups.
@@ -205,6 +210,15 @@ sub products {
     return $self->{products};
 }
 
+sub owner {
+    my $self = shift;
+    return $self->{owner} if exists $self->{owner};
+    if ($self->{owner_user_id}) {
+        $self->{owner} = Bugzilla::User->check({ id => $self->{owner_user_id}, cache => 1 });
+    }
+    return $self->{owner} || undef;
+}
+
 ###############################
 ####        Methods        ####
 ###############################
@@ -227,6 +241,13 @@ sub set_name        { $_[0]->set('name', $_[1]);        }
 sub set_user_regexp { $_[0]->set('userregexp', $_[1]);  }
 sub set_icon_url    { $_[0]->set('icon_url', $_[1]);    }
 
+sub set_owner {
+    my ($self, $owner_id) = @_;
+    $self->set('owner_user_id', $owner_id);
+    # Reset the default owner object.
+    delete $self->{owner};
+}
+
 sub update {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
@@ -519,6 +540,16 @@ sub _check_is_bug_group {
 
 sub _check_icon_url { return $_[1] ? clean_text($_[1]) : undef; }
 
+sub _check_owner {
+    my ($invocant, $owner, undef, $params) = @_;
+    return Bugzilla::User->check({ name => $owner, cache => 1 })->id if $owner;
+    # We require an owner if the group is a not a system group
+    if (blessed($invocant) && !$invocant->is_bug_group) {
+        return undef;
+    }
+    ThrowUserError('group_needs_owner');
+}
+
 1;
 
 __END__
@@ -541,6 +572,7 @@ Bugzilla::Group - Bugzilla group class.
     my $is_active    = $group->is_active;
     my $icon_url     = $group->icon_url;
     my $is_active_bug_group = $group->is_active_bug_group;
+    my $owner        = $group->owner;
 
     my $group_id = Bugzilla::Group::ValidateGroupName('admin', @users);
     my @groups   = Bugzilla::Group->get_all;
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index d3bb807b3..311b89098 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1051,6 +1051,11 @@ sub groups_in_sql {
     return Bugzilla->dbh->sql_in($field, $ids);
 }
 
+sub groups_owned {
+    my $self = shift;
+    return $self->{groups_owned} //= Bugzilla::Group->match({ owner_user_id => $self->id });
+}
+
 sub bless_groups {
     my $self = shift;
 
diff --git a/editgroups.cgi b/editgroups.cgi
index 086165bc8..52fa88b8c 100755
--- a/editgroups.cgi
+++ b/editgroups.cgi
@@ -213,6 +213,7 @@ if ($action eq 'new') {
         isactive    => scalar $cgi->param('isactive'),
         icon_url    => scalar $cgi->param('icon_url'),
         isbuggroup  => 1,
+        owner_user_id => scalar $cgi->param('owner'),
     });
 
     # Permit all existing products to use the new group if makeproductgroups.
@@ -397,6 +398,10 @@ sub doGroupChanges {
         $group->set_icon_url($cgi->param('icon_url'));
     }
 
+    if (defined $cgi->param('owner')) {
+        $group->set_owner($cgi->param('owner'));
+    }
+
     my $changes = $group->update();
 
     my $sth_insert = $dbh->prepare('INSERT INTO group_group_map
diff --git a/extensions/BMO/lib/Reports/Groups.pm b/extensions/BMO/lib/Reports/Groups.pm
index 3a5cd75dd..d1da19c5a 100644
--- a/extensions/BMO/lib/Reports/Groups.pm
+++ b/extensions/BMO/lib/Reports/Groups.pm
@@ -34,8 +34,8 @@ sub admins_report {
     my $groups = join(',', map { $dbh->quote($_) } @grouplist);
 
     my $query = "
-        SELECT groups.name, " .
-               $dbh->sql_group_concat('profiles.login_name', "','", 1) . "
+        SELECT groups.id, " .
+               $dbh->sql_group_concat('profiles.userid', "','", 1) . "
           FROM groups
                LEFT JOIN user_group_map
                     ON user_group_map.group_id = groups.id
@@ -48,14 +48,18 @@ sub admins_report {
       GROUP BY groups.name";
 
     my @groups;
-    foreach my $group (@{ $dbh->selectall_arrayref($query) }) {
+    foreach my $row (@{ $dbh->selectall_arrayref($query) }) {
+        my $group = Bugzilla::Group->new({ id => shift @$row, cache => 1});
         my @admins;
-        if ($group->[1]) {
-            foreach my $admin (split(/,/, $group->[1])) {
-                push(@admins, Bugzilla::User->new({ name => $admin }));
+        if (my $admin_ids = shift @$row) {
+            foreach my $uid (split(/,/, $admin_ids)) {
+                push(@admins, Bugzilla::User->new({ id => $uid, cache => 1 }));
             }
         }
-        push(@groups, { name => $group->[0], admins => \@admins });
+        push(@groups, { name        => $group->name,
+                        description => $group->description,
+                        owner       => $group->owner,
+                        admins      => \@admins });
     }
 
     $vars->{'groups'} = \@groups;
@@ -197,7 +201,7 @@ sub members_report {
     $group = '' unless grep { $_ eq $group } @group_names;
     return if $group eq '';
     my $group_obj = Bugzilla::Group->new({ name => $group });
-    $vars->{'group'} = $group;
+    $vars->{'group'} = $group_obj;
 
     my @types;
     my $members = $group_obj->members_complete();
diff --git a/extensions/BMO/template/en/default/pages/group_admins.html.tmpl b/extensions/BMO/template/en/default/pages/group_admins.html.tmpl
index 01bb744c4..c81c95748 100644
--- a/extensions/BMO/template/en/default/pages/group_admins.html.tmpl
+++ b/extensions/BMO/template/en/default/pages/group_admins.html.tmpl
@@ -28,14 +28,23 @@
   <table border="0" cellspacing="0" id="report" class="hover" width="100%">
     <tr id="report-header">
       <th align="left">Name</th>
+      <th align="left">Owner</th>
       <th align="left">Admins</th>
     </tr>
 
     [% FOREACH group = groups %]
       [% count = loop.count() %]
       <tr class="report_item [% count % 2 == 1 ? "report_row_odd" : "report_row_even" %]">
-        <td>
-          [% group.name FILTER html %]
+        <td nowrap>
+          <span title="[% group.description FILTER html %]">
+            [% group.name FILTER html %]</span>
+        </td>
+        <td nowrap>
+          [% IF group.owner.login == 'nobody@mozilla.org' %]
+            &ndash;
+          [% ELSE %]
+            [% INCLUDE global/user.html.tmpl who = group.owner %]
+          [% END %]
         </td>
         <td>
           [% FOREACH admin = group.admins %]
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 98679c1b7..b1f9b82c5 100644
--- a/extensions/BMO/template/en/default/pages/group_members.html.tmpl
+++ b/extensions/BMO/template/en/default/pages/group_members.html.tmpl
@@ -21,7 +21,7 @@
       <select name="group">
         [% FOREACH group_name = groups %]
           <option value="[% group_name FILTER html %]"
-                  [% "selected" IF group_name == group %]>
+                  [% "selected" IF group_name == group.name %]>
             [% group_name FILTER html %]</option>
         [% END %]
       </select>
@@ -39,7 +39,7 @@
 [% IF group != '' %]
 
   <p>
-    [% count FILTER none %] member[% count == 1 ? '' : 's' %] of the <b>[% group FILTER html %]</b> group:
+    [% count FILTER none %] member[% count == 1 ? '' : 's' %] of the <b>[% group.name FILTER html %]</b> group:
   </p>
 
   [% IF types.size > 0 %]
@@ -48,7 +48,7 @@
         <th>Type</th>
         <th>Count</th>
         <th>Members</th>
-        [% IF privileged %]
+        [% IF privileged || (user.id == group.owner.id) %]
           <th class="right">2FA, Last Seen (days ago)</th>
         [% END %]
       </tr>
@@ -111,7 +111,7 @@
       [% END %]
     </table>
 
-    <a href="page.cgi?id=group_members.json&amp;group=[% group FILTER uri %]
+    <a href="page.cgi?id=group_members.json&amp;group=[% group.name FILTER uri %]
        [% IF include_disabled %]&amp;include_disabled=1[% END %]">JSON</a>
   [% ELSE %]
     <p>
diff --git a/template/en/default/admin/groups/create.html.tmpl b/template/en/default/admin/groups/create.html.tmpl
index b3ac72372..e5ffb7819 100644
--- a/template/en/default/admin/groups/create.html.tmpl
+++ b/template/en/default/admin/groups/create.html.tmpl
@@ -29,6 +29,8 @@
   title = "Add group"
   subheader = "This page allows you to define a new user group."
   doc_section = "groups.html#create-groups"
+  generate_api_token = 1
+  javascript_urls = [ "js/field.js" ]
 %]
 
 <form method="post" action="editgroups.cgi">
@@ -47,6 +49,17 @@
     <th>Icon URL:</th>
     <td colspan="3"><input type="text" size="70" maxlength="255" id="icon_url" name="icon_url"></td>
   </tr>
+  <tr>
+    <th>Owner:</th>
+    <td colspan="3">
+      [% INCLUDE global/userselect.html.tmpl
+         name  => "owner"
+         id    => "owner"
+         size  => 30
+      %]
+    </td>
+  </tr>
+
   [% Hook.process('field') %]
   </table>
 
diff --git a/template/en/default/admin/groups/edit.html.tmpl b/template/en/default/admin/groups/edit.html.tmpl
index 9403c076d..fb54c2c0d 100644
--- a/template/en/default/admin/groups/edit.html.tmpl
+++ b/template/en/default/admin/groups/edit.html.tmpl
@@ -45,6 +45,8 @@
         padding-right: .5em;
     }
   "
+  generate_api_token = 1
+  javascript_urls = [ "js/field.js" ]
 %]
 
 <form method="post" action="editgroups.cgi">
@@ -97,6 +99,18 @@
       </td>
     </tr>
 
+    <tr>
+      <th>Owner:</th>
+      <td>
+        [% INCLUDE global/userselect.html.tmpl
+           id      => "owner"
+           name    => "owner"
+           value   => group.owner.login
+           size    => 30
+         %]
+      </td>
+    </tr>
+
     [% IF group.is_bug_group %]
       <tr>
         <th>Use For [% terms.Bugs %]:</th>
diff --git a/template/en/default/admin/users/edit.html.tmpl b/template/en/default/admin/users/edit.html.tmpl
index fbf283574..64eece345 100644
--- a/template/en/default/admin/users/edit.html.tmpl
+++ b/template/en/default/admin/users/edit.html.tmpl
@@ -136,6 +136,22 @@ $(function() {
     </td>
   </tr>
 
+  [% IF otheruser.groups_owned.size %]
+    <tr>
+      <th>Groups Owned:</th>
+      <td>
+        [% can_edit_groups = user.in_group('creategroups') %]
+        [% FOREACH group = otheruser.groups_owned %]
+          [% IF can_edit_groups %]
+            <a href="[% urlbase FILTER none %]editgroups.cgi?action=changeform&amp;group=[% group.id FILTER none %]">
+          [% END %]
+          [% group.name FILTER html %]
+          [% '</a>' IF can_edit_groups %]<br>
+        [% END %]
+      </td>
+    </tr>
+  [% END %]
+
   <tr>
     <th>Last Login:</th>
     <td>
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index d5d4a563d..83b20f4f6 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -409,6 +409,8 @@
             [% END %]
           [% CASE 'icon_url' %]
             <li>The group icon URL has been updated.</li>
+          [% CASE 'owner_user_id' %]
+            <li>The group owner was updated.</li>
           [% CASE 'members_add' %]
             <li>The following groups are now members of this group:
               [%+ changes.members_add.join(', ') FILTER html %]</li>
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 6f352e5ac..332075b69 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -541,7 +541,11 @@
   [% ELSIF error == "empty_group_name" %]
     [% title = "The group name can not be empty" %]
     You must enter a name for the group.
-  
+
+  [% ELSIF error == "group_needs_owner" %]
+    [% title = "Non-System Groups Require Default Owner" %]
+    You must enter a default owner login for non-system groups.
+
   [% ELSIF error == "entry_access_denied" %]
     [% title = "Permission Denied" %]
     [% admindocslinks = {'groups.html' => 'Group Security'} %]