Closed Bug 452793 Opened 11 years ago Closed 11 years ago

On bug creation, the first confirmed bug status should be selected by default for users with canconfirm privs, even if UNCONFIRMED is the bug status with the lowest sortkey

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: polidobj, Assigned: LpSolit)

References

Details

Attachments

(2 files, 2 obsolete files)

I'm not sure if this change was intended.  The sort order for status in bug lists was 
UNCO, NEW, ASSI, RESO. 
But now it's
NEW, UNCO, ASSI, RESO. 
The old order made more sense to me as that was the order a bug usually goes through in it's life cycle.  

I wonder if this change may be because you may want to review new bugs before triaging unconfirmed bugs on the bugs filed today bug list.  If that's the case then assigned bugs should also come before unconfirmed bugs.
Duplicate of this bug: 452812
Status: UNCONFIRMED → NEW
Ever confirmed: true
This was on purpose because of the first item in the list being picked as the default on the new bug form for people who have permission to set it, and not wanting their bugs to show up as UNCONFIRMED by default.  However, as noted on the dupe, this just got reverted (sorta fixing your original complaint) because it seemed to have set the default for people without privs, too, which we don't want.  Not entirely sure, but either way, we need to find a better way to set the default status for people with canconfirm privs on the new bug form.
Summary: Sort order for bug status changed → Sort order for bug status changed / bad defaults for people with canconfirm
(In reply to comment #2)
> [...] we need to find a better way to set
> the default status for people with canconfirm privs on the new bug form.

Maybe a preference, or then a "same as last time" status default?
I thought I remembered initial state being a distinct preference that I had set at one point, but I certainly can't find the preference any more.  Not sure if I'm just crazy or if it was a perhaps a bmo customization that was lost in the upgrade.
(In reply to comment #2)
> This was on purpose because of the first item in the list being picked as the
> default on the new bug form for people who have permission to set it, and not
> wanting their bugs to show up as UNCONFIRMED by default.  However, as noted on
> the dupe, this just got reverted (sorta fixing your original complaint) because
> it seemed to have set the default for people without privs, too, which we don't
> want.  Not entirely sure, but either way, we need to find a better way to set
> the default status for people with canconfirm privs on the new bug form.

Is this about bug 452799? Does this mean that this bug is being blocked by bug 452799?
> This was on purpose because of the first item in the list being picked as
> the default

I don't understand this reason.  The normal solution to that kind of problem is to use the SELECTED attribute.
(In reply to comment #6)
> I don't understand this reason.  The normal solution to that kind of problem is
> to use the SELECTED attribute.

Which selected attribute? There is none when you file a bug. Bugzilla cannot pre-select a bug status for you. Some users prefer to file bugs as NEW (I suppose most users with privs) and some as UNCO (one well-known guy comes to mind). We could configure Bugzilla to select the first non-UNCONFIRMED bug status. This wouldn't that hard to do, and non-invasive.
(In reply to comment #7)
> We could configure Bugzilla to select the first non-UNCONFIRMED bug
> status. This wouldn't that hard to do, and non-invasive.

  I think that's the best solution, and we should do it upstream.
Assignee: nobody → create-and-change
Component: Bugzilla: Other b.m.o Issues → Creating/Changing Bugs
OS: Windows XP → All
Product: mozilla.org → Bugzilla
QA Contact: other-bmo-issues → default-qa
Hardware: PC → All
Target Milestone: --- → Bugzilla 3.2
Version: other → 3.2
OK, taking.
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attached patch patch, v1Splinter Review
Attachment #336143 - Flags: review?(mkanat)
Attachment #336143 - Flags: review?(justdave)
Attachment #336143 - Flags: review?(justdave) → review+
Attachment #336143 - Flags: review?(mkanat)
Flags: approval3.2+
Flags: approval+
Summary: Sort order for bug status changed / bad defaults for people with canconfirm → The first confirmed bug status should be selected by default for users with canconfirm privs, even if UNCONFIRMED is the bug status with the lowest sortkey
tip:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.162; previous revision: 1.161
done

3.2rc1:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.160.2.1; previous revision: 1.160
done
Summary: The first confirmed bug status should be selected by default for users with canconfirm privs, even if UNCONFIRMED is the bug status with the lowest sortkey → On bug creation, the first confirmed bug status should be selected by default for users with canconfirm privs, even if UNCONFIRMED is the bug status with the lowest sortkey
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to comment #4)
> I thought I remembered initial state being a distinct preference that I had set
> at one point, but I certainly can't find the preference any more.  Not sure if

I do remember setting such a preference too...

> I'm just crazy or if it was a perhaps a bmo customization that was lost in the
> upgrade.
This is in production on bmo since yesterday, and we're still getting people with canconfirm privs unintentionally filing bugs as UNCONFIRMED, especially when using custom forms which don't provide an initial status option on the form, so whatever we did here didn't work.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ah, the patch here only affects the defaults fed to the bug entry template, which doesn't affect anything if the template doesn't use them (like if it doesn't have an initial status field).  Same change needs to be made to post_bug.cgi I suspect.
Attached patch Patch v1, part 2 (obsolete) — Splinter Review
Attachment #336791 - Flags: review?(LpSolit)
Attachment #336791 - Flags: review?(LpSolit) → review-
Comment on attachment 336791 [details] [diff] [review]
Patch v1, part 2

>-            $new_status ||= $valid_statuses[0];
>+            $new_status ||= ($valid_statuses[0] ne 'UNCONFIRMED') ? $valid_statuses[0] : $valid_statuses[1];

I know this is very unlikely, but you could edit the workflow to only allow UNCONFIRMED as a valid bug status for new bugs. In this case, $valid_statuses[1] doesn't exist. So you also have to make sure @valid_statuses has more than one element.

Out of curiosity, which custom form are you talking about (the one with no bug status field)??
(In reply to comment #16)
> (From update of attachment 336791 [details] [diff] [review])
> >-            $new_status ||= $valid_statuses[0];
> >+            $new_status ||= ($valid_statuses[0] ne 'UNCONFIRMED') ? $valid_statuses[0] : $valid_statuses[1];
> 
> I know this is very unlikely, but you could edit the workflow to only allow
> UNCONFIRMED as a valid bug status for new bugs. In this case,
> $valid_statuses[1] doesn't exist. So you also have to make sure
> @valid_statuses has more than one element.

In which case, your original patch for enter_bug has the same problem, because I basically copied the code from there and adjusted for variable names.

> Out of curiosity, which custom form are you talking about (the one with no bug
> status field)??

The IT Request form, the Trademark form, probably others.
(In reply to comment #17)
> In which case, your original patch for enter_bug has the same problem

No, because my patch includes |if (scalar @status == 1)|.
Attached patch Patch v1, part 2 v2 (obsolete) — Splinter Review
Ah so...  In that case, this should work a little better.
Attachment #336791 - Attachment is obsolete: true
Attachment #336873 - Flags: review?(LpSolit)
Attachment #336873 - Flags: review?(LpSolit) → review-
Comment on attachment 336873 [details] [diff] [review]
Patch v1, part 2 v2

>+            if (scalar(@valid_statuses) == 1) {
>+                $new_status ||= $valid_statuses[0];
>+            }
>+            else {
>+                $new_status ||= ($valid_statuses[0] ne 'UNCONFIRMED') ? $valid_statuses[0] : $valid_statuses[1];
>+            }

This doesn't work. @valid_statuses contains a list of bug status objects, not strings. So you cannot compare them directly to 'UNCONFIRMED'. It must be $valid_statuses[0]->name.

Also, wouldn't it make more sense to write:

  unless ($new_status) {
      if (scalar(@valid_statuses) == 1) {
          $new_status = $valid_statuses[0];
      }
      else {
          $new_status = ($valid_statuses[0]->name ne 'UNCONFIRMED') ?
                         $valid_statuses[0] : $valid_statuses[1];
      }
  }

Note that I also fixed the too long line.
/me assumes this is better
Attachment #336873 - Attachment is obsolete: true
Attachment #337901 - Flags: review?(LpSolit)
Attachment #337901 - Flags: review?(LpSolit) → review+
3.2:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.241.2.12; previous revision: 1.241.2.11
done

trunk:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.260; previous revision: 1.259
done
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.