From e0a4e9c1bb0d943dbf74d6a487c9d8306fcbd576 Mon Sep 17 00:00:00 2001
From: Max Kanat-Alexander <mkanat@bugzilla.org>
Date: Tue, 1 Mar 2011 07:55:11 -0800
Subject: Bug 637662: Make all the boolean charts work as expected for the
 "blocks" and "dependson" fields. r=mkanat, a=mkanat (module owner)

---
 Bugzilla/Search.pm                       | 63 +++++++-------------------------
 xt/lib/Bugzilla/Test/Search/Constants.pm |  9 -----
 2 files changed, 13 insertions(+), 59 deletions(-)

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 0262e7178..329a82500 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -274,9 +274,7 @@ use constant OPERATOR_FIELD_OVERRIDE => {
     'attachments' => {
         _non_changed => \&_attachments,
     },
-    blocked => {
-        _non_changed => \&_blocked_nonchanged,
-    },
+    blocked   => MULTI_SELECT_OVERRIDE,
     bug_group => MULTI_SELECT_OVERRIDE,
     classification => {
         _non_changed => \&_classification_nonchanged,
@@ -292,10 +290,8 @@ use constant OPERATOR_FIELD_OVERRIDE => {
     days_elapsed => {
         _default => \&_days_elapsed,
     },
-    dependson => {
-        _non_changed => \&_dependson_nonchanged,
-    },
-    keywords => MULTI_SELECT_OVERRIDE,
+    dependson => MULTI_SELECT_OVERRIDE,
+    keywords  => MULTI_SELECT_OVERRIDE,
     'flagtypes.name' => {
         _default => \&_flagtypes_name,
     },    
@@ -2525,47 +2521,6 @@ sub _classification_nonchanged {
         "classifications.id", "classifications", $term);
 }
 
-# XXX This should be combined with blocked_nonchanged.
-sub _dependson_nonchanged {
-    my ($self, $args) = @_;
-    my ($chart_id, $joins, $field, $operator) =
-        @$args{qw(chart_id joins field operator)};
-    
-    my $table = "dependson_$chart_id";
-    my $full_field = "$table.$field";
-    $args->{full_field} = $full_field;
-    $self->_do_operator_function($args);
-    my $term = $args->{term};
-    my $dep_join = {
-        table => 'dependencies',
-        as    => $table,
-        to    => 'blocked',
-        extra => [$term],
-    };
-    push(@$joins, $dep_join);
-    $args->{term} = "$full_field IS NOT NULL";
-}
-
-sub _blocked_nonchanged {
-    my ($self, $args) = @_;
-    my ($chart_id, $joins, $field, $operator) =
-        @$args{qw(chart_id joins field operator)};
-
-    my $table = "blocked_$chart_id";
-    my $full_field = "$table.$field";
-    $args->{full_field} = $full_field;
-    $self->_do_operator_function($args);
-    my $term = $args->{term};
-    my $dep_join = {
-        table => 'dependencies',
-        as    => $table,
-        to    => 'dependson',
-        extra => [$term],
-    };
-    push(@$joins, $dep_join);
-    $args->{term} = "$full_field IS NOT NULL";
-}
-
 sub _alias_nonchanged {
     my ($self, $args) = @_;
     $args->{full_field} = "COALESCE(bugs.alias, '')";
@@ -2617,8 +2572,9 @@ sub _multiselect_negative {
     $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 bug_id FROM $table WHERE $term)";
+        "bugs.bug_id NOT IN (SELECT $select FROM $table WHERE $term)";
 }
 
 sub _multiselect_multiple {
@@ -2667,6 +2623,12 @@ sub _multiselect_table {
         return "bug_group_map INNER JOIN groups
                                       ON bug_group_map.group_id = groups.id";
     }
+    elsif ($field eq 'blocked' or $field eq 'dependson') {
+        my $select = $field eq 'blocked' ? 'dependson' : 'blocked';
+        $args->{_select_field} = $select;
+        $args->{full_field} = "dependencies.$field";
+        return "dependencies";
+    }
     my $table = "bug_$field";
     $args->{full_field} = "bug_$field.value";
     return $table;
@@ -2677,7 +2639,8 @@ sub _multiselect_term {
     my $table = $self->_multiselect_table($args);
     $self->_do_operator_function($args);
     my $term = $args->{term};
-    return "bugs.bug_id IN (SELECT bug_id FROM $table WHERE $term)";
+    my $select = $args->{_select_field} || 'bug_id';
+    return "bugs.bug_id IN (SELECT $select FROM $table WHERE $term)";
 }
 
 sub _multiselect_nonchanged {
diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm
index ad057e58c..16d77a1d1 100644
--- a/xt/lib/Bugzilla/Test/Search/Constants.pm
+++ b/xt/lib/Bugzilla/Test/Search/Constants.pm
@@ -193,11 +193,6 @@ use constant SUBSTR_NO_FIELD_ADD => FIELD_TYPE_DATETIME, qw(
 
 # Certain fields fail all the "negative" search tests:
 #
-# Blocked and Dependson "notequals" only finds bugs that have
-# values for the field, but where the dependency list doesn't contain
-# the bug you listed. It doesn't find bugs that fully lack values for
-# the fields, as it should.
-#
 # bug_file_loc can be NULL, so it gets missed by the normal
 # notequals search.
 #
@@ -220,10 +215,8 @@ use constant NEGATIVE_BROKEN => (
     'attachments.description' => { contains => [5] },
     'attachments.filename'    => { contains => [5] },
     'attachments.mimetype'    => { contains => [5] },
-    blocked      => { contains => [3,4,5] },
     bug_file_loc => { contains => [5] },
     deadline     => { contains => [5] },
-    dependson    => { contains => [2,4,5] },
     longdesc     => { contains => [1] },
     'longdescs.isprivate'   => { contains => [1] },
     # Custom fields are busted because they can be NULL.
@@ -486,8 +479,6 @@ 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] },
-    "dependson"      => { contains => [2, 4, 5] },
     "flagtypes.name" => { contains => [1 .. 5] },
 );
 
-- 
cgit v1.2.3-24-g4f1b