Skip to content

bugzilla
bugzilla
Commit 09b34ef6 authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org
Browse files
Options

Bug 384664: Make keywords use Bugzilla::Bug in process_bug

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
parent ef86c422
Show whitespace changes
Inline Side-by-side
Showing with 139 additions and 119 deletions
Bugzilla/Bug.pm
View file @ 09b34ef6
......@@ -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,10 +883,14 @@ 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)) {
......@@ -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'};
......
process_bug.cgi
View file @ 09b34ef6
......@@ -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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment