Bug 638518: Make all boolean chart operators work with attach_data.thedata

r=mkanat, a=mkanat (module owner)

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index ce4b1605ef35b286f0225534e0ae359f4a88eea9..5c12e2ad6393881a4963ceb80995d628648f5665 100644 (file)
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -267,11 +267,9 @@ use constant OPERATOR_FIELD_OVERRIDE => {
     alias => {
         _non_changed => \&_alias_nonchanged,
     },
-    'attach_data.thedata' => {
-        _non_changed => \&_attach_data_thedata,
-    },
     # We check all attachment fields against this.
     attachments => MULTI_SELECT_OVERRIDE,
+    'attach_data.thedata' => MULTI_SELECT_OVERRIDE,
     blocked     => MULTI_SELECT_OVERRIDE,
     bug_group   => MULTI_SELECT_OVERRIDE,
     classification => {
@@ -2326,29 +2324,6 @@ sub _percentage_complete {
     $self->_add_extra_column('actual_time');
 }
 
-sub _attach_data_thedata {
-    my ($self, $args) = @_;
-    my ($chart_id, $joins) = @$args{qw(chart_id joins)};
-    
-    my $attach_table = "attachments_$chart_id";
-    my $data_table = "attachdata_$chart_id";
-    my $extra = $self->_user->is_insider
-                ? [] : ["$attach_table.isprivate = 0"];
-    my $attachments_join = {
-        table => 'attachments',
-        as    => $attach_table,
-        extra => $extra,
-    };
-    my $data_join = {
-        table => 'attach_data',
-        as    => $data_table,
-        from  => "$attach_table.attach_id",
-        to    => "id",
-    };
-    push(@$joins, $attachments_join, $data_join);
-    $args->{full_field} = "$data_table.thedata";
-}
-
 sub _join_flag_tables {
     my ($self, $args) = @_;
     my ($joins, $chart_id) = @$args{qw(joins chart_id)};
@@ -2601,6 +2576,12 @@ sub _multiselect_table {
         $args->{full_field} = $1;
         return "attachments";
     }
+    elsif ($field eq 'attach_data.thedata') {
+        $args->{_extra_where} = " AND attachments.isprivate = 0"
+            if !$self->_user->is_insider;
+        return "attachments INNER JOIN attach_data "
+               . " ON attachments.attach_id = attach_data.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 82b78bd3590693e0bac75ff08cca03d7e5f52d07..a5ec6e2b1756b898f8fb2a1337dabf078427ecca 100644 (file)
--- a/xt/lib/Bugzilla/Test/Search/Constants.pm
+++ b/xt/lib/Bugzilla/Test/Search/Constants.pm
@@ -47,7 +47,6 @@ our @EXPORT = qw(
     NUM_SEARCH_TESTS
     OR_BROKEN
     OR_SKIP
-    PG_BROKEN
     SKIP_FIELDS
     SPECIAL_PARAM_TESTS
     SUBSTR_NO_FIELD_ADD
@@ -203,7 +202,6 @@ use constant SUBSTR_NO_FIELD_ADD => FIELD_TYPE_DATETIME, qw(
 #
 # requestees.login_name doesn't find bugs that fully lack requestees.
 use constant NEGATIVE_BROKEN => (
-    'attach_data.thedata'     => { contains => [5] },
     bug_file_loc => { contains => [5] },
     deadline     => { contains => [5] },
     # Custom fields are busted because they can be NULL.
@@ -302,19 +300,13 @@ use constant KNOWN_BROKEN => {
     'allwordssubstr-<1>' => { ALLWORDS_BROKEN },
     # flagtypes.name does not work here, probably because they all try to
     # match against a single flag.
-    # Same for attach_data.thedata.
     'allwords-<1>' => {
         ALLWORDS_BROKEN,
         'flagtypes.name' => { contains => [1] },
     },
 
     nowordssubstr => { NOWORDS_BROKEN },
-    # attach_data.thedata doesn't match properly with any of the plain
-    # "words" searches. Also, bug 5 doesn't match because it lacks
-    # attachments.
-    nowords => {
-        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.
@@ -387,16 +379,6 @@ use constant KNOWN_BROKEN => {
     },
 };
 
-# This tracks things that are broken in different ways on Pg compared to
-# MySQL. Actually, in some of these cases, Pg is behaving correctly
-# where MySQL isn't, but the result is still a bit surprising to the user.
-use constant PG_BROKEN => {
-    'attach_data.thedata' => {
-        notregexp => { contains => [5] },
-        nowords   => { contains => [5] },
-    },
-};
-
 ###################
 # Broken NotTests #
 ###################
@@ -404,7 +386,6 @@ use constant PG_BROKEN => {
 # These are fields that are broken in the same way for pretty much every
 # NOT test that is broken.
 use constant COMMON_BROKEN_NOT => (
-    "attach_data.thedata"     => { contains => [5] },
     "bug_file_loc"            => { contains => [5] },
     "deadline"                => { contains => [5] },
     "flagtypes.name"          => { contains => [5] },
@@ -449,7 +430,6 @@ use constant BROKEN_NOT => {
         "flagtypes.name"      => { contains => [1,5] },
     },
     'allwords-<1> <2>' => {
-        'attach_data.thedata' => { contains => [5] },
         cc => { },
         'flagtypes.name'      => { contains => [5] },
     },
@@ -467,9 +447,6 @@ use constant BROKEN_NOT => {
     anywords => {
         COMMON_BROKEN_NOT,
     },
-    'anywords-<1> <2>' => {
-        'attach_data.thedata' => { contains => [5] },
-    },
     anywordssubstr => {
         COMMON_BROKEN_NOT,
     },
@@ -537,7 +514,6 @@ use constant BROKEN_NOT => {
     },
     nowordssubstr  => {
         NEGATIVE_BROKEN_NOT,
-        "attach_data.thedata" => { },
         "flagtypes.name" => { },
     },
     regexp         => {

diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
index 400ca32632cefc07a57787032c779e5cee43ff0c..02e0df06ca9c2c9b4871a1b29f5e6ec955edd685 100644 (file)
--- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm
+++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
@@ -191,13 +191,6 @@ sub _known_broken {
         $value_name .= "-$extra_name";
     }    
     
-    if (!$skip_pg_check and Bugzilla->dbh->isa('Bugzilla::DB::Pg')) {
-        my $field_broken = PG_BROKEN->{$field}->{$operator};
-        return $field_broken if $field_broken;
-        my $pg_value_broken = PG_BROKEN->{$field}->{$value_name};
-        return $pg_value_broken if $pg_value_broken;
-    }
-    
     my $value_broken = $constant->{$value_name}->{$field};
     $value_broken ||= $constant->{$value_name}->{$type};
     return $value_broken if $value_broken;