From e1b433e3d54504dceb151213d4addac42a1e5ca9 Mon Sep 17 00:00:00 2001
From: "mkanat%bugzilla.org" <>
Date: Sat, 12 Dec 2009 21:55:15 +0000
Subject: [PATCH] Bug 512606: Make statuses currently available for the user to
change this bug to be controlled by Bugzilla::Bug, not the template Patch by
Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
---
Bugzilla/Bug.pm | 80 +++++++++++-----
Bugzilla/Status.pm | 33 -------
template/en/default/bug/knob.html.tmpl | 123 +++++++------------------
3 files changed, 91 insertions(+), 145 deletions(-)
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 83e95aecd..2c3a11e8c 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1110,7 +1110,7 @@ sub _check_bug_status {
my $old_status; # Note that this is undef for new bugs.
if (ref $invocant) {
- @valid_statuses = @{$invocant->status->can_change_to};
+ @valid_statuses = @{$invocant->statuses_available};
$product = $invocant->product_obj;
$old_status = $invocant->status;
my $comments = $invocant->{added_comments} || [];
@@ -1118,12 +1118,11 @@ sub _check_bug_status {
}
else {
@valid_statuses = @{Bugzilla::Status->can_change_to()};
- }
-
- if (!$product->votes_to_confirm) {
- # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0,
- # even if you are in editbugs.
- @valid_statuses = grep {$_->name ne 'UNCONFIRMED'} @valid_statuses;
+ if (!$product->votes_to_confirm) {
+ # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0,
+ # even if you are in editbugs.
+ @valid_statuses = grep {$_->name ne 'UNCONFIRMED'} @valid_statuses;
+ }
}
# Check permissions for users filing new bugs.
@@ -2167,6 +2166,8 @@ sub set_status {
my $old_status = $self->status;
$self->set('bug_status', $status);
delete $self->{'status'};
+ delete $self->{'statuses_available'};
+ delete $self->{'choices'};
my $new_status = $self->status;
if ($new_status->is_open) {
@@ -2680,11 +2681,6 @@ sub isopened {
return is_open_state($self->{bug_status}) ? 1 : 0;
}
-sub isunconfirmed {
- my $self = shift;
- return ($self->bug_status eq 'UNCONFIRMED') ? 1 : 0;
-}
-
sub keywords {
my ($self) = @_;
return join(', ', (map { $_->name } @{$self->keyword_objects}));
@@ -2805,6 +2801,31 @@ sub status {
return $self->{'status'};
}
+sub statuses_available {
+ my $self = shift;
+ return [] if $self->{'error'};
+ return $self->{'statuses_available'}
+ if defined $self->{'statuses_available'};
+
+ my @statuses = @{ $self->status->can_change_to };
+
+ # UNCONFIRMED is only a valid status if it is enabled in this product.
+ if (!$self->product_obj->votes_to_confirm) {
+ @statuses = grep { $_->name ne 'UNCONFIRMED' } @statuses;
+ }
+
+ my @available;
+ foreach my $status (@statuses) {
+ # Make sure this is a legal status transition
+ next if !$self->check_can_change_field(
+ 'bug_status', $self->status->name, $status->name);
+ push(@available, $status);
+ }
+
+ $self->{'statuses_available'} = \@available;
+ return $self->{'statuses_available'};
+}
+
sub show_attachment_flags {
my ($self) = @_;
return $self->{'show_attachment_flags'}
@@ -2958,9 +2979,10 @@ sub choices {
}
my %choices = (
- product => \@products,
- component => $self->product_obj->components,
- version => $self->product_obj->versions,
+ bug_status => $self->statuses_available,
+ product => \@products,
+ component => $self->product_obj->components,
+ version => $self->product_obj->versions,
target_milestone => $self->product_obj->milestones,
);
@@ -3451,12 +3473,7 @@ sub check_can_change_field {
}
# *Only* users with (product-specific) "canconfirm" privs can confirm bugs.
- if ($field eq 'canconfirm'
- || ($field eq 'everconfirmed' && $newvalue)
- || ($field eq 'bug_status'
- && $oldvalue eq 'UNCONFIRMED'
- && is_open_state($newvalue)))
- {
+ if ($self->_changes_everconfirmed($field, $oldvalue, $newvalue)) {
$$PrivilegesRequired = 3;
return $user->in_group('canconfirm', $self->{'product_id'});
}
@@ -3532,6 +3549,24 @@ sub check_can_change_field {
return 0;
}
+# A helper for check_can_change_field
+sub _changes_everconfirmed {
+ my ($self, $field, $old, $new) = @_;
+ return 1 if $field eq 'everconfirmed';
+ if ($field eq 'bug_status') {
+ if ($self->everconfirmed) {
+ # Moving a confirmed bug to UNCONFIRMED will change everconfirmed.
+ return 1 if $new eq 'UNCONFIRMED';
+ }
+ else {
+ # Moving an unconfirmed bug to an open state that isn't
+ # UNCONFIRMED will confirm the bug.
+ return 1 if (is_open_state($new) and $new ne 'UNCONFIRMED');
+ }
+ }
+ return 0;
+}
+
#
# Field Validation
#
@@ -3626,8 +3661,7 @@ sub _validate_attribute {
my @valid_attributes = (
# Miscellaneous properties and methods.
qw(error groups product_id component_id
- comments milestoneurl attachments
- isopened isunconfirmed
+ comments milestoneurl attachments isopened
flag_types num_attachment_flag_types
show_attachment_flags any_flags_requesteeble),
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index 4720dc129..4d1281e7e 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -176,28 +176,6 @@ sub can_change_to {
return $self->{'can_change_to'};
}
-sub can_change_from {
- my $self = shift;
- my $dbh = Bugzilla->dbh;
-
- if (!defined $self->{'can_change_from'}) {
- my $old_status_ids = $dbh->selectcol_arrayref('SELECT old_status
- FROM status_workflow
- INNER JOIN bug_status
- ON id = old_status
- WHERE isactive = 1
- AND new_status = ?
- AND old_status IS NOT NULL',
- undef, $self->id);
-
- # Allow the bug status to remain unchanged.
- push(@$old_status_ids, $self->id);
- $self->{'can_change_from'} = Bugzilla::Status->new_from_list($old_status_ids);
- }
-
- return $self->{'can_change_from'};
-}
-
sub comment_required_on_change_from {
my ($self, $old_status) = @_;
my ($cond, $values) = $self->_status_condition($old_status);
@@ -300,17 +278,6 @@ below.
Returns: A list of Bugzilla::Status objects.
-=item C<can_change_from>
-
- Description: Returns the list of active statuses a bug can be changed from
- given the new bug status. If the bug status is available on
- bug creation, this method doesn't return this information.
- You have to call C<can_change_to> instead.
-
- Params: none.
-
- Returns: A list of Bugzilla::Status objects.
-
=item C<comment_required_on_change_from>
=over
diff --git a/template/en/default/bug/knob.html.tmpl b/template/en/default/bug/knob.html.tmpl
index 10d58e51b..ac14e6dc0 100644
--- a/template/en/default/bug/knob.html.tmpl
+++ b/template/en/default/bug/knob.html.tmpl
@@ -23,71 +23,33 @@
[% PROCESS global/variables.none.tmpl %]
<div id="status">
- [% initial_action_shown = 0 %]
- [% show_resolution = 0 %]
- [% bug_status_select_displayed = 0 %]
+ [% PROCESS bug/field.html.tmpl
+ no_tds = 1
+ field = bug_fields.bug_status
+ value = bug.bug_status
+ override_legal_values = bug.choices.bug_status
+ editable = bug.choices.bug_status.size > 1
+ %]
- [% closed_status_array = [] %]
- [%# These actions are based on the current custom workflow. %]
- [% FOREACH bug_status = bug.status.can_change_to %]
- [% NEXT IF bug.isunconfirmed && bug_status.is_open && !bug.user.canconfirm %]
- [% NEXT IF bug.isopened && !bug.isunconfirmed && bug_status.is_open && !bug.user.canedit %]
- [% NEXT IF (!bug_status.is_open || !bug.isopened) && !bug.user.canedit && !bug.user.isreporter %]
- [%# Special hack to only display UNCO or REOP when reopening, but not both;
- # for compatibility with older versions. %]
- [% NEXT IF !bug.isopened && (bug.everconfirmed && bug_status.name == "UNCONFIRMED"
- || !bug.everconfirmed && bug_status.name == "REOPENED") %]
- [% IF NOT bug_status_select_displayed %]
- <select name="bug_status" id="bug_status">
- [% bug_status_select_displayed = 1 %]
- [% END %]
- [% PROCESS initial_action %]
- [% NEXT IF bug_status.name == bug.bug_status %]
- <option value="[% bug_status.name FILTER html %]">
- [% display_value("bug_status", bug_status.name) FILTER html %]
- </option>
- [% IF !bug_status.is_open %]
- [% show_resolution = 1 %]
- [% filtered_status = bug_status.name FILTER js %]
- [% closed_status_array.push( filtered_status ) %]
- [% END %]
+ [% IF bug.resolution
+ OR bug.check_can_change_field('resolution', bug.resolution, 1)
+ %]
+ <noscript><br>resolved as </noscript>
[% END %]
- [%# These actions are special and are independent of the workflow. %]
- [% IF bug.user.canedit || bug.user.isreporter %]
- [% IF NOT bug_status_select_displayed %]
- <select name="bug_status" id="bug_status">
- [% bug_status_select_displayed = 1 %]
- [% END %]
- [% IF bug.isopened %]
- [% IF bug.resolution %]
- [% PROCESS initial_action %]
- [% END %]
- [% ELSIF bug.resolution != "MOVED" || bug.user.canmove %]
- [% PROCESS initial_action %]
- [% show_resolution = 1 %]
- [% END %]
- [% END %]
- [% IF bug_status_select_displayed %]
- </select>
- [% ELSE %]
- [% display_value("bug_status", bug.bug_status) FILTER html %]
- [% IF bug.resolution %]
- [%+ display_value("resolution", bug.resolution) FILTER html %]
- [% IF bug.dup_id %]
- <span id="duplicate_display">of
- [% "${terms.bug} ${bug.dup_id}" FILTER bug_link(bug.dup_id) FILTER none %]</span>
- [% END %]
- [% END %]
- [% END %]
- [% IF bug.user.canedit || bug.user.isreporter %]
- [% IF show_resolution %]
- <noscript><br>resolved as </noscript>
- <span id="resolution_settings">[% PROCESS select_resolution %]</span>
- [% END %]
+ <span id="resolution_settings">
+ [% PROCESS bug/field.html.tmpl
+ no_tds = 1
+ field = bug_fields.resolution
+ value = bug.resolution
+ override_legal_values = bug.choices.resolution
+ editable = bug.check_can_change_field('resolution', bug.resolution, 1)
+ %]
+ </span>
+
+ [% IF bug.check_can_change_field('dup_id', 0, 1) %]
<noscript><br> duplicate</noscript>
-
- <span id="duplicate_settings">of
+ <span id="duplicate_settings">of
<span id="dup_id_container" class="bz_default_hidden">
[% "${terms.bug} ${bug.dup_id}" FILTER bug_link(bug.dup_id) FILTER none %]
(<a href="#" id="dup_id_edit_action">edit</a>)
@@ -101,11 +63,20 @@
<div id="dup_id_discoverable" class="bz_default_hidden">
<a href="#" id="dup_id_discoverable_action">Mark as Duplicate</a>
</div>
+ [% ELSIF bug.dup_id %]
+ <noscript><br> duplicate</noscript>
+ <span id="duplicate_display">of
+ [% "${terms.bug} ${bug.dup_id}" FILTER bug_link(bug.dup_id) FILTER none %]</span>
[% END %]
</div>
+
<script type="text/javascript">
- var close_status_array = new Array("[% closed_status_array.join('", "') FILTER replace(',$', '')
- FILTER none %]");
+ var close_status_array = [
+ [% FOREACH status = bug.choices.bug_status %]
+ [% NEXT IF status.is_open %]
+ '[% status.name FILTER js %]'[% ',' UNLESS loop.last %]
+ [% END %]
+ ];
YAHOO.util.Dom.removeClass('dup_id_discoverable', 'bz_default_hidden');
hideEditableField( "dup_id_container", "dup_id", 'dup_id_edit_action',
'dup_id', '[% bug.dup_id FILTER js %]' )
@@ -127,29 +98,3 @@
[% INCLUDE "bug/field-events.js.tmpl" field = select_fields.bug_status %]
[% INCLUDE "bug/field-events.js.tmpl" field = select_fields.resolution %]
</script>
-
-[%# Common actions %]
-
-[% BLOCK initial_action %]
- [% IF !initial_action_shown %]
- <option selected value="[% bug.bug_status FILTER html %]">
- [% display_value("bug_status", bug.bug_status) FILTER html %]
- </option>
- [% IF !bug.isopened %]
- [% show_resolution = 1 %]
- [% filtered_status = bug.bug_status FILTER js %]
- [% closed_status_array.push(filtered_status) %]
- [% END %]
- [% initial_action_shown = 1 %]
- [% END %]
-[% END %]
-
-[% BLOCK select_resolution %]
- <select name="resolution" id="resolution">
- [% FOREACH r = bug.choices.resolution %]
- <option value="[% r.name FILTER html %]"
- [% ' selected="selected"' IF r.name == bug.resolution %]>
- [% display_value("resolution", r.name) FILTER html %]</option>
- [% END %]
- </select>
-[% END %]
--
2.24.1