From dddef33ef5bbbaba4e82d00ebe87877e474f5ea2 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Tue, 1 Mar 2011 07:41:44 -0800 Subject: Bug 637426: Fix every boolean chart operator type for the bug_group search field in Search.pm. (This also makes OR searches work against the bug_group field.) r=mkanat, a=mkanat --- Bugzilla/Search.pm | 33 +++++-------------------- xt/lib/Bugzilla/Test/Search/Constants.pm | 42 ++++++-------------------------- 2 files changed, 14 insertions(+), 61 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 3af5b98cd..0262e7178 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -277,9 +277,7 @@ use constant OPERATOR_FIELD_OVERRIDE => { blocked => { _non_changed => \&_blocked_nonchanged, }, - bug_group => { - _non_changed => \&_bug_group_nonchanged, - }, + bug_group => MULTI_SELECT_OVERRIDE, classification => { _non_changed => \&_classification_nonchanged, }, @@ -2364,30 +2362,6 @@ sub _percentage_complete { $self->_add_extra_column('actual_time'); } -sub _bug_group_nonchanged { - my ($self, $args) = @_; - my ($chart_id, $joins, $field) = @$args{qw(chart_id joins field)}; - - my $map_table = "bug_group_map_$chart_id"; - - push(@$joins, { table => 'bug_group_map', as => $map_table }); - - my $groups_table = "groups_$chart_id"; - my $full_field = "$groups_table.name"; - $args->{full_field} = $full_field; - $self->_do_operator_function($args); - my $term = $args->{term}; - my $groups_join = { - table => 'groups', - as => $groups_table, - from => "$map_table.group_id", - to => 'id', - extra => [$term], - }; - push(@$joins, $groups_join); - $args->{term} = "$full_field IS NOT NULL"; -} - sub _attach_data_thedata { my ($self, $args) = @_; my ($chart_id, $joins) = @$args{qw(chart_id joins)}; @@ -2688,6 +2662,11 @@ sub _multiselect_table { return "bug_tag INNER JOIN tags ON bug_tag.tag_id = tags.id" . " AND user_id = " . $self->_user->id; } + elsif ($field eq 'bug_group') { + $args->{full_field} = 'groups.name'; + return "bug_group_map INNER JOIN groups + ON bug_group_map.group_id = groups.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 6cb25126e..ad057e58c 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -198,12 +198,6 @@ use constant SUBSTR_NO_FIELD_ADD => FIELD_TYPE_DATETIME, qw( # the bug you listed. It doesn't find bugs that fully lack values for # the fields, as it should. # -# cc "not" matches if any CC'ed user matches, and it fails to match -# if there are no CCs on the bug. -# -# bug_group notequals doesn't find bugs that fully lack groups, -# and matches if there is one group that isn't equal. -# # bug_file_loc can be NULL, so it gets missed by the normal # notequals search. # @@ -228,7 +222,6 @@ use constant NEGATIVE_BROKEN => ( 'attachments.mimetype' => { contains => [5] }, blocked => { contains => [3,4,5] }, bug_file_loc => { contains => [5] }, - bug_group => { contains => [1,5] }, deadline => { contains => [5] }, dependson => { contains => [2,4,5] }, longdesc => { contains => [1] }, @@ -245,13 +238,12 @@ use constant NEGATIVE_BROKEN => ( # As with other fields, longdescs greaterthan matches if any comment # matches (which might be OK). # -# Same for keywords, bug_group, and cc. Logically, all of these might +# Same for keywords, and cc. Logically, all of these might # be OK, but it makes the operation not the logical reverse of # lessthaneq. What we're really saying here by marking these broken # is that there ought to be some way of searching "all ccs" vs "any cc" # (and same for the other fields). use constant GREATERTHAN_BROKEN => ( - bug_group => { contains => [1] }, cc => { contains => [1] }, longdesc => { contains => [1] }, ); @@ -260,9 +252,8 @@ use constant GREATERTHAN_BROKEN => ( # # allwordssubstr on longdescs fields matches against a single comment, # instead of matching against all comments on a bug. Same is true -# for cc and bug_group. +# for cc. use constant ALLWORDS_BROKEN => ( - bug_group => { contains => [1] }, cc => { contains => [1] }, longdesc => { contains => [1] }, ); @@ -270,15 +261,14 @@ use constant ALLWORDS_BROKEN => ( # nowords and nowordssubstr have these broken tests in common. # # flagtypes.name doesn't match bugs without flags. -# longdescs.isprivate, and bug_group actually work properly in +# longdescs.isprivate actually works properly in # terms of excluding bug 1 (since we exclude all values in the search, -# on our test), but still fail at including bug 5. +# on our test), but still fails at including bug 5. # The longdesc* fields, coincidentally, work completely # correctly, possibly because there's only one comment on bug 5. use constant NOWORDS_BROKEN => ( NEGATIVE_BROKEN, 'flagtypes.name' => { contains => [5] }, - bug_group => { contains => [5] }, longdesc => {}, 'longdescs.isprivate' => {}, ); @@ -496,10 +486,9 @@ use constant CHANGED_FROM_TO_BROKEN_NOT => ( # Common broken tests for the "not" or "no" operators. use constant NEGATIVE_BROKEN_NOT => ( - "blocked" => { contains => [3, 4, 5] }, - "bug_group" => { contains => [5] }, - "dependson" => { contains => [2, 4, 5] }, - "flagtypes.name" => { contains => [1 .. 5] }, + "blocked" => { contains => [3, 4, 5] }, + "dependson" => { contains => [2, 4, 5] }, + "flagtypes.name" => { contains => [1 .. 5] }, ); # These are field/operator combinations that are broken when run under NOT(). @@ -507,13 +496,11 @@ use constant BROKEN_NOT => { allwords => { COMMON_BROKEN_NOT, cc => { contains => [1] }, - bug_group => { contains => [1] }, "flagtypes.name" => { contains => [1,5] }, longdesc => { contains => [1] }, }, 'allwords-<1> <2>' => { 'attach_data.thedata' => { contains => [5] }, - bug_group => { }, cc => { }, 'flagtypes.name' => { contains => [5] }, 'longdesc' => { }, @@ -521,12 +508,10 @@ use constant BROKEN_NOT => { }, allwordssubstr => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, cc => { contains => [1] }, longdesc => { contains => [1] }, }, 'allwordssubstr-<1>,<2>' => { - bug_group => { }, cc => { }, "longdesc" => { }, "longdescs.isprivate" => { }, @@ -536,9 +521,6 @@ use constant BROKEN_NOT => { "flagtypes.name" => { contains => [1, 2, 5] }, "longdesc" => { contains => [1, 2] }, }, - 'anyexact-<1>, <2>' => { - bug_group => { contains => [1] }, - }, anywords => { COMMON_BROKEN_NOT, }, @@ -550,11 +532,9 @@ use constant BROKEN_NOT => { }, casesubstring => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, longdesc => { contains => [1] }, }, 'casesubstring-<1>-lc' => { - bug_group => { }, longdesc => { }, }, changedafter => { @@ -593,7 +573,6 @@ use constant BROKEN_NOT => { }, equals => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, "flagtypes.name" => { contains => [1, 5] }, longdesc => { contains => [1] }, }, @@ -611,12 +590,8 @@ use constant BROKEN_NOT => { longdesc => { contains => [1] }, 'longdescs.isprivate' => { }, }, - 'lessthan-2' => { - bug_group => { contains => [1] }, - }, lessthaneq => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, longdesc => { contains => [1] }, 'longdescs.isprivate' => { }, }, @@ -634,7 +609,6 @@ use constant BROKEN_NOT => { }, regexp => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, "flagtypes.name" => { contains => [1,5] }, longdesc => { contains => [1] }, }, @@ -643,7 +617,6 @@ use constant BROKEN_NOT => { }, substring => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, longdesc => { contains => [1] }, }, }; @@ -697,6 +670,7 @@ use constant GREATERTHAN_OVERRIDE => ( classification => { contains => [2,3,4,5] }, assigned_to => { contains => [2,3,4,5] }, bug_id => { contains => [2,3,4,5] }, + bug_group => { contains => [1,2,3,4] }, bug_severity => { contains => [2,3,4,5] }, bug_status => { contains => [2,3,4,5] }, component => { contains => [2,3,4,5] }, -- cgit v1.2.3-24-g4f1b