Bug 45862: Make searching comments behave sensibly for all boolean charts,

including "nowordssubstr" and "notequals".
r=mkanat, a=mkanat (module owner)

2 files changed, 21 insertions, 45 deletions

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 329a82500..73235ded3 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -296,10 +296,10 @@ use constant OPERATOR_FIELD_OVERRIDE => {
         _default => \&_flagtypes_name,
     },    
     longdesc => {
+        %{ MULTI_SELECT_OVERRIDE() },
         changedby     => \&_long_desc_changedby,
         changedbefore => \&_long_desc_changedbefore_after,
         changedafter  => \&_long_desc_changedbefore_after,
-        _default      => \&_long_desc,
     },
     'longdescs.count' => {
         changedby     => \&_long_desc_changedby,
@@ -2286,12 +2286,6 @@ sub _join_longdescs {
     return $table;
 }
 
-sub _long_desc {
-    my ($self, $args) = @_;
-    my $table = $self->_join_longdescs($args);
-    $args->{full_field} = "$table.thetext";
-}
-
 sub _long_descs_count {
     my ($self, $args) = @_;
     my ($chart_id, $joins) = @$args{qw(chart_id joins)};
@@ -2568,13 +2562,8 @@ sub _multiselect_negative {
     my ($self, $args) = @_;
     my ($field, $operator) = @$args{qw(field operator)};
 
-    my $table = $self->_multiselect_table($args);
     $args->{operator} = $self->_reverse_operator($operator);
-    $self->_do_operator_function($args);
-    my $term = $args->{term};
-    my $select = $args->{_select_field} || 'bug_id';
-    $args->{term} =
-        "bugs.bug_id NOT IN (SELECT $select FROM $table WHERE $term)";
+    $args->{term} = $self->_multiselect_term($args, 1);
 }
 
 sub _multiselect_multiple {
@@ -2605,6 +2594,13 @@ sub _multiselect_multiple {
     }
 }
 
+sub _multiselect_nonchanged {
+    my ($self, $args) = @_;
+    my ($chart_id, $joins, $field, $operator) =
+        @$args{qw(chart_id joins field operator)};
+    $args->{term} = $self->_multiselect_term($args)
+}
+
 sub _multiselect_table {
     my ($self, $args) = @_;
     my ($field, $chart_id) = @$args{qw(field chart_id)};
@@ -2626,28 +2622,29 @@ sub _multiselect_table {
     elsif ($field eq 'blocked' or $field eq 'dependson') {
         my $select = $field eq 'blocked' ? 'dependson' : 'blocked';
         $args->{_select_field} = $select;
-        $args->{full_field} = "dependencies.$field";
+        $args->{full_field} = $field;
         return "dependencies";
     }
+    elsif ($field eq 'longdesc') {
+        $args->{_extra_where} = " AND isprivate = 0"
+            if !$self->_user->is_insider;
+        $args->{full_field} = 'thetext';
+        return "longdescs";
+    }
     my $table = "bug_$field";
     $args->{full_field} = "bug_$field.value";
     return $table;
 }
 
 sub _multiselect_term {
-    my ($self, $args) = @_;
+    my ($self, $args, $not) = @_;
     my $table = $self->_multiselect_table($args);
     $self->_do_operator_function($args);
     my $term = $args->{term};
+    $term .= $args->{_extra_where} || '';
     my $select = $args->{_select_field} || 'bug_id';
-    return "bugs.bug_id IN (SELECT $select FROM $table WHERE $term)";
-}
-
-sub _multiselect_nonchanged {
-    my ($self, $args) = @_;
-    my ($chart_id, $joins, $field, $operator) =
-        @$args{qw(chart_id joins field operator)};
-    $args->{term} = $self->_multiselect_term($args);
+    my $not_sql = $not ? "NOT " : '';
+    return "bugs.bug_id ${not_sql}IN (SELECT $select FROM $table WHERE $term)";
 }
 
 ###############################
diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm
index 16d77a1d1..ed7ff8f74 100644
--- a/xt/lib/Bugzilla/Test/Search/Constants.pm
+++ b/xt/lib/Bugzilla/Test/Search/Constants.pm
@@ -196,9 +196,6 @@ use constant SUBSTR_NO_FIELD_ADD => FIELD_TYPE_DATETIME, qw(
 # bug_file_loc can be NULL, so it gets missed by the normal
 # notequals search.
 #
-# longdescs "notequals" matches if *any* of the values
-# are not equal to the string provided.
-#
 # attachments.* notequals doesn't find bugs that lack attachments.
 #
 # deadline notequals does not find bugs that lack deadlines
@@ -217,7 +214,6 @@ use constant NEGATIVE_BROKEN => (
     'attachments.mimetype'    => { contains => [5] },
     bug_file_loc => { contains => [5] },
     deadline     => { contains => [5] },
-    longdesc     => { contains => [1] },
     'longdescs.isprivate'   => { contains => [1] },
     # Custom fields are busted because they can be NULL.
     FIELD_TYPE_FREETEXT, { contains => [5] },
@@ -238,7 +234,6 @@ use constant NEGATIVE_BROKEN => (
 # (and same for the other fields).
 use constant GREATERTHAN_BROKEN => (
     cc        => { contains => [1] },
-    longdesc  => { contains => [1] },
 );
 
 # allwords and allwordssubstr have these broken tests in common.
@@ -248,7 +243,6 @@ use constant GREATERTHAN_BROKEN => (
 # for cc.
 use constant ALLWORDS_BROKEN => (
     cc        => { contains => [1] },
-    longdesc  => { contains => [1] },
 );
 
 # nowords and nowordssubstr have these broken tests in common.
@@ -262,7 +256,6 @@ use constant ALLWORDS_BROKEN => (
 use constant NOWORDS_BROKEN => (
     NEGATIVE_BROKEN,
     'flagtypes.name' => { contains => [5] },
-    longdesc         => {},
     'longdescs.isprivate' => {},
 );
 
@@ -488,29 +481,24 @@ use constant BROKEN_NOT => {
         COMMON_BROKEN_NOT,
         cc => { contains => [1] },
         "flagtypes.name"      => { contains => [1,5] },
-        longdesc => { contains => [1] },
     },
     'allwords-<1> <2>' => {
         'attach_data.thedata' => { contains => [5] },
         cc => { },
         'flagtypes.name'      => { contains => [5] },
-        'longdesc' => { },
         'longdescs.isprivate' => { },
     },
     allwordssubstr => {
         COMMON_BROKEN_NOT,
         cc => { contains => [1] },
-        longdesc => { contains => [1] },
     },
     'allwordssubstr-<1>,<2>' => {
         cc => { },
-        "longdesc" => { },
         "longdescs.isprivate" => { },
     },
     anyexact => {
         COMMON_BROKEN_NOT,
         "flagtypes.name" => { contains => [1, 2, 5] },
-        "longdesc"       => { contains => [1, 2] },
     },
     anywords => {
         COMMON_BROKEN_NOT,
@@ -523,10 +511,6 @@ use constant BROKEN_NOT => {
     },
     casesubstring => {
         COMMON_BROKEN_NOT,
-        longdesc  => { contains => [1] },
-    },
-    'casesubstring-<1>-lc' => {
-        longdesc => { },
     },
     changedafter => {
         "attach_data.thedata"   => { contains => [2, 3, 4] },
@@ -551,7 +535,6 @@ use constant BROKEN_NOT => {
         'attach_data.thedata' => { },
         blocked         => { contains => [1, 2] },
         dependson       => { contains => [1, 3] },
-        longdesc        => { },
         work_time       => { contains => [1] },
         FIELD_TYPE_BUG_ID, { contains => [1 .. 4] },
         
@@ -565,7 +548,6 @@ use constant BROKEN_NOT => {
     equals => {
         COMMON_BROKEN_NOT,
         "flagtypes.name" => { contains => [1, 5] },
-        longdesc  => { contains => [1] },
     },
     greaterthan => {
         COMMON_BROKEN_NOT,
@@ -578,12 +560,10 @@ use constant BROKEN_NOT => {
     },
     lessthan => {
         COMMON_BROKEN_NOT,
-        longdesc => { contains => [1] },
         'longdescs.isprivate' => { },
     },
     lessthaneq => {
         COMMON_BROKEN_NOT,
-        longdesc  => { contains => [1] },
         'longdescs.isprivate' => { },
     },
     notequals      => { NEGATIVE_BROKEN_NOT },
@@ -601,14 +581,12 @@ use constant BROKEN_NOT => {
     regexp         => {
         COMMON_BROKEN_NOT,
         "flagtypes.name" => { contains => [1,5] },
-        longdesc  => { contains => [1] },
     },
     'regexp-^1-' => {
         "flagtypes.name" => { contains => [5] },
     },
     substring      => {
         COMMON_BROKEN_NOT,
-        longdesc  => { contains => [1] },
     },
 };
 
@@ -668,6 +646,7 @@ use constant GREATERTHAN_OVERRIDE => (
     commenter    => { contains => [2,3,4,5] },
     # keywords matches if *any* keyword matches
     keywords     => { contains => [1,2,3,4] },
+    longdesc     => { contains => [1,2,3,4] },
     op_sys       => { contains => [2,3,4,5] },
     priority     => { contains => [2,3,4,5] },
     product      => { contains => [2,3,4,5] },