The default bug view has changed. See this FAQ.

Confirming bugs requires NEW state to exist

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Creating/Changing Bugs
P1
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: bbaetz, Assigned: Frédéric Buclin)

Tracking

Bugzilla 3.2
Bug Flags:
approval +
approval3.4 +
blocking3.4 +
approval3.2 +
blocking3.2.4 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

6.39 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
6.14 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
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

8 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

8 years ago
They should instead go into the confirmed state that sorts the lowest in the list.
Target Milestone: --- → Bugzilla 3.2
(Assignee)

Updated

8 years ago
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Flags: blocking3.4+
Flags: blocking3.2.4+
Priority: -- → P1
(Assignee)

Comment 3

8 years ago
Created attachment 385665 [details] [diff] [review]
patch, v1

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

8 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

8 years ago
Should this just be a param?

Comment 6

8 years ago
  No, it's too small of a functionality to deserve a parameter. Maybe when voting becomes a plugin.
(Assignee)

Comment 7

8 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

8 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

8 years ago
Created attachment 386166 [details] [diff] [review]
patch, v2

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

8 years ago
Comment on attachment 386166 [details] [diff] [review]
patch, v2

Looks good to me.
Attachment #386166 - Flags: review?(mkanat) → review+

Updated

8 years ago
Flags: approval3.4+
Flags: approval3.2+
Flags: approval+
(Assignee)

Comment 11

8 years ago
Created attachment 386173 [details] [diff] [review]
patch for 3.2, v1

Backport for 3.2. CheckIfVotedConfirmed() is still in editproducts.cgi instead of Product.pm.
Attachment #386173 - Flags: review?(mkanat)

Updated

8 years ago
Attachment #386173 - Flags: review?(mkanat) → review+
(Assignee)

Comment 12

8 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
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Blocks: 502682
You need to log in before you can comment on or make changes to this bug.