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_remove>
=over
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" %]