summaryrefslogtreecommitdiffstats
path: root/Bugzilla
diff options
context:
space:
mode:
authorlpsolit%gmail.com <>2006-08-25 00:56:39 +0200
committerlpsolit%gmail.com <>2006-08-25 00:56:39 +0200
commit41e381d9d5d1fe53fbf92127c3f65eac4f531f36 (patch)
treef1d680fa63706d55f3a9720e3d9bb821e9d2eca4 /Bugzilla
parent6d154983302359ba9d38e1ff659c580853f68c2d (diff)
downloadbugzilla-41e381d9d5d1fe53fbf92127c3f65eac4f531f36.tar.gz
bugzilla-41e381d9d5d1fe53fbf92127c3f65eac4f531f36.tar.xz
Bug 343809: Merge FlagType::validate() with Flag::validate() - Patch by Frédéric Buclin <LpSolit@gmail.com> a=myk
Diffstat (limited to 'Bugzilla')
-rw-r--r--Bugzilla/Attachment.pm12
-rw-r--r--Bugzilla/Flag.pm301
-rw-r--r--Bugzilla/FlagType.pm137
3 files changed, 186 insertions, 264 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 90ec68974..ab2562521 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -729,8 +729,8 @@ sub insert_attachment_for_bug {
$isurl = 0;
}
- # The order of these function calls is important, as both Flag::validate
- # and FlagType::validate assume User::match_field has ensured that the
+ # 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' },
@@ -744,13 +744,11 @@ sub insert_attachment_for_bug {
$$hr_vars->{'message'} = 'user_match_multiple';
}
- # FlagType::validate() and 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.
+ # 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.
# XXX needs $throw_error treatment
Bugzilla::Flag::validate($cgi, $bug->bug_id, -1);
- Bugzilla::FlagType::validate($cgi, $bug->bug_id, -1);
# Escape characters in strings that will be used in SQL statements.
my $description = $cgi->param('description');
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 04623524e..8b09102d1 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -96,6 +96,14 @@ Returns the ID of the flag.
Returns the name of the flagtype the flag belongs to.
+=item C<bug_id>
+
+Returns the ID of the bug this flag belongs to.
+
+=item C<attach_id>
+
+Returns the ID of the attachment this flag belongs to, if any.
+
=item C<status>
Returns the status '+', '-', '?' of the flag.
@@ -106,6 +114,8 @@ Returns the status '+', '-', '?' of the flag.
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'}; }
###############################
@@ -235,152 +245,203 @@ to -1 to force its check anyway.
sub validate {
my ($cgi, $bug_id, $attach_id) = @_;
- my $user = Bugzilla->user;
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-nnn", where "nnn" is the ID,
- # and returning just the ID portion of matching field names.
- my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
+ # 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));
- return unless scalar(@ids);
-
# No flag reference should exist when changing several bugs at once.
ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
- # No reference to existing flags should exist when creating a new
- # attachment.
- if ($attach_id && ($attach_id < 0)) {
- ThrowCodeError("flags_not_available", { type => 'a' });
+ # 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().
+
+ # All new flags must belong to active flag types.
+ if (scalar(@flagtype_ids)) {
+ my $inactive_flagtypes =
+ $dbh->selectrow_array('SELECT 1 FROM flagtypes
+ WHERE id IN (' . join(',', @flagtype_ids) . ')
+ AND is_active = 0 ' .
+ $dbh->sql_limit(1));
+
+ ThrowCodeError('flag_type_inactive') if $inactive_flagtypes;
}
- # Make sure all 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 id IN (" . join(',', @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 });
+ 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' });
+ }
+
+ # 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 id IN (" . join(',', @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 });
+ }
}
- foreach my $id (@ids) {
+ # 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;
+
+ # Don't bother validating types the user didn't touch.
+ next if $status eq 'X';
+
+ # Make sure the flag type exists.
+ my $flag_type = new Bugzilla::FlagType($id);
+ $flag_type || ThrowCodeError('flag_type_nonexistent', { id => $id });
+
+ _validate(undef, $flag_type, $status, \@requestees, $private_attachment,
+ $bug_id, $attach_id);
+ }
+
+ # 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;
# Make sure the flag exists.
my $flag = new Bugzilla::Flag($id);
$flag || ThrowCodeError("flag_nonexistent", { id => $id });
- # 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 was requested before it became unrequestable, leave it
- # as is.
- if ($status eq '?'
- && $flag->status ne '?'
- && !$flag->type->is_requestable)
- {
- ThrowCodeError("flag_status_invalid",
- { id => $id, status => $status });
- }
+ _validate($flag, $flag->type, $status, \@requestees, $private_attachment);
+ }
+}
- # Make sure the user didn't specify a requestee unless the flag
- # is specifically requestable. 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->requestee ? $flag->requestee->login : '';
- my $new_requestee = join('', @requestees);
- if ($new_requestee && $new_requestee ne $old_requestee) {
- ThrowCodeError("flag_requestee_disabled",
- { type => $flag->type });
- }
- }
+sub _validate {
+ my ($flag, $flag_type, $status, $requestees, $private_attachment,
+ $bug_id, $attach_id) = @_;
- # 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 });
+ my $user = 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)
+ {
+ ThrowCodeError('flag_status_invalid',
+ { id => $id, status => $status });
+ }
+
+ # 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 });
}
+ }
- # 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->requestee ? $flag->requestee->login : '';
- foreach my $login (@requestees) {
- next if $login eq $old_requestee;
-
- # We know the requestee exists because we ran
- # Bugzilla::User::match_field before getting here.
- my $requestee = new Bugzilla::User({ name => $login });
-
- # 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)) {
- ThrowUserError("flag_requestee_unauthorized",
- { flag_type => $flag->type,
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id => $attach_id
- });
- }
-
- # 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
- && $cgi->param('isprivate')
- && Bugzilla->params->{"insidergroup"}
- && !$requestee->in_group(Bugzilla->params->{"insidergroup"}))
- {
- ThrowUserError("flag_requestee_unauthorized_attachment",
- { flag_type => $flag->type,
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id => $attach_id
- });
- }
+ # 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 });
+ }
+
+ # 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 : '';
+ foreach my $login (@$requestees) {
+ next if $login eq $old_requestee;
+
+ # We know the requestee exists because we ran
+ # Bugzilla::User::match_field before getting here.
+ my $requestee = new Bugzilla::User({ name => $login });
+
+ # 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)) {
+ ThrowUserError('flag_requestee_unauthorized',
+ { flag_type => $flag_type,
+ requestee => $requestee,
+ bug_id => $bug_id,
+ attach_id => $attach_id });
}
- }
- # Make sure the user is authorized to modify flags, see bug 180879
- # - The flag is unchanged
- next if ($status eq $flag->status);
-
- # - User in the request_group can clear pending requests and set flags
- # and can rerequest set flags.
- next if (($status eq 'X' || $status eq '?')
- && (!$flag->type->request_group
- || $user->in_group_id($flag->type->request_group->id)));
-
- # - User in the grant_group can set/clear flags, including "+" and "-".
- next if (!$flag->type->grant_group
- || $user->in_group_id($flag->type->grant_group->id));
-
- # - Any other flag modification is denied
- ThrowUserError("flag_update_denied",
- { name => $flag->type->name,
- status => $status,
- old_status => $flag->status });
+ # 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'}))
+ {
+ ThrowUserError('flag_requestee_unauthorized_attachment',
+ { flag_type => $flag_type,
+ requestee => $requestee,
+ bug_id => $bug_id,
+ attach_id => $attach_id });
+ }
+ }
}
+
+ # 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));
+
+ # - User in the request_group can clear pending requests and set flags
+ # and can rerequest set flags.
+ return if (($status eq 'X' || $status eq '?')
+ && (!$flag_type->request_group
+ || $user->in_group_id($flag_type->request_group->id)));
+
+ # - User in the grant_group can set/clear flags, including "+" and "-".
+ return if (!$flag_type->grant_group
+ || $user->in_group_id($flag_type->grant_group->id));
+
+ # - Any other flag modification is denied
+ ThrowUserError('flag_update_denied',
+ { name => $flag_type->name,
+ status => $status,
+ old_status => $flag ? $flag->status : 'X' });
}
sub snapshot {
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 47efbd68a..1504be87d 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -359,143 +359,6 @@ sub count {
return $count;
}
-=pod
-
-=over
-
-=item C<validate($cgi, $bug_id, $attach_id)>
-
-Get a list of flag types to validate. Uses the "map" function
-to extract flag type IDs from form field names by matching columns
-whose name looks like "flag_type-nnn", where "nnn" is the ID,
-and returning just the ID portion of matching field names.
-
-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 ($cgi, $bug_id, $attach_id) = @_;
-
- my $user = Bugzilla->user;
- my $dbh = Bugzilla->dbh;
-
- my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
-
- return unless scalar(@ids);
-
- # No flag reference should exist when changing several bugs at once.
- ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
-
- # We don't check that these flag types are valid for
- # this bug/attachment. This check will be done later when
- # processing new flags, see Flag::FormToNewFlags().
-
- # All flag types have to be active
- my $inactive_flagtypes =
- $dbh->selectrow_array("SELECT 1 FROM flagtypes
- WHERE id IN (" . join(',', @ids) . ")
- AND is_active = 0 " .
- $dbh->sql_limit(1));
-
- ThrowCodeError("flag_type_inactive") if $inactive_flagtypes;
-
- foreach my $id (@ids) {
- my $status = $cgi->param("flag_type-$id");
- my @requestees = $cgi->param("requestee_type-$id");
-
- # Don't bother validating types the user didn't touch.
- next if $status eq "X";
-
- # Make sure the flag type exists.
- my $flag_type = new Bugzilla::FlagType($id);
- $flag_type
- || ThrowCodeError("flag_type_nonexistent", { id => $id });
-
- # Make sure the value of the field is 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 ($status eq '?' && !$flag_type->is_requestable) {
- ThrowCodeError("flag_status_invalid",
- { id => $id , status => $status });
- }
-
- # Make sure the user didn't specify a requestee unless the flag
- # is specifically requestable.
- if ($status eq '?'
- && !$flag_type->is_requesteeble
- && scalar(@requestees) > 0)
- {
- ThrowCodeError("flag_requestee_disabled", { type => $flag_type });
- }
-
- # 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 });
- }
-
- # 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) {
- foreach my $login (@requestees) {
- # We know the requestee exists because we ran
- # Bugzilla::User::match_field before getting here.
- my $requestee = new Bugzilla::User({ name => $login });
-
- # Throw an error if the user can't see the bug.
- if (!$requestee->can_see_bug($bug_id)) {
- ThrowUserError("flag_requestee_unauthorized",
- { flag_type => $flag_type,
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id => $attach_id });
- }
-
- # 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
- && Bugzilla->params->{"insidergroup"}
- && $cgi->param('isprivate')
- && !$requestee->in_group(Bugzilla->params->{"insidergroup"}))
- {
- ThrowUserError("flag_requestee_unauthorized_attachment",
- { flag_type => $flag_type,
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id => $attach_id });
- }
- }
- }
-
- # Make sure the user is authorized to modify flags, see bug 180879
- # - User in the grant_group can set flags, including "+" and "-".
- next if (!$flag_type->grant_group
- || $user->in_group_id($flag_type->grant_group->id));
-
- # - User in the request_group can request flags.
- next if ($status eq '?'
- && (!$flag_type->request_group
- || $user->in_group_id($flag_type->request_group->id)));
-
- # - Any other flag modification is denied
- ThrowUserError("flag_update_denied",
- { name => $flag_type->name,
- status => $status,
- old_status => "X" });
- }
-}
-
######################################################################
# Private Functions
######################################################################