sub _check_groups {
my ($invocant, $group_names, undef, $params) = @_;
+
+ my $bug_id = blessed($invocant) ? $invocant->id : undef;
my $product = blessed($invocant) ? $invocant->product_obj
: $params->{product};
my %add_groups;
if !ref $group_names;
# First check all the groups they chose to set.
+ my %args = ( product => $product->name, bug_id => $bug_id, action => 'add' );
foreach my $name (@$group_names) {
- my $group = Bugzilla::Group->check(
- { name => $name, product => $product,
- _error => 'group_restriction_not_allowed' });
+ my $group = Bugzilla::Group->check_no_disclose({ %args, name => $name });
if (!$product->group_is_settable($group)) {
- ThrowUserError('group_restriction_not_allowed',
- { name => $name, product => $product });
+ ThrowUserError('group_restriction_not_allowed', { %args, name => $name });
}
$add_groups{$group->id} = $group;
}
$self->set_target_milestone($tm_name);
}
}
-
+
if ($product_changed) {
- # Remove groups that can't be set in the new product, or that aren't
- # active.
- #
+ # Remove groups that can't be set in the new product.
# We copy this array because the original array is modified while we're
# working, and that confuses "foreach".
my @current_groups = @{$self->groups_in};
foreach my $group (@current_groups) {
- if (!$group->is_active or !$product->group_is_valid($group)) {
+ if (!$product->group_is_valid($group)) {
$self->remove_group($group);
}
}
-
+
# Make sure the bug is in all the mandatory groups for the new product.
foreach my $group (@{$product->groups_mandatory}) {
$self->add_group($group);
sub add_group {
my ($self, $group) = @_;
- # Invalid ids are silently ignored. (We can't tell people whether
- # or not a group exists.)
- $group = Bugzilla::Group->check($group) if !blessed $group;
- return if !$group->is_active or !$group->is_bug_group;
+ # If the user enters "FoO" but the DB has "Foo", $group->name would
+ # return "Foo" and thus revealing the existence of the group name.
+ # So we have to store and pass the name as entered by the user to
+ # the error message, if we have it.
+ my $group_name = blessed($group) ? $group->name : $group;
+ my $args = { name => $group_name, product => $self->product,
+ bug_id => $self->id, action => 'add' };
+
+ $group = Bugzilla::Group->check_no_disclose($args) if !blessed $group;
+
+ # If the bug is already in this group, then there is nothing to do.
+ return if $self->in_group($group);
+
# Make sure that bugs in this product can actually be restricted
# to this group by the current user.
$self->product_obj->group_is_settable($group)
- || ThrowUserError('group_invalid_restriction',
- { product => $self->product, group => $group,
- bug => $self });
+ || ThrowUserError('group_restriction_not_allowed', $args);
# OtherControl people can add groups only during a product change,
# and only when the group is not NA for them.
if (!$self->{_old_product_name}
|| $controls->{othercontrol} == CONTROLMAPNA)
{
- ThrowUserError('group_change_denied',
- { bug => $self, group => $group });
+ ThrowUserError('group_restriction_not_allowed', $args);
}
}
my $current_groups = $self->groups_in;
- if (!grep($group->id == $_->id, @$current_groups)) {
- push(@$current_groups, $group);
- }
+ push(@$current_groups, $group);
}
sub remove_group {
my ($self, $group) = @_;
- $group = Bugzilla::Group->check($group) if !blessed $group;
-
- # First, check if this is a valid group for this product.
- # You can *always* remove a group that is not valid for this product
- # or that is not active, so we don't do any other checks if either of
- # those are the case. (Users might remove inactive groups, and set_product
- # removes groups that aren't valid for this product.)
- #
- # This particularly happens when isbuggroup is no longer 1, and we're
- # moving a bug to a new product.
- if ($group->is_active and $self->product_obj->group_is_valid($group)) {
+
+ # See add_group() for the reason why we store the user input.
+ my $group_name = blessed($group) ? $group->name : $group;
+ my $args = { name => $group_name, product => $self->product,
+ bug_id => $self->id, action => 'remove' };
+
+ $group = Bugzilla::Group->check_no_disclose($args) if !blessed $group;
+
+ # If the bug isn't in this group, then either the name is misspelled,
+ # or the group really doesn't exist. Let the user know about this problem.
+ $self->in_group($group) || ThrowUserError('group_invalid_removal', $args);
+
+ # Check if this is a valid group for this product. You can *always*
+ # remove a group that is not valid for this product (set_product does this).
+ # This particularly happens when we're moving a bug to a new product.
+ # You still have to be a member of an inactive group to remove it.
+ if ($self->product_obj->group_is_valid($group)) {
my $controls = $self->product_obj->group_controls->{$group->id};
- # Nobody can ever remove a Mandatory group.
- if ($controls->{membercontrol} == CONTROLMAPMANDATORY) {
- ThrowUserError('group_invalid_removal',
- { product => $self->product, group => $group,
- bug => $self });
+ # Nobody can ever remove a Mandatory group, unless it became inactive.
+ if ($controls->{membercontrol} == CONTROLMAPMANDATORY && $group->is_active) {
+ ThrowUserError('group_invalid_removal', $args);
}
# OtherControl people can remove groups only during a product change,
|| $controls->{othercontrol} == CONTROLMAPMANDATORY
|| $controls->{othercontrol} == CONTROLMAPNA)
{
- ThrowUserError('group_change_denied',
- { bug => $self, group => $group });
+ ThrowUserError('group_invalid_removal', $args);
}
}
}
-
+
my $current_groups = $self->groups_in;
@$current_groups = grep { $_->id != $group->id } @$current_groups;
}
return $self->{'groups_in'};
}
+sub in_group {
+ my ($self, $group) = @_;
+ return grep($_->id == $group->id, @{$self->groups_in}) ? 1 : 0;
+}
+
sub user {
my $self = shift;
return $self->{'user'} if exists $self->{'user'};
return $ret;
}
+sub check_no_disclose {
+ my ($class, $params) = @_;
+ my $action = delete $params->{action};
+
+ $action =~ /^(?:add|remove)$/
+ or ThrowCodeError('bad_arg', { argument => $action,
+ function => "${class}::check_no_disclose" });
+
+ $params->{_error} = ($action eq 'add') ? 'group_restriction_not_allowed'
+ : 'group_invalid_removal';
+
+ my $group = $class->check($params);
+ return $group;
+}
+
###############################
### Validators ###
###############################
=over
+=item C<check_no_disclose>
+
+=over
+
+=item B<Description>
+
+Throws an error if the user cannot add or remove this group to/from a given
+bug, but doesn't specify if this is because the group doesn't exist, or the
+user is not allowed to edit this group restriction.
+
+=item B<Params>
+
+This method takes a single hashref as argument, with the following keys:
+
+=over
+
+=item C<name>
+
+C<string> The name of the group to add or remove.
+
+=item C<bug_id>
+
+C<integer> The ID of the bug to which the group change applies.
+
+=item C<product>
+
+C<string> The name of the product the bug belongs to.
+
+=item C<action>
+
+C<string> Must be either C<add> or C<remove>, depending on whether the group
+must be added or removed from the bug. Any other value will generate an error.
+
+=back
+
+=item C<Returns>
+
+A C<Bugzilla::Group> object on success, else an error is thrown.
+
+=back
+
=item C<check_members_are_visible>
Throws an error if this group is not visible (according to
in the database which refer to it. All references to this group must
be removed before you can remove it.
- [% ELSIF error == "group_change_denied" %]
- [% title = "Cannot Add/Remove That Group" %]
- You tried to add or remove the '[% group.name FILTER html %]' group
- from [% terms.bug %] [%+ bug.id FILTER html %], but you do not
- have permissions to do so.
-
[% ELSIF error == "group_exists" %]
[% title = "The group already exists" %]
The group [% name FILTER html %] already exists.
[% ELSIF error == "group_invalid_removal" %]
- You tried to remove [% terms.bug %] [%+ bug.id FILTER html %]
- from the '[% group.name FILTER html %]' group, but [% terms.bugs %]
- in the '[% product FILTER html %]' product can not be removed from that
- group.
-
- [% ELSIF error == "group_invalid_restriction" %]
- You tried to restrict [% terms.bug %] [%+ bug.id FILTER html %] to
- to the '[% group.name FILTER html %]' group, but [% terms.bugs %] in the
- '[% product FILTER html %]' product can not be restricted to
- that group.
+ You tried to remove [% terms.bug %] [%+ bug_id FILTER html %]
+ from the '[% name FILTER html %]' group, but either this group does not exist,
+ or you are not allowed to remove [% terms.bugs %] from this group in the
+ '[% product FILTER html %]' product.
[% ELSIF error == "group_restriction_not_allowed" %]
[% title = "Group Restriction Not Allowed" %]
- You tried to restrict [% terms.abug %] to the "[% name FILTER html %]"
- group, but either this group does not exist, or you are not allowed
- to restrict [% terms.bugs %] to this group in the "[% product.name FILTER html %]"
- product.
+ You tried to restrict [% bug_id ? "$terms.bug $bug_id" : terms.abug FILTER html %]
+ to the '[% name FILTER html %]' group, but either this group does not exist,
+ or you are not allowed to restrict [% terms.bugs %] to this group in the
+ '[% product FILTER html %]' product.
[% ELSIF error == "group_not_specified" %]
[% title = "Group not specified" %]
projects / thirdparty / bugzilla.git / commitdiff
