Skip to content

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

Bug 240398: Make flagtypes.name work properly with all the boolean chart

operators. r=mkanat, a=mkanat (module owner)
parent 96298832
Show whitespace changes
Inline Side-by-side
Showing with 9 additions and 138 deletions
Bugzilla/Search.pm
View file @ d275f99f
......@@ -290,9 +290,7 @@ use constant OPERATOR_FIELD_OVERRIDE => {
},
dependson => MULTI_SELECT_OVERRIDE,
keywords => MULTI_SELECT_OVERRIDE,
'flagtypes.name' => {
_default => \&_flagtypes_name,
},
'flagtypes.name' => MULTI_SELECT_OVERRIDE,
longdesc => {
%{ MULTI_SELECT_OVERRIDE() },
changedby => \&_long_desc_changedby,
......@@ -2355,57 +2353,6 @@ sub _join_flag_tables {
push(@$joins, $attachments_join, $flags_join);
}
sub _flagtypes_name {
my ($self, $args) = @_;
my ($chart_id, $operator, $joins, $field, $having) =
@$args{qw(chart_id operator joins field having)};
my $dbh = Bugzilla->dbh;
# Matches bugs by flag name/status.
# Note that--for the purposes of querying--a flag comprises
# its name plus its status (i.e. a flag named "review"
# with a status of "+" can be found by searching for "review+").
# Don't do anything if this condition is about changes to flags,
# as the generic change condition processors can handle those.
return if $operator =~ /^changed/;
# Add the flags and flagtypes tables to the query. We do
# a left join here so bugs without any flags still match
# negative conditions (f.e. "flag isn't review+").
$self->_join_flag_tables($args);
my $flags = "flags_$chart_id";
my $flagtypes = "flagtypes_$chart_id";
my $flagtypes_join = {
table => 'flagtypes',
as => $flagtypes,
from => "$flags.type_id",
to => 'id',
};
push(@$joins, $flagtypes_join);
my $full_field = $dbh->sql_string_concat("$flagtypes.name",
"$flags.status");
$args->{full_field} = $full_field;
$self->_do_operator_function($args);
my $term = $args->{term};
# If this is a negative condition (f.e. flag isn't "review+"),
# we only want bugs where all flags match the condition, not
# those where any flag matches, which needs special magic.
# Instead of adding the condition to the WHERE clause, we select
# the number of flags matching the condition and the total number
# of flags on each bug, then compare them in a HAVING clause.
# If the numbers are the same, all flags match the condition,
# so this bug should be included.
if ($operator =~ /^not/) {
push(@$having,
"SUM(CASE WHEN $full_field IS NOT NULL THEN 1 ELSE 0 END) = " .
"SUM(CASE WHEN $term THEN 1 ELSE 0 END)");
$args->{term} = '';
}
}
sub _days_elapsed {
my ($self, $args) = @_;
my $dbh = Bugzilla->dbh;
......@@ -2562,6 +2509,8 @@ sub _multiselect_nonchanged {
sub _multiselect_table {
my ($self, $args) = @_;
my ($field, $chart_id) = @$args{qw(field chart_id)};
my $dbh = Bugzilla->dbh;
if ($field eq 'keywords') {
$args->{full_field} = 'keyworddefs.name';
return "keywords INNER JOIN keyworddefs".
......@@ -2610,6 +2559,11 @@ sub _multiselect_table {
return "attachments INNER JOIN attach_data "
. " ON attachments.attach_id = attach_data.id"
}
elsif ($field eq 'flagtypes.name') {
$args->{full_field} = $dbh->sql_string_concat("flagtypes.name",
"flags.status");
return "flags INNER JOIN flagtypes ON flags.type_id = flagtypes.id";
}
my $table = "bug_$field";
$args->{full_field} = "bug_$field.value";
return $table;
......
xt/lib/Bugzilla/Test/Search/Constants.pm
View file @ d275f99f
......@@ -46,7 +46,6 @@ our @EXPORT = qw(
NUM_BUGS
NUM_SEARCH_TESTS
OR_BROKEN
OR_SKIP
SKIP_FIELDS
SPECIAL_PARAM_TESTS
SUBSTR_NO_FIELD_ADD
......@@ -134,13 +133,6 @@ use constant SKIP_FIELDS => qw(
days_elapsed
);
# During OR tests, we skip these fields. They basically just don't work
# right in OR tests, and it's too much work to document the exact tests
# that they cause to fail.
use constant OR_SKIP => qw(
flagtypes.name
);
# All the fields that represent users.
use constant USER_FIELDS => qw(
assigned_to
......@@ -212,13 +204,6 @@ use constant ALLWORDS_BROKEN => (
cc => { contains => [1] },
);
# nowords and nowordssubstr have these broken tests in common.
#
# flagtypes.name doesn't match bugs without flags.
use constant NOWORDS_BROKEN => (
'flagtypes.name' => { contains => [5] },
);
# Fields that don't generally work at all with changed* searches, but
# probably should.
use constant CHANGED_BROKEN => (
......@@ -272,16 +257,10 @@ use constant KNOWN_BROKEN => {
greaterthaneq => { GREATERTHAN_BROKEN },
'allwordssubstr-<1>' => { ALLWORDS_BROKEN },
# flagtypes.name does not work here, probably because they all try to
# match against a single flag.
'allwords-<1>' => {
ALLWORDS_BROKEN,
'flagtypes.name' => { contains => [1] },
},
nowordssubstr => { NOWORDS_BROKEN },
nowords => { NOWORDS_BROKEN },
# setters.login_name and requestees.login name aren't tracked individually
# in bugs_activity, so can't be searched using this method.
#
......@@ -357,12 +336,6 @@ use constant KNOWN_BROKEN => {
# Broken NotTests #
###################
# These are fields that are broken in the same way for pretty much every
# NOT test that is broken.
use constant COMMON_BROKEN_NOT => (
"flagtypes.name" => { contains => [5] },
);
# Common BROKEN_NOT values for the changed* fields.
use constant CHANGED_BROKEN_NOT => (
"attach_data.thedata" => { contains => [1] },
......@@ -385,42 +358,20 @@ use constant CHANGED_FROM_TO_BROKEN_NOT => (
FIELD_TYPE_MULTI_SELECT, { contains => [1] },
);
# Common broken tests for the "not" or "no" operators.
use constant NEGATIVE_BROKEN_NOT => (
"flagtypes.name" => { contains => [1 .. 5] },
);
# These are field/operator combinations that are broken when run under NOT().
use constant BROKEN_NOT => {
allwords => {
COMMON_BROKEN_NOT,
cc => { contains => [1] },
"flagtypes.name" => { contains => [1,5] },
},
'allwords-<1> <2>' => {
cc => { },
'flagtypes.name' => { contains => [5] },
},
allwordssubstr => {
COMMON_BROKEN_NOT,
cc => { contains => [1] },
},
'allwordssubstr-<1>,<2>' => {
cc => { },
},
anyexact => {
COMMON_BROKEN_NOT,
"flagtypes.name" => { contains => [1, 2, 5] },
},
anywords => {
COMMON_BROKEN_NOT,
},
anywordssubstr => {
COMMON_BROKEN_NOT,
},
casesubstring => {
COMMON_BROKEN_NOT,
},
changedafter => {
"attach_data.thedata" => { contains => [2, 3, 4] },
"classification" => { contains => [2, 3, 4] },
......@@ -454,45 +405,11 @@ use constant BROKEN_NOT => {
longdesc => { contains => [1] },
"remaining_time" => { contains => [1] },
},
equals => {
COMMON_BROKEN_NOT,
"flagtypes.name" => { contains => [1, 5] },
},
greaterthan => {
COMMON_BROKEN_NOT,
cc => { contains => [1] },
},
greaterthaneq => {
COMMON_BROKEN_NOT,
cc => { contains => [1] },
"flagtypes.name" => { contains => [2, 5] },
},
lessthan => {
COMMON_BROKEN_NOT,
},
lessthaneq => {
COMMON_BROKEN_NOT,
},
notequals => { NEGATIVE_BROKEN_NOT },
notregexp => { NEGATIVE_BROKEN_NOT },
notsubstring => { NEGATIVE_BROKEN_NOT },
nowords => {
NEGATIVE_BROKEN_NOT,
"flagtypes.name" => { },
},
nowordssubstr => {
NEGATIVE_BROKEN_NOT,
"flagtypes.name" => { },
},
regexp => {
COMMON_BROKEN_NOT,
"flagtypes.name" => { contains => [1,5] },
},
'regexp-^1-' => {
"flagtypes.name" => { contains => [5] },
},
substring => {
COMMON_BROKEN_NOT,
},
};
......
xt/lib/Bugzilla/Test/Search/OrTest.pm
View file @ d275f99f
......@@ -70,7 +70,7 @@ sub debug_value {
# SKIP & TODO Messages #
########################
sub _join_skip { OR_SKIP }
sub _join_skip { () }
sub _join_broken_constant { OR_BROKEN }
sub field_not_yet_implemented {
......
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