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

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] },