Bug 298688: Forbid the deletion of values used by default for the priority, severity, platform and OS and update data/params when editing them - Patch by Frédéric Buclin <LpSolit@gmail.com> r=GavinS a=justdave

5 files changed, 108 insertions, 42 deletions

diff --git a/editvalues.cgi b/editvalues.cgi
index 897498db3..5b45aac92 100755
--- a/editvalues.cgi
+++ b/editvalues.cgi
@@ -12,6 +12,7 @@
 # The Original Code is the Bugzilla Bug Tracking System.
 #
 # Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 # This is a script to edit the values of fields that have drop-down
 # or select boxes. It is largely a copy of editmilestones.cgi, but 
@@ -20,11 +21,12 @@
 use strict;
 use lib ".";
 
+require "globals.pl";
 use Bugzilla;
 use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::Constants;
-use Bugzilla::Config qw(:DEFAULT :locations);
+use Bugzilla::Config qw(:DEFAULT :admin :locations);
 
 # List of different tables that contain the changeable field values
 # (the old "enums.") Keep them in alphabetical order by their 
@@ -115,6 +117,13 @@ my $value   = trim($cgi->param('value')   || '');
 my $sortkey = trim($cgi->param('sortkey') || '0');
 my $action  = trim($cgi->param('action')  || '');
 
+# Gives the name of the parameter associated with the field
+# and representing its default value.
+my %defaults = {};
+$defaults{'op_sys'} = 'defaultopsys';
+$defaults{'rep_platform'} = 'defaultplatform';
+$defaults{'priority'} = 'defaultpriority';
+$defaults{'bug_severity'} = 'defaultseverity';
 
 #
 # field = '' -> Show nice list of fields
@@ -148,6 +157,7 @@ unless ($action) {
                                  {Slice =>{}});
     $vars->{'field'} = $field;
     $vars->{'values'} = $fieldvalues;
+    $vars->{'default'} = Param($defaults{$field});
     $template->process("admin/fieldvalues/list.html.tmpl",
                        $vars)
       || ThrowTemplateError($template->error());
@@ -237,6 +247,7 @@ if ($action eq 'del') {
 
     $vars->{'value'} = $value;
     $vars->{'field'} = $field;
+    $vars->{'param_name'} = $defaults{$field};
     $template->process("admin/fieldvalues/confirm-delete.html.tmpl",
                        $vars)
       || ThrowTemplateError($template->error());
@@ -250,6 +261,11 @@ if ($action eq 'del') {
 #
 if ($action eq 'delete') {
     ValueMustExist($field, $value);
+    if ($value eq Param($defaults{$field})) {
+        ThrowUserError('fieldvalue_is_default', {field      => $field,
+                                                 value      => $value,
+                                                 param_name => $defaults{$field}})
+    }
     trick_taint($field);
     trick_taint($value);
 
@@ -366,7 +382,20 @@ if ($action eq 'update') {
         $vars->{'updated_value'} = 1;
     }
 
-    $dbh->bz_unlock_tables(); 
+    $dbh->bz_unlock_tables();
+
+    # If the old value was the default value for the field,
+    # update data/params accordingly.
+    # This update is done while tables are unlocked due to the
+    # annoying call to GetVersionTable in Bugzilla/Config/Common.pm.
+    if ($value ne $valueold
+        && $valueold eq Param($defaults{$field}))
+    {
+        SetParam($defaults{$field}, $value);
+        WriteParams();
+        unlink "$datadir/versioncache";
+        $vars->{'default_value_updated'} = 1;
+    }
 
     $vars->{'value'} = $value;
     $vars->{'field'} = $field;
diff --git a/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl b/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl
index d1a016d8d..31deb547f 100644
--- a/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl
+++ b/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl
@@ -20,6 +20,8 @@
   # value_count: number; The number of values left for this field, including
   #              this value.
   # field: string; The name of the field.
+  # param_name: string; The name of the parameter (defaultxxx) associated
+  #             with the field.
   #%]
 
 [% title = BLOCK %]Delete Value '[% value FILTER html %]' from the 
@@ -27,6 +29,7 @@
 
 [% PROCESS global/header.html.tmpl
   title = title
+  style_urls = ['skins/standard/admin.css']
 %]
 
 <table border="1" cellpadding="4" cellspacing="0">
@@ -57,49 +60,58 @@
 </table>
 
 <h2>Confirmation</h2>
-  
-[% IF bug_count %]
 
-    Sorry, there
+[% IF (Param(param_name) == value) || bug_count || (value_count == 1) %]
 
-    [% IF bug_count > 1 %] 
-      are [% bug_count FILTER html %] [%+ terms.bugs %] 
-    [% ELSE %]
-      is [% bug_count FILTER html %] [%+ terms.bug %] 
-    [% END %]
+  <p>Sorry, but the '[% value FILTER html %]' value cannot be deleted
+  from the '[% field FILTER html %]' field for the following reason(s):</p>
 
-    that still have this field value. You must change the field value on
-    <a title="List of [% terms.bugs %] where '[% field FILTER html %]' is '[% value FILTER html %]'"
-       href="buglist.cgi?[% field FILTER url_quote %]=[%- value FILTER url_quote %]">
-    [% IF bug_count > 1 %]
-       those [% terms.bugs %] 
-    [% ELSE %]
-       that [% terms.bug %]
+  <ul class="warningmessages">
+    [% IF Param(param_name) == value %]
+      <li>'[% value FILTER html %]' is the default value for
+          the '[% field FILTER html %]' field.
+          [% IF user.groups.tweakparams %]
+            You first have to <a href="editparams.cgi?section=bugfields#
+            [%- param_name FILTER url_quote %]">change the default value</a> for
+            this field before you can delete this value.
+          [% END %]
     [% END %]
-    </a>
-    to another value before you can delete this value.
-
-[% END %]
 
-[% IF value_count == 1 %]
-
-    <p>Sorry, but '[% value FILTER html %]' is the last value for
-    '[%- field FILTER html %]', and so it can not be deleted.</p>
+    [% IF bug_count %]
+      <li>There
+          [% IF bug_count > 1 %] 
+            are [% bug_count FILTER html %] [%+ terms.bugs %] 
+          [% ELSE %]
+            is 1 [% terms.bug %] 
+          [% END %]
+          with this field value. You must change the field value on
+          <a title="List of [% terms.bugs %] where '[% field FILTER html %]' is '[% value FILTER html %]'"
+             href="buglist.cgi?[% field FILTER url_quote %]=[% value FILTER url_quote %]">
+            [% IF bug_count > 1 %]
+              those [% terms.bugs %] 
+            [% ELSE %]
+              that [% terms.bug %]
+            [% END %]
+          </a>
+          to another value before you can delete this value.
+    [% END %]
 
-  [% ELSE %]
+    [% IF value_count == 1 %]
+      <li>'[% value FILTER html %]' is the last value for
+          '[%- field FILTER html %]', and so it can not be deleted.
+    [% END %]
+  </ul>
 
-    [% IF bug_count == 0 %]
+[% ELSE %]
 
-      <p>Do you really want to delete this value?<p>
-  
-      <form method="post" action="editvalues.cgi">
-        <input type="submit" value="Yes, delete">
-        <input type="hidden" name="action" value="delete">
-        <input type="hidden" name="field" value="[% field FILTER html %]">
-        <input type="hidden" name="value" value="[% value FILTER html %]">
-      </form>
+  <p>Do you really want to delete this value?</p>
 
-  [% END %]
+  <form method="post" action="editvalues.cgi">
+    <input type="submit" value="Yes, delete">
+    <input type="hidden" name="action" value="delete">
+    <input type="hidden" name="field" value="[% field FILTER html %]">
+    <input type="hidden" name="value" value="[% value FILTER html %]">
+  </form>
 
 [% END %]
 
diff --git a/template/en/default/admin/fieldvalues/list.html.tmpl b/template/en/default/admin/fieldvalues/list.html.tmpl
index b661653de..d25adf2b1 100644
--- a/template/en/default/admin/fieldvalues/list.html.tmpl
+++ b/template/en/default/admin/fieldvalues/list.html.tmpl
@@ -50,18 +50,28 @@
        name => "sortkey"
        heading => "Sortkey"
      },
-   ]
+     {
+       name => "action"
+       heading => "Action"
+       content => "Delete"
+       contentlink => delete_contentlink
+     } ]
 %]
 
-[% columns.push({
-     heading => "Action"
-     content => "Delete"
-     contentlink => delete_contentlink
-   }) %]
+[% overrides.action = [ {
+     match_value => "$default"
+     match_field => 'name'
+     override_content => 1
+     content => "(Default value)"
+     override_contentlink => 1
+     contentlink => undef
+   } ]
+%] 
 
 [% PROCESS admin/table.html.tmpl
      columns = columns
      data = values
+     overrides = overrides
 %]
 
 <p>
diff --git a/template/en/default/admin/fieldvalues/updated.html.tmpl b/template/en/default/admin/fieldvalues/updated.html.tmpl
index fc9fcafbc..c7f676e67 100644
--- a/template/en/default/admin/fieldvalues/updated.html.tmpl
+++ b/template/en/default/admin/fieldvalues/updated.html.tmpl
@@ -24,6 +24,8 @@
   # value & updated_value: the name of the field value
   # sortkey & updated_sortkey: the field value sortkey
   # field: string; the field that the value belongs to
+  # default_value_updated: boolean; whether the default value for
+  #                        this field has been updated
   #%]
   
 [% title = BLOCK %]Updating Value '[% value FILTER html %]' of the
@@ -34,6 +36,10 @@
 
 [% IF updated_value %]
   <p>Updated field value to: '[% value FILTER html %]'.</p>
+  [% IF default_value_updated %]
+    <p>Note that this value is the default for this field.
+    All references to the default value will now point to this new value.</p>
+  [% END %]
 [% END %]
 
 [% IF updated_sortkey %]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 09f3d6e8a..334f1c54e 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -384,6 +384,15 @@
     The value '[% value FILTER html %]' does not exist for
     the '[% field FILTER html %]' field.
 
+  [% ELSIF error == "fieldvalue_is_default" %]
+    [% title = "Specified Field Value Is Default" %]
+    '[% value FILTER html %]' is the default value for
+    the '[% field FILTER html %]' field and cannot be deleted.
+    [% IF user.groups.tweakparams %]
+      You have to <a href="editparams.cgi?section=bugfields#
+      [%- param_name FILTER url_quote %]">change</a> the default value first.
+    [% END %]
+
   [% ELSIF error == "fieldvalue_name_too_long" %]
     [% title = "Field Value Is Too Long" %]
     The value of a field is limited to 60 characters.