Skip to content

Commit d275f99

Browse files
author
Max Kanat-Alexander
committed
Bug 240398: Make flagtypes.name work properly with all the boolean chart
operators. r=mkanat, a=mkanat (module owner)
1 parent 9629883 commit d275f99

3 files changed

Lines changed: 11 additions & 140 deletions

File tree

‎Bugzilla/Search.pm‎

Lines changed: 10 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,9 @@ use constant OPERATOR_FIELD_OVERRIDE => {
288288
days_elapsed => {
289289
_default => \&_days_elapsed,
290290
},
291-
dependson => MULTI_SELECT_OVERRIDE,
292-
keywords => MULTI_SELECT_OVERRIDE,
293-
'flagtypes.name' => {
294-
_default => \&_flagtypes_name,
295-
},
291+
dependson => MULTI_SELECT_OVERRIDE,
292+
keywords => MULTI_SELECT_OVERRIDE,
293+
'flagtypes.name' => MULTI_SELECT_OVERRIDE,
296294
longdesc => {
297295
%{ MULTI_SELECT_OVERRIDE() },
298296
changedby => \&_long_desc_changedby,
@@ -2355,57 +2353,6 @@ sub _join_flag_tables {
23552353
push(@$joins, $attachments_join, $flags_join);
23562354
}
23572355

2358-
sub _flagtypes_name {
2359-
my ($self, $args) = @_;
2360-
my ($chart_id, $operator, $joins, $field, $having) =
2361-
@$args{qw(chart_id operator joins field having)};
2362-
my $dbh = Bugzilla->dbh;
2363-
2364-
# Matches bugs by flag name/status.
2365-
# Note that--for the purposes of querying--a flag comprises
2366-
# its name plus its status (i.e. a flag named "review"
2367-
# with a status of "+" can be found by searching for "review+").
2368-
2369-
# Don't do anything if this condition is about changes to flags,
2370-
# as the generic change condition processors can handle those.
2371-
return if $operator =~ /^changed/;
2372-
2373-
# Add the flags and flagtypes tables to the query. We do
2374-
# a left join here so bugs without any flags still match
2375-
# negative conditions (f.e. "flag isn't review+").
2376-
$self->_join_flag_tables($args);
2377-
my $flags = "flags_$chart_id";
2378-
my $flagtypes = "flagtypes_$chart_id";
2379-
my $flagtypes_join = {
2380-
table => 'flagtypes',
2381-
as => $flagtypes,
2382-
from => "$flags.type_id",
2383-
to => 'id',
2384-
};
2385-
push(@$joins, $flagtypes_join);
2386-
2387-
my $full_field = $dbh->sql_string_concat("$flagtypes.name",
2388-
"$flags.status");
2389-
$args->{full_field} = $full_field;
2390-
$self->_do_operator_function($args);
2391-
my $term = $args->{term};
2392-
2393-
# If this is a negative condition (f.e. flag isn't "review+"),
2394-
# we only want bugs where all flags match the condition, not
2395-
# those where any flag matches, which needs special magic.
2396-
# Instead of adding the condition to the WHERE clause, we select
2397-
# the number of flags matching the condition and the total number
2398-
# of flags on each bug, then compare them in a HAVING clause.
2399-
# If the numbers are the same, all flags match the condition,
2400-
# so this bug should be included.
2401-
if ($operator =~ /^not/) {
2402-
push(@$having,
2403-
"SUM(CASE WHEN $full_field IS NOT NULL THEN 1 ELSE 0 END) = " .
2404-
"SUM(CASE WHEN $term THEN 1 ELSE 0 END)");
2405-
$args->{term} = '';
2406-
}
2407-
}
2408-
24092356
sub _days_elapsed {
24102357
my ($self, $args) = @_;
24112358
my $dbh = Bugzilla->dbh;
@@ -2562,6 +2509,8 @@ sub _multiselect_nonchanged {
25622509
sub _multiselect_table {
25632510
my ($self, $args) = @_;
25642511
my ($field, $chart_id) = @$args{qw(field chart_id)};
2512+
my $dbh = Bugzilla->dbh;
2513+
25652514
if ($field eq 'keywords') {
25662515
$args->{full_field} = 'keyworddefs.name';
25672516
return "keywords INNER JOIN keyworddefs".
@@ -2610,6 +2559,11 @@ sub _multiselect_table {
26102559
return "attachments INNER JOIN attach_data "
26112560
. " ON attachments.attach_id = attach_data.id"
26122561
}
2562+
elsif ($field eq 'flagtypes.name') {
2563+
$args->{full_field} = $dbh->sql_string_concat("flagtypes.name",
2564+
"flags.status");
2565+
return "flags INNER JOIN flagtypes ON flags.type_id = flagtypes.id";
2566+
}
26132567
my $table = "bug_$field";
26142568
$args->{full_field} = "bug_$field.value";
26152569
return $table;

‎xt/lib/Bugzilla/Test/Search/Constants.pm‎

Lines changed: 0 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ our @EXPORT = qw(
4646
NUM_BUGS
4747
NUM_SEARCH_TESTS
4848
OR_BROKEN
49-
OR_SKIP
5049
SKIP_FIELDS
5150
SPECIAL_PARAM_TESTS
5251
SUBSTR_NO_FIELD_ADD
@@ -134,13 +133,6 @@ use constant SKIP_FIELDS => qw(
134133
days_elapsed
135134
);
136135

137-
# During OR tests, we skip these fields. They basically just don't work
138-
# right in OR tests, and it's too much work to document the exact tests
139-
# that they cause to fail.
140-
use constant OR_SKIP => qw(
141-
flagtypes.name
142-
);
143-
144136
# All the fields that represent users.
145137
use constant USER_FIELDS => qw(
146138
assigned_to
@@ -212,13 +204,6 @@ use constant ALLWORDS_BROKEN => (
212204
cc => { contains => [1] },
213205
);
214206

215-
# nowords and nowordssubstr have these broken tests in common.
216-
#
217-
# flagtypes.name doesn't match bugs without flags.
218-
use constant NOWORDS_BROKEN => (
219-
'flagtypes.name' => { contains => [5] },
220-
);
221-
222207
# Fields that don't generally work at all with changed* searches, but
223208
# probably should.
224209
use constant CHANGED_BROKEN => (
@@ -272,16 +257,10 @@ use constant KNOWN_BROKEN => {
272257
greaterthaneq => { GREATERTHAN_BROKEN },
273258

274259
'allwordssubstr-<1>' => { ALLWORDS_BROKEN },
275-
# flagtypes.name does not work here, probably because they all try to
276-
# match against a single flag.
277260
'allwords-<1>' => {
278261
ALLWORDS_BROKEN,
279-
'flagtypes.name' => { contains => [1] },
280262
},
281263

282-
nowordssubstr => { NOWORDS_BROKEN },
283-
nowords => { NOWORDS_BROKEN },
284-
285264
# setters.login_name and requestees.login name aren't tracked individually
286265
# in bugs_activity, so can't be searched using this method.
287266
#
@@ -357,12 +336,6 @@ use constant KNOWN_BROKEN => {
357336
# Broken NotTests #
358337
###################
359338

360-
# These are fields that are broken in the same way for pretty much every
361-
# NOT test that is broken.
362-
use constant COMMON_BROKEN_NOT => (
363-
"flagtypes.name" => { contains => [5] },
364-
);
365-
366339
# Common BROKEN_NOT values for the changed* fields.
367340
use constant CHANGED_BROKEN_NOT => (
368341
"attach_data.thedata" => { contains => [1] },
@@ -385,42 +358,20 @@ use constant CHANGED_FROM_TO_BROKEN_NOT => (
385358
FIELD_TYPE_MULTI_SELECT, { contains => [1] },
386359
);
387360

388-
# Common broken tests for the "not" or "no" operators.
389-
use constant NEGATIVE_BROKEN_NOT => (
390-
"flagtypes.name" => { contains => [1 .. 5] },
391-
);
392-
393361
# These are field/operator combinations that are broken when run under NOT().
394362
use constant BROKEN_NOT => {
395363
allwords => {
396-
COMMON_BROKEN_NOT,
397364
cc => { contains => [1] },
398-
"flagtypes.name" => { contains => [1,5] },
399365
},
400366
'allwords-<1> <2>' => {
401367
cc => { },
402-
'flagtypes.name' => { contains => [5] },
403368
},
404369
allwordssubstr => {
405-
COMMON_BROKEN_NOT,
406370
cc => { contains => [1] },
407371
},
408372
'allwordssubstr-<1>,<2>' => {
409373
cc => { },
410374
},
411-
anyexact => {
412-
COMMON_BROKEN_NOT,
413-
"flagtypes.name" => { contains => [1, 2, 5] },
414-
},
415-
anywords => {
416-
COMMON_BROKEN_NOT,
417-
},
418-
anywordssubstr => {
419-
COMMON_BROKEN_NOT,
420-
},
421-
casesubstring => {
422-
COMMON_BROKEN_NOT,
423-
},
424375
changedafter => {
425376
"attach_data.thedata" => { contains => [2, 3, 4] },
426377
"classification" => { contains => [2, 3, 4] },
@@ -454,45 +405,11 @@ use constant BROKEN_NOT => {
454405
longdesc => { contains => [1] },
455406
"remaining_time" => { contains => [1] },
456407
},
457-
equals => {
458-
COMMON_BROKEN_NOT,
459-
"flagtypes.name" => { contains => [1, 5] },
460-
},
461408
greaterthan => {
462-
COMMON_BROKEN_NOT,
463409
cc => { contains => [1] },
464410
},
465411
greaterthaneq => {
466-
COMMON_BROKEN_NOT,
467412
cc => { contains => [1] },
468-
"flagtypes.name" => { contains => [2, 5] },
469-
},
470-
lessthan => {
471-
COMMON_BROKEN_NOT,
472-
},
473-
lessthaneq => {
474-
COMMON_BROKEN_NOT,
475-
},
476-
notequals => { NEGATIVE_BROKEN_NOT },
477-
notregexp => { NEGATIVE_BROKEN_NOT },
478-
notsubstring => { NEGATIVE_BROKEN_NOT },
479-
nowords => {
480-
NEGATIVE_BROKEN_NOT,
481-
"flagtypes.name" => { },
482-
},
483-
nowordssubstr => {
484-
NEGATIVE_BROKEN_NOT,
485-
"flagtypes.name" => { },
486-
},
487-
regexp => {
488-
COMMON_BROKEN_NOT,
489-
"flagtypes.name" => { contains => [1,5] },
490-
},
491-
'regexp-^1-' => {
492-
"flagtypes.name" => { contains => [5] },
493-
},
494-
substring => {
495-
COMMON_BROKEN_NOT,
496413
},
497414
};
498415

‎xt/lib/Bugzilla/Test/Search/OrTest.pm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ sub debug_value {
7070
# SKIP & TODO Messages #
7171
########################
7272

73-
sub _join_skip { OR_SKIP }
73+
sub _join_skip { () }
7474
sub _join_broken_constant { OR_BROKEN }
7575

7676
sub field_not_yet_implemented {

0 commit comments

Comments
 (0)