Skip to content

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

Bug 384651: Make CC adding and removal use Bugzilla::Bug in process_bug

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
parent f32106f8
Hide whitespace changes
Inline Side-by-side
Showing with 119 additions and 88 deletions
Bugzilla/Bug.pm
View file @ ee2eb7de
......@@ -471,6 +471,40 @@ sub update {
return $changes;
}
# XXX Temporary hack until all of process_bug uses update().
sub update_cc {
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_cc = map {$_->id} @{$old_bug->cc_users};
my @new_cc = map {$_->id} @{$self->cc_users};
my ($removed, $added) = diff_arrays(\@old_cc, \@new_cc);
if (scalar @$removed) {
$dbh->do('DELETE FROM cc WHERE bug_id = ? AND who IN (' .
join(',', @$removed) . ')', undef, $self->id);
}
foreach my $user_id (@$added) {
$dbh->do('INSERT INTO cc (bug_id, who) VALUES (?,?)',
undef, $self->id, $user_id);
}
my $removed_users = Bugzilla::User->new_from_list($removed);
my $added_users = Bugzilla::User->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 {$_->login} @$removed_users);
my $added_names = join ', ', (map {$_->login} @$added_users);
LogActivityEntry($self->id, "cc", $removed_names, $added_names,
Bugzilla->user->id, $delta_ts);
}
return ($removed_users, $added_users);
}
# This is the correct way to delete bugs from the DB.
# No bug should be deleted from anywhere else except from here.
#
......@@ -1034,6 +1068,38 @@ sub set_status {
# "Add/Remove" Methods #
########################
# 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 {
# XXX $product is a temporary hack until all of process_bug uses Bug
# objects for updating.
my ($self, $user_or_name, $product) = @_;
return if !$user_or_name;
my $user = ref $user_or_name ? $user_or_name
: Bugzilla::User::check($user_or_name);
my $product_id = $product ? $product->id : $self->{product_id};
if (Bugzilla->params->{strict_isolation}
&& !$user->can_edit_product($product_id))
{
ThrowUserError('invalid_user_group', { users => $user->login,
bug_id => $self->id });
}
my $cc_users = $self->cc_users;
push(@$cc_users, $user) if !grep($_->id == $user->id, @$cc_users);
}
# Accepts a User object or a username. Removes the User if they exist
# in the list, but doesn't throw an error if they don't exist.
sub remove_cc {
my ($self, $user_or_name) = @_;
my $user = ref $user_or_name ? $user_or_name
: Bugzilla::User::check($user_or_name);
my $cc_users = $self->cc_users;
@$cc_users = grep { $_->id != $user->id } @$cc_users;
}
# $bug->add_comment("comment", {isprivate => 1, work_time => 10.5,
# type => CMT_NORMAL, extra_data => $data});
sub add_comment {
......@@ -1185,6 +1251,19 @@ sub cc {
return $self->{'cc'};
}
# XXX Eventually this will become the standard "cc" method used everywhere.
sub cc_users {
my $self = shift;
return $self->{'cc_users'} if exists $self->{'cc_users'};
return [] if $self->{'error'};
my $dbh = Bugzilla->dbh;
my $cc_ids = $dbh->selectcol_arrayref(
'SELECT who FROM cc WHERE bug_id = ?', undef, $self->id);
$self->{'cc_users'} = Bugzilla::User->new_from_list($cc_ids);
return $self->{'cc_users'};
}
sub component {
my ($self) = @_;
return $self->{component} if exists $self->{component};
......
Bugzilla/User.pm
View file @ ee2eb7de
......@@ -131,6 +131,14 @@ sub new {
return $class->SUPER::new(@_);
}
sub check {
my ($username) = @_;
$username = trim($username);
my $user = new Bugzilla::User({ name => $username })
|| ThrowUserError('invalid_username', { name => $username });
return $user;
}
sub update {
my $self = shift;
my $changes = $self->SUPER::update(@_);
......@@ -2072,6 +2080,11 @@ Params: login_name - B<Required> The login name for the new user.
disable_mail - If 1, bug-related mail will not be sent to this user;
if 0, mail will be sent depending on the user's email preferences.
=item C<check>
Takes a username as its only argument. Throws an error if there is no
user with that username. Returns a C<Bugzilla::User> object.
=item C<is_available_username>
Returns a boolean indicating whether or not the supplied username is
......
process_bug.cgi
View file @ ee2eb7de
......@@ -27,6 +27,7 @@
# Frédéric Buclin <LpSolit@gmail.com>
# Lance Larsh <lance.larsh@oracle.com>
# Akamai Technologies <bugzilla-dev@akamai.com>
# Max Kanat-Alexander <mkanat@bugzilla.org>
# Implementation notes for this file:
#
......@@ -107,7 +108,7 @@ sub comment_exists {
# named "id_x" where "x" is the bug number.
# For each bug being modified, make sure its ID is a valid bug number
# representing an existing bug that the user is authorized to access.
my @idlist;
my (@idlist, @bug_objects);
if (defined $cgi->param('id')) {
my $id = $cgi->param('id');
ValidateBugID($id);
......@@ -116,12 +117,15 @@ if (defined $cgi->param('id')) {
# lots of later code will need it, and will obtain it from there
$cgi->param('id', $id);
push @idlist, $id;
push(@bug_objects, new Bugzilla::Bug($id));
} else {
foreach my $i ($cgi->param()) {
if ($i =~ /^id_([1-9][0-9]*)/) {
my $id = $1;
ValidateBugID($id);
push @idlist, $id;
# We do this until we have Bugzilla::Bug->new_from_list.
push(@bug_objects, new Bugzilla::Bug($id));
}
}
}
......@@ -130,7 +134,7 @@ if (defined $cgi->param('id')) {
scalar(@idlist) || ThrowUserError("no_bugs_chosen");
# Build a bug object using the first bug id, for validations.
my $bug = new Bugzilla::Bug($idlist[0]);
my $bug = $bug_objects[0];
# Make sure form param 'dontchange' is defined so it can be compared to easily.
$cgi->param('dontchange','') unless defined $cgi->param('dontchange');
......@@ -613,7 +617,7 @@ if ($cgi->param('product') ne $cgi->param('dontchange')) {
}
my $component;
my (%cc_add, %cc_remove);
my (@cc_add, @cc_remove);
if ($cgi->param('component') ne $cgi->param('dontchange')) {
if (scalar(@newprod_ids) > 1) {
......@@ -634,7 +638,7 @@ if ($cgi->param('component') ne $cgi->param('dontchange')) {
# 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;
push(@cc_add, $cc->login);
}
}
}
......@@ -710,13 +714,15 @@ if ( defined $cgi->param('id') &&
}
}
# 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.
# 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 arrays of
# users involved in this CC change. Then those arrays can be used later
# on for the actual change.
if (defined $cgi->param('newcc')
|| defined $cgi->param('addselfcc')
|| defined $cgi->param('removecc')
|| defined $cgi->param('masscc')) {
# If masscc is defined, then we came from buglist and need to either add or
# remove cc's... otherwise, we came from bugform and may need to do both.
my ($cc_add, $cc_remove) = "";
......@@ -735,23 +741,15 @@ if (defined $cgi->param('newcc')
}
}
if ($cc_add) {
$cc_add =~ s/[\s,]+/ /g; # Change all delimiters to a single space
foreach my $person ( split(" ", $cc_add) ) {
my $pid = login_to_id($person, THROW_ERROR);
$cc_add{$pid} = $person;
}
}
if ($cgi->param('addselfcc')) {
$cc_add{$whoid} = $user->login;
}
if ($cc_remove) {
$cc_remove =~ s/[\s,]+/ /g; # Change all delimiters to a single space
foreach my $person ( split(" ", $cc_remove) ) {
my $pid = login_to_id($person, THROW_ERROR);
$cc_remove{$pid} = $person;
}
}
push(@cc_add, split(/[\s,]+/, $cc_add)) if $cc_add;
push(@cc_add, Bugzilla->user) if $cgi->param('addselfcc');
push(@cc_remove, split(/[\s,]+/, $cc_remove)) if $cc_remove;
}
foreach my $b (@bug_objects) {
$b->remove_cc($_) foreach @cc_remove;
$b->add_cc($_, $product) foreach @cc_add;
}
# Store the new assignee and QA contact IDs (if any). This is the
......@@ -991,25 +989,6 @@ sub LogDependencyActivity {
return 0;
}
if (Bugzilla->params->{"strict_isolation"}) {
my @blocked_cc = ();
foreach my $pid (keys %cc_add) {
$usercache{$pid} ||= Bugzilla::User->new($pid);
my $cc_user = $usercache{$pid};
foreach my $product_id (@newprod_ids) {
if (!$cc_user->can_edit_product($product_id)) {
push (@blocked_cc, $cc_user->login);
last;
}
}
}
if (scalar(@blocked_cc)) {
ThrowUserError("invalid_user_group",
{'users' => \@blocked_cc,
'bug_id' => (scalar(@idlist) > 1) ? undef : $idlist[0]});
}
}
if ($prod_changed && Bugzilla->params->{"strict_isolation"}) {
my $sth_cc = $dbh->prepare("SELECT who
FROM cc
......@@ -1061,6 +1040,8 @@ if ($prod_changed && Bugzilla->params->{"strict_isolation"}) {
}
my %bug_objects = map {$_->id => $_} @bug_objects;
# This loop iterates once for each bug to be processed (i.e. all the
# bugs selected when this script is called with multiple bugs selected
# from buglist.cgi, or just the one bug when called from
......@@ -1432,51 +1413,9 @@ foreach my $id (@idlist) {
$bug_changed = 1;
}
my @ccRemoved = ();
if (defined $cgi->param('newcc')
|| defined $cgi->param('addselfcc')
|| defined $cgi->param('removecc')
|| defined $cgi->param('masscc')) {
# Get the current CC list for this bug
my %oncc;
my $cc_list = $dbh->selectcol_arrayref(
q{SELECT who FROM cc WHERE bug_id = ?}, undef, $id);
foreach my $who (@$cc_list) {
$oncc{$who} = 1;
}
my ($cc_removed) = $bug_objects{$id}->update_cc($timestamp);
$cc_removed = [map {$_->login} @$cc_removed];
my (@added, @removed) = ();
my $sth_insert = $dbh->prepare(q{INSERT INTO cc (bug_id, who)
VALUES (?, ?)});
foreach my $pid (keys %cc_add) {
# If this person isn't already on the cc list, add them
if (! $oncc{$pid}) {
$sth_insert->execute($id, $pid);
push (@added, $cc_add{$pid});
$oncc{$pid} = 1;
}
}
my $sth_delete = $dbh->prepare(q{DELETE FROM cc
WHERE bug_id = ? AND who = ?});
foreach my $pid (keys %cc_remove) {
# If the person is on the cc list, remove them
if ($oncc{$pid}) {
$sth_delete->execute($id, $pid);
push (@removed, $cc_remove{$pid});
$oncc{$pid} = 0;
}
}
# If any changes were found, record it in the activity log
if (scalar(@removed) || scalar(@added)) {
my $removed = join(", ", @removed);
my $added = join(", ", @added);
LogActivityEntry($id,"cc",$removed,$added,$whoid,$timestamp);
$bug_changed = 1;
}
@ccRemoved = @removed;
}
# We need to send mail for dependson/blocked bugs if the dependencies
# change or the status or resolution change. This var keeps track of that.
......@@ -1677,7 +1616,7 @@ foreach my $id (@idlist) {
# all concerned users, including the bug itself, but also the
# duplicated bug and dependent bugs, if any.
$vars->{'mailrecipients'} = { 'cc' => \@ccRemoved,
$vars->{'mailrecipients'} = { 'cc' => $cc_removed,
'owner' => $origOwner,
'qacontact' => $origQaContact,
'changer' => Bugzilla->user->login };
......
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