610767 - contrib/convert-workflow.pl should add transitions from RESOLVED and VERIFIED to CONFIRMED (if transitions to REOPENED were present)

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect, P1)

Description

[Frank Becker]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_4; de-de) AppleWebKit/533.18.1 (KHTML, like Gecko) Version/5.0.2 Safari/533.18.5
Build Identifier: 4.0rc1+

If use migrate from 3.6 to 4.0 the following transitions are not included

Reproducible: Always

Comment 1

[Frank Becker]

Sorry I hit the wrong key here are the transitions

RESOLVED -> CONFIRMED
VERIFIED -> CONFIRMED

so if you do an clean install you get an other workflow.

Comment 2

[Max Kanat-Alexander]

Okay. Those transitions are actually missing from the Status Workflow table? That doesn't sound right--that would mean that the RESOLVED -> NEW and VERIFIED -> NEW transitions were missing from that table also.

Comment 3

[Frank Becker]

(In reply to comment #2)
> Okay. Those transitions are actually missing from the Status Workflow table?
> That doesn't sound right--that would mean that the RESOLVED -> NEW and VERIFIED
> -> NEW transitions were missing from that table also.

Yes I drop the database in an 3.6 version and create it with checksetup.pl.
RESOLVED -> NEW and VERIFIED -> NEW transitions are not checked

I think if someone use an unchanged 3.6 workflow the result after convert-workflow.pl should be the same to create an database with 4.0.

Comment 4

[Frédéric Buclin]

Confirmed. Note that the output of convert-workflow.pl mentions:

"Done. There are some things you may want to fix, now:

  * You may have to fix the Status Workflow using the Status Workflow
    panel in "Administration"."

So the admin shouldn't be completely surprised that he cannot reopen bugs. But I agree that the two missing transitions mentioned in comment 1 should be added, as CONFIRMED is what REOPENED has been converted into.

(I would mark this bug as a blocker if this script wasn't in contrib/.)

Comment 5

[Matt Selsky [:selsky]]

Attachment: Add missing transitions, v1

Transitions will be added from {RESOLVED,VERIFIED} -> CONFIRMED, if REOPENED was present.

Comment 6

[Matt Selsky [:selsky]]

A 3.6 -> 4.0 upgrade also contains an extra transition:
VERIFIED -> RESOLVED

The original transition of "VERIFIED -> CLOSED" from 3.6 became "VERIFIED -> CLOSED" as part of the conversion.

This transition is not part of the default workflow documented at http://www.bugzilla.org/docs/tip/en/images/bzLifecycle.png

Should this script remove that transition?

Comment 7

[Frédéric Buclin]

(In reply to Matt Selsky [:selsky] from comment #6)
> Should this script remove that transition?

No need. This transition doesn't hurt.

Comment 8

[Frédéric Buclin]

Comment on attachment 610205 [details] [diff] [review]
Add missing transitions, v1

>=== modified file 'contrib/convert-workflow.pl'

>+if ($reopened) {

Rather than looking for REOPENED itself, you should look for the transitions involving this bug status in the status_workflow table before the deletion happens. If the bug status is simply renamed, then the workflow is not affected (this is the case by default for NEW -> CONFIRMED and ASSIGNED -> IN_PROGRESS). So this is only a problem if the bug status is deleted. So your code should go into this IF block, line 94:

    # If the new status already exists, just delete the old one, but retain
    # the workflow items from it.
    if (my $existing = new Bugzilla::Status({ name => $to })) {
        $dbh->do('DELETE FROM bug_status WHERE value = ?', undef, $from);
    }

Currently, the comment is wrong as it states that the workflow is retained. So before this deletion happens, you must make all the transitions involving the old bug status point to the new one (and make sure you don't add a transition from a bug status to itself, and that you don't add duplicated entries in the status_workflow table, as this would violate the UNIQUE index in the DB). So what I'm saying is that we shouldn't just blindly add the RESOLVED/VERIFIED -> CONFIRMED transitions, but fix it for all deleted bug statuses in a more general way.

Comment 9

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #4)
> (I would mark this bug as a blocker if this script wasn't in contrib/.)

Bah, it's one of our scripts, after all.

Comment 10

[Frédéric Buclin]

I haven't seen selsky for the last few weeks, so I'm going to fix this bug myself.

Comment 11

[Frédéric Buclin]

Attachment: patch, v1

Comment 12

[Gervase Markham [:gerv]]

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

Sorry, I don't have time to properly review this and it's not obvious by inspection. :-|

Gerv

Comment 13

[David Lawrence [:dkl]]

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

Review of attachment 671795 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and works as expected. r=dkl

Comment 14

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified contrib/convert-workflow.pl
Committed revision 8447.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified contrib/convert-workflow.pl
Committed revision 8433.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified contrib/convert-workflow.pl
Committed revision 8157.