Skip to content

bugzilla
bugzilla
Commit 9de3481e authored by lpsolit%gmail.com's avatar lpsolit%gmail.com
Browse files
Options

Bug 347213: When moving a bug to another product, Bugzilla should let us…

Bug 347213: When moving a bug to another product, Bugzilla should let us preemptively put the bug into a (valid) group which is not a default one for this product Bug 303183: Bugzilla fails to warn when a product change would force the removal of an optional group Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=LpSolit
parent 1ca7c677
Show whitespace changes
Inline Side-by-side
Showing with 149 additions and 272 deletions
process_bug.cgi
View file @ 9de3481e
......@@ -78,34 +78,6 @@ local our $PrivilegesRequired = 0;
# Subroutines
######################################################################
sub BugInGroupId {
my ($bug_id, $group_id) = @_;
detaint_natural($bug_id);
detaint_natural($group_id);
my ($in_group) = Bugzilla->dbh->selectrow_array(
"SELECT CASE WHEN bug_id != 0 THEN 1 ELSE 0 END
FROM bug_group_map
WHERE bug_id = ? AND group_id = ?", undef, ($bug_id, $group_id));
return $in_group;
}
# This function checks if there are any default groups defined.
# If so, then groups may have to be changed when bugs move from
# one bug to another.
sub AnyDefaultGroups {
my $dbh = Bugzilla->dbh;
my $any_default =
$dbh->selectrow_array('SELECT 1
FROM group_control_map
INNER JOIN groups
ON groups.id = group_control_map.group_id
WHERE isactive != 0
AND (membercontrol = ? OR othercontrol = ?) ' .
$dbh->sql_limit(1),
undef, (CONTROLMAPDEFAULT, CONTROLMAPDEFAULT));
return $any_default;
}
# Used to send email when an update is done.
sub send_results {
my ($bug_id, $vars) = @_;
......@@ -329,8 +301,7 @@ if ((defined $cgi->param('id') && $cgi->param('product') ne $bug->product)
# shown. Also show the verification form if the product-specific fields
# somehow still need to be verified, or if we need to verify whether or not
# to add the bugs to their new product's group.
if (!$vok || !$cok || !$mok || !defined $cgi->param('confirm_product_change')
|| (AnyDefaultGroups() && !defined $cgi->param('addtonewgroup'))) {
if (!$vok || !$cok || !$mok || !defined $cgi->param('confirm_product_change')) {
if (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
if (!$vok) {
......@@ -350,10 +321,7 @@ if ((defined $cgi->param('id') && $cgi->param('product') ne $bug->product)
}
}
if (!$vok || !$cok || !$mok
|| !defined $cgi->param('confirm_product_change'))
{
$vars->{'verify_fields'} = 1;
$vars->{'product'} = $product;
my %defaults;
# We set the defaults to these fields to the old value,
# if it's a valid option, otherwise we use the default where
......@@ -375,7 +343,6 @@ if ((defined $cgi->param('id') && $cgi->param('product') ne $bug->product)
}
if (Bugzilla->params->{"usetargetmilestone"}) {
$vars->{'use_target_milestone'} = 1;
$vars->{'milestones'} = \@milestone_names;
if ($mok) {
$defaults{'target_milestone'} = $cgi->param('target_milestone');
......@@ -383,17 +350,22 @@ if ((defined $cgi->param('id') && $cgi->param('product') ne $bug->product)
$defaults{'target_milestone'} = $product->default_milestone;
}
}
else {
$vars->{'use_target_milestone'} = 0;
}
$vars->{'defaults'} = \%defaults;
}
else {
$vars->{'verify_fields'} = 0;
}
$vars->{'verify_bug_group'} = (AnyDefaultGroups()
&& !defined $cgi->param('addtonewgroup'));
# Get the ID of groups which are no longer valid in the new product.
my $gids =
$dbh->selectcol_arrayref('SELECT bgm.group_id
FROM bug_group_map AS bgm
WHERE bgm.bug_id IN (' . join(', ', @idlist) . ')
AND bgm.group_id NOT IN
(SELECT gcm.group_id
FROM group_control_map AS gcm
WHERE gcm.product_id = ?
AND ((gcm.membercontrol != ?
AND gcm.group_id IN (' . $grouplist . '))
OR gcm.othercontrol != ?))',
undef, ($product->id, CONTROLMAPNA, CONTROLMAPNA));
$vars->{'old_groups'} = Bugzilla::Group->new_from_list($gids);
$template->process("bug/process/verify-new-product.html.tmpl", $vars)
|| ThrowTemplateError($template->error());
......@@ -578,36 +550,6 @@ sub DoComma {
$::comma = ",";
}
# Changing this so that it will process groups from checkboxes instead of
# select lists. This means that instead of looking for the bit-X values in
# the form, we need to loop through all the bug groups this user has access
# to, and for each one, see if it's selected.
# If the form element isn't present, or the user isn't in the group, leave
# it as-is
my @groupAdd = ();
my @groupDel = ();
my $groups = $dbh->selectall_arrayref(
qq{SELECT groups.id, isactive FROM groups
WHERE id IN($grouplist) AND isbuggroup = 1});
foreach my $group (@$groups) {
my ($b, $isactive) = @$group;
# The multiple change page may not show all groups a bug is in
# (eg product groups when listing more than one product)
# Only consider groups which were present on the form. We can't do this
# for single bug changes because non-checked checkboxes aren't present.
# All the checkboxes should be shown in that case, though, so it isn't
# an issue there
if (defined $cgi->param('id') || defined $cgi->param("bit-$b")) {
if (!$cgi->param("bit-$b")) {
push(@groupDel, $b);
} elsif ($cgi->param("bit-$b") == 1 && $isactive) {
push(@groupAdd, $b);
}
}
}
foreach my $field ("rep_platform", "priority", "bug_severity",
"bug_file_loc", "short_desc", "version", "op_sys",
"target_milestone", "status_whiteboard") {
......@@ -966,7 +908,6 @@ if (!grep($keywordaction eq $_, qw(add delete makeexact))) {
}
if ($::comma eq ""
&& (! @groupAdd) && (! @groupDel)
&& (!Bugzilla::Keyword::keyword_count()
|| (0 == @keywordlist && $keywordaction ne "makeexact"))
&& defined $cgi->param('masscc') && ! $cgi->param('masscc')
......@@ -1285,18 +1226,21 @@ foreach my $id (@idlist) {
{ product => $oldhash{'product'} });
}
# If we are editing a single bug or if bugs are being moved into
# a specific product, $product is defined. If $product is undefined,
# then we don't move bugs, so we can use their current product.
my $new_product = $product || new Bugzilla::Product({name => $oldhash{'product'}});
if ($requiremilestone) {
# musthavemilestoneonaccept applies only if at least two
# target milestones are defined for the current product.
my $old_product = new Bugzilla::Product({name => $oldhash{'product'}});
if (scalar(@{$old_product->milestones}) > 1) {
# target milestones are defined for the product.
if (scalar(@{$new_product->milestones}) > 1) {
my $value = $cgi->param('target_milestone');
if (!defined $value || $value eq $cgi->param('dontchange')) {
$value = $oldhash{'target_milestone'};
}
# if musthavemilestoneonaccept == 1, then the target
# milestone must be different from the default one.
if ($value eq $old_product->default_milestone) {
if ($value eq $new_product->default_milestone) {
ThrowUserError("milestone_required", { bug_id => $id });
}
}
......@@ -1415,60 +1359,69 @@ foreach my $id (@idlist) {
$dbh->do(q{DELETE FROM duplicates WHERE dupe = ?}, undef, $id);
}
my %groupsrequired = ();
my %groupsforbidden = ();
my $group_controls =
$dbh->selectall_arrayref(q{SELECT id, membercontrol
FROM groups
LEFT JOIN group_control_map
ON id = group_id
AND product_id = ?
WHERE isactive != 0},
undef, $oldhash{'product_id'});
foreach my $group_control (@$group_controls) {
my ($group, $control) = @$group_control;
$control ||= 0;
unless ($control > CONTROLMAPNA) {
$groupsforbidden{$group} = 1;
# First of all, get all groups the bug is currently restricted to.
my $initial_groups =
$dbh->selectcol_arrayref('SELECT group_id, name
FROM bug_group_map
INNER JOIN groups
ON groups.id = bug_group_map.group_id
WHERE bug_id = ?', {Columns=>[1,2]}, $id);
my %original_groups = @$initial_groups;
my %updated_groups = %original_groups;
# Now let's see which groups have to be added or removed.
foreach my $gid (keys %{$new_product->group_controls}) {
my $group = $new_product->group_controls->{$gid};
# Leave inactive groups alone.
next unless $group->{group}->is_active;
if ($group->{membercontrol} == CONTROLMAPMANDATORY
|| ($group->{othercontrol} == CONTROLMAPMANDATORY && !$user->in_group_id($gid)))
{
$updated_groups{$gid} = $group->{group}->name;
}
elsif ($group->{membercontrol} == CONTROLMAPNA
|| ($group->{othercontrol} == CONTROLMAPNA && !$user->in_group_id($gid)))
{
delete $updated_groups{$gid};
}
# When editing several bugs at once, only consider groups which
# have been displayed.
elsif (defined $cgi->param('id') || defined $cgi->param("bit-$gid")) {
if (!$cgi->param("bit-$gid")) {
delete $updated_groups{$gid};
}
# Note that == 1 is important, because == -1 means "ignore this group".
elsif ($cgi->param("bit-$gid") == 1) {
$updated_groups{$gid} = $group->{group}->name;
}
}
if ($control == CONTROLMAPMANDATORY) {
$groupsrequired{$group} = 1;
}
# We also have to remove groups which are not legal in the new product.
foreach my $gid (keys %updated_groups) {
delete $updated_groups{$gid}
unless exists $new_product->group_controls->{$gid};
}
my ($removed, $added) = diff_arrays([keys %original_groups], [keys %updated_groups]);
my @groupAddNames = ();
my @groupAddNamesAll = ();
my $sth = $dbh->prepare(q{INSERT INTO bug_group_map (bug_id, group_id)
VALUES (?, ?)});
foreach my $grouptoadd (@groupAdd, keys %groupsrequired) {
next if $groupsforbidden{$grouptoadd};
my $group_obj = new Bugzilla::Group($grouptoadd);
push(@groupAddNamesAll, $group_obj->name);
if (!BugInGroupId($id, $grouptoadd)) {
push(@groupAddNames, $group_obj->name);
$sth->execute($id, $grouptoadd);
}
}
my @groupDelNames = ();
my @groupDelNamesAll = ();
$sth = $dbh->prepare(q{DELETE FROM bug_group_map
WHERE bug_id = ? AND group_id = ?});
foreach my $grouptodel (@groupDel, keys %groupsforbidden) {
my $group_obj = new Bugzilla::Group($grouptodel);
push(@groupDelNamesAll, $group_obj->name);
next if $groupsrequired{$grouptodel};
if (BugInGroupId($id, $grouptodel)) {
push(@groupDelNames, $group_obj->name);
}
$sth->execute($id, $grouptodel);
}
my $groupDelNames = join(',', @groupDelNames);
my $groupAddNames = join(',', @groupAddNames);
if ($groupDelNames ne $groupAddNames) {
LogActivityEntry($id, "bug_group", $groupDelNames, $groupAddNames,
$whoid, $timestamp);
# We can now update the DB.
if (scalar(@$removed)) {
$dbh->do('DELETE FROM bug_group_map WHERE bug_id = ?
AND group_id IN (' . join(', ', @$removed) . ')',
undef, $id);
}
if (scalar(@$added)) {
my $sth = $dbh->prepare('INSERT INTO bug_group_map
(bug_id, group_id) VALUES (?, ?)');
$sth->execute($id, $_) foreach @$added;
}
# Add the changes to the bug_activity table.
if (scalar(@$removed) || scalar(@$added)) {
my @removed_names = map { $original_groups{$_} } @$removed;
my @added_names = map { $updated_groups{$_} } @$added;
LogActivityEntry($id, 'bug_group', join(',', @removed_names),
join(',', @added_names), $whoid, $timestamp);
$bug_changed = 1;
}
......@@ -1581,127 +1534,6 @@ foreach my $id (@idlist) {
}
}
# When a bug changes products and the old or new product is associated
# with a bug group, it may be necessary to remove the bug from the old
# group or add it to the new one. There are a very specific series of
# conditions under which these activities take place, more information
# about which can be found in comments within the conditionals below.
# Check if the user has changed the product to which the bug belongs;
if ($cgi->param('product') ne $cgi->param('dontchange')
&& $cgi->param('product') ne $oldhash{'product'})
{
# Depending on the "addtonewgroup" variable, groups with
# defaults will change.
#
# For each group, determine
# - The group id and if it is active
# - The control map value for the old product and this group
# - The control map value for the new product and this group
# - Is the user in this group?
# - Is the bug in this group?
my $groups = $dbh->selectall_arrayref(
qq{SELECT DISTINCT groups.id, isactive,
oldcontrolmap.membercontrol,
newcontrolmap.membercontrol,
CASE WHEN groups.id IN ($grouplist) THEN 1 ELSE 0 END,
CASE WHEN bug_group_map.group_id IS NOT NULL
THEN 1 ELSE 0 END
FROM groups
LEFT JOIN group_control_map AS oldcontrolmap
ON oldcontrolmap.group_id = groups.id
AND oldcontrolmap.product_id = ?
LEFT JOIN group_control_map AS newcontrolmap
ON newcontrolmap.group_id = groups.id
AND newcontrolmap.product_id = ?
LEFT JOIN bug_group_map
ON bug_group_map.group_id = groups.id
AND bug_group_map.bug_id = ?},
undef, $oldhash{'product_id'}, $product->id, $id);
my @groupstoremove = ();
my @groupstoadd = ();
my @defaultstoremove = ();
my @defaultstoadd = ();
my @allgroups = ();
my $buginanydefault = 0;
my $buginanychangingdefault = 0;
foreach my $group (@$groups) {
my ($groupid, $isactive, $oldcontrol, $newcontrol,
$useringroup, $bugingroup) = @$group;
# An undefined newcontrol is none.
$newcontrol = CONTROLMAPNA unless $newcontrol;
$oldcontrol = CONTROLMAPNA unless $oldcontrol;
push(@allgroups, $groupid);
if (($bugingroup) && ($isactive)
&& ($oldcontrol == CONTROLMAPDEFAULT)) {
# Bug was in a default group.
$buginanydefault = 1;
if (($newcontrol != CONTROLMAPDEFAULT)
&& ($newcontrol != CONTROLMAPMANDATORY)) {
# Bug was in a default group that no longer is.
$buginanychangingdefault = 1;
push (@defaultstoremove, $groupid);
}
}
if (($isactive) && (!$bugingroup)
&& ($newcontrol == CONTROLMAPDEFAULT)
&& ($useringroup)) {
push (@defaultstoadd, $groupid);
}
if (($bugingroup) && ($isactive) && ($newcontrol == CONTROLMAPNA)) {
# Group is no longer permitted.
push(@groupstoremove, $groupid);
}
if ((!$bugingroup) && ($isactive)
&& ($newcontrol == CONTROLMAPMANDATORY)) {
# Group is now required.
push(@groupstoadd, $groupid);
}
}
# If addtonewgroups = "yes", old default groups will be removed
# and new default groups will be added.
# If addtonewgroups = "yesifinold", old default groups will be removed
# and new default groups will be added only if the bug was in ANY
# of the old default groups.
# If addtonewgroups = "no", old default groups will be removed and not
# replaced.
push(@groupstoremove, @defaultstoremove);
if (AnyDefaultGroups()
&& (($cgi->param('addtonewgroup') eq 'yes')
|| (($cgi->param('addtonewgroup') eq 'yesifinold')
&& ($buginanydefault)))) {
push(@groupstoadd, @defaultstoadd);
}
# Now actually update the bug_group_map.
my @DefGroupsAdded = ();
my @DefGroupsRemoved = ();
my $sth_insert =
$dbh->prepare(q{INSERT INTO bug_group_map (bug_id, group_id)
VALUES (?, ?)});
my $sth_delete = $dbh->prepare(q{DELETE FROM bug_group_map
WHERE bug_id = ?
AND group_id = ?});
foreach my $groupid (@allgroups) {
my $thisadd = grep( ($_ == $groupid), @groupstoadd);
my $thisdel = grep( ($_ == $groupid), @groupstoremove);
if ($thisadd) {
my $group_obj = new Bugzilla::Group($groupid);
push(@DefGroupsAdded, $group_obj->name);
$sth_insert->execute($id, $groupid);
} elsif ($thisdel) {
my $group_obj = new Bugzilla::Group($groupid);
push(@DefGroupsRemoved, $group_obj->name);
$sth_delete->execute($id, $groupid);
}
}
if ((@DefGroupsAdded) || (@DefGroupsRemoved)) {
LogActivityEntry($id, "bug_group",
join(', ', @DefGroupsRemoved),
join(', ', @DefGroupsAdded),
$whoid, $timestamp);
}
}
# get a snapshot of the newly set values out of the database,
# and then generate any necessary bug activity entries by seeing
# what has changed since before we wrote out the new values.
......
template/en/default/bug/process/verify-new-product.html.tmpl
View file @ 9de3481e
......@@ -17,20 +17,16 @@
# Rights Reserved.
#
# Contributor(s): Myk Melez <myk@mozilla.org>
# Frédéric Buclin <LpSolit@gmail.com>
#%]
[%# INTERFACE:
# verify_fields: boolean; whether or not to verify
# the version, component, and target milestone fields
# product: object; the new product.
# versions: array; versions for the new product.
# components: array; components for the new product.
# milestones: array; milestones for the new product.
# defaults: hash; keys are names of fields, values are defaults for
# those fields
# verify_bug_group: boolean; whether or not to ask the user
# if they want to add the bug to its new product's group
# use_target_milestone: boolean; whether or not to use
# the target milestone field
#%]
[%# The global Bugzilla->cgi object is used to obtain form variable values. %]
......@@ -45,15 +41,14 @@
<form action="process_bug.cgi" method="post">
[% PROCESS "global/hidden-fields.html.tmpl"
exclude=(verify_fields ? "^version|component|target_milestone$" : "") %]
exclude=("^version|component|target_milestone|bit-\\d+$") %]
<input type="hidden" name="confirm_product_change" value="1">
[%# Verify the version, component, and target milestone fields. %]
[% IF verify_fields %]
<h3>Verify Version, Component[% ", Target Milestone" IF use_target_milestone %]</h3>
<h3>Verify Version, Component[% ", Target Milestone" IF Param("usetargetmilestone") %]</h3>
<p>
[% IF use_target_milestone %]
[% IF Param("usetargetmilestone") %]
You are moving the [% terms.bug %](s) to the product
<b>[% cgi.param("product") FILTER html %]</b>,
and the version, component, and/or target milestone fields are no longer
......@@ -84,7 +79,7 @@
default=defaults.component
size=10 %]
</td>
[% IF use_target_milestone %]
[% IF Param("usetargetmilestone") %]
<td>
<b>Target Milestone:</b><br>
[% PROCESS "global/select-menu.html.tmpl"
......@@ -97,22 +92,72 @@
</tr>
</table>
[% END %]
[% IF verify_bug_group %]
<h3>Verify [% terms.Bug %] Group</h3>
<p>
Do you want to add the [% terms.bug %] to its new product's default groups (if any)?
[% IF old_groups.size %]
<p>These groups are not legal for the '[% product.name FILTER html %]'
product or you are not allowed to restrict [% terms.bugs %] to these groups.
[%+ 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 %]">
[% group.name FILTER html %]: [% group.description FILTER html %]
</label>
<br>
[% END %]
</p>
[% END %]
<p>
<input type="radio" name="addtonewgroup" value="no"><b>no</b><br>
<input type="radio" name="addtonewgroup" value="yes"><b>yes</b><br>
<input type="radio" name="addtonewgroup" value="yesifinold" checked="checked">
<b>yes, but only if the [% terms.bug %] was in any of its old product's default groups</b><br>
[% mandatory_groups = [] %]
[% optional_groups = [] %]
[% FOREACH gid = product.group_controls.keys %]
[% group = product.group_controls.$gid %]
[% NEXT UNLESS group.group.is_active %]
[% IF group.membercontrol == constants.CONTROLMAPMANDATORY
|| (group.othercontrol == constants.CONTROLMAPMANDATORY && !user.in_group(group.group.name)) %]
[% mandatory_groups.push(group) %]
[% ELSIF (group.membercontrol != constants.CONTROLMAPNA && user.in_group(group.group.name))
|| group.othercontrol != constants.CONTROLMAPNA %]
[% optional_groups.push(group) %]
[% END %]
[% END %]
[% IF optional_groups.size %]
<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="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))
|| (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 %]">
[% group.group.name FILTER html %]: [% group.group.description FILTER html %]
</label>
<br>
[% END %]
</p>
[% END %]
[% END %]
[% IF mandatory_groups.size %]
<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 %]">
[% group.group.name FILTER html %]: [% group.group.description FILTER html %]
</label>
<br>
[% END %]
</p>
[% END %]
<input type="submit" id="change_product" value="Commit">
......
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