From d275f99f6cce590b7a0bb8268a2407ae13ea8d77 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Thu, 3 Mar 2011 13:57:22 -0800 Subject: Bug 240398: Make flagtypes.name work properly with all the boolean chart operators. r=mkanat, a=mkanat (module owner) --- Bugzilla/Search.pm | 66 ++++--------------------- xt/lib/Bugzilla/Test/Search/Constants.pm | 83 -------------------------------- xt/lib/Bugzilla/Test/Search/OrTest.pm | 2 +- 3 files changed, 11 insertions(+), 140 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 57b8fe748..24abc4379 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -288,11 +288,9 @@ use constant OPERATOR_FIELD_OVERRIDE => { days_elapsed => { _default => \&_days_elapsed, }, - dependson => MULTI_SELECT_OVERRIDE, - keywords => MULTI_SELECT_OVERRIDE, - 'flagtypes.name' => { - _default => \&_flagtypes_name, - }, + dependson => MULTI_SELECT_OVERRIDE, + keywords => MULTI_SELECT_OVERRIDE, + '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; diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index 353f9a3bf..deca77a72 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -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, }, }; diff --git a/xt/lib/Bugzilla/Test/Search/OrTest.pm b/xt/lib/Bugzilla/Test/Search/OrTest.pm index 35653d0f0..42dc30698 100644 --- a/xt/lib/Bugzilla/Test/Search/OrTest.pm +++ b/xt/lib/Bugzilla/Test/Search/OrTest.pm @@ -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 { -- cgit v1.2.3-24-g4f1b