Skip to content

bugzilla
bugzilla
Commit 0d39e7b4 authored by Max Kanat-Alexander's avatar Max Kanat-Alexander
Browse files
Options

Bug 573173: Make Bugzilla::Bug's add_group and remove_group take group

names instead of ids r=dkl, a=mkanat
parent ed5c3b39
Hide whitespace changes
Inline Side-by-side
Showing with 53 additions and 59 deletions
Bugzilla/Bug.pm
View file @ 0d39e7b4
......@@ -2682,8 +2682,7 @@ sub add_group {
my ($self, $group) = @_;
# Invalid ids are silently ignored. (We can't tell people whether
# or not a group exists.)
$group = new Bugzilla::Group($group) unless ref $group;
return unless $group;
$group = Bugzilla::Group->check($group) if !blessed $group;
return if !$group->is_active or !$group->is_bug_group;
......@@ -2691,7 +2690,7 @@ sub add_group {
# to this group by the current user.
$self->product_obj->group_is_settable($group)
|| ThrowUserError('group_invalid_restriction',
{ product => $self->product, group_id => $group->id,
{ product => $self->product, group => $group,
bug => $self });
# OtherControl people can add groups only during a product change,
......@@ -2702,7 +2701,7 @@ sub add_group {
|| $controls->{othercontrol} == CONTROLMAPNA)
{
ThrowUserError('group_change_denied',
{ bug => $self, group_id => $group->id });
{ bug => $self, group => $group });
}
}
......@@ -2714,8 +2713,7 @@ sub add_group {
sub remove_group {
my ($self, $group) = @_;
$group = new Bugzilla::Group($group) unless ref $group;
return unless $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
......@@ -2731,7 +2729,7 @@ sub remove_group {
# Nobody can ever remove a Mandatory group.
if ($controls->{membercontrol} == CONTROLMAPMANDATORY) {
ThrowUserError('group_invalid_removal',
{ product => $self->product, group_id => $group->id,
{ product => $self->product, group => $group,
bug => $self });
}
......@@ -2743,7 +2741,7 @@ sub remove_group {
|| $controls->{othercontrol} == CONTROLMAPNA)
{
ThrowUserError('group_change_denied',
{ bug => $self, group_id => $group->id });
{ bug => $self, group => $group });
}
}
}
......
process_bug.cgi
View file @ 0d39e7b4
......@@ -337,27 +337,10 @@ foreach my $field (grep(/^defined_isprivate/, $cgi->param())) {
}
$set_all_fields{comment_is_private} = \%is_private;
my %groups = ( add => [], remove => [] );
my %checked_bit; # Used to avoid adding groups twice (defined_ + actual bit-)
foreach my $param_name (grep(/bit-\d+$/, $cgi->param())) {
$param_name =~ /bit-(\d+)$/;
my $gid = $1;
next if $checked_bit{$gid};
my $bit_param = "bit-$gid";
if (should_set($bit_param, 1)) {
# Check ! first to avoid having to check defined below.
if (!$cgi->param($bit_param)) {
push(@{ $groups{remove} }, $gid);
}
# "== 1" is important because mass-change uses -1 to mean
# "don't change this restriction"
elsif ($cgi->param($bit_param) == 1) {
push(@{ $groups{add} }, $gid);
}
}
$checked_bit{$gid} = 1;
}
$set_all_fields{groups} = \%groups;
my @check_groups = $cgi->param('defined_groups');
my @set_groups = $cgi->param('groups');
my ($removed_groups) = diff_arrays(\@check_groups, \@set_groups);
$set_all_fields{groups} = { add => \@set_groups, remove => $removed_groups };
my @custom_fields = Bugzilla->active_custom_fields;
foreach my $field (@custom_fields) {
......
template/en/default/bug/edit.html.tmpl
View file @ 0d39e7b4
......@@ -654,14 +654,15 @@
[% END %]
[% IF group.ingroup %]
<input type="hidden" name="defined_bit-[% group.bit %]" value="1">
<input type="hidden" name="defined_groups"
value="[% group.name FILTER html %]">
[% END %]
<input type="checkbox" value="1" name="bit-[% group.bit %]"
id="bit-[% group.bit %]"
<input type="checkbox" value="[% group.name FILTER html %]"
name="groups" id="group_[% group.bit %]"
[% ' checked="checked"' IF group.ison %]
[% ' disabled="disabled"' IF NOT group.ingroup %]>
<label for="bit-[% group.bit %]">
<label for="group_[% group.bit %]">
[%- group.description FILTER html_light %]</label>
<br>
[% END %]
......
template/en/default/bug/process/verify-new-product.html.tmpl
View file @ 0d39e7b4
......@@ -40,7 +40,7 @@
[% SET exclude_items = ['version', 'component', 'target_milestone'] %]
[% IF verify_bug_groups %]
[% exclude_items.push('bit-\d+') %]
[% exclude_items.push('groups', 'defined_groups') %]
[% END %]
[% Hook.process('exclude') %]
......@@ -124,9 +124,10 @@
[%+ terms.Bugs %] will no longer be restricted to these groups and may become
public if no other group applies:<br>
[% FOREACH group = old_groups %]
<input type="checkbox" id="bit-[% group.id FILTER html %]"
name="bit-[% group.id FILTER html %]" disabled="disabled" value="1">
<label for="bit-[% group.id FILTER html %]">
<input type="checkbox" id="group_[% group.group.id FILTER html %]"
name="groups" disabled="disabled"
value="[% group.group.name FILTER html %]">
<label for="group_[% group.group.id FILTER html %]">
[% group.name FILTER html %]: [% group.description FILTER html %]
</label>
<br>
......@@ -154,16 +155,15 @@
<p>These groups are optional. You can decide to restrict [% terms.bugs %] to
one or more of the following groups:<br>
[% FOREACH group = optional_groups %]
<input type="hidden" name="defined_bit-[% group.group.id FILTER html %]"
value="1">
<input type="checkbox" id="bit-[% group.group.id FILTER html %]"
name="bit-[% group.group.id FILTER html %]"
[%+ ((group.membercontrol == constants.CONTROLMAPDEFAULT && user.in_group(group.group.name))
<input type="hidden" name="defined_groups"
value="[% group.group.name FILTER html %]">
<input type="checkbox" id="group_[% group.group.id FILTER html %]"
name="groups"
[% ' checked="checked"' IF ((group.membercontrol == constants.CONTROLMAPDEFAULT && user.in_group(group.group.name))
|| (group.othercontrol == constants.CONTROLMAPDEFAULT && !user.in_group(group.group.name))
|| cgi.param("bit-$group.group.id") == 1) ?
'checked="checked"' : ''
%] value="1">
<label for="bit-[% group.group.id FILTER html %]">
|| cgi.param("groups").contains(group.group.name)) %]
%] value="[% group.group.name FILTER html %]">
<label for="group_[% group.group.id FILTER html %]">
[% group.group.name FILTER html %]: [% group.group.description FILTER html %]
</label>
<br>
......@@ -175,9 +175,10 @@
<p>These groups are mandatory and [% terms.bugs %] will be automatically
restricted to these groups:<br>
[% FOREACH group = mandatory_groups %]
<input type="checkbox" id="bit-[% group.group.id FILTER html %]" checked="checked"
name="bit-[% group.group.id FILTER html %]" value="1" disabled="disabled">
<label for="bit-[% group.group.id FILTER html %]">
<input type="checkbox" id="group_[% group.group.id FILTER html %]"
checked="checked" disabled="disabled"
name="groups" value="[% group.group.name FILTER html %]">
<label for="group_[% group.group.id FILTER html %]">
[% group.group.name FILTER html %]: [% group.group.description FILTER html %]
</label>
<br>
......
template/en/default/global/user-error.html.tmpl
View file @ 0d39e7b4
......@@ -701,7 +701,7 @@
[% ELSIF error == "group_change_denied" %]
[% title = "Cannot Add/Remove That Group" %]
You tried to add or remove group id [% group_id FILTER html %]
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.
......@@ -726,13 +726,13 @@
[% ELSIF error == "group_invalid_removal" %]
You tried to remove [% terms.bug %] [%+ bug.id FILTER html %]
from group id [% group_id FILTER html %], but [% terms.bugs %] in the
'[% product FILTER html %]' product can not be removed from that
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 group id [% group_id FILTER html %], but [% terms.bugs %] in the
to the '[% group.name FILTER html %]' group, but [% terms.bugs %] in the
'[% product FILTER html %]' product can not be restricted to
that group.
......
template/en/default/list/edit-multiple.html.tmpl
View file @ 0d39e7b4
......@@ -309,10 +309,18 @@
[% IF groups.size > 0 %]
<script type="text/javascript">
function turn_off(myself, id) {
var other_checkbox = document.getElementById(id);
if (myself.checked && other_checkbox) {
other_checkbox.checked = false;
}
}
</script>
<b>Groups:</b><br>
<table border="1">
<tr>
<th>Don't<br>change<br>this group<br>restriction</th>
<th>Remove<br>[% terms.bugs %]<br>from this<br>group</th>
<th>Add<br>[% terms.bugs %]<br>to this<br>group</th>
<th>Group Name:</th>
......@@ -321,14 +329,17 @@
[% FOREACH group = groups %]
<tr>
<td align="center">
<input type="radio" name="bit-[% group.id %]" value="-1" checked="checked">
</td>
<td align="center">
<input type="radio" name="bit-[% group.id %]" value="0">
<input type="checkbox" name="defined_groups"
id="defined_group_[% group.id %]"
value="[% group.name FILTER html %]"
onchange="turn_off(this, 'group_[% group.id %]')">
</td>
[% IF group.is_active %]
<td align="center">
<input type="radio" name="bit-[% group.id %]" value="1">
<input type="checkbox" name="groups"
id="group_[% group.id FILTER html %]"
value="[% group.name FILTER html %]"
onchange="turn_off(this, 'defined_group_[% group.id %]')">
</td>
[% ELSE %]
<td>&nbsp;</td>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment