From edf97ecb4b942f6ed515d16d381c212668e96513 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Thu, 20 Dec 2007 00:59:31 +0000 Subject: Bug 388147: Move assigned_to and qa_contact updating into Bugzilla::Bug Patch By Max Kanat-Alexander r=LpSolit, a=LpSolit --- process_bug.cgi | 362 +++++++++++++++----------------------------------------- 1 file changed, 97 insertions(+), 265 deletions(-) (limited to 'process_bug.cgi') diff --git a/process_bug.cgi b/process_bug.cgi index a8df416cd..91d430aa9 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -293,6 +293,13 @@ sub DuplicateUserConfirm { exit; } +my @set_fields = qw(op_sys rep_platform priority bug_severity + component target_milestone version + bug_file_loc status_whiteboard short_desc + deadline remaining_time estimated_time); +push(@set_fields, 'assigned_to') if !$cgi->param('set_default_assignee'); +push(@set_fields, 'qa_contact') if !$cgi->param('set_default_qa_contact'); + my %methods = ( bug_severity => 'set_severity', rep_platform => 'set_platform', @@ -303,97 +310,19 @@ foreach my $b (@bug_objects) { # Component, target_milestone, and version are in here just in case # the 'product' field wasn't defined in the CGI. It doesn't hurt to set # them twice. - foreach my $field_name (qw(op_sys rep_platform priority bug_severity - component target_milestone version - bug_file_loc status_whiteboard short_desc - deadline remaining_time estimated_time)) - { + foreach my $field_name (@set_fields) { if (should_set($field_name)) { my $method = $methods{$field_name}; $method ||= "set_" . $field_name; $b->$method($cgi->param($field_name)); } } + $b->reset_assigned_to if $cgi->param('set_default_assignee'); + $b->reset_qa_contact if $cgi->param('set_default_qa_contact'); } my $action = trim($cgi->param('action') || ''); -if ($action eq Bugzilla->params->{'move-button-text'}) { - Bugzilla->params->{'move-enabled'} || ThrowUserError("move_bugs_disabled"); - - $user->is_mover || ThrowUserError("auth_failure", {action => 'move', - object => 'bugs'}); - - my @multi_select_locks = map {'bug_' . $_->name . " WRITE"} - Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT, - obsolete => 0 }); - - $dbh->bz_lock_tables('bugs WRITE', 'bugs_activity WRITE', 'duplicates WRITE', - 'longdescs WRITE', 'profiles READ', 'groups READ', - 'bug_group_map READ', 'group_group_map READ', - 'user_group_map READ', 'classifications READ', - 'products READ', 'components READ', 'votes READ', - 'cc READ', 'fielddefs READ', 'bug_status READ', - 'status_workflow READ', 'resolution READ', @multi_select_locks); - - # First update all moved bugs. - foreach my $bug (@bug_objects) { - $bug->add_comment(scalar $cgi->param('comment'), - { type => CMT_MOVED_TO, extra_data => $user->login }); - } - # Don't export the new status and resolution. We want the current ones. - local $Storable::forgive_me = 1; - my $bugs = dclone(\@bug_objects); - foreach my $bug (@bug_objects) { - my ($status, $resolution) = $bug->get_new_status_and_resolution('move'); - $bug->set_status($status); - $bug->set_resolution($resolution); - } - $_->update() foreach @bug_objects; - $dbh->bz_unlock_tables(); - - # Now send emails. - foreach my $id (@idlist) { - $vars->{'mailrecipients'} = { 'changer' => $user->login }; - $vars->{'id'} = $id; - $vars->{'type'} = "move"; - send_results($id, $vars); - } - # Prepare and send all data about these bugs to the new database - my $to = Bugzilla->params->{'move-to-address'}; - $to =~ s/@/\@/; - my $from = Bugzilla->params->{'moved-from-address'}; - $from =~ s/@/\@/; - my $msg = "To: $to\n"; - $msg .= "From: Bugzilla <" . $from . ">\n"; - $msg .= "Subject: Moving bug(s) " . join(', ', @idlist) . "\n\n"; - - my @fieldlist = (Bugzilla::Bug->fields, 'group', 'long_desc', - 'attachment', 'attachmentdata'); - my %displayfields; - foreach (@fieldlist) { - $displayfields{$_} = 1; - } - - $template->process("bug/show.xml.tmpl", { bugs => $bugs, - displayfields => \%displayfields, - }, \$msg) - || ThrowTemplateError($template->error()); - - $msg .= "\n"; - MessageToMTA($msg); - - # End the response page. - unless (Bugzilla->usage_mode == USAGE_MODE_EMAIL) { - $template->process("bug/navigate.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - $template->process("global/footer.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - } - exit; -} - - $::query = "UPDATE bugs SET"; $::comma = ""; local our @values; @@ -498,83 +427,93 @@ if (defined $cgi->param('newcc') foreach my $b (@bug_objects) { $b->remove_cc($_) foreach @cc_remove; $b->add_cc($_) foreach @cc_add; + # Theoretically you could move a product without ever specifying + # a new assignee or qa_contact, or adding/removing any CCs. So, + # we have to check that the current assignee, qa, and CCs are still + # valid if we've switched products, under strict_isolation. We can only + # do that here. There ought to be some better way to do this, + # architecturally, but I haven't come up with it. + if ($product_change) { + $b->_check_strict_isolation(); + } } -# Store the new assignee and QA contact IDs (if any). This is the -# only way to keep these informations when bugs are reassigned by -# component as $cgi->param('assigned_to') and $cgi->param('qa_contact') -# are not the right fields to look at. -# If the assignee or qacontact is changed, the new one is checked when -# changed information is validated. If not, then the unchanged assignee -# or qacontact may have to be validated later. - -my $assignee; -my $qacontact; -my $qacontact_checked = 0; -my $assignee_checked = 0; - -my %usercache = (); - -if (should_set('assigned_to') && !$cgi->param('set_default_assignee')) { - 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"); +if ($action eq Bugzilla->params->{'move-button-text'}) { + Bugzilla->params->{'move-enabled'} || ThrowUserError("move_bugs_disabled"); + + $user->is_mover || ThrowUserError("auth_failure", {action => 'move', + object => 'bugs'}); + + my @multi_select_locks = map {'bug_' . $_->name . " WRITE"} + Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT, + obsolete => 0 }); + + $dbh->bz_lock_tables('bugs WRITE', 'bugs_activity WRITE', 'duplicates WRITE', + 'longdescs WRITE', 'profiles READ', 'groups READ', + 'bug_group_map READ', 'group_group_map READ', + 'user_group_map READ', 'classifications READ', + 'products READ', 'components READ', 'votes READ', + 'cc READ', 'fielddefs READ', 'bug_status READ', + 'status_workflow READ', 'resolution READ', @multi_select_locks); + + # First update all moved bugs. + foreach my $bug (@bug_objects) { + $bug->add_comment(scalar $cgi->param('comment'), + { type => CMT_MOVED_TO, extra_data => $user->login }); } - DoComma(); - $::query .= "assigned_to = ?"; - push(@values, $assignee); - $assignee_checked = 1; -}; + # Don't export the new status and resolution. We want the current ones. + local $Storable::forgive_me = 1; + my $bugs = dclone(\@bug_objects); + foreach my $bug (@bug_objects) { + my ($status, $resolution) = $bug->get_new_status_and_resolution('move'); + $bug->set_status($status); + $bug->set_resolution($resolution); + } + $_->update() foreach @bug_objects; + $dbh->bz_unlock_tables(); -if (should_set('qa_contact') && !$cgi->param('set_default_qa_contact')) { - my $name = trim($cgi->param('qa_contact')); - $qacontact = login_to_id($name, THROW_ERROR) if ($name ne ""); - if ($qacontact && Bugzilla->params->{"strict_isolation"} - && !(defined $cgi->param('id') && $bug->qa_contact - && $qacontact == $bug->qa_contact->id)) - { - $usercache{$qacontact} ||= Bugzilla::User->new($qacontact); - my $qa_user = $usercache{$qacontact}; - foreach my $product_id (@newprod_ids) { - if (!$qa_user->can_edit_product($product_id)) { - my $product_name = Bugzilla::Product->new($product_id)->name; - ThrowUserError('invalid_user_group', - {'users' => $qa_user->login, - 'product' => $product_name, - 'bug_id' => (scalar(@idlist) > 1) - ? undef : $idlist[0] - }); - } - } + # Now send emails. + foreach my $id (@idlist) { + $vars->{'mailrecipients'} = { 'changer' => $user->login }; + $vars->{'id'} = $id; + $vars->{'type'} = "move"; + send_results($id, $vars); } - $qacontact_checked = 1; - DoComma(); - if($qacontact) { - $::query .= "qa_contact = ?"; - push(@values, $qacontact); + # Prepare and send all data about these bugs to the new database + my $to = Bugzilla->params->{'move-to-address'}; + $to =~ s/@/\@/; + my $from = Bugzilla->params->{'moved-from-address'}; + $from =~ s/@/\@/; + my $msg = "To: $to\n"; + $msg .= "From: Bugzilla <" . $from . ">\n"; + $msg .= "Subject: Moving bug(s) " . join(', ', @idlist) . "\n\n"; + + my @fieldlist = (Bugzilla::Bug->fields, 'group', 'long_desc', + 'attachment', 'attachmentdata'); + my %displayfields; + foreach (@fieldlist) { + $displayfields{$_} = 1; } - else { - $::query .= "qa_contact = NULL"; + + $template->process("bug/show.xml.tmpl", { bugs => $bugs, + displayfields => \%displayfields, + }, \$msg) + || ThrowTemplateError($template->error()); + + $msg .= "\n"; + MessageToMTA($msg); + + # End the response page. + unless (Bugzilla->usage_mode == USAGE_MODE_EMAIL) { + $template->process("bug/navigate.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + $template->process("global/footer.html.tmpl", $vars) + || ThrowTemplateError($template->error()); } + exit; } + if (($cgi->param('set_default_assignee') || $cgi->param('set_default_qa_contact')) && Bugzilla->params->{'commentonreassignbycomponent'} && !comment_exists()) { @@ -661,56 +600,6 @@ sub SnapShotBug { my $timestamp; -if ($product_change && Bugzilla->params->{"strict_isolation"}) { - my $sth_cc = $dbh->prepare("SELECT who - FROM cc - WHERE bug_id = ?"); - my $sth_bug = $dbh->prepare("SELECT assigned_to, qa_contact - FROM bugs - WHERE bug_id = ?"); - - foreach my $id (@idlist) { - $sth_cc->execute($id); - my @blocked_cc = (); - while (my ($pid) = $sth_cc->fetchrow_array) { - # Ignore deleted accounts. They will never get notification. - $usercache{$pid} ||= Bugzilla::User->new($pid) || next; - my $cc_user = $usercache{$pid}; - if (!$cc_user->can_edit_product($product->id)) { - push (@blocked_cc, $cc_user->login); - } - } - if (scalar(@blocked_cc)) { - ThrowUserError('invalid_user_group', - {'users' => \@blocked_cc, - 'bug_id' => $id, - 'product' => $product->name}); - } - $sth_bug->execute($id); - my ($assignee, $qacontact) = $sth_bug->fetchrow_array; - if (!$assignee_checked) { - $usercache{$assignee} ||= Bugzilla::User->new($assignee) || next; - my $assign_user = $usercache{$assignee}; - if (!$assign_user->can_edit_product($product->id)) { - ThrowUserError('invalid_user_group', - {'users' => $assign_user->login, - 'bug_id' => $id, - 'product' => $product->name}); - } - } - if (!$qacontact_checked && $qacontact) { - $usercache{$qacontact} ||= Bugzilla::User->new($qacontact) || next; - my $qa_user = $usercache{$qacontact}; - if (!$qa_user->can_edit_product($product->id)) { - ThrowUserError('invalid_user_group', - {'users' => $qa_user->login, - 'bug_id' => $id, - 'product' => $product->name}); - } - } - } -} - my %bug_objects = map {$_->id => $_} @bug_objects; # This loop iterates once for each bug to be processed (i.e. all the @@ -751,35 +640,6 @@ foreach my $id (@idlist) { $comma = ','; } - # We have to check whether the bug is moved to another product - # and/or component before reassigning. - if ($cgi->param('set_default_assignee')) { - my $new_comp_id = $bug_objects{$id}->component_id; - $assignee = $dbh->selectrow_array('SELECT initialowner - FROM components - WHERE components.id = ?', - undef, $new_comp_id); - $query .= "$comma assigned_to = ?"; - push(@bug_values, $assignee); - $comma = ','; - } - - if (Bugzilla->params->{'useqacontact'} && $cgi->param('set_default_qa_contact')) { - my $new_comp_id = $bug_objects{$id}->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 $bug_changed = 0; my $write = "WRITE"; # Might want to make a param to control # whether we do LOW_PRIORITY ... @@ -826,14 +686,11 @@ foreach my $id (@idlist) { $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'}; - $formhash{'assigned_to'} = $assignee; - # This hash is required by Bug::check_can_change_field(). my $cgi_hash = {'dontchange' => scalar $cgi->param('dontchange')}; foreach my $col (@editable_bug_fields) { + # XXX - Ugly workaround which has to go away before 3.1.3. + next if ($col eq 'assigned_to' || $col eq 'qa_contact'); if (exists $formhash{$col} && !$old_bug_obj->check_can_change_field($col, $oldhash{$col}, $formhash{$col}, \$PrivilegesRequired, $cgi_hash)) @@ -844,11 +701,6 @@ foreach my $id (@idlist) { $vars->{'oldvalue'} = $old_bug_obj->component; $vars->{'newvalue'} = $cgi->param('component'); $vars->{'field'} = 'component'; - } elsif ($col eq 'assigned_to' || $col eq 'qa_contact') { - # Display the assignee or QA contact email address - $vars->{'oldvalue'} = user_id_to_login($oldhash{$col}); - $vars->{'newvalue'} = user_id_to_login($formhash{$col}); - $vars->{'field'} = $col; } else { $vars->{'oldvalue'} = $oldhash{$col}; $vars->{'newvalue'} = $formhash{$col}; @@ -1034,11 +886,6 @@ foreach my $id (@idlist) { $newhash{$col} = $newvalues[$i]; $i++; } - # for passing to Bugzilla::BugMail to ensure that when someone is removed - # from one of these fields, they get notified of that fact (if desired) - # - my $origOwner = ""; - my $origQaContact = ""; # $msgs will store emails which have to be sent to voters, if any. my $msgs; @@ -1051,27 +898,10 @@ foreach my $id (@idlist) { my $new = shift @newvalues; if ($old ne $new) { - # save off the old value for passing to Bugzilla::BugMail so - # the old assignee can be notified - # - if ($col eq 'assigned_to') { - $old = ($old) ? user_id_to_login($old) : ""; - $new = ($new) ? user_id_to_login($new) : ""; - $origOwner = $old; - } - - # ditto for the old qa contact - # - if ($col eq 'qa_contact') { - $old = ($old) ? user_id_to_login($old) : ""; - $new = ($new) ? user_id_to_login($new) : ""; - $origQaContact = $old; - } - # Bugzilla::Bug does these for us already. next if grep($_ eq $col, qw(keywords op_sys rep_platform priority product_id component_id version - target_milestone + target_milestone assigned_to qa_contact bug_severity short_desc alias deadline estimated_time remaining_time reporter_accessible cclist_accessible @@ -1147,10 +977,12 @@ foreach my $id (@idlist) { # all concerned users, including the bug itself, but also the # duplicated bug and dependent bugs, if any. - $vars->{'mailrecipients'} = { 'cc' => $cc_removed, - 'owner' => $origOwner, - 'qacontact' => $origQaContact, - 'changer' => Bugzilla->user->login }; + my $orig_qa = $old_bug_obj->qa_contact; + $vars->{'mailrecipients'} = { + cc => $cc_removed, + owner => $old_bug_obj->assigned_to->login, + qacontact => $orig_qa ? $orig_qa->login : '', + changer => Bugzilla->user->login }; $vars->{'id'} = $id; $vars->{'type'} = "bug"; -- cgit v1.2.1