Skip to content

Commit dac9873

Browse files
globauglobau
committed
Bug 1151745: add ui to minimise steps required to move bugs between products
1 parent 0d15453 commit dac9873

File tree

11 files changed

+591
-82
lines changed

11 files changed

+591
-82
lines changed

‎Bugzilla/Bug.pm

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ use Bugzilla::Comment;
5252
use Bugzilla::BugUrl;
5353
use Bugzilla::BugUserLastVisit;
5454

55-
use List::MoreUtils qw(firstidx uniq part);
55+
use List::MoreUtils qw(firstidx uniq part any);
5656
use List::Util qw(min max first);
5757
use Storable qw(dclone);
5858
use URI;
@@ -2689,7 +2689,32 @@ sub _set_product {
26892689
# other part of Bugzilla that checks $@.
26902690
undef $@;
26912691
Bugzilla->error_mode($old_error_mode);
2692-
2692+
2693+
my $invalid_groups;
2694+
my @idlist = ($self->id);
2695+
push(@idlist, map { $_->id } @{ $params->{other_bugs} })
2696+
if $params->{other_bugs};
2697+
@idlist = uniq @idlist;
2698+
2699+
# BMO - if everything is ok then we can skip the verfication page
2700+
if ($component_ok && $version_ok && $milestone_ok) {
2701+
$invalid_groups = $self->get_invalid_groups({ bug_ids => \@idlist, product => $product });
2702+
my $has_invalid_group = 0;
2703+
foreach my $group (@$invalid_groups) {
2704+
if (any { $_ eq $group->name } @{ $params->{groups}->{add} }) {
2705+
$has_invalid_group = 1;
2706+
last;
2707+
}
2708+
}
2709+
$params->{product_change_confirmed} =
2710+
# always check for invalid groups
2711+
!$has_invalid_group
2712+
# never skip verification when changing multiple bugs
2713+
&& scalar(@idlist) == 1
2714+
# ensure the user has seen the group ui for private bugs
2715+
&& (!@{ $self->groups_in } || Bugzilla->input_params->{group_verified});
2716+
}
2717+
26932718
my $verified = $params->{product_change_confirmed};
26942719
my %vars;
26952720
if (!$verified || !$component_ok || !$version_ok || !$milestone_ok) {
@@ -2709,27 +2734,9 @@ sub _set_product {
27092734

27102735
if (!$verified) {
27112736
$vars{verify_bug_groups} = 1;
2712-
my $dbh = Bugzilla->dbh;
2713-
my @idlist = ($self->id);
2714-
push(@idlist, map {$_->id} @{ $params->{other_bugs} })
2715-
if $params->{other_bugs};
2716-
# Get the ID of groups which are no longer valid in the new product.
2717-
my $gids = $dbh->selectcol_arrayref(
2718-
'SELECT bgm.group_id
2719-
FROM bug_group_map AS bgm
2720-
WHERE bgm.bug_id IN (' . join(',', ('?') x @idlist) . ')
2721-
AND bgm.group_id NOT IN
2722-
(SELECT gcm.group_id
2723-
FROM group_control_map AS gcm
2724-
WHERE gcm.product_id = ?
2725-
AND ( (gcm.membercontrol != ?
2726-
AND gcm.group_id IN ('
2727-
. Bugzilla->user->groups_as_string . '))
2728-
OR gcm.othercontrol != ?) )',
2729-
undef, (@idlist, $product->id, CONTROLMAPNA, CONTROLMAPNA));
2730-
$vars{'old_groups'} = Bugzilla::Group->new_from_list($gids);
2737+
$vars{old_groups} = $invalid_groups || $self->get_invalid_groups({ bug_ids => \@idlist, product => $product });
27312738
}
2732-
2739+
27332740
if (%vars) {
27342741
$vars{product} = $product;
27352742
$vars{bug} = $self;
@@ -4301,6 +4308,27 @@ sub map_fields {
43014308
return \%field_values;
43024309
}
43034310

4311+
# Return the groups which are no longer valid in the specified product
4312+
sub get_invalid_groups {
4313+
my ($invocant, $params) = @_;
4314+
my @idlist = @{ $params->{bug_ids} };
4315+
my $product = $params->{product};
4316+
my $gids = Bugzilla->dbh->selectcol_arrayref(
4317+
'SELECT bgm.group_id
4318+
FROM bug_group_map AS bgm
4319+
WHERE bgm.bug_id IN (' . join(',', ('?') x @idlist) . ')
4320+
AND bgm.group_id NOT IN
4321+
(SELECT gcm.group_id
4322+
FROM group_control_map AS gcm
4323+
WHERE gcm.product_id = ?
4324+
AND ( (gcm.membercontrol != ?
4325+
AND gcm.group_id IN ('
4326+
. Bugzilla->user->groups_as_string . '))
4327+
OR gcm.othercontrol != ?) )',
4328+
undef, (@idlist, $product->id, CONTROLMAPNA, CONTROLMAPNA));
4329+
return Bugzilla::Group->new_from_list($gids);
4330+
}
4331+
43044332
################################################################################
43054333
# check_can_change_field() defines what users are allowed to change. You
43064334
# can add code here for site-specific policy changes, according to the

‎extensions/BugModal/lib/WebService.pm

Lines changed: 210 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@ use Bugzilla::Bug;
1515
use Bugzilla::Constants;
1616
use Bugzilla::Error;
1717
use Bugzilla::Field;
18+
use Bugzilla::Group;
1819
use Bugzilla::Keyword;
1920
use Bugzilla::Milestone;
21+
use Bugzilla::Product;
2022
use Bugzilla::Version;
23+
use List::MoreUtils qw(any first_value);
2124

2225
# these methods are much lighter than our public API calls
2326

@@ -33,6 +36,7 @@ sub rest_resources {
3336
},
3437
},
3538
},
39+
3640
# returns pre-formatted html, enabling reuse of the user template
3741
qr{^/bug_modal/cc/(\d+)$}, {
3842
GET => {
@@ -42,6 +46,18 @@ sub rest_resources {
4246
},
4347
},
4448
},
49+
50+
# returns fields that require touching when the product is changed
51+
qw{^/bug_modal/new_product/(\d+)$}, {
52+
GET => {
53+
method => 'new_product',
54+
params => sub {
55+
# products with slashes in their name means we have to grab
56+
# the product from the query-string instead of the path
57+
return { id => $_[0], product_name => Bugzilla->input_params->{product} }
58+
},
59+
},
60+
},
4561
]
4662
}
4763

@@ -59,15 +75,15 @@ sub edit {
5975
unless (grep { $_->id == $bug->product_id } @products) {
6076
unshift @products, $bug->product_obj;
6177
}
62-
$options{product} = [ map { { name => $_->name, description => $_->description } } @products ];
78+
$options{product} = [ map { { name => $_->name } } @products ];
6379

64-
$options{component} = _name_desc($bug->component, $bug->product_obj->components);
65-
$options{version} = _name($bug->version, $bug->product_obj->versions);
66-
$options{target_milestone} = _name($bug->target_milestone, $bug->product_obj->milestones);
67-
$options{priority} = _name($bug->priority, 'priority');
68-
$options{bug_severity} = _name($bug->bug_severity, 'bug_severity');
69-
$options{rep_platform} = _name($bug->rep_platform, 'rep_platform');
70-
$options{op_sys} = _name($bug->op_sys, 'op_sys');
80+
$options{component} = _name($bug->product_obj->components, $bug->component);
81+
$options{version} = _name($bug->product_obj->versions, $bug->version);
82+
$options{target_milestone} = _name($bug->product_obj->milestones, $bug->target_milestone);
83+
$options{priority} = _name('priority', $bug->priority);
84+
$options{bug_severity} = _name('bug_severity', $bug->bug_severity);
85+
$options{rep_platform} = _name('rep_platform', $bug->rep_platform);
86+
$options{op_sys} = _name('op_sys', $bug->op_sys);
7187

7288
# custom select fields
7389
my @custom_fields =
@@ -93,27 +109,15 @@ sub edit {
93109
}
94110

95111
sub _name {
96-
my ($current, $values) = @_;
112+
my ($values, $current) = @_;
97113
# values can either be an array-ref of values, or a field name, which
98114
# result in that field's legal-values being used.
99115
if (!ref($values)) {
100116
$values = Bugzilla::Field->new({ name => $values, cache => 1 })->legal_values;
101117
}
102118
return [
103119
map { { name => $_->name } }
104-
grep { $_->name eq $current || $_->is_active }
105-
@$values
106-
];
107-
}
108-
109-
sub _name_desc {
110-
my ($current, $values) = @_;
111-
if (!ref($values)) {
112-
$values = Bugzilla::Field->new({ name => $values, cache => 1 })->legal_values;
113-
}
114-
return [
115-
map { { name => $_->name, description => $_->description } }
116-
grep { $_->name eq $current || $_->is_active }
120+
grep { (defined $current && $_->name eq $current) || $_->is_active }
117121
@$values
118122
];
119123
}
@@ -135,4 +139,188 @@ sub cc {
135139
return { html => $html };
136140
}
137141

142+
sub new_product {
143+
my ($self, $params) = @_;
144+
my $dbh = Bugzilla->dbh;
145+
my $user = Bugzilla->user;
146+
my $bug = Bugzilla::Bug->check({ id => $params->{id} });
147+
my $product = Bugzilla::Product->check({ name => $params->{product_name}, cache => 1 });
148+
my $true = $self->type('boolean', 1);
149+
my %result;
150+
151+
# components
152+
153+
my $components = _name($product->components);
154+
my $current_component = $bug->component;
155+
if (my $component = first_value { $_->{name} eq $current_component} @$components) {
156+
# identical component in both products
157+
$component->{selected} = $true;
158+
}
159+
else {
160+
# default to a blank value
161+
unshift @$components, {
162+
name => '',
163+
selected => $true,
164+
};
165+
}
166+
$result{component} = $components;
167+
168+
# milestones
169+
170+
my $milestones = _name($product->milestones);
171+
my $current_milestone = $bug->target_milestone;
172+
if ($bug->check_can_change_field('target_milestone', 0, 1)
173+
&& (my $milestone = first_value { $_->{name} eq $current_milestone} @$milestones))
174+
{
175+
# identical milestone in both products
176+
$milestone->{selected} = $true;
177+
}
178+
else {
179+
# use default milestone
180+
my $default_milestone = $product->default_milestone;
181+
my $milestone = first_value { $_->{name} eq $default_milestone } @$milestones;
182+
$milestone->{selected} = $true;
183+
}
184+
$result{target_milestone} = $milestones;
185+
186+
# versions
187+
188+
my $versions = _name($product->versions);
189+
my $current_version = $bug->version;
190+
my $selected_version;
191+
if (my $version = first_value { $_->{name} eq $current_version } @$versions) {
192+
# identical version in both products
193+
$version->{selected} = $true;
194+
$selected_version = $version;
195+
}
196+
elsif (
197+
$current_version =~ /^(\d+) Branch$/
198+
|| $current_version =~ /^Firefox (\d+)$/
199+
|| $current_version =~ /^(\d+)$/)
200+
{
201+
# firefox, with its three version naming schemes
202+
my $branch = $1;
203+
foreach my $test_version ("$branch Branch", "Firefox $branch", $branch) {
204+
if (my $version = first_value { $_->{name} eq $test_version } @$versions) {
205+
$version->{selected} = $true;
206+
$selected_version = $version;
207+
last;
208+
}
209+
}
210+
}
211+
if (!$selected_version) {
212+
# "unspecified", "other"
213+
foreach my $test_version ("unspecified", "other") {
214+
if (my $version = first_value { lc($_->{name}) eq $test_version } @$versions) {
215+
$version->{selected} = $true;
216+
$selected_version = $version;
217+
last;
218+
}
219+
}
220+
}
221+
if (!$selected_version) {
222+
# default to a blank value
223+
unshift @$versions, {
224+
name => '',
225+
selected => $true,
226+
};
227+
}
228+
$result{version} = $versions;
229+
230+
# groups
231+
232+
my @groups;
233+
234+
# find invalid groups
235+
push @groups,
236+
map {{
237+
type => 'invalid',
238+
group => $_,
239+
checked => 0,
240+
}}
241+
@{ Bugzilla::Bug->get_invalid_groups({ bug_ids => [ $bug->id ], product => $product }) };
242+
243+
# logic lifted from bug/process/verify-new-product.html.tmpl
244+
my $current_groups = $bug->groups_in;
245+
my $group_controls = $product->group_controls;
246+
foreach my $group_id (keys %$group_controls) {
247+
my $group_control = $group_controls->{$group_id};
248+
if ($group_control->{membercontrol} == CONTROLMAPMANDATORY
249+
|| ($group_control->{othercontrol} == CONTROLMAPMANDATORY && !$user->in_group($group_control->{name})))
250+
{
251+
# mandatory, always checked
252+
push @groups, {
253+
type => 'mandatory',
254+
group => $group_control->{group},
255+
checked => 1,
256+
};
257+
}
258+
elsif (
259+
($group_control->{membercontrol} != CONTROLMAPNA && $user->in_group($group_control->{name}))
260+
|| $group_control->{othercontrol} != CONTROLMAPNA)
261+
{
262+
# optional, checked if..
263+
my $group = $group_control->{group};
264+
my $checked =
265+
# same group as current product
266+
(any { $_->id == $group->id } @$current_groups)
267+
# member default
268+
|| $group_control->{membercontrol} == CONTROLMAPDEFAULT && $user->in_group($group_control->{name})
269+
# or other default
270+
|| $group_control->{othercontrol} == CONTROLMAPDEFAULT && !$user->in_group($group_control->{name})
271+
;
272+
push @groups, {
273+
type => 'optional',
274+
group => $group_control->{group},
275+
checked => $checked || 0,
276+
};
277+
}
278+
}
279+
280+
my $default_group_name = $product->default_security_group;
281+
if (my $default_group = first_value { $_->{group}->name eq $default_group_name } @groups) {
282+
# because we always allow the default product group to be selected, it's never invalid
283+
$default_group->{type} = 'optional' if $default_group->{type} eq 'invalid';
284+
}
285+
else {
286+
# add the product's default group if it's missing
287+
unshift @groups, {
288+
type => 'optional',
289+
group => $product->default_security_group_obj,
290+
checked => 0,
291+
};
292+
}
293+
294+
# if the bug is currently in a group, ensure a group is checked by default
295+
# by checking the product's default group if no other groups apply
296+
if (@$current_groups && !any { $_->{checked} } @groups) {
297+
foreach my $g (@groups) {
298+
next unless $g->{group}->name eq $default_group_name;
299+
$g->{checked} = 1;
300+
last;
301+
}
302+
}
303+
304+
# group by type and flatten
305+
my $vars = {
306+
product => $product,
307+
groups => { invalid => [], mandatory => [], optional => [] },
308+
};
309+
foreach my $g (@groups) {
310+
push @{ $vars->{groups}->{$g->{type}} }, {
311+
id => $g->{group}->id,
312+
name => $g->{group}->name,
313+
description => $g->{group}->description,
314+
checked => $g->{checked},
315+
};
316+
}
317+
318+
# build group selection html
319+
my $template = Bugzilla->template;
320+
$template->process('bug_modal/new_product_groups.html.tmpl', $vars, \$result{groups})
321+
|| ThrowTemplateError($template->error);
322+
323+
return \%result;
324+
}
325+
138326
1;

0 commit comments

Comments
 (0)