summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorlpsolit%gmail.com <>2009-08-05 14:35:50 +0200
committerlpsolit%gmail.com <>2009-08-05 14:35:50 +0200
commitd0002e9626b97df6fad2c597b89c8ec31f7c308a (patch)
treeb14fd7ecb63ad9931b5c72e9c666ef499f0daa1d
parent8b2db148f30d74283d3a80ebd77691c94a1ca4a7 (diff)
downloadbugzilla-d0002e9626b97df6fad2c597b89c8ec31f7c308a.tar.gz
bugzilla-d0002e9626b97df6fad2c597b89c8ec31f7c308a.tar.xz
Bug 415541: Implement $bug->set_flags() and $attachment->set_flags() - Patch by Frédéric Buclin <LpSolit@gmail.com> a=LpSolit
-rw-r--r--Bugzilla/Attachment.pm58
-rw-r--r--Bugzilla/Bug.pm27
-rw-r--r--Bugzilla/Flag.pm1245
-rw-r--r--Bugzilla/Hook.pm4
-rwxr-xr-xattachment.cgi49
-rwxr-xr-xeditflagtypes.cgi12
-rwxr-xr-xeditusers.cgi62
-rw-r--r--extensions/example/code/flag-end_of_update.pl6
-rwxr-xr-xpost_bug.cgi12
-rwxr-xr-xprocess_bug.cgi20
-rw-r--r--template/en/default/global/code-error.html.tmpl13
-rw-r--r--template/en/default/global/user-error.html.tmpl22
-rw-r--r--template/en/default/request/email.txt.tmpl19
13 files changed, 692 insertions, 857 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 9748d17f4..752ddce9a 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -101,7 +101,6 @@ use constant UPDATE_COLUMNS => qw(
ispatch
isprivate
mimetype
- modification_time
);
use constant VALIDATORS => {
@@ -445,9 +444,9 @@ flags that have been set on the attachment
sub flags {
my $self = shift;
- return $self->{flags} if exists $self->{flags};
- $self->{flags} = Bugzilla::Flag->match({ 'attach_id' => $self->id });
+ # Don't cache it as it must be in sync with ->flag_types.
+ $self->{flags} = [map { @{$_->{flags}} } @{$self->flag_types}];
return $self->{flags};
}
@@ -471,7 +470,7 @@ sub flag_types {
component_id => $self->bug->component_id,
attach_id => $self->id };
- $self->{flag_types} = Bugzilla::Flag::_flag_types($vars);
+ $self->{flag_types} = Bugzilla::Flag->_flag_types($vars);
return $self->{flag_types};
}
@@ -482,10 +481,34 @@ sub flag_types {
sub set_content_type { $_[0]->set('mimetype', $_[1]); }
sub set_description { $_[0]->set('description', $_[1]); }
sub set_filename { $_[0]->set('filename', $_[1]); }
-sub set_is_obsolete { $_[0]->set('isobsolete', $_[1]); }
sub set_is_patch { $_[0]->set('ispatch', $_[1]); }
sub set_is_private { $_[0]->set('isprivate', $_[1]); }
+sub set_is_obsolete {
+ my ($self, $obsolete) = @_;
+
+ my $old = $self->isobsolete;
+ $self->set('isobsolete', $obsolete);
+ my $new = $self->isobsolete;
+
+ # If the attachment is being marked as obsolete, cancel pending requests.
+ if ($new && $old != $new) {
+ my @requests = grep { $_->status eq '?' } @{$self->flags};
+ return unless scalar @requests;
+
+ my %flag_ids = map { $_->id => 1 } @requests;
+ foreach my $flagtype (@{$self->flag_types}) {
+ @{$flagtype->{flags}} = grep { !$flag_ids{$_->id} } @{$flagtype->{flags}};
+ }
+ }
+}
+
+sub set_flags {
+ my ($self, $flags, $new_flags) = @_;
+
+ Bugzilla::Flag->set_flag($self, $_) foreach (@$flags, @$new_flags);
+}
+
sub _check_bug {
my ($invocant, $bug) = @_;
my $user = Bugzilla->user;
@@ -799,7 +822,7 @@ Params: takes a hashref with the following keys:
parameter has no effect.
C<mimetype> - string - a valid MIME type.
C<creation_ts> - string (optional) - timestamp of the insert
- as returned by SELECT NOW().
+ as returned by SELECT LOCALTIMESTAMP(0).
C<ispatch> - boolean (optional, default false) - true if the
attachment is a patch.
C<isprivate> - boolean (optional, default false) - true if
@@ -887,7 +910,7 @@ sub run_create_validators {
$params->{ispatch} = $params->{ispatch} ? 1 : 0;
$params->{filename} = $class->_check_filename($params->{filename}, $params->{isurl});
$params->{mimetype} = $class->_check_content_type($params->{mimetype});
- $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
+ $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
$params->{modification_time} = $params->{creation_ts};
$params->{submitter_id} = Bugzilla->user->id || ThrowCodeError('invalid_user');
@@ -898,14 +921,14 @@ sub update {
my $self = shift;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user;
- my $bug = $self->bug;
-
- my $timestamp = shift || $dbh->selectrow_array('SELECT NOW()');
- $self->{modification_time} = $timestamp;
+ my $timestamp = shift || $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
my ($changes, $old_self) = $self->SUPER::update(@_);
- # Ignore this change.
- delete $changes->{modification_time};
+
+ my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_self, $timestamp);
+ if ($removed || $added) {
+ $changes->{'flagtypes.name'} = [$removed, $added];
+ }
# Record changes in the activity table.
my $sth = $dbh->prepare('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
@@ -914,14 +937,17 @@ sub update {
foreach my $field (keys %$changes) {
my $change = $changes->{$field};
- my $fieldid = get_field_id("attachments.$field");
- $sth->execute($bug->id, $self->id, $user->id, $timestamp,
+ $field = "attachments.$field" unless $field eq "flagtypes.name";
+ my $fieldid = get_field_id($field);
+ $sth->execute($self->bug_id, $self->id, $user->id, $timestamp,
$fieldid, $change->[0], $change->[1]);
}
if (scalar(keys %$changes)) {
+ $dbh->do('UPDATE attachments SET modification_time = ? WHERE attach_id = ?',
+ undef, ($timestamp, $self->id));
$dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
- undef, $timestamp, $bug->id);
+ undef, ($timestamp, $self->bug_id));
}
return $changes;
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 64a53b8a1..64627f4e9 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -590,7 +590,7 @@ sub run_create_validators {
# Callers cannot set Reporter, currently.
$params->{reporter} = $class->_check_reporter();
- $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
+ $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
$params->{delta_ts} = $params->{creation_ts};
if ($params->{estimated_time}) {
@@ -646,7 +646,7 @@ sub update {
my $dbh = Bugzilla->dbh;
# XXX This is just a temporary hack until all updating happens
# inside this function.
- my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()");
+ my $delta_ts = shift || $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
my ($changes, $old_bug) = $self->SUPER::update(@_);
@@ -774,7 +774,13 @@ sub update {
$changes->{'bug_group'} = [join(', ', @removed_names),
join(', ', @added_names)];
}
-
+
+ # Flags
+ my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_bug, $delta_ts);
+ if ($removed || $added) {
+ $changes->{'flagtypes.name'} = [$removed, $added];
+ }
+
# Comments
foreach my $comment (@{$self->{added_comments} || []}) {
my $columns = join(',', keys %$comment);
@@ -1931,6 +1937,11 @@ sub set_dup_id {
}
sub set_estimated_time { $_[0]->set('estimated_time', $_[1]); }
sub _set_everconfirmed { $_[0]->set('everconfirmed', $_[1]); }
+sub set_flags {
+ my ($self, $flags, $new_flags) = @_;
+
+ Bugzilla::Flag->set_flag($self, $_) foreach (@$flags, @$new_flags);
+}
sub set_op_sys { $_[0]->set('op_sys', $_[1]); }
sub set_platform { $_[0]->set('rep_platform', $_[1]); }
sub set_priority { $_[0]->set('priority', $_[1]); }
@@ -2632,10 +2643,18 @@ sub flag_types {
component_id => $self->{component_id},
bug_id => $self->bug_id };
- $self->{'flag_types'} = Bugzilla::Flag::_flag_types($vars);
+ $self->{'flag_types'} = Bugzilla::Flag->_flag_types($vars);
return $self->{'flag_types'};
}
+sub flags {
+ my $self = shift;
+
+ # Don't cache it as it must be in sync with ->flag_types.
+ $self->{flags} = [map { @{$_->{flags}} } @{$self->flag_types}];
+ return $self->{flags};
+}
+
sub isopened {
my $self = shift;
return is_open_state($self->{bug_status}) ? 1 : 0;
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 66c392198..6efe0b431 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -53,6 +53,8 @@ whose names start with _ or a re specifically noted as being private.
=cut
+use Scalar::Util qw(blessed);
+
use Bugzilla::FlagType;
use Bugzilla::Hook;
use Bugzilla::User;
@@ -69,21 +71,44 @@ use base qw(Bugzilla::Object Exporter);
#### Initialization ####
###############################
-use constant DB_COLUMNS => qw(
- flags.id
- flags.type_id
- flags.bug_id
- flags.attach_id
- flags.requestee_id
- flags.setter_id
- flags.status
-);
-
use constant DB_TABLE => 'flags';
use constant LIST_ORDER => 'id';
use constant SKIP_REQUESTEE_ON_ERROR => 1;
+use constant DB_COLUMNS => qw(
+ id
+ type_id
+ bug_id
+ attach_id
+ requestee_id
+ setter_id
+ status
+);
+
+use constant REQUIRED_CREATE_FIELDS => qw(
+ attach_id
+ bug_id
+ setter_id
+ status
+ type_id
+);
+
+use constant UPDATE_COLUMNS => qw(
+ requestee_id
+ setter_id
+ status
+ type_id
+);
+
+use constant VALIDATORS => {
+};
+
+use constant UPDATE_VALIDATORS => {
+ setter => \&_check_setter,
+ status => \&_check_status,
+};
+
###############################
#### Accessors ######
###############################
@@ -116,11 +141,14 @@ Returns the status '+', '-', '?' of the flag.
=cut
-sub id { return $_[0]->{'id'}; }
-sub name { return $_[0]->type->name; }
-sub bug_id { return $_[0]->{'bug_id'}; }
-sub attach_id { return $_[0]->{'attach_id'}; }
-sub status { return $_[0]->{'status'}; }
+sub id { return $_[0]->{'id'}; }
+sub name { return $_[0]->type->name; }
+sub type_id { return $_[0]->{'type_id'}; }
+sub bug_id { return $_[0]->{'bug_id'}; }
+sub attach_id { return $_[0]->{'attach_id'}; }
+sub status { return $_[0]->{'status'}; }
+sub setter_id { return $_[0]->{'setter_id'}; }
+sub requestee_id { return $_[0]->{'requestee_id'}; }
###############################
#### Methods ####
@@ -184,6 +212,14 @@ sub attachment {
return $self->{'attachment'};
}
+sub bug {
+ my $self = shift;
+
+ require Bugzilla::Bug;
+ $self->{'bug'} ||= new Bugzilla::Bug($self->bug_id);
+ return $self->{'bug'};
+}
+
################################
## Searching/Retrieving Flags ##
################################
@@ -268,251 +304,171 @@ sub count {
# Creating and Modifying
######################################################################
-=pod
-
-=over
-
-=item C<validate($bug_id, $attach_id, $skip_requestee_on_error)>
-
-Validates fields containing flag modifications.
-
-If the attachment is new, it has no ID yet and $attach_id is set
-to -1 to force its check anyway.
-
-=back
-
-=cut
-
-sub validate {
- my ($bug_id, $attach_id, $skip_requestee_on_error) = @_;
- my $cgi = Bugzilla->cgi;
- my $dbh = Bugzilla->dbh;
-
- # Get a list of flags to validate. Uses the "map" function
- # to extract flag IDs from form field names by matching fields
- # whose name looks like "flag_type-nnn" (new flags) or "flag-nnn"
- # (existing flags), where "nnn" is the ID, and returning just
- # the ID portion of matching field names.
- my @flagtype_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
- my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
-
- return unless (scalar(@flagtype_ids) || scalar(@flag_ids));
-
- # No flag reference should exist when changing several bugs at once.
- ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
+sub set_flag {
+ my ($class, $obj, $params) = @_;
- # We don't check that these new flags are valid for this bug/attachment,
- # because the bug may be moved into another product meanwhile.
- # This check will be done later when creating new flags, see FormToNewFlags().
+ my ($bug, $attachment);
+ if (blessed($obj) && $obj->isa('Bugzilla::Attachment')) {
+ $attachment = $obj;
+ $bug = $attachment->bug;
+ }
+ elsif (blessed($obj) && $obj->isa('Bugzilla::Bug')) {
+ $bug = $obj;
+ }
+ else {
+ ThrowCodeError('flag_unexpected_object', { 'caller' => ref $obj });
+ }
- if (scalar(@flag_ids)) {
- # No reference to existing flags should exist when creating a new
- # attachment.
- if ($attach_id && ($attach_id < 0)) {
- ThrowCodeError('flags_not_available', { type => 'a' });
+ # Update (or delete) an existing flag.
+ if ($params->{id}) {
+ my $flag = $class->check({ id => $params->{id} });
+
+ # Security check: make sure the flag belongs to the bug/attachment.
+ # We don't check that the user editing the flag can see
+ # the bug/attachment. That's the job of the caller.
+ ($attachment && $flag->attach_id && $attachment->id == $flag->attach_id)
+ || (!$attachment && !$flag->attach_id && $bug->id == $flag->bug_id)
+ || ThrowCodeError('invalid_flag_association',
+ { bug_id => $bug->id,
+ attach_id => $attachment ? $attachment->id : undef });
+
+ # Extract the current flag object from the object.
+ my ($obj_flagtype) = grep { $_->id == $flag->type_id } @{$obj->flag_types};
+ # If no flagtype can be found for this flag, this means the bug is being
+ # moved into a product/component where the flag is no longer valid.
+ # So either we can attach the flag to another flagtype having the same
+ # name, or we remove the flag.
+ if (!$obj_flagtype) {
+ my $success = $flag->retarget($obj);
+ return unless $success;
+
+ ($obj_flagtype) = grep { $_->id == $flag->type_id } @{$obj->flag_types};
+ push(@{$obj_flagtype->{flags}}, $flag);
}
+ my ($obj_flag) = grep { $_->id == $flag->id } @{$obj_flagtype->{flags}};
+ # If the flag has the correct type but cannot be found above, this means
+ # the flag is going to be removed (e.g. because this is a pending request
+ # and the attachment is being marked as obsolete).
+ return unless $obj_flag;
- # Make sure all existing flags belong to the bug/attachment
- # they pretend to be.
- my $field = ($attach_id) ? "attach_id" : "bug_id";
- my $field_id = $attach_id || $bug_id;
- my $not = ($attach_id) ? "" : "NOT";
-
- my $invalid_data =
- $dbh->selectrow_array(
- "SELECT 1 FROM flags
- WHERE "
- . $dbh->sql_in('id', \@flag_ids)
- . " AND ($field != ? OR attach_id IS $not NULL) "
- . $dbh->sql_limit(1), undef, $field_id);
-
- if ($invalid_data) {
- ThrowCodeError('invalid_flag_association',
- { bug_id => $bug_id,
- attach_id => $attach_id });
- }
+ $class->_validate($obj_flag, $obj_flagtype, $params, $bug, $attachment);
}
-
- # Validate new flags.
- foreach my $id (@flagtype_ids) {
- my $status = $cgi->param("flag_type-$id");
- my @requestees = $cgi->param("requestee_type-$id");
- my $private_attachment = $cgi->param('isprivate') ? 1 : 0;
-
+ # Create a new flag.
+ elsif ($params->{type_id}) {
# Don't bother validating types the user didn't touch.
- next if $status eq 'X';
-
- # Make sure the flag type exists. If it doesn't, FormToNewFlags()
- # will ignore it, so it's safe to ignore it here.
- my $flag_type = new Bugzilla::FlagType($id);
- next unless $flag_type;
+ return if $params->{status} eq 'X';
+
+ my $flagtype = Bugzilla::FlagType->check({ id => $params->{type_id} });
+ # Security check: make sure the flag type belongs to the bug/attachment.
+ ($attachment && $flagtype->target_type eq 'attachment'
+ && scalar(grep { $_->id == $flagtype->id } @{$attachment->flag_types}))
+ || (!$attachment && $flagtype->target_type eq 'bug'
+ && scalar(grep { $_->id == $flagtype->id } @{$bug->flag_types}))
+ || ThrowCodeError('invalid_flag_association',
+ { bug_id => $bug->id,
+ attach_id => $attachment ? $attachment->id : undef });
# Make sure the flag type is active.
- unless ($flag_type->is_active) {
- ThrowCodeError('flag_type_inactive', {'type' => $flag_type->name});
- }
-
- _validate(undef, $flag_type, $status, undef, \@requestees, $private_attachment,
- $bug_id, $attach_id, $skip_requestee_on_error);
- }
+ $flagtype->is_active
+ || ThrowCodeError('flag_type_inactive', { type => $flagtype->name });
- # Validate existing flags.
- foreach my $id (@flag_ids) {
- my $status = $cgi->param("flag-$id");
- my @requestees = $cgi->param("requestee-$id");
- my $private_attachment = $cgi->param('isprivate') ? 1 : 0;
+ # Extract the current flagtype object from the object.
+ my ($obj_flagtype) = grep { $_->id == $flagtype->id } @{$obj->flag_types};
- # Make sure the flag exists. If it doesn't, process() will ignore it,
- # so it's safe to ignore it here.
- my $flag = new Bugzilla::Flag($id);
- next unless $flag;
+ # We cannot create a new flag if there is already one and this
+ # flag type is not multiplicable.
+ if (!$flagtype->is_multiplicable) {
+ if (scalar @{$obj_flagtype->{flags}}) {
+ ThrowUserError('flag_type_not_multiplicable', { type => $flagtype });
+ }
+ }
- _validate($flag, $flag->type, $status, undef, \@requestees, $private_attachment,
- undef, undef, $skip_requestee_on_error);
+ $class->_validate(undef, $obj_flagtype, $params, $bug, $attachment);
+ }
+ else {
+ ThrowCodeError('param_required', { function => $class . '->set_flag',
+ param => 'id/type_id' });
}
}
sub _validate {
- my ($flag, $flag_type, $status, $setter, $requestees, $private_attachment,
- $bug_id, $attach_id, $skip_requestee_on_error) = @_;
-
- # By default, the flag setter (or requester) is the current user.
- $setter ||= Bugzilla->user;
-
- my $id = $flag ? $flag->id : $flag_type->id; # Used in the error messages below.
- $bug_id ||= $flag->bug_id;
- $attach_id ||= $flag->attach_id if $flag; # Maybe it's a bug flag.
-
- # Make sure the user chose a valid status.
- grep($status eq $_, qw(X + - ?))
- || ThrowCodeError('flag_status_invalid',
- { id => $id, status => $status });
-
- # Make sure the user didn't request the flag unless it's requestable.
- # If the flag existed and was requested before it became unrequestable,
- # leave it as is.
- if ($status eq '?'
- && (!$flag || $flag->status ne '?')
- && !$flag_type->is_requestable)
+ my ($class, $flag, $flag_type, $params, $bug, $attachment) = @_;
+
+ # If it's a new flag, let's create it now.
+ my $obj_flag = $flag || bless({ type_id => $flag_type->id,
+ status => '',
+ bug_id => $bug->id,
+ attach_id => $attachment ?
+ $attachment->id : undef},
+ $class);
+
+ my $old_status = $obj_flag->status;
+ my $old_requestee_id = $obj_flag->requestee_id;
+
+ $obj_flag->_set_status($params->{status});
+ $obj_flag->_set_requestee($params->{requestee}, $attachment, $params->{skip_roe});
+
+ # The setter field MUST NOT be updated if neither the status
+ # nor the requestee fields changed.
+ if (($obj_flag->status ne $old_status)
+ # The requestee ID can be undefined.
+ || (($obj_flag->requestee_id || 0) != ($old_requestee_id || 0)))
{
- ThrowCodeError('flag_status_invalid',
- { id => $id, status => $status });
+ $obj_flag->_set_setter($params->{setter});
}
- # Make sure the user didn't specify a requestee unless the flag
- # is specifically requestable. For existing flags, if the requestee
- # was set before the flag became specifically unrequestable, don't
- # let the user change the requestee, but let the user remove it by
- # entering an empty string for the requestee.
- if ($status eq '?' && !$flag_type->is_requesteeble) {
- my $old_requestee = ($flag && $flag->requestee) ?
- $flag->requestee->login : '';
- my $new_requestee = join('', @$requestees);
- if ($new_requestee && $new_requestee ne $old_requestee) {
- ThrowCodeError('flag_requestee_disabled',
- { type => $flag_type });
- }
+ # If the flag is deleted, remove it from the list.
+ if ($obj_flag->status eq 'X') {
+ @{$flag_type->{flags}} = grep { $_->id != $obj_flag->id } @{$flag_type->{flags}};
}
-
- # Make sure the user didn't enter multiple requestees for a flag
- # that can't be requested from more than one person at a time.
- if ($status eq '?'
- && !$flag_type->is_multiplicable
- && scalar(@$requestees) > 1)
- {
- ThrowUserError('flag_not_multiplicable', { type => $flag_type });
+ # Add the newly created flag to the list.
+ elsif (!$obj_flag->id) {
+ push(@{$flag_type->{flags}}, $obj_flag);
}
+}
- # Make sure the requestees are authorized to access the bug
- # (and attachment, if this installation is using the "insider group"
- # feature and the attachment is marked private).
- if ($status eq '?' && $flag_type->is_requesteeble) {
- my $old_requestee = ($flag && $flag->requestee) ?
- $flag->requestee->login : '';
-
- my @legal_requestees;
- foreach my $login (@$requestees) {
- if ($login eq $old_requestee) {
- # This requestee was already set. Leave him alone.
- push(@legal_requestees, $login);
- next;
- }
+=pod
- # We know the requestee exists because we ran
- # Bugzilla::User::match_field before getting here.
- my $requestee = new Bugzilla::User({ name => $login });
+=over
- # Throw an error if the user can't see the bug.
- # Note that if permissions on this bug are changed,
- # can_see_bug() will refer to old settings.
- if (!$requestee->can_see_bug($bug_id)) {
- next if $skip_requestee_on_error;
- ThrowUserError('flag_requestee_unauthorized',
- { flag_type => $flag_type,
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id => $attach_id });
- }
+=item C<create($flag, $timestamp)>
- # Throw an error if the target is a private attachment and
- # the requestee isn't in the group of insiders who can see it.
- if ($attach_id
- && $private_attachment
- && Bugzilla->params->{'insidergroup'}
- && !$requestee->in_group(Bugzilla->params->{'insidergroup'}))
- {
- next if $skip_requestee_on_error;
- ThrowUserError('flag_requestee_unauthorized_attachment',
- { flag_type => $flag_type,
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id => $attach_id });
- }
+Creates a flag record in the database.
- # Throw an error if the user won't be allowed to set the flag.
- if (!$requestee->can_set_flag($flag_type)) {
- next if $skip_requestee_on_error;
- ThrowUserError('flag_requestee_needs_privs',
- {'requestee' => $requestee,
- 'flagtype' => $flag_type});
- }
+=back
- # This requestee can be set.
- push(@legal_requestees, $login);
- }
+=cut
- # Update the requestee list for this flag.
- if (scalar(@legal_requestees) < scalar(@$requestees)) {
- my $field_name = 'requestee_type-' . $flag_type->id;
- Bugzilla->cgi->delete($field_name);
- Bugzilla->cgi->param(-name => $field_name, -value => \@legal_requestees);
- }
- }
+sub create {
+ my ($class, $flag, $timestamp) = @_;
+ $timestamp ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
- # Make sure the user is authorized to modify flags, see bug 180879
- # - The flag exists and is unchanged.
- return if ($flag && ($status eq $flag->status));
+ my $params = {};
+ my @columns = grep { $_ ne 'id' } $class->DB_COLUMNS;
+ $params->{$_} = $flag->{$_} foreach @columns;
- # - User in the request_group can clear pending requests and set flags
- # and can rerequest set flags.
- return if (($status eq 'X' || $status eq '?')
- && $setter->can_request_flag($flag_type));
+ $params->{creation_date} = $params->{modification_date} = $timestamp;
+ $flag = $class->SUPER::create($params);
+ return $flag;
+}
+
+sub update {
+ my $self = shift;
+ my $dbh = Bugzilla->dbh;
+ my $timestamp = shift || $dbh->selectrow_array('SELECT NOW()');
- # - User in the grant_group can set/clear flags, including "+" and "-".
- return if $setter->can_set_flag($flag_type);
+ my $changes = $self->SUPER::update(@_);
- # - Any other flag modification is denied
- ThrowUserError('flag_update_denied',
- { name => $flag_type->name,
- status => $status,
- old_status => $flag ? $flag->status : 'X' });
+ if (scalar(keys %$changes)) {
+ $dbh->do('UPDATE flags SET modification_date = ? WHERE id = ?',
+ undef, ($timestamp, $self->id));
+ }
+ return $changes;
}
sub snapshot {
- my ($class, $bug_id, $attach_id) = @_;
+ my ($class, $flags) = @_;
- my $flags = $class->match({ 'bug_id' => $bug_id,
- 'attach_id' => $attach_id });
my @summaries;
foreach my $flag (@$flags) {
my $summary = $flag->setter->nick . ':' . $flag->type->name . $flag->status;
@@ -522,479 +478,378 @@ sub snapshot {
return @summaries;
}
+sub update_activity {
+ my ($class, $old_summaries, $new_summaries) = @_;
-=pod
-
-=over
-
-=item C<process($bug, $attachment, $timestamp, $hr_vars)>
-
-Processes changes to flags.
-
-The bug and/or the attachment objects are the ones this flag is about,
-the timestamp is the date/time the bug was last touched (so that changes
-to the flag can be stamped with the same date/time).
-
-=back
-
-=cut
-
-sub process {
- my ($class, $bug, $attachment, $timestamp, $hr_vars) = @_;
- my $dbh = Bugzilla->dbh;
- my $cgi = Bugzilla->cgi;
-
- # Make sure the bug (and attachment, if given) exists and is accessible
- # to the current user. Moreover, if an attachment object is passed,
- # make sure it belongs to the given bug.
- return if ($bug->error || ($attachment && $bug->bug_id != $attachment->bug_id));
-
- my $bug_id = $bug->bug_id;
- my $attach_id = $attachment ? $attachment->id : undef;
-
- # Use the date/time we were given if possible (allowing calling code
- # to synchronize the comment's timestamp with those of other records).
- $timestamp ||= $dbh->selectrow_array('SELECT NOW()');
-
- # Take a snapshot of flags before any changes.
- my @old_summaries = $class->snapshot($bug_id, $attach_id);
+ my ($removed, $added) = diff_arrays($old_summaries, $new_summaries);
+ if (scalar @$removed || scalar @$added) {
+ # Remove flag requester/setter information
+ foreach (@$removed, @$added) { s/^[^:]+:// }
- # Cancel pending requests if we are obsoleting an attachment.
- if ($attachment && $cgi->param('isobsolete')) {
- $class->CancelRequests($bug, $attachment);
+ $removed = join(", ", @$removed);
+ $added = join(", ", @$added);
+ return ($removed, $added);
}
+ return ();
+}
- # Create new flags and update existing flags.
- my $new_flags = FormToNewFlags($bug, $attachment, $cgi, $hr_vars);
- foreach my $flag (@$new_flags) { create($flag, $bug, $attachment, $timestamp) }
- modify($bug, $attachment, $cgi, $timestamp);
+sub update_flags {
+ my ($class, $self, $old_self, $timestamp) = @_;
- # In case the bug's product/component has changed, clear flags that are
- # no longer valid.
- my $flag_ids = $dbh->selectcol_arrayref(
- "SELECT DISTINCT flags.id
- FROM flags
- INNER JOIN bugs
- ON flags.bug_id = bugs.bug_id
- LEFT JOIN flaginclusions AS i
- ON flags.type_id = i.type_id
- AND (bugs.product_id = i.product_id OR i.product_id IS NULL)
- AND (bugs.component_id = i.component_id OR i.component_id IS NULL)
- WHERE bugs.bug_id = ?
- AND i.type_id IS NULL",
- undef, $bug_id);
+ my @old_summaries = $class->snapshot($old_self->flags);
+ my %old_flags = map { $_->id => $_ } @{$old_self->flags};
- my $flags = Bugzilla::Flag->new_from_list($flag_ids);
- foreach my $flag (@$flags) {
- my $is_retargetted = retarget($flag, $bug);
- unless ($is_retargetted) {
- clear($flag, $bug, $flag->attachment);
- $hr_vars->{'message'} = 'flag_cleared';
+ foreach my $new_flag (@{$self->flags}) {
+ if (!$new_flag->id) {
+ # This is a new flag.
+ my $flag = $class->create($new_flag, $timestamp);
+ $new_flag->{id} = $flag->id;
+ $class->notify($new_flag, undef, $self);
+ }
+ else {
+ $new_flag->update($timestamp);
+ $class->notify($new_flag, $old_flags{$new_flag->id}, $self);
+ delete $old_flags{$new_flag->id};
}
}
-
- $flag_ids = $dbh->selectcol_arrayref(
- "SELECT DISTINCT flags.id
- FROM flags, bugs, flagexclusions e
- WHERE bugs.bug_id = ?
- AND flags.bug_id = bugs.bug_id
- AND flags.type_id = e.type_id
- AND (bugs.product_id = e.product_id OR e.product_id IS NULL)
- AND (bugs.component_id = e.component_id OR e.component_id IS NULL)",
- undef, $bug_id);
-
- $flags = Bugzilla::Flag->new_from_list($flag_ids);
- foreach my $flag (@$flags) {
- my $is_retargetted = retarget($flag, $bug);
- clear($flag, $bug, $flag->attachment) unless $is_retargetted;
+ # These flags have been deleted.
+ foreach my $old_flag (values %old_flags) {
+ $class->notify(undef, $old_flag, $self);
+ $old_flag->remove_from_db();
}
- # Take a snapshot of flags after changes.
- my @new_summaries = $class->snapshot($bug_id, $attach_id);
+ # If the bug has been moved into another product or component,
+ # we must also take care of attachment flags which are no longer valid,
+ # as well as all bug flags which haven't been forgotten above.
+ if ($self->isa('Bugzilla::Bug')
+ && ($self->{_old_product_name} || $self->{_old_component_name}))
+ {
+ my @removed = $class->force_cleanup($self);
+ push(@old_summaries, @removed);
+ }
- update_activity($bug_id, $attach_id, $timestamp, \@old_summaries, \@new_summaries);
+ my @new_summaries = $class->snapshot($self->flags);
+ my @changes = $class->update_activity(\@old_summaries, \@new_summaries);
- Bugzilla::Hook::process('flag-end_of_update', { bug => $bug,
+ Bugzilla::Hook::process('flag-end_of_update', { object => $self,
timestamp => $timestamp,
old_flags => \@old_summaries,
new_flags => \@new_summaries,
});
+ return @changes;
}
-sub update_activity {
- my ($bug_id, $attach_id, $timestamp, $old_summaries, $new_summaries) = @_;
- my $dbh = Bugzilla->dbh;
+sub retarget {
+ my ($self, $obj) = @_;
- my ($removed, $added) = diff_arrays($old_summaries, $new_summaries);
- if (scalar @$removed || scalar @$added) {
- # Remove flag requester/setter information
- foreach (@$removed, @$added) { s/^[^:]+:// }
+ my @flagtypes = grep { $_->name eq $self->type->name } @{$obj->flag_types};
- $removed = join(", ", @$removed);
- $added = join(", ", @$added);
- my $field_id = get_field_id('flagtypes.name');
- $dbh->do('INSERT INTO bugs_activity
- (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES (?, ?, ?, ?, ?, ?, ?)',
- undef, ($bug_id, $attach_id, Bugzilla->user->id,
- $timestamp, $field_id, $removed, $added));
-
- $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
- undef, ($timestamp, $bug_id));
+ my $success = 0;
+ foreach my $flagtype (@flagtypes) {
+ next if !$flagtype->is_active;
+ next if (!$flagtype->is_multiplicable && scalar @{$flagtype->{flags}});
+
+ $self->{type_id} = $flagtype->id;
+ delete $self->{type};
+ $success = 1;
+ last;
}
+ return $success;
}
-=pod
-
-=over
-
-=item C<create($flag, $bug, $attachment, $timestamp)>
+# In case the bug's product/component has changed, clear flags that are
+# no longer valid.
+sub force_cleanup {
+ my ($class, $bug) = @_;
+ my $dbh = Bugzilla->dbh;
-Creates a flag record in the database.
+ my $flag_ids = $dbh->selectcol_arrayref(
+ 'SELECT DISTINCT flags.id
+ FROM flags
+ INNER JOIN bugs
+ ON flags.bug_id = bugs.bug_id
+ LEFT JOIN flaginclusions AS i
+ ON flags.type_id = i.type_id
+ AND (bugs.product_id = i.product_id OR i.product_id IS NULL)
+ AND (bugs.component_id = i.component_id OR i.component_id IS NULL)
+ WHERE bugs.bug_id = ? AND i.type_id IS NULL',
+ undef, $bug->id);
-=back
+ my @removed = $class->force_retarget($flag_ids, $bug);
-=cut
+ $flag_ids = $dbh->selectcol_arrayref(
+ 'SELECT DISTINCT flags.id
+ FROM flags, bugs, flagexclusions e
+ WHERE bugs.bug_id = ?
+ AND flags.bug_id = bugs.bug_id
+ AND flags.type_id = e.type_id
+ AND (bugs.product_id = e.product_id OR e.product_id IS NULL)
+ AND (bugs.component_id = e.component_id OR e.component_id IS NULL)',
+ undef, $bug->id);
+
+ push(@removed , $class->force_retarget($flag_ids, $bug));
+ return @removed;
+}
-sub create {
- my ($flag, $bug, $attachment, $timestamp) = @_;
+sub force_retarget {
+ my ($class, $flag_ids, $bug) = @_;
my $dbh = Bugzilla->dbh;
- my $attach_id = $attachment ? $attachment->id : undef;
- my $requestee_id;
- # Be careful! At this point, $flag is *NOT* yet an object!
- $requestee_id = $flag->{'requestee'}->id if $flag->{'requestee'};
-
- $dbh->do('INSERT INTO flags (type_id, bug_id, attach_id, requestee_id,
- setter_id, status, creation_date, modification_date)
- VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
- undef, ($flag->{'type'}->id, $bug->bug_id,
- $attach_id, $requestee_id, $flag->{'setter'}->id,
- $flag->{'status'}, $timestamp, $timestamp));
-
- # Now that the new flag has been added to the DB, create a real flag object.
- # This is required to call notify() correctly.
- my $flag_id = $dbh->bz_last_key('flags', 'id');
- $flag = new Bugzilla::Flag($flag_id);
-
- # Send an email notifying the relevant parties about the flag creation.
- if ($flag->requestee && $flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) {
- $flag->{'addressee'} = $flag->requestee;
+ my $flags = $class->new_from_list($flag_ids);
+ my @removed;
+ foreach my $flag (@$flags) {
+ # $bug is undefined when e.g. editing inclusion and exclusion lists.
+ my $obj = $flag->attachment || $bug || $flag->bug;
+ my $is_retargetted = $flag->retarget($obj);
+ if ($is_retargetted) {
+ $dbh->do('UPDATE flags SET type_id = ? WHERE id = ?',
+ undef, ($flag->type_id, $flag->id));
+ }
+ else {
+ # Track deleted attachment flags.
+ push(@removed, $class->snapshot([$flag])) if $flag->attach_id;
+ $class->notify(undef, $flag, $bug || $flag->bug);
+ $flag->remove_from_db();
+ }
}
-
- notify($flag, $bug, $attachment);
-
- # Return the new flag object.
- return $flag;
+ return @removed;
}
-=pod
-
-=over
-
-=item C<modify($bug, $attachment, $cgi, $timestamp)>
-
-Modifies flags in the database when a user changes them.
-
-=back
-
-=cut
-
-sub modify {
- my ($bug, $attachment, $cgi, $timestamp) = @_;
- my $setter = Bugzilla->user;
- my $dbh = Bugzilla->dbh;
-
- # Extract a list of flags from the form data.
- my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
+###############################
+#### Validators ######
+###############################
- # Loop over flags and update their record in the database if necessary.
- # Two kinds of changes can happen to a flag: it can be set to a different
- # state, and someone else can be asked to set it. We take care of both
- # those changes.
- my @flags;
- foreach my $id (@ids) {
- my $flag = new Bugzilla::Flag($id);
- # If the flag no longer exists, ignore it.
- next unless $flag;
+sub _set_requestee {
+ my ($self, $requestee, $attachment, $skip_requestee_on_error) = @_;
- my $status = $cgi->param("flag-$id");
+ # Used internally to check if the requestee is retargetting the request.
+ $self->{_old_requestee_id} = $self->requestee ? $self->requestee->id : 0;
+ $self->{requestee} =
+ $self->_check_requestee($requestee, $attachment, $skip_requestee_on_error);
- # If the user entered more than one name into the requestee field
- # (i.e. they want more than one person to set the flag) we can reuse
- # the existing flag for the first person (who may well be the existing
- # requestee), but we have to create new flags for each additional.
- my @requestees = $cgi->param("requestee-$id");
- my $requestee_email;
- if ($status eq "?"
- && scalar(@requestees) > 1
- && $flag->type->is_multiplicable)
- {
- # The first person, for which we'll reuse the existing flag.
- $requestee_email = shift(@requestees);
+ $self->{requestee_id} =
+ $self->{requestee} ? $self->{requestee}->id : undef;
+}
- # Create new flags like the existing one for each additional person.
- foreach my $login (@requestees) {
- create({ type => $flag->type,
- setter => $setter,
- status => "?",
- requestee => new Bugzilla::User({ name => $login }) },
- $bug, $attachment, $timestamp);
- }
- }
- else {
- $requestee_email = trim($cgi->param("requestee-$id") || '');
- }
+sub _set_setter {
+ my ($self, $setter) = @_;
- # Ignore flags the user didn't change. There are two components here:
- # either the status changes (trivial) or the requestee changes.
- # Change of either field will cause full update of the flag.
+ $self->set('setter', $setter);
+ $self->{setter_id} = $self->setter->id;
+}
- my $status_changed = ($status ne $flag->status);
+sub _set_status {
+ my ($self, $status) = @_;
- # Requestee is considered changed, if all of the following apply:
- # 1. Flag status is '?' (requested)
- # 2. Flag can have a requestee
- # 3. The requestee specified on the form is different from the
- # requestee specified in the db.
+ # Store the old flag status. It's needed by _check_setter().
+ $self->{_old_status} = $self->status;
+ $self->set('status', $status);
+}
- my $old_requestee = $flag->requestee ? $flag->requestee->login : '';
+sub _check_requestee {
+ my ($self, $requestee, $attachment, $skip_requestee_on_error) = @_;
- my $requestee_changed =
- ($status eq "?" &&
- $flag->type->is_requesteeble &&
- $old_requestee ne $requestee_email);
+ # If the flag status is not "?", then no requestee can be defined.
+ return undef if ($self->status ne '?');
- next unless ($status_changed || $requestee_changed);
+ # Store this value before updating the flag object.
+ my $old_requestee = $self->requestee ? $self->requestee->login : '';
- # Since the status is validated, we know it's safe, but it's still
- # tainted, so we have to detaint it before using it in a query.
- trick_taint($status);
+ if ($self->status eq '?' && $requestee) {
+ $requestee = Bugzilla::User->check($requestee);
+ }
+ else {
+ undef $requestee;
+ }
- if ($status eq '+' || $status eq '-') {
- $dbh->do('UPDATE flags
- SET setter_id = ?, requestee_id = NULL,
- status = ?, modification_date = ?
- WHERE id = ?',
- undef, ($setter->id, $status, $timestamp, $flag->id));
-
- # If the status of the flag was "?", we have to notify
- # the requester (if he wants to).
- my $requester;
- if ($flag->status eq '?') {
- $requester = $flag->setter;
- $flag->{'requester'} = $requester;
+ if ($requestee && $requestee->login ne $old_requestee) {
+ # Make sure the user didn't specify a requestee unless the flag
+ # is specifically requestable. For existing flags, if the requestee
+ # was set before the flag became specifically unrequestable, the
+ # user can either remove him or leave him alone.
+ ThrowCodeError('flag_requestee_disabled', { type => $self->type })
+ if !$self->type->is_requesteeble;
+
+ # Make sure the requestee can see the bug.
+ # Note that can_see_bug() will query the DB, so if the bug
+ # is being added/removed from some groups and these changes
+ # haven't been committed to the DB yet, they won't be taken
+ # into account here. In this case, old restrictions matters.
+ if (!$requestee->can_see_bug($self->bug_id)) {
+ if ($skip_requestee_on_error) {
+ undef $requestee;
}
- # Now update the flag object with its new values.
- $flag->{'setter'} = $setter;
- $flag->{'requestee'} = undef;
- $flag->{'requestee_id'} = undef;
- $flag->{'status'} = $status;
-
- # Send an email notifying the relevant parties about the fulfillment,
- # including the requester.
- if ($requester && $requester->wants_mail([EVT_REQUESTED_FLAG])) {
- $flag->{'addressee'} = $requester;
+ else {
+ ThrowUserError('flag_requestee_unauthorized',
+ { flag_type => $self->type,
+ requestee => $requestee,
+ bug_id => $self->bug_id,
+ attach_id => $self->attach_id });
}
-
- notify($flag, $bug, $attachment);
}
- elsif ($status eq '?') {
- # If the one doing the change is the requestee, then this means he doesn't
- # want to reply to the request and he simply reassigns the request to
- # someone else. In this case, we keep the requester unaltered.
- my $new_setter = $setter;
- if ($flag->requestee && $flag->requestee->id == $setter->id) {
- $new_setter = $flag->setter;
- }
-
- # Get the requestee, if any.
- my $requestee_id;
- if ($requestee_email) {
- $requestee_id = login_to_id($requestee_email);
- $flag->{'requestee'} = new Bugzilla::User($requestee_id);
- $flag->{'requestee_id'} = $requestee_id;
+ # Make sure the requestee can see the private attachment.
+ elsif ($self->attach_id && $attachment->isprivate && !$requestee->is_insider) {
+ if ($skip_requestee_on_error) {
+ undef $requestee;
}
else {
- # If the status didn't change but we only removed the
- # requestee, we have to clear the requestee field.
- $flag->{'requestee'} = undef;
- $flag->{'requestee_id'} = undef;
- }
-
- # Update the database with the changes.
- $dbh->do('UPDATE flags
- SET setter_id = ?, requestee_id = ?,
- status = ?, modification_date = ?
- WHERE id = ?',
- undef, ($new_setter->id, $requestee_id, $status,
- $timestamp, $flag->id));
-
- # Now update the flag object with its new values.
- $flag->{'setter'} = $new_setter;
- $flag->{'status'} = $status;
-
- # Send an email notifying the relevant parties about the request.
- if ($flag->requestee && $flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) {
- $flag->{'addressee'} = $flag->requestee;
+ ThrowUserError('flag_requestee_unauthorized_attachment',
+ { flag_type => $self->type,
+ requestee => $requestee,
+ bug_id => $self->bug_id,
+ attach_id => $self->attach_id });
}
-
- notify($flag, $bug, $attachment);
}
- elsif ($status eq 'X') {
- clear($flag, $bug, $attachment);
+ # Make sure the user is allowed to set the flag.
+ elsif (!$requestee->can_set_flag($self->type)) {
+ if ($skip_requestee_on_error) {
+ undef $requestee;
+ }
+ else {
+ ThrowUserError('flag_requestee_needs_privs',
+ {'requestee' => $requestee,
+ 'flagtype' => $self->type});
+ }
}
-
- push(@flags, $flag);
}
-
- return \@flags;
+ return $requestee;
}
-=pod
-
-=over
-
-=item C<retarget($flag, $bug)>
+sub _check_setter {
+ my ($self, $setter) = @_;
-Change the type of the flag, if possible. The new flag type must have
-the same name as the current flag type, must exist in the product and
-component the bug is in, and the current settings of the flag must pass
-validation. If no such flag type can be found, the type remains unchanged.
+ # By default, the currently logged in user is the setter.
+ $setter ||= Bugzilla->user;
+ (blessed($setter) && $setter->isa('Bugzilla::User') && $setter->id)
+ || ThrowCodeError('invalid_user');
-Retargetting flags is a good way to keep flags when moving bugs from one
-product where a flag type is available to another product where the flag
-type is unavailable, but another flag type having the same name exists.
-Most of the time, if they have the same name, this means that they have
-the same meaning, but with different settings.
+ # set_status() has already been called. So this refers
+ # to the new flag status.
+ my $status = $self->status;
-=back
-
-=cut
+ # Make sure the user is authorized to modify flags, see bug 180879:
+ # - The flag exists and is unchanged.
+ # - Users in the request_group can clear pending requests and set flags
+ # and can rerequest set flags.
+ # - Users in the grant_group can set/clear flags, including "+" and "-".
+ unless (($status eq $self->{_old_status})
+ || (($status eq 'X' || $status eq '?')
+ && $setter->can_request_flag($self->type))
+ || $setter->can_set_flag($self->type))
+ {
+ ThrowUserError('flag_update_denied',
+ { name => $self->type->name,
+ status => $status,
+ old_status => $self->{_old_status} });
+ }
-sub retarget {
- my ($flag, $bug) = @_;
- my $dbh = Bugzilla->dbh;
+ # If the requester is retargetting the request, we don't
+ # update the setter, so that the setter gets the notification.
+ if ($status eq '?' && $self->{_old_requestee_id} == $setter->id) {
+ return $self->setter;
+ }
+ return $setter;
+}
- # We are looking for flagtypes having the same name as the flagtype
- # to which the current flag belongs, and being in the new product and
- # component of the bug.
- my $flagtypes = Bugzilla::FlagType::match(
- {'name' => $flag->name,
- 'target_type' => $flag->type->target_type,
- 'is_active' => 1,
- 'product_id' => $bug->product_id,
- 'component_id' => $bug->component_id});
-
- # If we found no flagtype, the flag will be deleted.
- return 0 unless scalar(@$flagtypes);
-
- # If we found at least one, change the type of the flag,
- # assuming the setter/requester is allowed to set/request flags
- # belonging to this flagtype.
- my $requestee = $flag->requestee ? [$flag->requestee->login] : [];
- my $is_private = ($flag->attachment) ? $flag->attachment->isprivate : 0;
- my $is_retargetted = 0;
-
- foreach my $flagtype (@$flagtypes) {
- # Get the number of flags of this type already set for this target.
- my $has_flags = __PACKAGE__->count(
- { 'type_id' => $flagtype->id,
- 'bug_id' => $bug->bug_id,
- 'attach_id' => $flag->attach_id });
+sub _check_status {
+ my ($self, $status) = @_;
- # Do not create a new flag of this type if this flag type is
- # not multiplicable and already has a flag set.
- next if (!$flagtype->is_multiplicable && $has_flags);
-
- # Check user privileges.
- my $error_mode_cache = Bugzilla->error_mode;
- Bugzilla->error_mode(ERROR_MODE_DIE);
- eval {
- _validate(undef, $flagtype, $flag->status, $flag->setter,
- $requestee, $is_private, $bug->bug_id, $flag->attach_id);
- };
- Bugzilla->error_mode($error_mode_cache);
- # If the validation failed, then we cannot use this flagtype.
- next if ($@);
-
- # Checks are successful, we can retarget the flag to this flagtype.
- $dbh->do('UPDATE flags SET type_id = ? WHERE id = ?',
- undef, ($flagtype->id, $flag->id));
-
- $is_retargetted = 1;
- last;
+ # - Make sure the status is valid.
+ # - Make sure the user didn't request the flag unless it's requestable.
+ # If the flag existed and was requested before it became unrequestable,
+ # leave it as is.
+ if (!grep($status eq $_ , qw(X + - ?))
+ || ($status eq '?' && $self->status ne '?' && !$self->type->is_requestable))
+ {
+ ThrowCodeError('flag_status_invalid', { id => $self->id,
+ status => $status });
}
- return $is_retargetted;
+ return $status;
}
+######################################################################
+# Utility Functions
+######################################################################
+
=pod
=over
-=item C<clear($flag, $bug, $attachment)>
+=item C<extract_flags_from_cgi($bug, $attachment, $hr_vars)>
-Remove a flag from the DB.
+Checks whether or not there are new flags to create and returns an
+array of hashes. This array is then passed to Flag::create().
=back
=cut
-sub clear {
- my ($flag, $bug, $attachment) = @_;
- my $dbh = Bugzilla->dbh;
+sub extract_flags_from_cgi {
+ my ($class, $bug, $attachment, $vars, $skip) = @_;
+ my $cgi = Bugzilla->cgi;
- $dbh->do('DELETE FROM flags WHERE id = ?', undef, $flag->id);
+ my $match_status = Bugzilla::User::match_field($cgi, {
+ '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
+ }, $skip);
- # If we cancel a pending request, we have to notify the requester
- # (if he wants to).
- my $requester;
- if ($flag->status eq '?') {
- $requester = $flag->setter;
- $flag->{'requester'} = $requester;
+ $vars->{'match_field'} = 'requestee';
+ if ($match_status == USER_MATCH_FAILED) {
+ $vars->{'message'} = 'user_match_failed';
}
-
- # Now update the flag object to its new values. The last
- # requester/setter and requestee are kept untouched (for the
- # record). Else we could as well delete the flag completely.
- $flag->{'exists'} = 0;
- $flag->{'status'} = "X";
-
- if ($requester && $requester->wants_mail([EVT_REQUESTED_FLAG])) {
- $flag->{'addressee'} = $requester;
+ elsif ($match_status == USER_MATCH_MULTIPLE) {
+ $vars->{'message'} = 'user_match_multiple';
}
- notify($flag, $bug, $attachment);
-}
-
-
-######################################################################
-# Utility Functions
-######################################################################
-
-=pod
+ # Extract a list of flag type IDs from field names.
+ my @flagtype_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
+ @flagtype_ids = grep($cgi->param("flag_type-$_") ne 'X', @flagtype_ids);
-=over
+ # Extract a list of existing flag IDs.
+ my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
-=item C<FormToNewFlags($bug, $attachment, $cgi, $hr_vars)>
+ return () if (!scalar(@flagtype_ids) && !scalar(@flag_ids));
-Checks whether or not there are new flags to create and returns an
-array of flag objects. This array is then passed to Flag::create().
+ my (@new_flags, @flags);
+ foreach my $flag_id (@flag_ids) {
+ my $flag = $class->new($flag_id);
+ # If the flag no longer exists, ignore it.
+ next unless $flag;
-=back
+ my $status = $cgi->param("flag-$flag_id");
-=cut
+ # If the user entered more than one name into the requestee field
+ # (i.e. they want more than one person to set the flag) we can reuse
+ # the existing flag for the first person (who may well be the existing
+ # requestee), but we have to create new flags for each additional requestee.
+ my @requestees = $cgi->param("requestee-$flag_id");
+ my $requestee_email;
+ if ($status eq "?"
+ && scalar(@requestees) > 1
+ && $flag->type->is_multiplicable)
+ {
+ # The first person, for which we'll reuse the existing flag.
+ $requestee_email = shift(@requestees);
-sub FormToNewFlags {
- my ($bug, $attachment, $cgi, $hr_vars) = @_;
- my $dbh = Bugzilla->dbh;
- my $setter = Bugzilla->user;
-
- # Extract a list of flag type IDs from field names.
- my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
- @type_ids = grep($cgi->param("flag_type-$_") ne 'X', @type_ids);
+ # Create new flags like the existing one for each additional person.
+ foreach my $login (@requestees) {
+ push(@new_flags, { type_id => $flag->type_id,
+ status => "?",
+ requestee => $login,
+ skip_roe => $skip });
+ }
+ }
+ elsif ($status eq "?" && scalar(@requestees)) {
+ # If there are several requestees and the flag type is not multiplicable,
+ # this will fail. But that's the job of the validator to complain. All
+ # we do here is to extract and convert data from the CGI.
+ $requestee_email = trim($cgi->param("requestee-$flag_id") || '');
+ }
- return () unless scalar(@type_ids);
+ push(@flags, { id => $flag_id,
+ status => $status,
+ requestee => $requestee_email,
+ skip_roe => $skip });
+ }
# Get a list of active flag types available for this product/component.
my $flag_types = Bugzilla::FlagType::match(
@@ -1002,15 +857,14 @@ sub FormToNewFlags {
'component_id' => $bug->{'component_id'},
'is_active' => 1 });
- foreach my $type_id (@type_ids) {
+ foreach my $flagtype_id (@flagtype_ids) {
# Checks if there are unexpected flags for the product/component.
- if (!scalar(grep { $_->id == $type_id } @$flag_types)) {
- $hr_vars->{'message'} = 'unexpected_flag_types';
+ if (!scalar(grep { $_->id == $flagtype_id } @$flag_types)) {
+ $vars->{'message'} = 'unexpected_flag_types';
last;
}
}
- my @flags;
foreach my $flag_type (@$flag_types) {
my $type_id = $flag_type->id;
@@ -1019,10 +873,10 @@ sub FormToNewFlags {
next unless ($flag_type->target_type eq 'bug' xor $attachment);
# We are only interested in flags the user tries to create.
- next unless scalar(grep { $_ == $type_id } @type_ids);
+ next unless scalar(grep { $_ == $type_id } @flagtype_ids);
# Get the number of flags of this type already set for this target.
- my $has_flags = __PACKAGE__->count(
+ my $has_flags = $class->count(
{ 'type_id' => $type_id,
'target_type' => $attachment ? 'attachment' : 'bug',
'bug_id' => $bug->bug_id,
@@ -1036,65 +890,23 @@ sub FormToNewFlags {
trick_taint($status);
my @logins = $cgi->param("requestee_type-$type_id");
- if ($status eq "?" && scalar(@logins) > 0) {
+ if ($status eq "?" && scalar(@logins)) {
foreach my $login (@logins) {
- push (@flags, { type => $flag_type ,
- setter => $setter ,
- status => $status ,
- requestee =>
- new Bugzilla::User({ name => $login }) });
+ push (@new_flags, { type_id => $type_id,
+ status => $status,
+ requestee => $login,
+ skip_roe => $skip });
last unless $flag_type->is_multiplicable;
}
}
else {
- push (@flags, { type => $flag_type ,
- setter => $setter ,
- status => $status });
+ push (@new_flags, { type_id => $type_id,
+ status => $status });
}
}
- # Return the list of flags.
- return \@flags;
-}
-
-# This is a helper to set flags on a new bug or attachment.
-# For existing bugs and attachments, errors must be reported.
-sub set_flags {
- my ($class, $bug, $attachment, $timestamp, $vars) = @_;
- my $cgi = Bugzilla->cgi;
-
- # The order of these function calls is important, as Flag::validate
- # assumes User::match_field has ensured that the
- # values in the requestee fields are legitimate user email addresses.
- my $match_status = Bugzilla::User::match_field($cgi, {
- '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
- }, MATCH_SKIP_CONFIRM);
-
- $vars->{'match_field'} = 'requestee';
- if ($match_status == USER_MATCH_FAILED) {
- $vars->{'message'} = 'user_match_failed';
- }
- elsif ($match_status == USER_MATCH_MULTIPLE) {
- $vars->{'message'} = 'user_match_multiple';
- }
-
- # 1. Add flags, if any. To avoid dying if something goes wrong
- # while processing flags, we will eval() flag validation.
- #
- # 2. Flag::validate() should not detect any reference to existing flags
- # when creating a new attachment. Setting the third param to -1 will
- # force this function to check this point.
- my $error_mode_cache = Bugzilla->error_mode;
- Bugzilla->error_mode(ERROR_MODE_DIE);
- eval {
- validate($bug->bug_id, $attachment ? -1 : undef, SKIP_REQUESTEE_ON_ERROR);
- $class->process($bug, $attachment, $timestamp, $vars);
- };
- Bugzilla->error_mode($error_mode_cache);
- if ($@) {
- $vars->{'message'} = 'flag_creation_failed';
- $vars->{'flag_creation_error'} = $@;
- }
+ # Return the list of flags to update and/or to create.
+ return (\@flags, \@new_flags);
}
=pod
@@ -1111,10 +923,41 @@ or deleted.
=cut
sub notify {
- my ($flag, $bug, $attachment) = @_;
+ my ($class, $flag, $old_flag, $obj) = @_;
- # There is nobody to notify.
- return unless ($flag->{'addressee'} || $flag->type->cc_list);
+ my ($bug, $attachment);
+ if (blessed($obj) && $obj->isa('Bugzilla::Attachment')) {
+ $attachment = $obj;
+ $bug = $attachment->bug;
+ }
+ elsif (blessed($obj) && $obj->isa('Bugzilla::Bug')) {
+ $bug = $obj;
+ }
+ else {
+ # Not a good time to throw an error.
+ return;
+ }
+
+ my $addressee;
+ # If the flag is set to '?', maybe the requestee wants a notification.
+ if ($flag && $flag->requestee_id
+ && (!$old_flag || ($old_flag->requestee_id || 0) != $flag->requestee_id))
+ {
+ if ($flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) {
+ $addressee = $flag->requestee;
+ }
+ }
+ elsif ($old_flag && $old_flag->status eq '?'
+ && (!$flag || $flag->status ne '?'))
+ {
+ if ($old_flag->setter->wants_mail([EVT_REQUESTED_FLAG])) {
+ $addressee = $old_flag->setter;
+ }
+ }
+
+ my $cc_list = $flag ? $flag->type->cc_list : $old_flag->type->cc_list;
+ # Is there someone to notify?
+ return unless ($addressee || $cc_list);
# If the target bug is restricted to one or more groups, then we need
# to make sure we don't send email about it to unauthorized users
@@ -1124,7 +967,7 @@ sub notify {
my $attachment_is_private = $attachment ? $attachment->isprivate : undef;
my %recipients;
- foreach my $cc (split(/[, ]+/, $flag->type->cc_list)) {
+ foreach my $cc (split(/[, ]+/, $cc_list)) {
my $ccuser = new Bugzilla::User({ name => $cc });
next if (scalar(@bug_in_groups) && (!$ccuser || !$ccuser->can_see_bug($bug->bug_id)));
next if $attachment_is_private && (!$ccuser || !$ccuser->is_insider);
@@ -1134,16 +977,15 @@ sub notify {
}
# Only notify if the addressee is allowed to receive the email.
- if ($flag->{'addressee'} && $flag->{'addressee'}->email_enabled) {
- $recipients{$flag->{'addressee'}->email} = $flag->{'addressee'};
+ if ($addressee && $addressee->email_enabled) {
+ $recipients{$addressee->email} = $addressee;
}
# Process and send notification for each recipient.
# If there are users in the CC list who don't have an account,
# use the default language for email notifications.
my $default_lang;
if (grep { !$_ } values %recipients) {
- my $default_user = new Bugzilla::User();
- $default_lang = $default_user->settings->{'lang'}->{'value'};
+ $default_lang = Bugzilla::User->new()->settings->{'lang'}->{'value'};
}
foreach my $to (keys %recipients) {
@@ -1152,6 +994,7 @@ sub notify {
my $thread_user_id = $recipients{$to} ? $recipients{$to}->id : 0;
my $vars = { 'flag' => $flag,
+ 'old_flag' => $old_flag,
'to' => $to,
'bug' => $bug,
'attachment' => $attachment,
@@ -1170,43 +1013,12 @@ sub notify {
}
}
-# Cancel all request flags from the attachment being obsoleted.
-sub CancelRequests {
- my ($class, $bug, $attachment, $timestamp) = @_;
- my $dbh = Bugzilla->dbh;
-
- my $request_ids =
- $dbh->selectcol_arrayref("SELECT flags.id
- FROM flags
- LEFT JOIN attachments ON flags.attach_id = attachments.attach_id
- WHERE flags.attach_id = ?
- AND flags.status = '?'
- AND attachments.isobsolete = 0",
- undef, $attachment->id);
-
- return if (!scalar(@$request_ids));
-
- # Take a snapshot of flags before any changes.
- my @old_summaries = $class->snapshot($bug->bug_id, $attachment->id)
- if ($timestamp);
- my $flags = Bugzilla::Flag->new_from_list($request_ids);
- foreach my $flag (@$flags) { clear($flag, $bug, $attachment) }
-
- # If $timestamp is undefined, do not update the activity table
- return unless ($timestamp);
-
- # Take a snapshot of flags after any changes.
- my @new_summaries = $class->snapshot($bug->bug_id, $attachment->id);
- update_activity($bug->bug_id, $attachment->id, $timestamp,
- \@old_summaries, \@new_summaries);
-}
-
# This is an internal function used by $bug->flag_types
# and $attachment->flag_types to collect data about available
# flag types and existing flags set on them. You should never
# call this function directly.
sub _flag_types {
- my $vars = shift;
+ my ($class, $vars) = @_;
my $target_type = $vars->{target_type};
my $flags;
@@ -1214,15 +1026,15 @@ sub _flag_types {
# Retrieve all existing flags for this bug/attachment.
if ($target_type eq 'bug') {
my $bug_id = delete $vars->{bug_id};
- $flags = Bugzilla::Flag->match({target_type => 'bug', bug_id => $bug_id});
+ $flags = $class->match({target_type => 'bug', bug_id => $bug_id});
}
elsif ($target_type eq 'attachment') {
my $attach_id = delete $vars->{attach_id};
- $flags = Bugzilla::Flag->match({attach_id => $attach_id});
+ $flags = $class->match({attach_id => $attach_id});
}
else {
ThrowCodeError('bad_arg', {argument => 'target_type',
- function => 'Bugzilla::Flag::_flag_types'});
+ function => $class . '->_flag_types'});
}
# Get all available flag types for the given product and component.
@@ -1231,10 +1043,11 @@ sub _flag_types {
$_->{flags} = [] foreach @$flag_types;
my %flagtypes = map { $_->id => $_ } @$flag_types;
- # Group existing flags per type.
- # Call the internal 'type_id' variable instead of the method
- # to not create a flagtype object.
- push(@{$flagtypes{$_->{type_id}}->{flags}}, $_) foreach @$flags;
+ # Group existing flags per type, and skip those becoming invalid
+ # (which can happen when a bug is being moved into a new product
+ # or component).
+ @$flags = grep { exists $flagtypes{$_->type_id} } @$flags;
+ push(@{$flagtypes{$_->type_id}->{flags}}, $_) foreach @$flags;
return [sort {$a->sortkey <=> $b->sortkey || $a->name cmp $b->name} values %flagtypes];
}
diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm
index 5bc2e7716..a8f61a415 100644
--- a/Bugzilla/Hook.pm
+++ b/Bugzilla/Hook.pm
@@ -377,7 +377,7 @@ Params:
=head2 flag-end_of_update
-This happens at the end of L<Bugzilla::Flag/process>, after all other changes
+This happens at the end of L<Bugzilla::Flag/update_flags>, after all other changes
are made to the database and after emails are sent. It gives you a before/after
snapshot of flags so you can react to specific flag changes.
This generally occurs inside a database transaction.
@@ -389,7 +389,7 @@ Params:
=over
-=item C<bug> - The changed bug object.
+=item C<object> - The changed bug or attachment object.
=item C<timestamp> - The timestamp used for all updates in this transaction.
diff --git a/attachment.cgi b/attachment.cgi
index f6549841f..b62e4f23c 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -469,26 +469,16 @@ sub insert {
store_in_file => scalar $cgi->param('bigfile'),
});
- Bugzilla::Flag->set_flags($bug, $attachment, $timestamp, $vars);
-
- my $fieldid = get_field_id('attachments.isobsolete');
-
foreach my $obsolete_attachment (@obsolete_attachments) {
- # If the obsolete attachment has request flags, cancel them.
- # This call must be done before updating the 'attachments' table.
- Bugzilla::Flag->CancelRequests($bug, $obsolete_attachment, $timestamp);
-
- $dbh->do('UPDATE attachments SET isobsolete = 1, modification_time = ?
- WHERE attach_id = ?',
- undef, ($timestamp, $obsolete_attachment->id));
-
- $dbh->do('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
- fieldid, removed, added)
- VALUES (?,?,?,?,?,?,?)',
- undef, ($bug->bug_id, $obsolete_attachment->id, $user->id,
- $timestamp, $fieldid, 0, 1));
+ $obsolete_attachment->set_is_obsolete(1);
+ $obsolete_attachment->update($timestamp);
}
+ my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
+ $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR);
+ $attachment->set_flags($flags, $new_flags);
+ $attachment->update($timestamp);
+
# Insert a comment about the new attachment into the database.
my $comment = "Created an attachment (id=" . $attachment->id . ")\n" .
$attachment->description . "\n";
@@ -627,27 +617,18 @@ sub update {
$bug->add_comment($comment, { isprivate => $attachment->isprivate });
}
- # The order of these function calls is important, as Flag::validate
- # assumes User::match_field has ensured that the values in the
- # requestee fields are legitimate user email addresses.
- Bugzilla::User::match_field($cgi, {
- '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' }
- });
- Bugzilla::Flag::validate($bug->id, $attachment->id);
-
+ my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
+ $attachment->set_flags($flags, $new_flags);
# Figure out when the changes were made.
- my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
-
- # Update flags. We have to do this before committing changes
- # to attachments so that we can delete pending requests if the user
- # is obsoleting this attachment without deleting any requests
- # the user submits at the same time.
- Bugzilla::Flag->process($bug, $attachment, $timestamp, $vars);
+ my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
- $attachment->update($timestamp);
+ my $changes = $attachment->update($timestamp);
+ # If there are changes, we updated delta_ts in the DB. We have to
+ # reflect this change in the bug object.
+ $bug->{delta_ts} = $timestamp if scalar(keys %$changes);
# Commit the comment, if any.
- $bug->update();
+ $bug->update($timestamp);
# Commit the transaction now that we are finished updating the database.
$dbh->bz_commit_transaction();
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 4dbaae573..b730ae2e5 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -413,11 +413,7 @@ sub update {
WHERE flags.type_id = ?
AND i.type_id IS NULL',
undef, $id);
- my $flags = Bugzilla::Flag->new_from_list($flag_ids);
- foreach my $flag (@$flags) {
- my $bug = new Bugzilla::Bug($flag->bug_id);
- Bugzilla::Flag::clear($flag, $bug, $flag->attachment);
- }
+ Bugzilla::Flag->force_retarget($flag_ids);
$flag_ids = $dbh->selectcol_arrayref('SELECT DISTINCT flags.id
FROM flags
@@ -431,11 +427,7 @@ sub update {
AND (bugs.component_id = e.component_id
OR e.component_id IS NULL)',
undef, $id);
- $flags = Bugzilla::Flag->new_from_list($flag_ids);
- foreach my $flag (@$flags) {
- my $bug = new Bugzilla::Bug($flag->bug_id);
- Bugzilla::Flag::clear($flag, $bug, $flag->attachment);
- }
+ Bugzilla::Flag->force_retarget($flag_ids);
# Now silently remove requestees from flags which are no longer
# specifically requestable.
diff --git a/editusers.cgi b/editusers.cgi
index 3d8b66184..b415fd0b0 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -481,35 +481,36 @@ if ($action eq 'search') {
my $sth_set_bug_timestamp =
$dbh->prepare('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?');
- # Reference removals which need LogActivityEntry.
- my $statement_flagupdate = 'UPDATE flags set requestee_id = NULL
- WHERE bug_id = ?
- AND attach_id %s
- AND requestee_id = ?';
- my $sth_flagupdate_attachment =
- $dbh->prepare(sprintf($statement_flagupdate, '= ?'));
- my $sth_flagupdate_bug =
- $dbh->prepare(sprintf($statement_flagupdate, 'IS NULL'));
-
- my $buglist = $dbh->selectall_arrayref('SELECT DISTINCT bug_id, attach_id
- FROM flags
- WHERE requestee_id = ?',
- undef, $otherUserID);
-
- foreach (@$buglist) {
- my ($bug_id, $attach_id) = @$_;
- my @old_summaries = Bugzilla::Flag->snapshot($bug_id, $attach_id);
- if ($attach_id) {
- $sth_flagupdate_attachment->execute($bug_id, $attach_id, $otherUserID);
+ my $sth_updateFlag = $dbh->prepare('INSERT INTO bugs_activity
+ (bug_id, attach_id, who, bug_when, fieldid, removed, added)
+ VALUES (?, ?, ?, ?, ?, ?, ?)');
+
+ # Flags
+ my $flag_ids =
+ $dbh->selectcol_arrayref('SELECT id FROM flags WHERE requestee_id = ?',
+ undef, $otherUserID);
+
+ my $flags = Bugzilla::Flag->new_from_list($flag_ids);
+
+ $dbh->do('UPDATE flags SET requestee_id = NULL, modification_date = ?
+ WHERE requestee_id = ?', undef, ($timestamp, $otherUserID));
+
+ # We want to remove the requestee but leave the requester alone,
+ # so we have to log these changes manually.
+ my %bugs;
+ push(@{$bugs{$_->bug_id}->{$_->attach_id || 0}}, $_) foreach @$flags;
+ my $fieldid = get_field_id('flagtypes.name');
+ foreach my $bug_id (keys %bugs) {
+ foreach my $attach_id (keys %{$bugs{$bug_id}}) {
+ my @old_summaries = Bugzilla::Flag->snapshot($bugs{$bug_id}->{$attach_id});
+ $_->_set_requestee() foreach @{$bugs{$bug_id}->{$attach_id}};
+ my @new_summaries = Bugzilla::Flag->snapshot($bugs{$bug_id}->{$attach_id});
+ my ($removed, $added) =
+ Bugzilla::Flag->update_activity(\@old_summaries, \@new_summaries);
+ $sth_updateFlag->execute($bug_id, $attach_id || undef, $userid,
+ $timestamp, $fieldid, $removed, $added);
}
- else {
- $sth_flagupdate_bug->execute($bug_id, $otherUserID);
- }
- my @new_summaries = Bugzilla::Flag->snapshot($bug_id, $attach_id);
- # Let update_activity do all the dirty work, including setting
- # the bug timestamp.
- Bugzilla::Flag::update_activity($bug_id, $attach_id, $timestamp,
- \@old_summaries, \@new_summaries);
+ $sth_set_bug_timestamp->execute($timestamp, $bug_id);
$updatedbugs{$bug_id} = 1;
}
@@ -536,9 +537,8 @@ if ($action eq 'search') {
($otherUserID, $otherUserID));
# Deletions in referred tables which need LogActivityEntry.
- $buglist = $dbh->selectcol_arrayref('SELECT bug_id FROM cc
- WHERE who = ?',
- undef, $otherUserID);
+ my $buglist = $dbh->selectcol_arrayref('SELECT bug_id FROM cc WHERE who = ?',
+ undef, $otherUserID);
$dbh->do('DELETE FROM cc WHERE who = ?', undef, $otherUserID);
foreach my $bug_id (@$buglist) {
LogActivityEntry($bug_id, 'cc', $otherUser->login, '', $userid,
diff --git a/extensions/example/code/flag-end_of_update.pl b/extensions/example/code/flag-end_of_update.pl
index 6371bd154..e1705cc57 100644
--- a/extensions/example/code/flag-end_of_update.pl
+++ b/extensions/example/code/flag-end_of_update.pl
@@ -26,15 +26,15 @@ use Bugzilla::Util qw(diff_arrays);
# This code doesn't actually *do* anything, it's just here to show you
# how to use this hook.
my $args = Bugzilla->hook_args;
-my ($bug, $timestamp, $old_flags, $new_flags) =
- @$args{qw(bug timestamp old_flags new_flags)};
+my ($object, $timestamp, $old_flags, $new_flags) =
+ @$args{qw(object timestamp old_flags new_flags)};
my ($removed, $added) = diff_arrays($old_flags, $new_flags);
my ($granted, $denied) = (0, 0);
foreach my $new_flag (@$added) {
$granted++ if $new_flag =~ /\+$/;
$denied++ if $new_flag =~ /-$/;
}
-my $bug_id = $bug->id;
+my $bug_id = (ref $object eq 'Bugzilla::Bug') ? $object->id : $object->bug_id;
my $result = "$granted flags were granted and $denied flags were denied"
. " on bug $bug_id at $timestamp.";
# Uncomment this line to see $result in your webserver's error log whenever
diff --git a/post_bug.cgi b/post_bug.cgi
index 9a933fc1a..997b621ad 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -104,7 +104,7 @@ if (defined $cgi->param('maketemplate')) {
umask 0;
# get current time
-my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
+my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
# Group Validation
my @selected_groups;
@@ -219,7 +219,10 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
if ($attachment) {
# Set attachment flags.
- Bugzilla::Flag->set_flags($bug, $attachment, $timestamp, $vars);
+ my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
+ $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR);
+ $attachment->set_flags($flags, $new_flags);
+ $attachment->update($timestamp);
# Update the comment to include the new attachment ID.
# This string is hardcoded here because Template::quoteUrls()
@@ -246,7 +249,10 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
}
# Set bug flags.
-Bugzilla::Flag->set_flags($bug, undef, $timestamp, $vars);
+my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($bug, undef, $vars,
+ SKIP_REQUESTEE_ON_ERROR);
+$bug->set_flags($flags, $new_flags);
+$bug->update($timestamp);
# Email everyone the details of the new bug
$vars->{'mailrecipients'} = {'changer' => $user->login};
diff --git a/process_bug.cgi b/process_bug.cgi
index 9faaf7445..f10467d4e 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -143,22 +143,13 @@ if (defined $cgi->param('dontchange')) {
}
# do a match on the fields if applicable
-
-# The order of these function calls is important, as Flag::validate
-# assumes User::match_field has ensured that the values
-# in the requestee fields are legitimate user email addresses.
-&Bugzilla::User::match_field($cgi, {
+Bugzilla::User::match_field($cgi, {
'qa_contact' => { 'type' => 'single' },
'newcc' => { 'type' => 'multi' },
'masscc' => { 'type' => 'multi' },
'assigned_to' => { 'type' => 'single' },
- '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
});
-# Validate flags in all cases. validate() should not detect any
-# reference to flags if $cgi->param('id') is undefined.
-Bugzilla::Flag::validate($cgi->param('id'));
-
print $cgi->header() unless Bugzilla->usage_mode == USAGE_MODE_EMAIL;
# Check for a mid-air collision. Currently this only works when updating
@@ -280,6 +271,12 @@ foreach my $bug (@bug_objects) {
$product_change ||= $changed;
}
+# Flags should be set AFTER the bug has been moved into another product/component.
+if ($cgi->param('id')) {
+ my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($first_bug, undef, $vars);
+ $first_bug->set_flags($flags, $new_flags);
+}
+
if ($cgi->param('id') && (defined $cgi->param('dependson')
|| defined $cgi->param('blocked')) )
{
@@ -586,9 +583,6 @@ foreach my $bug (@bug_objects) {
CheckIfVotedConfirmed($bug->id);
}
- # Set and update flags.
- Bugzilla::Flag->process($bug, undef, $timestamp, $vars);
-
$dbh->bz_commit_transaction();
###############
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 91207a20e..97fd59d21 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -222,15 +222,10 @@
but you tried to flag it as obsolete while creating a new attachment to
[% terms.bug %] [%+ my_bug_id FILTER html %].
- [% ELSIF error == "flags_not_available" %]
- [% title = "Flag Editing not Allowed" %]
- [% IF type == "b" %]
- Flags cannot be set or changed when
- changing several [% terms.bugs %] at once.
- [% ELSE %]
- References to existing flags when creating
- a new attachment are invalid.
- [% END %]
+ [% ELSIF error == "flag_unexpected_object" %]
+ [% title = "Object Not Recognized" %]
+ Flags cannot be set for objects of type [% caller FILTER html %].
+ They can only be set for [% terms.bugs %] and attachments.
[% ELSIF error == "flag_requestee_disabled" %]
[% title = "Flag not Requestable from Specific Person" %]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 69811f210..a30f29706 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -568,12 +568,6 @@
<br>Alternately, if your attachment is an image, you could convert
it to a compressible format like JPG or PNG and try again.
- [% ELSIF error == "flag_not_multiplicable" %]
- [% docslinks = {'flags-overview.html' => 'An overview on Flags',
- 'flags.html' => 'Using Flags'} %]
- You can't ask more than one person at a time for
- <em>[% type.name FILTER html %]</em>.
-
[% ELSIF error == "flag_requestee_needs_privs" %]
[% title = "Flag Requestee Needs Privileges" %]
[% requestee.identity FILTER html %] does not have permission to set the
@@ -632,6 +626,12 @@
The name <em>[% name FILTER html %]</em> must be 1-50 characters long
and must not contain any spaces or commas.
+ [% ELSIF error == "flag_type_not_multiplicable" %]
+ [% docslinks = {'flags-overview.html' => 'An overview on Flags',
+ 'flags.html' => 'Using Flags'} %]
+ You cannot have several <em>[% type.name FILTER html %]</em> flags
+ for this [% IF attachment %] attachment [% ELSE %] [%+ terms.bug %] [% END %].
+
[% ELSIF error == "flag_update_denied" %]
[% title = "Flag Modification Denied" %]
[% admindocslinks = {'flags-overview.html#flags-admin' => 'Administering Flags',
@@ -1158,9 +1158,11 @@
[% ELSIF error == "no_bugs_in_list" %]
[% title = "Delete Tag?" %]
This will remove all [% terms.bugs %] from the
- [% tag FILTER html %] tag. This will delete the tag completely. Click
+ <em>[% name FILTER html %]</em> tag. This will delete the tag completely. Click
<a href="buglist.cgi?cmdtype=dorem&amp;remaction=forget&amp;namedcmd=
- [%- tag FILTER url_quote %]">here</a> if you really want to delete it.
+ [%- name FILTER url_quote %]&amp;token=
+ [%- issue_hash_token([query_id, name]) FILTER url_quote %]">here</a>
+ if you really want to delete it.
[% ELSIF error == "no_bugs_to_remove" %]
[% title = "No Tag Selected" %]
@@ -1742,6 +1744,10 @@
milestone
[% ELSIF class == "Bugzilla::Status" %]
status
+ [% ELSIF class == "Bugzilla::Flag" %]
+ flag
+ [% ELSIF class == "Bugzilla::FlagType" %]
+ flagtype
[% ELSIF class == "Bugzilla::Field" %]
field
[% ELSIF ( matches = class.match('^Bugzilla::Field::Choice::(.+)') ) %]
diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl
index 81948c42c..adc0faa1c 100644
--- a/template/en/default/request/email.txt.tmpl
+++ b/template/en/default/request/email.txt.tmpl
@@ -24,28 +24,31 @@
[% bugidsummary = bug.bug_id _ ': ' _ bug.short_desc %]
[% attidsummary = attachment.id _ ': ' _ attachment.description %]
+[% flagtype_name = flag ? flag.type.name : old_flag.type.name %]
[% statuses = { '+' => "granted" , '-' => 'denied' , 'X' => "canceled" ,
'?' => "asked" } %]
[% to_identity = "" %]
[% on_behalf_of = 0 %]
-[% IF flag.status == '?' %]
+[% action = flag.status || 'X' %]
+
+[% IF flag && flag.status == '?' %]
[% subject_status = "requested" %]
- [% IF flag.setter.id == user.id %]
+ [% IF flag.setter_id == user.id %]
[% to_identity = flag.requestee.identity _ " for" %]
[% ELSE %]
[% on_behalf_of = 1 %]
[% IF flag.requestee %][% to_identity = " to " _ flag.requestee.identity %][% END %]
[% END %]
[% ELSE %]
- [% IF flag.requester %]
- [% to_identity = flag.requester.identity _ "'s request for" %]
+ [% IF old_flag && old_flag.status == '?' %]
+ [% to_identity = old_flag.setter.identity _ "'s request for" %]
[% END %]
- [% subject_status = statuses.${flag.status} %]
+ [% subject_status = statuses.$action %]
[% END %]
From: [% Param('mailfrom') %]
To: [% to %]
-Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ bug.bug_id %]] [% bug.short_desc %]
+Subject: [% flagtype_name %] [%+ subject_status %]: [[% terms.Bug %] [%+ bug.bug_id %]] [% bug.short_desc %]
[%- IF attachment %] :
[Attachment [% attachment.id %]] [% attachment.description %][% END %]
X-Bugzilla-Type: request
@@ -55,10 +58,10 @@ X-Bugzilla-Type: request
[%- FILTER bullet = wrap(80) -%]
[% IF on_behalf_of %]
-[% user.identity %] has reassigned [% flag.setter.identity %]'s request for [% flag.type.name %]
+[% user.identity %] has reassigned [% flag.setter.identity %]'s request for [% flagtype_name %]
[% to_identity %]:
[% ELSE %]
-[% user.identity %] has [% statuses.${flag.status} %] [%+ to_identity %] [%+ flag.type.name %]:
+[% user.identity %] has [% statuses.$action %] [%+ to_identity %] [%+ flagtype_name %]:
[% END %]
[% terms.Bug %] [%+ bugidsummary %]