344964 - Workflow Centralization: initial bug statuses in enter_bug.cgi

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[A. Karl Kornel]

enter_bug.cgi and post_bug.cgi both check to see what bug states are available for a new bug.  The code that does the checking should be implemented in Bugzilla::Workflow->new_bug_statuses().

Comment 1

[Max Kanat-Alexander]

Seems better to have a Bugzilla::Status object than Bugzilla::Workflow. The real "object" is a status and how that status relates to other statuses as a whole--the workflow is not a single object.

You could also create a get_status like we have get_fields in Bugzilla.pm.

Comment 2

[A. Karl Kornel]

(In reply to comment #1)
> Seems better to have a Bugzilla::Status object than Bugzilla::Workflow. The
> real "object" is a status and how that status relates to other statuses as a
> whole--the workflow is not a single object.
> 

That's an interesting idea.  I don't know how well that would work with multiple workflows, though.  If you have multiple workflows, each workflow is going to have a different idea of which statuses are valid as the initial status for a bug.

Comment 3

[Max Kanat-Alexander]

That's fine. You just have an new_bug boolean column in the bug_status table to represent that that status can be put right onto a new bug. Everything can be represented in the DB.

You can also have a table for bug_status_relation, which shows which statuses each status can go to and from.

Comment 4

[Max Kanat-Alexander]

Looks like Firefox did some strange selecting of the group radio boxes on the mass-change page when I clicked refresh in my browser before that last mass-change. This is not really a security bug. :-)

Comment 5

[Frédéric Buclin]

This bug is retargetted to Bugzilla 3.2 for one of the following reasons:

- it has no assignee (except the default one)
- we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1)
- it's not a blocker

If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0.

If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug.

If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".

Comment 6

[Frédéric Buclin]

post_bug.cgi no longer look at bug statuses. This code has already been moved into Bug.pm.

Comment 7

[Frédéric Buclin]

Attachment: patch, v1

This patch is based and depends on bug 344965. So you need the patch from bug 344965 to be applied first.

Comment 8

[Frédéric Buclin]

Attachment: patch, v2

Comment 9

[Gervase Markham [:gerv]]

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

>+my $initial_statuses = Bugzilla::Status->can_change_to();
>+# Exclude closed states from the UI, even if the workflow allows them.
>+# The back-end code will still accept them, though.
>+# XXX - We do this for now so that we don't have to add a resolution field.

I think this is entirely reasonable. We don't have to support opening a bug and closing it again in a single action - it's so rare. I've never even seen anyone do it. So remove the XXX line and just let the code stand as it is.

>+@$initial_statuses = grep { $_->is_open } @$initial_statuses;
>+
>+my @status = map { $_->name } @$initial_statuses;
>+# UNCONFIRMED is illegal if votes_to_confirm = 0.
>+@status = grep {$_ ne 'UNCONFIRMED'} @status unless $product->votes_to_confirm;

<sigh> We really don't want special code for certain states. But I guess eliminating the votes_to_confirm stuff is a separate bug.

>Index: Bugzilla/Bug.pm
>===================================================================
>-        elsif (!$product->votes_to_confirm) {
>-            # Without privs, products that don't support UNCONFIRMED default 
>-            # to NEW.
>-            $status = 'NEW';
>+           # If the user with privs hasn't selected another status,
>+           # select the first one of the list.
>+           $status ||= $valid_statuses[0];
>         }
>         else {
>-            $status = 'UNCONFIRMED';
>+            # A user with no privs cannot choose the initial status.
>+            $status = $valid_statuses[0];

Isn't this duplicating the function of code in enter_bug.cgi?

If you can persuade me the code above doesn't need factoring out, r=gerv.

Gerv

Comment 10

[Frédéric Buclin]

(In reply to comment #9)
> Isn't this duplicating the function of code in enter_bug.cgi?

It's very similar but slightly different as this one has to deal with power users not providing any bug status, while the code in enter_bug.cgi doesn't need to handle this case as it accepts no input from the user yet.

Comment 11

[Frédéric Buclin]

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.157; previous revision: 1.156
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.182; previous revision: 1.181
done
Checking in Bugzilla/Status.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Status.pm,v  <--  Status.pm
new revision: 1.2; previous revision: 1.1
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.213; previous revision: 1.212
done