diff options
| -rwxr-xr-x | Bugzilla/Bug.pm | 160 | ||||
| -rwxr-xr-x | process_bug.cgi | 100 |
2 files changed, 140 insertions, 120 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 56018cebd..10c595d94 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -388,7 +388,7 @@ sub run_create_validators { $params->{version} = $class->_check_version($product, $params->{version}); - $params->{keywords} = $class->_check_keywords($product, $params->{keywords}); + $params->{keywords} = $class->_check_keywords($params->{keywords}, $product); $params->{groups} = $class->_check_groups($product, $params->{groups}); @@ -496,8 +496,8 @@ sub update_cc { # If any changes were found, record it in the activity log if (scalar @$removed || scalar @$added) { - my $removed_names = join ', ', (map {$_->login} @$removed_users); - my $added_names = join ', ', (map {$_->login} @$added_users); + my $removed_names = join(', ', (map {$_->login} @$removed_users)); + my $added_names = join(', ', (map {$_->login} @$added_users)); LogActivityEntry($self->id, "cc", $removed_names, $added_names, Bugzilla->user->id, $delta_ts); } @@ -505,6 +505,45 @@ sub update_cc { return ($removed_users, $added_users); } +# XXX Temporary hack until all of process_bug uses update() +sub update_keywords { + my $self = shift; + + my $dbh = Bugzilla->dbh; + my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()"); + + my $old_bug = $self->new($self->id); + my @old_ids = map { $_->id } @{$old_bug->keyword_objects}; + my @new_ids = map { $_->id } @{$self->keyword_objects}; + + my ($removed, $added) = diff_arrays(\@old_ids, \@new_ids); + + if (scalar @$removed) { + $dbh->do('DELETE FROM keywords WHERE bug_id = ? AND keywordid IN (' + . join(',', @$removed) . ')', undef, $self->id); + } + foreach my $keyword_id (@$added) { + $dbh->do('INSERT INTO keywords (bug_id, keywordid) VALUES (?,?)', + undef, $self->id, $keyword_id); + } + + $dbh->do('UPDATE bugs SET keywords = ? WHERE bug_id = ?', undef, + $self->keywords, $self->id); + + my $removed_keywords = Bugzilla::Keyword->new_from_list($removed); + my $added_keywords = Bugzilla::Keyword->new_from_list($added); + + # If any changes were found, record it in the activity log + if (scalar @$removed || scalar @$added) { + my $removed_names = join(', ', (map {$_->name} @$removed_keywords)); + my $added_names = join(', ', (map {$_->name} @$added_keywords)); + LogActivityEntry($self->id, "keywords", $removed_names, + $added_names, Bugzilla->user->id, $delta_ts); + } + + return [$removed_keywords, $added_keywords]; +} + # This is the correct way to delete bugs from the DB. # No bug should be deleted from anywhere else except from here. # @@ -626,9 +665,7 @@ sub _check_bug_status { my @valid_statuses; if (ref $invocant) { - $invocant->{'prod_obj'} ||= - new Bugzilla::Product({name => $invocant->product}); - $product = $invocant->{'prod_obj'}; + $product = $invocant->product_obj; @valid_statuses = map { $_->name } @{$invocant->status->can_change_to}; } else { @@ -846,11 +883,15 @@ sub _check_groups { } sub _check_keywords { - my ($invocant, $product, $keyword_string) = @_; + my ($invocant, $keyword_string, $product) = @_; $keyword_string = trim($keyword_string); - return [] if (!$keyword_string - || !Bugzilla->user->in_group('editbugs', $product->id)); - + return [] if !$keyword_string; + + # On creation, only editbugs users can set keywords. + if (!ref $invocant) { + return [] if !Bugzilla->user->in_group('editbugs', $product->id); + } + my %keywords; foreach my $keyword (split(/[\s,]+/, $keyword_string)) { next unless $keyword; @@ -1068,6 +1109,8 @@ sub set_status { # "Add/Remove" Methods # ######################## +# These are in alphabetical order by field name. + # Accepts a User object or a username. Adds the user only if they # don't already exist as a CC on the bug. sub add_cc { @@ -1142,6 +1185,57 @@ sub add_comment { push(@{$self->{added_comments}}, $add_comment); } +# There was a lot of duplicate code when I wrote this as three separate +# functions, so I just combined them all into one. This is also easier for +# process_bug to use. +sub modify_keywords { + my ($self, $keywords, $action) = @_; + + $action ||= "makeexact"; + if (!grep($action eq $_, qw(add delete makeexact))) { + $action = "makeexact"; + } + + $keywords = $self->_check_keywords($keywords); + + my (@result, $any_changes); + if ($action eq 'makeexact') { + @result = @$keywords; + # Check if anything was added or removed. + my @old_ids = map { $_->id } @{$self->keyword_objects}; + my @new_ids = map { $_->id } @result; + my ($removed, $added) = diff_arrays(\@old_ids, \@new_ids); + $any_changes = scalar @$removed || scalar @$added; + } + else { + # We're adding or deleting specific keywords. + my %keys = map {$_->id => $_} @{$self->keyword_objects}; + if ($action eq 'add') { + $keys{$_->id} = $_ foreach @$keywords; + } + else { + delete $keys{$_->id} foreach @$keywords; + } + @result = values %keys; + $any_changes = scalar @$keywords; + } + # Make sure we retain the sort order. + @result = sort {lc($a->name) cmp lc($b->name)} @result; + + if ($any_changes) { + my $privs; + my $new = join(', ', (map {$_->name} @result)); + my $check = $self->check_can_change_field('keywords', 0, 1, \$privs) + || ThrowUserError('illegal_change', { field => 'keywords', + oldvalue => $self->keywords, + newvalue => $new, + privs => $privs }); + } + + $self->{'keyword_objects'} = \@result; + return $any_changes; +} + ##################################################################### # Instance Accessors ##################################################################### @@ -1339,20 +1433,20 @@ sub isunconfirmed { sub keywords { my ($self) = @_; - return $self->{'keywords'} if exists $self->{'keywords'}; - return () if $self->{'error'}; + return join(', ', (map { $_->name } @{$self->keyword_objects})); +} - my $dbh = Bugzilla->dbh; - my $list_ref = $dbh->selectcol_arrayref( - "SELECT keyworddefs.name - FROM keyworddefs, keywords - WHERE keywords.bug_id = ? - AND keyworddefs.id = keywords.keywordid - ORDER BY keyworddefs.name", - undef, ($self->bug_id)); +# XXX At some point, this should probably replace the normal "keywords" sub. +sub keyword_objects { + my $self = shift; + return $self->{'keyword_objects'} if defined $self->{'keyword_objects'}; + return [] if $self->{'error'}; - $self->{'keywords'} = join(', ', @$list_ref); - return $self->{'keywords'}; + my $dbh = Bugzilla->dbh; + my $ids = $dbh->selectcol_arrayref( + "SELECT keywordid FROM keywords WHERE bug_id = ?", undef, $self->id); + $self->{'keyword_objects'} = Bugzilla::Keyword->new_from_list($ids); + return $self->{'keyword_objects'}; } sub longdescs { @@ -1368,8 +1462,7 @@ sub milestoneurl { return $self->{'milestoneurl'} if exists $self->{'milestoneurl'}; return '' if $self->{'error'}; - $self->{'prod_obj'} ||= new Bugzilla::Product({name => $self->product}); - $self->{'milestoneurl'} = $self->{'prod_obj'}->milestone_url; + $self->{'milestoneurl'} = $self->product_obj->milestone_url; return $self->{'milestoneurl'}; } @@ -1383,6 +1476,14 @@ sub product { return $self->{product}; } +# XXX This should eventually replace the "product" subroutine. +sub product_obj { + my $self = shift; + return {} if $self->{error}; + $self->{prod_obj} ||= new Bugzilla::Product($self->{product_id}); + return $self->{prod_obj}; +} + sub qa_contact { my ($self) = @_; return $self->{'qa_contact'} if exists $self->{'qa_contact'}; @@ -1443,10 +1544,8 @@ sub use_votes { my ($self) = @_; return 0 if $self->{'error'}; - $self->{'prod_obj'} ||= new Bugzilla::Product({name => $self->product}); - return Bugzilla->params->{'usevotes'} - && $self->{'prod_obj'}->votes_per_user > 0; + && $self->product_obj->votes_per_user > 0; } sub groups { @@ -1549,7 +1648,6 @@ sub choices { return {} if $self->{'error'}; $self->{'choices'} = {}; - $self->{prod_obj} ||= new Bugzilla::Product({name => $self->product}); my @prodlist = map {$_->name} @{Bugzilla->user->get_enterable_products}; # The current product is part of the popup, even if new bugs are no longer @@ -1571,9 +1669,9 @@ sub choices { 'op_sys' => get_legal_field_values('op_sys'), 'bug_status' => get_legal_field_values('bug_status'), 'resolution' => \@res, - 'component' => [map($_->name, @{$self->{prod_obj}->components})], - 'version' => [map($_->name, @{$self->{prod_obj}->versions})], - 'target_milestone' => [map($_->name, @{$self->{prod_obj}->milestones})], + 'component' => [map($_->name, @{$self->product_obj->components})], + 'version' => [map($_->name, @{$self->product_obj->versions})], + 'target_milestone' => [map($_->name, @{$self->product_obj->milestones})], }; return $self->{'choices'}; diff --git a/process_bug.cgi b/process_bug.cgi index 4b5d0ec54..253de665f 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -882,34 +882,18 @@ $requiremilestone = $vars->{requiremilestone}; DuplicateUserConfirm($bug, $duplicate) if $vars->{DuplicateUserConfirm}; _remove_remaining_time() if $vars->{remove_remaining_time}; -my @keywordlist; -my %keywordseen; - +my $any_keyword_changes; if (defined $cgi->param('keywords')) { - foreach my $keyword (split(/[\s,]+/, $cgi->param('keywords'))) { - if ($keyword eq '') { - next; - } - my $keyword_obj = new Bugzilla::Keyword({name => $keyword}); - if (!$keyword_obj) { - ThrowUserError("unknown_keyword", - { keyword => $keyword }); - } - if (!$keywordseen{$keyword_obj->id}) { - push(@keywordlist, $keyword_obj->id); - $keywordseen{$keyword_obj->id} = 1; - } + foreach my $b (@bug_objects) { + my $return = + $b->modify_keywords(scalar $cgi->param('keywords'), + scalar $cgi->param('keywordaction')); + $any_keyword_changes ||= $return; } } -my $keywordaction = $cgi->param('keywordaction') || "makeexact"; -if (!grep($keywordaction eq $_, qw(add delete makeexact))) { - $keywordaction = "makeexact"; -} - if ($::comma eq "" - && (!Bugzilla::Keyword::keyword_count() - || (0 == @keywordlist && $keywordaction ne "makeexact")) + && !$any_keyword_changes && defined $cgi->param('masscc') && ! $cgi->param('masscc') ) { if (!defined $cgi->param('comment') || $cgi->param('comment') =~ /^\s*$/) { @@ -1184,25 +1168,6 @@ foreach my $id (@idlist) { } } - # When editing multiple bugs, users can specify a list of keywords to delete - # from bugs. If the list matches the current set of keywords on those bugs, - # Bug::check_can_change_field will fail to check permissions because it thinks - # the list hasn't changed. To fix that, we have to call Bug::check_can_change_field - # again with old!=new if the keyword action is "delete" and old=new. - if ($keywordaction eq "delete" - && defined $cgi->param('keywords') - && length(@keywordlist) > 0 - && $cgi->param('keywords') eq $oldhash{keywords} - && !$old_bug_obj->check_can_change_field("keywords", "old is not", "equal to new", - \$PrivilegesRequired)) - { - $vars->{'oldvalue'} = $oldhash{keywords}; - $vars->{'newvalue'} = "no keywords"; - $vars->{'field'} = "keywords"; - $vars->{'privs'} = $PrivilegesRequired; - ThrowUserError("illegal_change", $vars); - } - $oldhash{'product'} = $old_bug_obj->product; if (!Bugzilla->user->can_edit_product($oldhash{'product_id'})) { ThrowUserError("product_edit_denied", @@ -1287,49 +1252,8 @@ foreach my $id (@idlist) { $bug_changed = 1; } - if (Bugzilla::Keyword::keyword_count() - && defined $cgi->param('keywords')) - { - # There are three kinds of "keywordsaction": makeexact, add, delete. - # For makeexact, we delete everything, and then add our things. - # For add, we delete things we're adding (to make sure we don't - # end up having them twice), and then we add them. - # For delete, we just delete things on the list. - my $changed = 0; - if ($keywordaction eq "makeexact") { - $dbh->do(q{DELETE FROM keywords WHERE bug_id = ?}, - undef, $id); - $changed = 1; - } - my $sth_delete = $dbh->prepare(q{DELETE FROM keywords - WHERE bug_id = ? - AND keywordid = ?}); - my $sth_insert = - $dbh->prepare(q{INSERT INTO keywords (bug_id, keywordid) - VALUES (?, ?)}); - foreach my $keyword (@keywordlist) { - if ($keywordaction ne "makeexact") { - $sth_delete->execute($id, $keyword); - $changed = 1; - } - if ($keywordaction ne "delete") { - $sth_insert->execute($id, $keyword); - $changed = 1; - } - } - if ($changed) { - my $list = $dbh->selectcol_arrayref( - q{SELECT keyworddefs.name - FROM keyworddefs - INNER JOIN keywords - ON keyworddefs.id = keywords.keywordid - WHERE keywords.bug_id = ? - ORDER BY keyworddefs.name}, - undef, $id); - $dbh->do("UPDATE bugs SET keywords = ? WHERE bug_id = ?", - undef, join(', ', @$list), $id); - } - } + $bug_objects{$id}->update_keywords($timestamp); + $query .= " WHERE bug_id = ?"; push(@bug_values, $id); @@ -1546,10 +1470,8 @@ foreach my $id (@idlist) { $origQaContact = $old; } - # If this is the keyword field, only record the changes, not everything. - if ($col eq 'keywords') { - ($old, $new) = diff_strings($old, $new); - } + # update_keywords does this for us already. + next if ($col eq 'keywords'); if ($col eq 'product') { # If some votes have been removed, RemoveVotes() returns |