From b3630da125fa112e04f6e6a15328f64e13a874c5 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Fri, 13 Apr 2007 19:53:23 +0000 Subject: Bug 92552: Separate reassignment from bug status change (they are now independent) - Patch by Frédéric Buclin r=gerv a=LpSolit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Bug.pm | 17 --- process_bug.cgi | 157 +++++++++++------------ template/en/default/bug/edit.html.tmpl | 29 ++++- template/en/default/bug/knob.html.tmpl | 43 ------- template/en/default/global/userselect.html.tmpl | 11 +- template/en/default/list/edit-multiple.html.tmpl | 49 ++++--- 6 files changed, 131 insertions(+), 175 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 777fca323..62be49564 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1511,15 +1511,6 @@ sub get_new_status_and_resolution { $status = $self->everconfirmed ? 'REOPENED' : 'UNCONFIRMED'; $resolution = ''; } - elsif ($action =~ /^reassign(?:bycomponent)?$/) { - if (!is_open_state($self->bug_status) || $self->bug_status eq 'UNCONFIRMED') { - $status = $self->bug_status; - } - else { - $status = 'NEW'; - } - $resolution = $self->resolution; - } elsif ($action eq 'duplicate') { # Only alter the bug status if the bug is currently open. $status = is_open_state($self->bug_status) ? 'RESOLVED' : $self->bug_status; @@ -2153,14 +2144,6 @@ sub check_can_change_field { return 1; } - # Ignore the assigned_to field if the bug is not being reassigned - if ($field eq 'assigned_to' - && $data->{'knob'} ne 'reassignbycomponent' - && $data->{'knob'} ne 'reassign') - { - return 1; - } - # If the user isn't allowed to change a field, we must tell him who can. # We store the required permission set into the $PrivilegesRequired # variable which gets passed to the error template. diff --git a/process_bug.cgi b/process_bug.cgi index 74468ada9..b2dd8c6bd 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -744,6 +744,8 @@ if ($cgi->param('product') ne $cgi->param('dontchange')) { } my $component; +my (%cc_add, %cc_remove); + if ($cgi->param('component') ne $cgi->param('dontchange')) { if (scalar(@newprod_ids) > 1) { ThrowUserError("no_component_change_for_multiple_products"); @@ -756,6 +758,16 @@ if ($cgi->param('component') ne $cgi->param('dontchange')) { DoComma(); $::query .= "component_id = ?"; push(@values, $component->id); + + # Add in the default CC list for the component if we are moving bugs. + if (!$cgi->param('id') || $component->id != $bug->component_id) { + foreach my $cc (@{$component->initial_cc}) { + # NewCC must be defined or the code below won't insert + # any CCs. + $cgi->param('newcc') || $cgi->param('newcc', ""); + $cc_add{$cc->id} = $cc->login; + } + } } # If this installation uses bug aliases, and the user is changing the alias, @@ -829,12 +841,9 @@ if ( defined $cgi->param('id') && } } -my $duplicate; - # We need to check the addresses involved in a CC change before we touch any bugs. # What we'll do here is formulate the CC data into two hashes of ID's involved # in this CC change. Then those hashes can be used later on for the actual change. -my (%cc_add, %cc_remove); if (defined $cgi->param('newcc') || defined $cgi->param('addselfcc') || defined $cgi->param('removecc') @@ -891,9 +900,38 @@ my $assignee_checked = 0; my %usercache = (); -if (defined $cgi->param('qa_contact') - && $cgi->param('knob') ne "reassignbycomponent") +if (defined $cgi->param('assigned_to') + && !$cgi->param('set_default_assignee') + && trim($cgi->param('assigned_to')) ne $cgi->param('dontchange')) { + my $name = trim($cgi->param('assigned_to')); + if ($name ne "") { + $assignee = login_to_id($name, THROW_ERROR); + if (Bugzilla->params->{"strict_isolation"}) { + $usercache{$assignee} ||= Bugzilla::User->new($assignee); + my $assign_user = $usercache{$assignee}; + foreach my $product_id (@newprod_ids) { + if (!$assign_user->can_edit_product($product_id)) { + my $product_name = Bugzilla::Product->new($product_id)->name; + ThrowUserError('invalid_user_group', + {'users' => $assign_user->login, + 'product' => $product_name, + 'bug_id' => (scalar(@idlist) > 1) + ? undef : $idlist[0] + }); + } + } + } + } else { + ThrowUserError("reassign_to_empty"); + } + DoComma(); + $::query .= "assigned_to = ?"; + push(@values, $assignee); + $assignee_checked = 1; +}; + +if (defined $cgi->param('qa_contact') && !$cgi->param('set_default_qa_contact')) { my $name = trim($cgi->param('qa_contact')); # The QA contact cannot be deleted from show_bug.cgi for a single bug! if ($name ne $cgi->param('dontchange')) { @@ -928,6 +966,12 @@ if (defined $cgi->param('qa_contact') } } +if ($cgi->param('set_default_assignee') || $cgi->param('set_default_qa_contact')) { + CheckonComment('reassignbycomponent'); +} + +my $duplicate; # It will store the ID of the bug we are pointing to, if any. + SWITCH: for ($cgi->param('knob')) { /^none$/ && do { last SWITCH; @@ -983,43 +1027,6 @@ SWITCH: for ($cgi->param('knob')) { } last SWITCH; }; - /^reassign$/ && CheckonComment( "reassign" ) && do { - if ($cgi->param('andconfirm')) { - DoConfirm($bug); - } - if (defined $cgi->param('assigned_to') - && trim($cgi->param('assigned_to')) ne "") { - $assignee = login_to_id(trim($cgi->param('assigned_to')), THROW_ERROR); - if (Bugzilla->params->{"strict_isolation"}) { - $usercache{$assignee} ||= Bugzilla::User->new($assignee); - my $assign_user = $usercache{$assignee}; - foreach my $product_id (@newprod_ids) { - if (!$assign_user->can_edit_product($product_id)) { - my $product_name = Bugzilla::Product->new($product_id)->name; - ThrowUserError('invalid_user_group', - {'users' => $assign_user->login, - 'product' => $product_name, - 'bug_id' => (scalar(@idlist) > 1) - ? undef : $idlist[0] - }); - } - } - } - } else { - ThrowUserError("reassign_to_empty"); - } - DoComma(); - $::query .= "assigned_to = ?"; - push(@values, $assignee); - $assignee_checked = 1; - last SWITCH; - }; - /^reassignbycomponent$/ && CheckonComment( "reassignbycomponent" ) && do { - if ($cgi->param('compconfirm')) { - DoConfirm($bug); - } - last SWITCH; - }; /^reopen$/ && CheckonComment( "reopen" ) && do { last SWITCH; }; @@ -1290,10 +1297,10 @@ foreach my $id (@idlist) { $comma = ','; } - if ($cgi->param('knob') eq 'reassignbycomponent') { - # We have to check whether the bug is moved to another product - # and/or component before reassigning. If $component is defined, - # use it; else use the product/component the bug is already in. + # We have to check whether the bug is moved to another product + # and/or component before reassigning. If $component is defined, + # use it; else use the product/component the bug is already in. + if ($cgi->param('set_default_assignee')) { my $new_comp_id = $component ? $component->id : $old_bug_obj->{'component_id'}; $assignee = $dbh->selectrow_array('SELECT initialowner FROM components @@ -1302,29 +1309,22 @@ foreach my $id (@idlist) { $query .= "$comma assigned_to = ?"; push(@bug_values, $assignee); $comma = ','; - if (Bugzilla->params->{"useqacontact"}) { - $qacontact = $dbh->selectrow_array('SELECT initialqacontact - FROM components - WHERE components.id = ?', - undef, $new_comp_id); - if ($qacontact) { - $query .= "$comma qa_contact = ?"; - push(@bug_values, $qacontact); - } - else { - $query .= "$comma qa_contact = NULL"; - } - } + } - # And add in the Default CC for the Component. - my $comp_obj = $component || new Bugzilla::Component($new_comp_id); - my @new_init_cc = @{$comp_obj->initial_cc}; - foreach my $cc (@new_init_cc) { - # NewCC must be defined or the code below won't insert - # any CCs. - $cgi->param('newcc') || $cgi->param('newcc', []); - $cc_add{$cc->id} = $cc->login; + if (Bugzilla->params->{'useqacontact'} && $cgi->param('set_default_qa_contact')) { + my $new_comp_id = $component ? $component->id : $old_bug_obj->{'component_id'}; + $qacontact = $dbh->selectrow_array('SELECT initialqacontact + FROM components + WHERE components.id = ?', + undef, $new_comp_id); + if ($qacontact) { + $query .= "$comma qa_contact = ?"; + push(@bug_values, $qacontact); } + else { + $query .= "$comma qa_contact = NULL"; + } + $comma = ','; } my %dependencychanged; @@ -1362,24 +1362,19 @@ foreach my $id (@idlist) { } $oldhash{$col} = $oldvalues[$i]; $formhash{$col} = $cgi->param($col) if defined $cgi->param($col); - # The status and resolution are defined by the workflow. - $formhash{$col} = $status if $col eq 'bug_status'; - $formhash{$col} = $resolution if $col eq 'resolution'; $i++; } - # If the user is reassigning bugs, we need to: - # - convert $newhash{'assigned_to'} and $newhash{'qa_contact'} - # email addresses into their corresponding IDs; + # The status and resolution are defined by the workflow. + $formhash{'bug_status'} = $status; + $formhash{'resolution'} = $resolution; + + # We need to convert $newhash{'assigned_to'} and $newhash{'qa_contact'} + # email addresses into their corresponding IDs; $formhash{'qa_contact'} = $qacontact if Bugzilla->params->{'useqacontact'}; - if ($cgi->param('knob') eq 'reassignbycomponent' - || $cgi->param('knob') eq 'reassign') { - $formhash{'assigned_to'} = $assignee; - } + $formhash{'assigned_to'} = $assignee; + # This hash is required by Bug::check_can_change_field(). - my $cgi_hash = { - 'dontchange' => scalar $cgi->param('dontchange'), - 'knob' => scalar $cgi->param('knob') - }; + my $cgi_hash = {'dontchange' => scalar $cgi->param('dontchange')}; foreach my $col (@editable_bug_fields) { if (exists $formhash{$col} && !$old_bug_obj->check_can_change_field($col, $oldhash{$col}, $formhash{$col}, diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl index c97077d69..037ea443d 100644 --- a/template/en/default/bug/edit.html.tmpl +++ b/template/en/default/bug/edit.html.tmpl @@ -667,7 +667,7 @@ [%############################################################################%] [% BLOCK section_people %] - +
- [% IF Param('useqacontact') %] - - + + + + [% IF Param("useqacontact") %] [% END %] @@ -336,27 +356,6 @@
[% END %] -[% knum = knum + 1 %] - - -[% INCLUDE global/userselect.html.tmpl - name => "assigned_to" - value => user.login - size => 32 - onchange => "document.forms.changeform.knob[$knum].checked=true;" -%]
- -[% knum = knum + 1 %] - -
- [% IF Param('move-enabled') && user.is_mover %] -- cgit v1.2.3-24-g4f1b
Reporter: @@ -678,20 +678,34 @@
- Assigned To: + + Assignee: - [% INCLUDE user_identity user => bug.assigned_to %] + [% IF bug.check_can_change_field("assigned_to", 0, 1) %] + [% INCLUDE global/userselect.html.tmpl + id => "assigned_to" + name => "assigned_to" + value => bug.assigned_to.login + size => 30 + %] +
+ + + [% ELSE %] + + [% INCLUDE user_identity user => bug.assigned_to %] + [% END %]
+ : + [% IF bug.check_can_change_field("qa_contact", 0, 1) %] [% INCLUDE global/userselect.html.tmpl id => "qa_contact" @@ -700,6 +714,9 @@ size => 30 emptyok => 1 %] +
+ + [% ELSE %] diff --git a/template/en/default/bug/knob.html.tmpl b/template/en/default/bug/knob.html.tmpl index 12f1adf65..0e1a928f4 100644 --- a/template/en/default/bug/knob.html.tmpl +++ b/template/en/default/bug/knob.html.tmpl @@ -76,49 +76,6 @@ [% PROCESS select_resolution %] [% PROCESS duplicate %] - - [% IF bug.user.canedit %] - - - [% safe_assigned_to = FILTER js; bug.assigned_to.login; END %] - [% INCLUDE global/userselect.html.tmpl - id => "assigned_to" - name => "assigned_to" - value => bug.assigned_to.login - size => 32 - onchange => "if ((this.value != '$safe_assigned_to') && (this.value != '')) { - document.changeform.knob[$knum].checked=true; - }" - %] -
- [% IF bug.isunconfirmed && bug.user.canconfirm %] -      - -
- [% END %] - [% knum = knum + 1 %] - - - -
- [% IF bug.isunconfirmed && bug.user.canconfirm %] -      - -
- [% END %] - [% knum = knum + 1 %] - [% END %] [% ELSE %] [% IF bug.resolution != "MOVED" || (bug.resolution == "MOVED" && bug.user.canmove) %] diff --git a/template/en/default/global/userselect.html.tmpl b/template/en/default/global/userselect.html.tmpl index 7a46891ab..398300b52 100644 --- a/template/en/default/global/userselect.html.tmpl +++ b/template/en/default/global/userselect.html.tmpl @@ -15,8 +15,6 @@ #%] [%# INTERFACE: - # userlist: select only; array reference with list of users and identities - # userlist is built by Bugzilla::User::get_userlist() # name: mandatory; field name # id: optional; field id # value: optional; default field value/selection @@ -26,7 +24,7 @@ # size: optional, input only; size attribute value # emptyok: optional, select only; if true, prepend menu option to start of select # multiple: optional, do multiselect box, value is size (height) of box - # + # do_not_change: optional, contains the string meaning "do not alter this role" #%] [% IF Param("usemenuforusers") %] @@ -40,6 +38,13 @@ [% IF emptyok %] [% END %] + + [% IF do_not_change %] + + [% END %] + [% FOREACH tmpuser = user.get_userlist %] [% IF tmpuser.visible OR value.match("\\b$tmpuser.login\\b") %]
+ [% INCLUDE global/userselect.html.tmpl + id => "assigned_to" + name => "assigned_to" + value => dontchange + do_not_change => dontchange + size => 32 + %] + + +
- + [% INCLUDE global/userselect.html.tmpl + id => "qa_contact" + name => "qa_contact" + value => dontchange + do_not_change => dontchange + size => 32 + %] + +