Bug 344965: Fix process_bug.cgi and bug/* templates to work with custom bug status workflow - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=LpSolit

7 files changed, 84 insertions, 148 deletions

diff --git a/template/en/default/admin/workflow/comment.html.tmpl b/template/en/default/admin/workflow/comment.html.tmpl
index 5e9a788d6..df9d5e872 100644
--- a/template/en/default/admin/workflow/comment.html.tmpl
+++ b/template/en/default/admin/workflow/comment.html.tmpl
@@ -50,24 +50,24 @@
     <th>&nbsp;</th>
     [% FOREACH status = statuses %]
       <th class="col-header[% status.is_open ? " open-status" : " closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
     [% END %]
   </tr>
 
   [%# This defines the entry point in the workflow %]
-  [% p = [{id => 0, value => "{Start}", is_open => 1}] %]
+  [% p = [{id => 0, name => "{Start}", is_open => 1}] %]
   [% FOREACH status = p.merge(statuses) %]
     <tr class="highlight">
       <th align="right" class="[% status.is_open ? "open-status" : "closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
 
       [% FOREACH new_status = statuses %]
         [% IF workflow.${status.id}.${new_status.id}.defined %]
           <td align="center" class="checkbox-cell
               [% " checked" IF workflow.${status.id}.${new_status.id} %]"
-              title="From [% status.value FILTER html %] to [% new_status.value FILTER html %]">
+              title="From [% status.name FILTER html %] to [% new_status.name FILTER html %]">
             <input type="checkbox" name="c_[% status.id %]_[% new_status.id %]"
                    id="c_[% status.id %]_[% new_status.id %]" onclick="toggle_cell(this)"
               [% " checked='checked'" IF workflow.${status.id}.${new_status.id} %]>
diff --git a/template/en/default/admin/workflow/edit.html.tmpl b/template/en/default/admin/workflow/edit.html.tmpl
index dee71c0a1..d602171a1 100644
--- a/template/en/default/admin/workflow/edit.html.tmpl
+++ b/template/en/default/admin/workflow/edit.html.tmpl
@@ -34,8 +34,12 @@
 </script>
 
 <p>
-  This page allows you to define which status transitions are valid
-  in your workflow.
+  This page allows you to define which status transitions are valid in your workflow.
+  For compatibility with older versions of [% terms.Bugzilla %], reopening [% terms.abug %]
+  will only display either UNCONFIRMED or REOPENED (if allowed by your workflow) but not
+  both. The decision depends on whether the [% terms.bug %] has ever been confirmed or not.
+  So it is a good idea to allow both transitions and let [% terms.Bugzilla %] select the
+  correct one.
 </p>
 
 <form id="workflow_form" method="POST" action="editworkflow.cgi">
@@ -50,24 +54,24 @@
     <th>&nbsp;</th>
     [% FOREACH status = statuses %]
       <th class="col-header[% status.is_open ? " open-status" : " closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
     [% END %]
   </tr>
 
   [%# This defines the entry point in the workflow %]
-  [% p = [{id => 0, value => "{Start}", is_open => 1}] %]
+  [% p = [{id => 0, name => "{Start}", is_open => 1}] %]
   [% FOREACH status = p.merge(statuses) %]
     <tr class="highlight">
       <th align="right" class="[% status.is_open ? "open-status" : "closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
 
       [% FOREACH new_status = statuses %]
         [% IF status.id != new_status.id %]
           <td align="center" class="checkbox-cell
               [% " checked" IF workflow.${status.id}.${new_status.id}.defined %]"
-              title="From [% status.value FILTER html %] to [% new_status.value FILTER html %]">
+              title="From [% status.name FILTER html %] to [% new_status.name FILTER html %]">
             <input type="checkbox" name="w_[% status.id %]_[% new_status.id %]"
                    id="w_[% status.id %]_[% new_status.id %]" onclick="toggle_cell(this)"
               [% " checked='checked'" IF workflow.${status.id}.${new_status.id}.defined %]>
diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index 619c594e1..fe3adbbe9 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -595,7 +595,7 @@
       <td align="right">
         <b><a href="page.cgi?id=fields.html#status">Status</a></b>:
       </td>
-      <td>[% status_descs.${bug.bug_status} FILTER html %]</td>
+      <td>[% get_status(bug.bug_status) FILTER html %]</td>
     </tr>
 
     <tr>
diff --git a/template/en/default/bug/knob.html.tmpl b/template/en/default/bug/knob.html.tmpl
index 0e1a928f4..99aed9c22 100644
--- a/template/en/default/bug/knob.html.tmpl
+++ b/template/en/default/bug/knob.html.tmpl
@@ -18,42 +18,39 @@
   #
   # Contributor(s): Gervase Markham <gerv@gerv.net>
   #                 Vaskin Kissoyan <vkissoyan@yahoo.com>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
 
-[%# *** Knob *** %]
-
 <br>
 <div id="knob">
   <div id="knob-options">
-
-  [% knum = 1 %]
   [% initial_action_shown = 0 %]
 
-  [% IF bug.isunconfirmed && bug.user.canconfirm %]
-    [% PROCESS initial_action %]
-    <input type="radio" id="knob-confirm" name="knob" value="confirm">
-    <label for="knob-confirm">
-      Confirm [% terms.bug %] (change status to <b>[% get_status("NEW") FILTER html %]</b>)
-    </label>
-    <br>
-    [% knum = knum + 1 %]
-  [% END %]
-
-  [% IF bug.isopened && bug.bug_status != "ASSIGNED" && bug.user.canedit
-        && (!bug.isunconfirmed || bug.user.canconfirm) %]
+  [%# These actions are based on the current custom workflow. %]
+  [% FOREACH bug_status = bug.status.can_change_to %]
+    [% NEXT IF bug.isunconfirmed && bug_status.is_open && !bug.user.canconfirm %]
+    [% NEXT IF bug.isopened && !bug.isunconfirmed && bug_status.is_open && !bug.user.canedit %]
+    [% NEXT IF !bug_status.is_open && !bug.user.canedit && !bug.user.isreporter %]
+    [% NEXT IF !bug_status.is_open && bug_status.is_open && !bug.user.canedit && !bug.user.isreporter %]
+    [%# Special hack to only display UNCO or REOP when reopening, but not both;
+      # for compatibility with older versions. %]
+    [% NEXT IF !bug.isopened && (bug.everconfirmed && bug_status.name == "UNCONFIRMED"
+                                 || !bug.everconfirmed && bug_status.name == "REOPENED") %]
     [% PROCESS initial_action %]
-    <input type="radio" id="knob-accept" name="knob" value="accept">
-    <label for="knob-accept">
-      Accept [% terms.bug %] (
-      [% IF bug.isunconfirmed %]confirm [% terms.bug %], and [% END %]change
-      status to <b>[% get_status("ASSIGNED") FILTER html %]</b>)
+    <input type="radio" id="knob_[% bug_status.id FILTER html %]" name="knob"
+           value="[% bug_status.name FILTER html %]">
+    <label for="knob_[% bug_status.id FILTER html %]">
+      Change status to <b>[% get_status(bug_status.name) FILTER html %]</b>
     </label>
+    [% IF bug.isopened && !bug_status.is_open %]
+      and set the resolution to [% PROCESS select_resolution field = "knob_${bug_status.id}" %]
+    [% END %]
     <br>
-    [% knum = knum + 1 %]
   [% END %]
 
+  [%# These actions are special and are independent of the workflow. %]
   [% IF bug.user.canedit || bug.user.isreporter %]
     [% IF bug.isopened %]
       [% IF bug.resolution %]
@@ -64,65 +61,27 @@
           <b>[% get_resolution(bug.resolution) FILTER html %]</b>)
         </label>
         <br>
-        [% knum = knum + 1 %]
       [% END %]
-
-      [% PROCESS initial_action %]
-      <input type="radio" id="knob-resolve" name="knob" value="resolve">
-      <label for="knob-resolve">
-        Resolve [% terms.bug %], changing 
-        <a href="page.cgi?id=fields.html#resolution">resolution</a> to
-      </label>  
-      [% PROCESS select_resolution %]
-
-      [% PROCESS duplicate %]
     [% ELSE %]
-      [% IF bug.resolution != "MOVED" ||
-           (bug.resolution == "MOVED" && bug.user.canmove) %]
+      [% IF bug.resolution != "MOVED" || bug.user.canmove %]
         [% PROCESS initial_action %]
-        <input type="radio" id="knob-change-resolution" name="knob" value="change_resolution">
-        <label for="knob-change-resolution">
+        <input type="radio" id="knob_change_resolution" name="knob" value="change_resolution">
+        <label for="knob_change_resolution">
           Change <a href="page.cgi?id=fields.html#resolution">resolution</a> to
         </label>
-        [% PROCESS select_resolution %]
-
-        [% PROCESS duplicate %]
-
-        <input type="radio" id="knob-reopen" name="knob" value="reopen">
-        <label for="knob-reopen">
-          Reopen [% terms.bug %]
-        </label>
-        <br>
-        [% knum = knum + 1 %]
-      [% END %]
-      [% IF bug.bug_status == "RESOLVED" %]
-        [% PROCESS initial_action %]
-        <input type="radio" id="knob-verify" name="knob" value="verify">
-        <label for="knob-verify">
-          Mark [% terms.bug %] as <b>[% get_status("VERIFIED") FILTER html %]</b>
-        </label>
+        [% PROCESS select_resolution field = "knob_change_resolution" %]
         <br>
-        [% knum = knum + 1 %]
-      [% END %]
-      [% IF bug.bug_status != "CLOSED" %]
-        [% PROCESS initial_action %]
-        <input type="radio" id="knob-close" name="knob" value="close">
-        <label for="knob-close">
-          Mark [% terms.bug %] as <b>[% get_status("CLOSED") FILTER html %]</b>
-        </label>
-        <br>
-        [% knum = knum + 1 %]
       [% END %]
     [% END %]
+
+    [% PROCESS duplicate %]
   [% END %]
   </div>
 
   <div id="knob-buttons">
-  <input type="submit" value="Commit" id="commit">
+    <input type="submit" value="Commit" id="commit">
     [% IF bug.user.canmove %]
-      &nbsp; <font size="+1"><b> | </b></font> &nbsp;
-      <input type="submit" name="action" id="action"
-             value="[% Param("move-button-text") %]">
+      <input type="submit" name="action" id="action" value="[% Param("move-button-text") %]">
     [% END %]
   </div>
 </div>
@@ -143,23 +102,20 @@
 [% END %]
 
 [% BLOCK select_resolution %]
-  <select name="resolution"
-          onchange="document.changeform.knob[[% knum %]].checked=true">
+  <select name="resolution_[% field FILTER html %]"
+          onchange="document.forms['changeform'].[% field FILTER html %].checked=true">
     [% FOREACH r = bug.choices.resolution %]
       <option value="[% r FILTER html %]">[% get_resolution(r) FILTER html %]</option>
     [% END %]
   </select>
-  <br>
-  [% knum = knum + 1 %]
 [% END %]
 
 [% BLOCK duplicate %]
-  <input type="radio" id="knob-duplicate" name="knob" value="duplicate">
-  <label for="knob-duplicate">
+  <input type="radio" id="knob_duplicate" name="knob" value="duplicate">
+  <label for="knob_duplicate">
     Mark the [% terms.bug %] as duplicate of [% terms.bug %] #
   </label>
   <input name="dup_id" size="6"
-         onchange="if (this.value != '') {document.changeform.knob[[% knum %]].checked=true}">
+         onchange="if (this.value != '') {document.forms['changeform'].knob_duplicate.checked=true}">
   <br>
-  [% knum = knum + 1 %]
 [% END %]
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index ed3d72503..b5ab0bf47 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -191,8 +191,7 @@
 
 'list/edit-multiple.html.tmpl' => [
   'group.id', 
-  'knum', 
-  'menuname', 
+  'menuname',
 ],
 
 'list/list.rdf.tmpl' => [
@@ -319,10 +318,6 @@
   'flag.status',
 ],
 
-'bug/knob.html.tmpl' => [
-  'knum', 
-],
-
 'bug/navigate.html.tmpl' => [
   'bug_list.first', 
   'bug_list.last', 
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 485f7c403..615499426 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -243,7 +243,13 @@
 
   [% ELSIF error == "comment_required" %]
     [% title = "Comment Required" %]
-    You have to specify a <b>comment</b> on this change.
+    You have to specify a <b>comment</b>
+    [% IF old.size && new %]
+      to change the [% terms.bug %] status from [% old.join(", ") FILTER html %]
+      to [% new FILTER html %].
+    [% ELSE %]
+      on this change.
+    [% END %]
     Please explain your change.
 
   [% ELSIF error == "comment_too_long" %]
@@ -633,7 +639,12 @@
     [% title = "Your Search Makes No Sense" %]
     The only legal values for the <em>Attachment is patch</em> field are
     0 and 1.
-         
+
+  [% ELSIF error == "illegal_bug_status_transition" %]
+    [% title = "Illegal $terms.Bug Status Change" %]
+    You are not allowed to change the [% terms.bug %] status from
+    [%+ old.join(", ") FILTER html %] to [%+ new FILTER html %].
+
   [% ELSIF error == "illegal_change" %]
     [% title = "Not allowed" %]
     You tried to change the 
diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl
index 668445995..28e513e7b 100644
--- a/template/en/default/list/edit-multiple.html.tmpl
+++ b/template/en/default/list/edit-multiple.html.tmpl
@@ -18,6 +18,7 @@
   #
   # Contributor(s): Myk Melez <myk@mozilla.org>
   #                 Max Kanat-Alexander <mkanat@bugzilla.org>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
@@ -301,66 +302,25 @@
 
 
 
-[% knum = 0 %]
 <input id="knob-none" type="radio" name="knob" value="none" checked="checked">
 <label for="knob-none">Do nothing else</label><br>
 
-[% IF bugstatuses.size == 1 && bugstatuses.0 == unconfirmedstate %]
-  [% knum = knum + 1 %]
-  <input id="knob-confirm" type="radio" name="knob" value="confirm">
-  <label for="knob-confirm">
-    Confirm [% terms.bugs %] (change status to <b>[% get_status("NEW") FILTER html %]</b>)
-  </label><br>
+[% FOREACH bug_status = new_bug_statuses %]
+    <input type="radio" id="knob_[% bug_status.id FILTER html %]" name="knob"
+           value="[% bug_status.name FILTER html %]">
+    <label for="knob_[% bug_status.id FILTER html %]">
+      Change status to <b>[% get_status(bug_status.name) FILTER html %]</b>
+    </label>
+    [% IF !bug_status.is_open %]
+      and set the resolution to [% PROCESS select_resolution field = "knob_${bug_status.id}" %]
+    [% END %]
+    <br>
 [% END %]
 
-[%# If all the bugs being changed are open, allow the user to accept them,
-    clear their resolution or resolve them. %]
-[% IF !bugstatuses.containsany(closedstates) %]
-  [% knum = knum + 1 %]
-  <input id="knob-accept" type="radio" name="knob" value="accept">
-  <label for="knob-accept">
-    Accept [% terms.bugs %] (change status to <b>[% get_status("ASSIGNED") FILTER html %]</b>)
-  </label><br>
-
-  [% knum = knum + 1 %]
+[%# If all the bugs being changed are open, allow the user to clear their resolution. %]
+[% IF !current_bug_statuses.containsany(closedstates) %]
   <input id="knob-clearresolution" type="radio" name="knob" value="clearresolution">
   <label for="knob-clearresolution">Clear the resolution</label><br>
-
-  [% knum = knum + 1 %]
-  <input id="knob-resolve" type="radio" name="knob" value="resolve">
-  <label for="knob-resolve">
-    Resolve [% terms.bugs %], changing <a href="page.cgi?id=fields.html#resolution">resolution</a> to
-  </label>
-  <select name="resolution" onchange="document.forms.changeform.knob[[% knum %]].checked=true">
-    [% FOREACH resolution = resolutions %]
-      [% NEXT IF !resolution %]
-      <option value="[% resolution FILTER html %]">
-        [% get_resolution(resolution) FILTER html %]
-      </option>
-    [% END %]
-  </select><br>
-
-[% END %]
-
-[%# If all the bugs are closed, allow the user to reopen them. %]
-[% IF !bugstatuses.containsany(openstates) %]
-  [% knum = knum + 1 %]
-  <input id="knob-reopen" type="radio" name="knob" value="reopen">
-  <label for="knob-reopen">Reopen [% terms.bugs %]</label><br>
-[% END %]
-
-[% IF bugstatuses.size == 1 %]
-  [% IF bugstatuses.contains('RESOLVED') %]
-    [% knum = knum + 1 %]
-    <input id="knob-verify" type="radio" name="knob" value="verify">
-    <label for="knob-verify">Mark [% terms.bugs %] as <b>[% get_status("VERIFIED") FILTER html %]</b></label><br>
-  [% END %]
-[% END %]
-
-[% IF !bugstatuses.containsany(openstates) AND !bugstatuses.contains('CLOSED') %]
-  [% knum = knum + 1 %]
-  <input id="knob-close" type="radio" name="knob" value="close">
-  <label for="knob-close">Mark [% terms.bugs %] as <b>[% get_status("CLOSED") FILTER html %]</b></label><br>
 [% END %]
 
 <input type="submit" id="commit" value="Commit">
@@ -384,3 +344,13 @@
     [% END %]
   </select>
 [% END %]
+
+[% BLOCK select_resolution %]
+  <select name="resolution"
+          onchange="document.forms['changeform'].[% field FILTER html %].checked=true">
+    [% FOREACH r = resolutions %]
+      [% NEXT IF !r %]
+      <option value="[% r FILTER html %]">[% get_resolution(r) FILTER html %]</option>
+    [% END %]
+  </select>
+[% END %]