Bug 324784: editgroups.cgi should make sure groups it edits/removes are not used by editparams.cgi (in data/params) - Patch by Frédéric Buclin <LpSolit@gmail.com> r=wicked a=justdave

3 files changed, 62 insertions, 3 deletions

diff --git a/editgroups.cgi b/editgroups.cgi
index 9c1280d5c..7b86df35a 100755
--- a/editgroups.cgi
+++ b/editgroups.cgi
@@ -31,6 +31,7 @@ use lib ".";
 
 use Bugzilla;
 use Bugzilla::Constants;
+use Bugzilla::Config qw(:DEFAULT :admin);
 use Bugzilla::Group;
 use Bugzilla::User;
 require "globals.pl";
@@ -338,6 +339,17 @@ if ($action eq 'del') {
     if (!$isbuggroup) {
         ThrowUserError("system_group_not_deletable", { name => $name });
     }
+    # Groups having a special role cannot be deleted.
+    my @special_groups;
+    foreach my $special_group ('chartgroup', 'insidergroup', 'timetrackinggroup') {
+        if ($name eq Param($special_group)) {
+            push(@special_groups, $special_group);
+        }
+    }
+    if (scalar(@special_groups)) {
+        ThrowUserError('group_has_special_role', {'name'  => $name,
+                                                  'groups' => \@special_groups});
+    }
 
     # Group inheritance no longer appears in user_group_map.
     my $grouplist = join(',', @{Bugzilla::User->flatten_group_membership($gid)});
@@ -391,6 +403,17 @@ if ($action eq 'delete') {
     if (!$isbuggroup) {
         ThrowUserError("system_group_not_deletable", { name => $name });
     }
+    # Groups having a special role cannot be deleted.
+    my @special_groups;
+    foreach my $special_group ('chartgroup', 'insidergroup', 'timetrackinggroup') {
+        if ($name eq Param($special_group)) {
+            push(@special_groups, $special_group);
+        }
+    }
+    if (scalar(@special_groups)) {
+        ThrowUserError('group_has_special_role', {'name'  => $name,
+                                                  'groups' => \@special_groups});
+    }
 
     my $cantdelete = 0;
 
@@ -596,6 +619,16 @@ sub doGroupChanges {
             $chgs = 1;
             $dbh->do('UPDATE groups SET name = ? WHERE id = ?',
                       undef, ($name, $gid));
+            # If the group is used by some parameters, we have to update
+            # these parameters too.
+            my $update_params = 0;
+            foreach my $group ('chartgroup', 'insidergroup', 'timetrackinggroup') {
+                if ($cgi->param('oldname') eq Param($group)) {
+                    SetParam($group, $name);
+                    $update_params = 1;
+                }
+            }
+            WriteParams() if $update_params;
         }
         if ($desc ne $cgi->param('olddesc')) {
             $chgs = 1;
diff --git a/template/en/default/admin/groups/list.html.tmpl b/template/en/default/admin/groups/list.html.tmpl
index f32e715d6..64e30f9e4 100644
--- a/template/en/default/admin/groups/list.html.tmpl
+++ b/template/en/default/admin/groups/list.html.tmpl
@@ -84,10 +84,22 @@
      content => " "
    }]
    overrides.action = [ {
-     match_value => "0"
-     match_field => 'isbuggroup'
+     match_value => Param("chartgroup")
+     match_field => 'name'
      override_content => 1
-     content => " "
+     content => "(used as the 'chartgroup')"
+   },
+   {
+     match_value => Param("insidergroup")
+     match_field => 'name'
+     override_content => 1
+     content => "(used as the 'insidergroup')"
+   },
+   {
+     match_value => Param("timetrackinggroup")
+     match_field => 'name'
+     override_content => 1
+     content => "(used as the 'timetrackinggroup')"
    },
    {
      match_value => "1"
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index ee7f6c17d..ac1bca1c2 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -519,6 +519,20 @@
     [% title = "The group already exists" %]
     The group [% name FILTER html %] already exists.
 
+  [% ELSIF error == "group_has_special_role" %]
+    [% title = "Group not deletable" %]
+    [% IF groups.size == 1 %]
+      [% attr = "it" %]
+      [% param = "parameter" %]
+    [% ELSE %]
+      [% attr = "them" %]
+      [% param = "parameters" %]
+    [% END %]
+    The group '[% name FILTER html %]' is used by the
+    '[% groups.join("' and '") FILTER html %]' [% param FILTER html %].
+    In order to delete this group, you first have to change the
+    [%+ param FILTER html %] to make [% attr FILTER html %] point to another group.
+
   [% ELSIF error == "group_not_specified" %]
     [% title = "Group not specified" %]
     No group was specified.