Bug 377485: Implement editworkflow.cgi - Patch by Frédéric Buclin <LpSolit@gmail.com> r=gerv r=mkanat a=mkanat

9 files changed, 489 insertions, 2 deletions

diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 844f0b0e8..e9205ba0b 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -583,6 +583,7 @@ use constant ABSTRACT_SCHEMA => {
             sortkey  => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0},
             isactive => {TYPE => 'BOOLEAN', NOTNULL => 1, 
                          DEFAULT => 'TRUE'},
+            is_open  => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'},
         ],
         INDEXES => [
             bug_status_value_idx  => {FIELDS => ['value'],
@@ -671,6 +672,19 @@ use constant ABSTRACT_SCHEMA => {
         ],
     },
 
+    status_workflow => {
+        FIELDS => [
+            # On bug creation, there is no old value.
+            old_status      => {TYPE => 'INT2'},
+            new_status      => {TYPE => 'INT2', NOTNULL => 1},
+            require_comment => {TYPE => 'INT1', NOTNULL => 1, DEFAULT => 0},
+        ],
+        INDEXES => [
+            status_workflow_idx  => {FIELDS => ['old_status', 'new_status'],
+                                     TYPE => 'UNIQUE'},
+        ],
+    },
+
     # USER INFO
     # ---------
 
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 24a885118..f1a148353 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -506,6 +506,9 @@ sub update_table_definitions {
     _fix_uppercase_custom_field_names();
     _fix_uppercase_index_names();
 
+    # 2007-05-17 LpSolit@gmail.com - Bug 344965
+    _initialize_workflow();
+
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################
@@ -2775,6 +2778,90 @@ sub _fix_uppercase_index_names {
     }
 }
 
+sub _initialize_workflow {
+    my $dbh = Bugzilla->dbh;
+
+    if (!$dbh->bz_column_info('bug_status', 'is_open')) {
+        $dbh->bz_add_column('bug_status', 'is_open',
+                            {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'});
+
+        # Till now, bug statuses were not customizable. Nevertheless, local
+        # changes are possible and so we will try to respect these changes.
+        # This means: get the status of bugs having a resolution different from ''
+        # and mark these statuses as 'closed', even if some of these statuses are
+        # expected to be open statuses. Bug statuses we have no information about
+        # are left as 'open'.
+        my @statuses =
+          @{$dbh->selectcol_arrayref('SELECT DISTINCT bug_status FROM bugs
+                                      WHERE resolution != ?', undef, '')};
+
+        # Append the default list of closed statuses. Duplicated statuses don't hurt.
+        @statuses = map {$dbh->quote($_)} (@statuses, qw(RESOLVED VERIFIED CLOSED));
+
+        print "Marking closed bug statuses as such...\n";
+        $dbh->do('UPDATE bug_status SET is_open = 0 WHERE value IN (' .
+                  join(', ', @statuses) . ')');
+    }
+
+    # Populate the status_workflow table. We do nothing if the table already
+    # has entries. If all bug status transitions have been deleted, the
+    # workflow will be restored to its default schema.
+    my $count = $dbh->selectrow_array('SELECT COUNT(*) FROM status_workflow');
+    return if $count;
+
+    my $create = Bugzilla->params->{'commentoncreate'};
+    my $confirm = Bugzilla->params->{'commentonconfirm'};
+    my $accept = Bugzilla->params->{'commentonaccept'};
+    my $resolve = Bugzilla->params->{'commentonresolve'};
+    my $verify = Bugzilla->params->{'commentonverify'};
+    my $close = Bugzilla->params->{'commentonclose'};
+    my $reopen = Bugzilla->params->{'commentonreopen'};
+    # This was till recently the only way to get back to NEW for
+    # confirmed bugs, so we use this parameter here.
+    my $reassign = Bugzilla->params->{'commentonreassign'};
+
+    # This is the default workflow.
+    my @workflow = ([undef, 'UNCONFIRMED', $create],
+                    [undef, 'NEW', $create],
+                    [undef, 'ASSIGNED', $create],
+                    ['UNCONFIRMED', 'NEW', $confirm],
+                    ['UNCONFIRMED', 'ASSIGNED', $accept],
+                    ['UNCONFIRMED', 'RESOLVED', $resolve],
+                    ['NEW', 'ASSIGNED', $accept],
+                    ['NEW', 'RESOLVED', $resolve],
+                    ['ASSIGNED', 'NEW', $reassign],
+                    ['ASSIGNED', 'RESOLVED', $resolve],
+                    ['REOPENED', 'NEW', $reassign],
+                    ['REOPENED', 'ASSIGNED', $accept],
+                    ['REOPENED', 'RESOLVED', $resolve],
+                    ['RESOLVED', 'UNCONFIRMED', $reopen],
+                    ['RESOLVED', 'REOPENED', $reopen],
+                    ['RESOLVED', 'VERIFIED', $verify],
+                    ['RESOLVED', 'CLOSED', $close],
+                    ['VERIFIED', 'UNCONFIRMED', $reopen],
+                    ['VERIFIED', 'REOPENED', $reopen],
+                    ['VERIFIED', 'CLOSED', $close],
+                    ['CLOSED', 'UNCONFIRMED', $reopen],
+                    ['CLOSED', 'REOPENED', $reopen]);
+
+    print "Now filling the 'status_workflow' table with valid bug status transitions...\n";
+    my $sth_select = $dbh->prepare('SELECT id FROM bug_status WHERE value = ?');
+    my $sth = $dbh->prepare('INSERT INTO status_workflow (old_status, new_status,
+                                         require_comment) VALUES (?, ?, ?)');
+
+    foreach my $transition (@workflow) {
+        my ($from, $to);
+        # If it's an initial state, there is no "old" value.
+        $from = $dbh->selectrow_array($sth_select, undef, $transition->[0])
+          if $transition->[0];
+        $to = $dbh->selectrow_array($sth_select, undef, $transition->[1]);
+        # If one of the bug statuses doesn't exist, the transition is invalid.
+        next if (($transition->[0] && !$from) || !$to);
+
+        $sth->execute($from, $to, $transition->[2] ? 1 : 0);
+    }
+}
+
 1;
 
 __END__
diff --git a/editworkflow.cgi b/editworkflow.cgi
new file mode 100644
index 000000000..9a369c974
--- /dev/null
+++ b/editworkflow.cgi
@@ -0,0 +1,147 @@
+#!/usr/bin/perl -wT
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Frédéric Buclin.
+# Portions created by Frédéric Buclin are Copyright (C) 2007
+# Frédéric Buclin. All Rights Reserved.
+#
+# Contributor(s): Frédéric Buclin <LpSolit@gmail.com>
+
+use strict;
+
+use lib qw(.);
+
+use Bugzilla;
+use Bugzilla::Constants;
+use Bugzilla::Error;
+use Bugzilla::Token;
+
+my $cgi = Bugzilla->cgi;
+my $dbh = Bugzilla->dbh;
+my $user = Bugzilla->login(LOGIN_REQUIRED);
+
+print $cgi->header();
+
+$user->in_group('admin')
+  || ThrowUserError('auth_failure', {group  => 'admin',
+                                     action => 'modify',
+                                     object => 'workflow'});
+
+my $action = $cgi->param('action') || 'edit';
+my $token = $cgi->param('token');
+
+sub get_statuses {
+    my $statuses = $dbh->selectall_arrayref('SELECT id, value, is_open FROM bug_status
+                                             ORDER BY sortkey, value', { Slice => {} });
+    return $statuses;
+}
+
+sub get_workflow {
+    my $workflow = $dbh->selectall_arrayref('SELECT old_status, new_status, require_comment
+                                             FROM status_workflow');
+    my %workflow;
+    foreach my $row (@$workflow) {
+        my ($old, $new, $type) = @$row;
+        $workflow{$old || 0}{$new} = $type;
+    }
+    return \%workflow;
+}
+
+sub load_template {
+    my ($filename, $message) = @_;
+    my $template = Bugzilla->template;
+    my $vars = {};
+
+    $vars->{'statuses'} = get_statuses();
+    $vars->{'workflow'} = get_workflow();
+    $vars->{'token'} = issue_session_token("workflow_$filename");
+    $vars->{'message'} = $message;
+
+    $template->process("admin/workflow/$filename.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
+    exit;
+}
+
+if ($action eq 'edit') {
+    load_template('edit');
+}
+elsif ($action eq 'update') {
+    check_token_data($token, 'workflow_edit');
+    my $statuses = get_statuses;
+    my $workflow = get_workflow();
+    my $initial_state = {id => 0};
+
+    my $sth_insert = $dbh->prepare('INSERT INTO status_workflow (old_status, new_status)
+                                    VALUES (?, ?)');
+    my $sth_delete = $dbh->prepare('DELETE FROM status_workflow
+                                    WHERE old_status = ? AND new_status = ?');
+    my $sth_delnul = $dbh->prepare('DELETE FROM status_workflow
+                                    WHERE old_status IS NULL AND new_status = ?');
+
+    foreach my $old ($initial_state, @$statuses) {
+        # Hashes cannot have undef as a key, so we use 0. But the DB
+        # must store undef, for referential integrity.
+        my $old_id_for_db = $old->{'id'} || undef;
+        foreach my $new (@$statuses) {
+            next if $old->{'id'} == $new->{'id'};
+
+            if ($cgi->param('w_' . $old->{'id'} . '_' . $new->{'id'})) {
+                $sth_insert->execute($old_id_for_db, $new->{'id'})
+                  unless defined $workflow->{$old->{'id'}}->{$new->{'id'}};
+            }
+            elsif ($old_id_for_db) {
+                $sth_delete->execute($old_id_for_db, $new->{'id'});
+            }
+            else {
+                $sth_delnul->execute($new->{'id'});
+            }
+        }
+    }
+    delete_token($token);
+    load_template('edit', 'workflow_updated');
+}
+elsif ($action eq 'edit_comment') {
+    load_template('comment');
+}
+elsif ($action eq 'update_comment') {
+    check_token_data($token, 'workflow_comment');
+    my $workflow = get_workflow();
+
+    my $sth_update = $dbh->prepare('UPDATE status_workflow SET require_comment = ?
+                                    WHERE old_status = ? AND new_status = ?');
+    my $sth_updnul = $dbh->prepare('UPDATE status_workflow SET require_comment = ?
+                                    WHERE old_status IS NULL AND new_status = ?');
+
+    foreach my $old (keys %$workflow) {
+        # Hashes cannot have undef as a key, so we use 0. But the DB
+        # must store undef, for referential integrity.
+        my $old_id_for_db = $old || undef;
+        foreach my $new (keys %{$workflow->{$old}}) {
+            my $comment_required = $cgi->param("c_${old}_$new") ? 1 : 0;
+            next if ($workflow->{$old}->{$new} == $comment_required);
+            if ($old_id_for_db) {
+                $sth_update->execute($comment_required, $old_id_for_db, $new);
+            }
+            else {
+                $sth_updnul->execute($comment_required, $new);
+            }
+        }
+    }
+    delete_token($token);
+    load_template('comment', 'workflow_updated');
+}
+else {
+    ThrowCodeError("action_unrecognized", {action => $action});
+}
diff --git a/skins/standard/admin.css b/skins/standard/admin.css
index 9fcb46c94..830b39952 100644
--- a/skins/standard/admin.css
+++ b/skins/standard/admin.css
@@ -65,3 +65,41 @@ td.admin_links dt.forbidden a, td.admin_links dd.forbidden a {
     color: inherit;
     cursor: default;
 }
+
+.col-header {
+    width: 8em;
+}
+
+.checkbox-cell {
+    border: 1px black solid;
+}
+
+/* Grey-green color */
+.open-status {
+    color: #286;
+}
+
+/* Brown-red color */
+.closed-status {
+    color: #a63;
+}
+
+/* Dark green color */
+.checked {
+    background-color: #5b4;
+}
+
+/* Dark red color */
+td.forbidden {
+    background-color: #811;
+}
+
+tr.highlight:hover {
+    background-color: yellow;
+}
+
+th.title {
+    font-size: larger;
+    text-align: center;
+    vertical-align: middle;
+}
diff --git a/template/en/default/admin/workflow/comment.html.tmpl b/template/en/default/admin/workflow/comment.html.tmpl
new file mode 100644
index 000000000..5e9a788d6
--- /dev/null
+++ b/template/en/default/admin/workflow/comment.html.tmpl
@@ -0,0 +1,93 @@
+[%# 1.0@bugzilla.org %]
+[%# The contents of this file are subject to the Mozilla Public
+  # License Version 1.1 (the "License"); you may not use this file
+  # except in compliance with the License. You may obtain a copy of
+  # the License at http://www.mozilla.org/MPL/
+  #
+  # Software distributed under the License is distributed on an "AS
+  # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+  # implied. See the License for the specific language governing
+  # rights and limitations under the License.
+  #
+  # The Original Code is the Bugzilla Bug Tracking System.
+  #
+  # Contributor(s): Frédéric Buclin <LpSolit@gmail.com>
+  #                 Gervase Markham <gerv@mozilla.org>
+  #%]
+
+[% PROCESS "global/field-descs.none.tmpl" %]
+
+[% INCLUDE global/header.html.tmpl
+   title = "Edit Actions Triggered by the Workflow"
+   style_urls = ['skins/standard/admin.css']
+%]
+
+<script type="text/javascript">
+<!--
+  function toggle_cell(cell) {
+    if (cell.checked)
+      cell.parentNode.className = "checkbox-cell checked";
+    else
+      cell.parentNode.className = "checkbox-cell";
+  }
+//-->
+</script>
+
+<p>
+  This page allows you to define which status transitions require a comment
+  by the user doing the change.
+</p>
+
+<form id="workflow_form" method="POST" action="editworkflow.cgi">
+<table>
+  <tr>
+    <th colspan="2">&nbsp;</th>
+    <th colspan="[% statuses.size FILTER html %]" class="title">To</th>
+  </tr>
+
+  <tr>
+    <th rowspan="[% statuses.size + 2 FILTER html %]" class="title">From</th>
+    <th>&nbsp;</th>
+    [% FOREACH status = statuses %]
+      <th class="col-header[% status.is_open ? " open-status" : " closed-status" %]">
+        [% status.value FILTER html %]
+      </th>
+    [% END %]
+  </tr>
+
+  [%# This defines the entry point in the workflow %]
+  [% p = [{id => 0, value => "{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 %]
+      </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 %]">
+            <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} %]>
+          </td>
+        [% ELSE %]
+          <td class="checkbox-cell forbidden">&nbsp;</td>
+        [% END %]
+      [% END %]
+    </tr>
+  [% END %]
+</table>
+
+<p align="center">
+  <input type="hidden" name="action" value="update_comment">
+  <input type="hidden" name="token" value="[% token FILTER html %]">
+  <input type="submit" value="Commit Changes"> -
+  <a href="editworkflow.cgi?action=edit_comment">Cancel Changes</a> -
+  <a href="editworkflow.cgi">View Current Workflow</a>
+</p>
+
+</form>
+
+[% INCLUDE global/footer.html.tmpl %]
diff --git a/template/en/default/admin/workflow/edit.html.tmpl b/template/en/default/admin/workflow/edit.html.tmpl
new file mode 100644
index 000000000..dee71c0a1
--- /dev/null
+++ b/template/en/default/admin/workflow/edit.html.tmpl
@@ -0,0 +1,93 @@
+[%# 1.0@bugzilla.org %]
+[%# The contents of this file are subject to the Mozilla Public
+  # License Version 1.1 (the "License"); you may not use this file
+  # except in compliance with the License. You may obtain a copy of
+  # the License at http://www.mozilla.org/MPL/
+  #
+  # Software distributed under the License is distributed on an "AS
+  # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+  # implied. See the License for the specific language governing
+  # rights and limitations under the License.
+  #
+  # The Original Code is the Bugzilla Bug Tracking System.
+  #
+  # Contributor(s): Frédéric Buclin <LpSolit@gmail.com>
+  #                 Gervase Markham <gerv@mozilla.org>
+  #%]
+
+[% PROCESS "global/field-descs.none.tmpl" %]
+
+[% INCLUDE global/header.html.tmpl
+   title = "Edit Workflow"
+   style_urls = ['skins/standard/admin.css']
+%]
+
+<script type="text/javascript">
+<!--
+  function toggle_cell(cell) {
+    if (cell.checked)
+      cell.parentNode.className = "checkbox-cell checked";
+    else
+      cell.parentNode.className = "checkbox-cell";
+  }
+//-->
+</script>
+
+<p>
+  This page allows you to define which status transitions are valid
+  in your workflow.
+</p>
+
+<form id="workflow_form" method="POST" action="editworkflow.cgi">
+<table>
+  <tr>
+    <th colspan="2">&nbsp;</th>
+    <th colspan="[% statuses.size FILTER html %]" class="title">To</th>
+  </tr>
+
+  <tr>
+    <th rowspan="[% statuses.size + 2 FILTER html %]" class="title">From</th>
+    <th>&nbsp;</th>
+    [% FOREACH status = statuses %]
+      <th class="col-header[% status.is_open ? " open-status" : " closed-status" %]">
+        [% status.value FILTER html %]
+      </th>
+    [% END %]
+  </tr>
+
+  [%# This defines the entry point in the workflow %]
+  [% p = [{id => 0, value => "{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 %]
+      </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 %]">
+            <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 %]>
+          </td>
+        [% ELSE %]
+          <td class="checkbox-cell forbidden">&nbsp;</td>
+        [% END %]
+      [% END %]
+    </tr>
+  [% END %]
+</table>
+
+<p align="center">
+  <input type="hidden" name="action" value="update">
+  <input type="hidden" name="token" value="[% token FILTER html %]">
+  <input type="submit" value="Commit Changes"> -
+  <a href="editworkflow.cgi">Cancel Changes</a> -
+  <a href="editworkflow.cgi?action=edit_comment">View Current Triggers</a>
+</p>
+
+</form>
+
+[% INCLUDE global/footer.html.tmpl %]
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index ac579115b..ed3d72503 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -543,6 +543,16 @@
   'comp.bug_count'
 ],
 
+'admin/workflow/edit.html.tmpl' => [
+  'status.id',
+  'new_status.id',
+],
+
+'admin/workflow/comment.html.tmpl' => [
+  'status.id',
+  'new_status.id',
+],
+
 'account/login.html.tmpl' => [
   'target', 
 ],
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 11fe0733c..3673a8d6e 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -491,7 +491,10 @@
     You entered a username that matched more than one
     user, so we have instead left the [% match_field FILTER html %]
     field blank.
-    
+
+  [% ELSIF message_tag == "workflow_updated" %]
+    The workflow has been updated.
+
   [% ELSE %]
     [%# Give sensible error if error functions are used incorrectly.
       #%]        
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 4fa138206..485f7c403 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -189,7 +189,7 @@
     [% ELSIF object == "settings" %]
       settings
     [% ELSIF object == "sudo_session" %]
-      an sudo session
+      a sudo session
     [% ELSIF object == "timetracking_summaries" %]
       time-tracking summary reports
     [% ELSIF object == "user" %]
@@ -198,6 +198,8 @@
       users
     [% ELSIF object == "versions" %]
       versions
+    [% ELSIF object == "workflow" %]
+      the workflow
     [% END %].
 
     [% Hook.process("auth_failure") %]