Closed
Bug 500900
Opened 15 years ago
Closed 15 years ago
Confirming bugs requires NEW state to exist
Categories
(Bugzilla :: Creating/Changing Bugs, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.2
People
(Reporter: bbaetz, Assigned: LpSolit)
References
Details
Attachments
(2 files, 1 obsolete file)
6.39 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
6.14 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
CheckIfVotedConfirmed does: $dbh->do("UPDATE bugs SET bug_status = 'NEW', everconfirmed = 1, " . "delta_ts = ? WHERE bug_id = ?", undef, ($timestamp, $id)); $dbh->do("INSERT INTO bugs_activity " . "(bug_id, who, bug_when, fieldid, removed, added) " . "VALUES (?, ?, ?, ?, ?, ?)", undef, ($id, $who, $timestamp, $fieldid, 'UNCONFIRMED', 'NEW')); UNCONFIRMED isn't allowed to be changed, but NEW can be, so if someone removes/renames the NEW state, you end up with a bug in an invalid state that it can't be moved out of....
Comment 1•15 years ago
|
||
We got hit by this at http://bugs.openembedded.org. Bradley was so kind to open this report. I can confirm the problem.
Comment 2•15 years ago
|
||
They should instead go into the confirmed state that sorts the lowest in the list.
Target Milestone: --- → Bugzilla 3.2
Assignee | ||
Updated•15 years ago
|
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Flags: blocking3.4+
Flags: blocking3.2.4+
Priority: -- → P1
Assignee | ||
Comment 3•15 years ago
|
||
The user ID passed to CheckIfVotedConfirmed() is always the currently logged in user, so this parameter can go away as $bug->update() already does this assumption. I also replaced direct SQL calls by setters, so that security and other validity checks can work correctly.
Attachment #385665 -
Flags: review?(mkanat)
Comment 4•15 years ago
|
||
Comment on attachment 385665 [details] [diff] [review] patch, v1 >Index: Bugzilla/Bug.pm >+ $bug->_set_everconfirmed(1); I assume this is to catch some other case where the bug is marked in a confirmed status but doesn't have everconfirmed set? That's an anomalous case, and we should let sanitycheck deal with it, and never expect it to happen otherwise. >+ [% ELSIF error == "no_open_bug_status" %] >+ [% title = "No Open Bug Status" %] >+ There is no way to confirm [% terms.abug %] in this installation. Mmmm, let's be more specific--say that there is no valid transition from UNCONFIRMED to an open state, meaning that bugs cannot be confirmed.
Attachment #385665 -
Flags: review?(mkanat) → review-
Reporter | ||
Comment 5•15 years ago
|
||
Should this just be a param?
Comment 6•15 years ago
|
||
No, it's too small of a functionality to deserve a parameter. Maybe when voting becomes a plugin.
Assignee | ||
Comment 7•15 years ago
|
||
(In reply to comment #4) > I assume this is to catch some other case where the bug is marked in a > confirmed status but doesn't have everconfirmed set? That's an anomalous case, > and we should let sanitycheck deal with it, and never expect it to happen > otherwise. No, this is for bugs which are in a closed state. If a bug is e.g. RESOLVED WONTFIX, we only set everconfirmed = 1, but leave the bug as RESOLVED. So this has nothing to do with an anomalous case. Max, was that the reason for your r-? If yes, please reconsider.
Comment 8•15 years ago
|
||
Ah, okay. Add a comment about that and change the error message and it'll be r+. I think it's OK to call _set_everconfirmed, because we are inside Bugzilla::Bug, after all. Eventually we should make it $bug->confirm_if_vote_confirmed.
Assignee | ||
Comment 9•15 years ago
|
||
Comment added + fixed the error message. code-error now calls field-descs.none.tmpl, as user-error does.
Attachment #385665 -
Attachment is obsolete: true
Attachment #386166 -
Flags: review?(mkanat)
Comment 10•15 years ago
|
||
Comment on attachment 386166 [details] [diff] [review] patch, v2 Looks good to me.
Attachment #386166 -
Flags: review?(mkanat) → review+
Updated•15 years ago
|
Flags: approval3.4+
Flags: approval3.2+
Flags: approval+
Assignee | ||
Comment 11•15 years ago
|
||
Backport for 3.2. CheckIfVotedConfirmed() is still in editproducts.cgi instead of Product.pm.
Attachment #386173 -
Flags: review?(mkanat)
Updated•15 years ago
|
Attachment #386173 -
Flags: review?(mkanat) → review+
Assignee | ||
Comment 12•15 years ago
|
||
tip: Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.418; previous revision: 1.417 done Checking in votes.cgi; /cvsroot/mozilla/webtools/bugzilla/votes.cgi,v <-- votes.cgi new revision: 1.58; previous revision: 1.57 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.281; previous revision: 1.280 done Checking in Bugzilla/Product.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Product.pm,v <-- Product.pm new revision: 1.38; previous revision: 1.37 done Checking in template/en/default/global/code-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v <-- code-error.html.tmpl new revision: 1.113; previous revision: 1.112 done 3.3.4: Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.417.2.1; previous revision: 1.417 done Checking in votes.cgi; /cvsroot/mozilla/webtools/bugzilla/votes.cgi,v <-- votes.cgi new revision: 1.57.2.1; previous revision: 1.57 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.276.2.5; previous revision: 1.276.2.4 done Checking in Bugzilla/Product.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Product.pm,v <-- Product.pm new revision: 1.34.2.1; previous revision: 1.34 done Checking in template/en/default/global/code-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v <-- code-error.html.tmpl new revision: 1.109.2.1; previous revision: 1.109 done 3.2.3: Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.142.2.3; previous revision: 1.142.2.2 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.410.2.3; previous revision: 1.410.2.2 done Checking in votes.cgi; /cvsroot/mozilla/webtools/bugzilla/votes.cgi,v <-- votes.cgi new revision: 1.55.2.1; previous revision: 1.55 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.241.2.19; previous revision: 1.241.2.18 done Checking in template/en/default/global/code-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v <-- code-error.html.tmpl new revision: 1.105.2.1; previous revision: 1.105 done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•