375191 - Remove ChangeStatus() and ChangeResolution() from process_bug.cgi

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Frédéric Buclin]

Attachment: patch, v1

These routines are big hacks and make customisation mostly impossible. They have to go away. What this patch does is to remove these two routines and prepare Bugzilla::Bug->get_status_and_resolution() for future customisation. Note that this is only a backend cleanup; you still cannot add new statuses. Also, existing statuses are still hardcoded as it's only a cleanup.

Max: this patch cannot be smaller; believe me. ;)

Comment 1

[Gervase Markham [:gerv]]

Comment on attachment 259511 [details] [diff] [review]
patch, v1

r=gerv, except that I have concern that the code currently in Bug.pm doesn't really belong there. I've discussed this with LpSolit, and he's going to discuss it with mkanat.

Gerv

Comment 2

[Max Kanat-Alexander]

Comment on attachment 259511 [details] [diff] [review]
patch, v1

Yeah, this looks generally fine. I'm sure it'll be modified when we get around to completing Bug->update().

Comment 3

[Olav Vitters]

Comment on attachment 259511 [details] [diff] [review]
patch, v1

Index: process_bug.cgi

@@ -1402,24 +1299,23 @@ foreach my $id (@idlist) {

             if ($qacontact) {
-                $query .= ", qa_contact = ?";
+                $query .= "$comma qa_contact = ?";

Useless change?
                 push(@bug_values, $qacontact);
             }
             else {
-                $query .= ", qa_contact = NULL";
+                $query .= "$comma qa_contact = NULL";

Same as before.


Index: Bugzilla/Bug.pm

@@ -1495,6 +1495,90 @@ sub bug_alias_to_id {

+sub get_status_and_resolution {
+    my ($bug, $action, $resolution) = @_;

I do not like that. Change $bug to $self please.

Some of the lines within this function exceed 80 cols.

+    elsif ($action =~ /^reassign(?:bycomponent)?$/) {

IMO an additional '|| $action eq ' is clearer.

+        if (!is_open_state($bug->bug_status) || $bug->bug_status eq 'UNCONFIRMED') {
+            $status = $bug->bug_status;
+        }
+        else {
+            $status = 'NEW';
+        }
+        $resolution = $bug->resolution;
+    }
+    elsif ($action eq 'duplicate') {
+        # Only alter the bug status if the bug is currently open.
+        $status = is_open_state($bug->bug_status) ? 'RESOLVED' : $bug->bug_status;

I thought the old code always changed the status back to RESOLVED?

 
Index: template/en/default/global/user-error.html.tmpl

+  [% ELSIF error == "missing_resolution" %]
+    [% title = "Resolution Required" %]
+    A valid resolution is required to mark [% terms.bugs %] as

Perhaps: "one or more bugs"

Comment 4

[Frédéric Buclin]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.357; previous revision: 1.356
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.177; previous revision: 1.176
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.208; previous revision: 1.207
done