92515 - Should be able to change resolution without re-opening...

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Stephen Rasku]

Quite often, users will use the wrong resolution when resolving a bug.  For
example, using (FIXED instead of INVALID or WORKSFORME).  In order to change the
resolution, I need to re-open and and re-resolve.  This generates two e-mails
for nitpicking so I don't usually do it.  

It should be possible to change the resolution without having to reopen.
Possibly there could be a permission which controlled who had this privilege.

Comment 1

[Dave Miller [:justdave]]

I've often wished for this myself. :-)

Comment 2

[Matthew Tuck [:CodeMachine]]

If we do this, then I'd say that anyone who can do it in two steps should be
able to do it in one.

Comment 3

[Stephen Rasku]

OK.  Fair enough.

Comment 4

[Matthew Tuck [:CodeMachine]]

I'm not sure about this request, but it may just be inertia - I can't think of
any reason not to do it.

Currently there's a field called "Priority, status, severity, and milestone
changes" on e-mail prefs.  If we did this we'd need to change this to include
resolutions.

Comment 5

[Dave Miller [:justdave]]

I'd just change the wording on the existing option.  For purposes of 
notifications a resolution change should be considered a status change.

Comment 6

[Diego Biurrun]

See also bug 87840, which is about changing the duplication target, so it's a
special case of this bug.

Comment 7

[Andreas Franke (gone)]

-> Bugzilla product, Changing Bugs component, reassigning.

Comment 8

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 9

[Matthew Tuck [:CodeMachine]]

*** Bug 136631 has been marked as a duplicate of this bug. ***

Comment 10

[Dave Miller [:justdave]]

*** Bug 179104 has been marked as a duplicate of this bug. ***

Comment 11

[Dave Miller [:justdave]]

*** Bug 187640 has been marked as a duplicate of this bug. ***

Comment 12

[Dave Miller [:justdave]]

Enhancements which don't currently have patches on them which are targetted at
2.18 are being retargetted to 2.20 because we're about to freeze for 2.18. 
Consideration will be taken for moving items back to 2.18 on a case-by-case
basis (but is unlikely for enhancements)

Comment 13

[Dave Miller [:justdave]]

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.

Comment 14

[Olav Vitters]

*** Bug 295724 has been marked as a duplicate of this bug. ***

Comment 15

[Olav Vitters]

*** Bug 285540 has been marked as a duplicate of this bug. ***

Comment 16

[Frédéric Buclin]

I'll take it as soon as we unfreeze if nobody else wants to. I want it in 2.24.

Comment 17

[Frédéric Buclin]

Attachment: patch, v1

Comment 18

[Teemu Mannermaa (:wicked)]

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

This block from CheckCanChangeField() should be dealt with too:
    # A resolution change is always accompanied by a status change. So, we
    # always OK resolution changes; if they really can't do this, we will
    # notice it when status is checked.
    if ($field eq "resolution") {
        return 1;
    }

>Index: process_bug.cgi
>===================================================================
>-    /^resolve$/ && CheckonComment( "resolve" ) && do {
>+    /^(resolve|change_resolution)$/ && CheckonComment( "resolve" ) && do {

Shouldn't use commentonresolve to mandate a comment when change_resolution is done because we are not actually resolving the bug anymore. Maybe add a commentonchangeresolution parameter?

>-        # Check here, because its the only place we require the resolution
>         check_field('resolution', scalar $cgi->param('resolution'),
>                     \@::settable_resolution);

Nit: Why remove the comment? Isn't it still valid?

>-        # Make sure the bug is not already marked as a dupe
>-        # (may appear in race condition)
>-        my $dupe_of =
>-            $dbh->selectrow_array("SELECT dupe_of FROM duplicates
>-                                   WHERE dupe = ?",
>-                                   undef, $cgi->param('id'));
>-        if ($dupe_of) {
>-            ThrowUserError("dupe_entry_found", { dupe_of => $dupe_of });
>-        }
>+        # If the bug was already marked as a duplicate, remove
>+        # the existing entry.
>+        $dbh->do('DELETE FROM duplicates WHERE dupe = ?',
>+                  undef, $cgi->param('id'));

This change is alarming. Bug state shouldn't be changed at this stage because a) it won't be logged and b) the script could still end with an error.

Comment 19

[Frédéric Buclin]

Attachment: patch, v2

Comment 20

[Teemu Mannermaa (:wicked)]

Comment on attachment 215053 [details] [diff] [review]
patch, v2

>Index: process_bug.cgi
>===================================================================
>-    /^resolve$/ && CheckonComment( "resolve" ) && do {
>+    /^(resolve|change_resolution)$/ && CheckonComment( "resolve" ) && do {

Nit: I still think a separate param would be better.

Looks like dupe_of changes are not logged in the activity log nor bugmailed about. This seems to happen even before this patch so I won't hold review for that. Although, one would have seen the change to REOPEN and then to RESOLVED DUPLICATE again previously. Resolution changes themself are logged and bugmailed about correctly.

There will be multiple duplicate notations but that's a different problem too. We should really stop doing these comments and simply show duplication states in the bug info.

Comment 21

[Frédéric Buclin]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.310; previous revision: 1.309
done
Checking in template/en/default/bug/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/knob.html.tmpl,v  <--  knob.html.tmpl
new revision: 1.20; previous revision: 1.19
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.162; previous revision: 1.161
done

Comment 22

[Max Kanat-Alexander]

Added to relnotes in bug 349423. Please let me know if I missed any critical information about this bug in the release notes.

Comment 23

[Frédéric Buclin]

Attachment: selenium test, v1

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.0/
modified t/test_bug_edit.t
Committed revision 175.

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/3.6/
modified t/test_bug_edit.t
Committed revision 146.