Bug 780820: Allows for multiple custom search criteria to match one field

9 files changed, 247 insertions, 54 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 2f708d065..5eb44c403 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -405,8 +405,10 @@ sub sql_string_until {
 }
 
 sub sql_in {
-    my ($self, $column_name, $in_list_ref) = @_;
-    return " $column_name IN (" . join(',', @$in_list_ref) . ") ";
+    my ($self, $column_name, $in_list_ref, $negate) = @_;
+    return " $column_name "
+             . ($negate ? "NOT " : "")
+             . "IN (" . join(',', @$in_list_ref) . ") ";
 }
 
 sub sql_fulltext_search {
diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm
index ebf59533f..7df2c09bb 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -216,16 +216,16 @@ sub sql_position {
 }
 
 sub sql_in {
-    my ($self, $column_name, $in_list_ref) = @_;
+    my ($self, $column_name, $in_list_ref, $negate) = @_;
     my @in_list = @$in_list_ref;
-    return $self->SUPER::sql_in($column_name, $in_list_ref) if $#in_list < 1000;
+    return $self->SUPER::sql_in($column_name, $in_list_ref, $negate) if $#in_list < 1000;
     my @in_str;
     while (@in_list) {
         my $length = $#in_list + 1;
         my $splice = $length > 1000 ? 1000 : $length;
         my @sub_in_list = splice(@in_list, 0, $splice);
         push(@in_str, 
-             $self->SUPER::sql_in($column_name, \@sub_in_list)); 
+             $self->SUPER::sql_in($column_name, \@sub_in_list, $negate));
     }
     return "( " . join(" OR ", @in_str) . " )";
 }
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 542b01045..8ba9cdc6b 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -48,6 +48,7 @@ use Bugzilla::Group;
 use Bugzilla::User;
 use Bugzilla::Field;
 use Bugzilla::Search::Clause;
+use Bugzilla::Search::ClauseGroup;
 use Bugzilla::Search::Condition qw(condition);
 use Bugzilla::Status;
 use Bugzilla::Keyword;
@@ -1116,8 +1117,8 @@ sub _translate_join {
     
     die "join with no table: " . Dumper($join_info) if !$join_info->{table};
     die "join with no 'as': " . Dumper($join_info) if !$join_info->{as};
-        
-    my $from_table = "bugs";
+
+    my $from_table = $join_info->{bugs_table} || "bugs";
     my $from  = $join_info->{from} || "bug_id";
     if ($from =~ /^(\w+)\.(\w+)$/) {
         ($from_table, $from) = ($1, $2);
@@ -1522,7 +1523,7 @@ sub _charts_to_conditions {
     my $clause = $self->_charts;
     my @joins;
     $clause->walk_conditions(sub {
-        my ($condition) = @_;
+        my ($clause, $condition) = @_;
         return if !$condition->translated;
         push(@joins, @{ $condition->translated->{joins} });
     });
@@ -1542,7 +1543,7 @@ sub _params_to_data_structure {
     my ($self) = @_;
     
     # First we get the "special" charts, representing all the normal
-    # field son the search page. This may modify _params, so it needs to
+    # fields on the search page. This may modify _params, so it needs to
     # happen first.
     my $clause = $self->_special_charts;
 
@@ -1551,7 +1552,7 @@ sub _params_to_data_structure {
     
     # And then process the modern "custom search" format.
     $clause->add( $self->_custom_search );
-   
+    
     return $clause;
 }
 
@@ -1582,7 +1583,7 @@ sub _boolean_charts {
                 my $identifier = "$chart_id-$and_id-$or_id";
                 my $field = $params->{"field$identifier"};
                 my $operator = $params->{"type$identifier"};
-                my $value = $params->{"value$identifier"};                
+                my $value = $params->{"value$identifier"};
                 $or_clause->add($field, $operator, $value);
             }
             $and_clause->add($or_clause);
@@ -1598,13 +1599,18 @@ sub _custom_search {
     my ($self) = @_;
     my $params = $self->_params;
 
-    my $current_clause = new Bugzilla::Search::Clause($params->{j_top});
+    my $joiner = $params->{j_top} || '';
+    my $current_clause = $joiner eq 'AND_G'
+        ? new Bugzilla::Search::ClauseGroup()
+        : new Bugzilla::Search::Clause($joiner);
     my @clause_stack;
     foreach my $id ($self->_field_ids) {
         my $field = $params->{"f$id"};
         if ($field eq 'OP') {
-            my $joiner = $params->{"j$id"};
-            my $new_clause = new Bugzilla::Search::Clause($joiner);
+            my $joiner = $params->{"j$id"} || '';
+            my $new_clause = $joiner eq 'AND_G'
+                ? new Bugzilla::Search::ClauseGroup()
+                : new Bugzilla::Search::Clause($joiner);
             $new_clause->negate($params->{"n$id"});
             $current_clause->add($new_clause);
             push(@clause_stack, $current_clause);
@@ -1643,14 +1649,12 @@ sub _field_ids {
 }
 
 sub _handle_chart {
-    my ($self, $chart_id, $condition) = @_;
+    my ($self, $chart_id, $clause, $condition) = @_;
     my $dbh = Bugzilla->dbh;
     my $params = $self->_params;
     my ($field, $operator, $value) = $condition->fov;
-
-    $field = FIELD_MAP->{$field} || $field;
-
     return if (!defined $field or !defined $operator or !defined $value);
+    $field = FIELD_MAP->{$field} || $field;
     
     my $string_value;
     if (ref $value eq 'ARRAY') {
@@ -1681,15 +1685,19 @@ sub _handle_chart {
     # on multiple values, like anyexact.
     
     my %search_args = (
-        chart_id   => $chart_id,
-        sequence   => $chart_id,
-        field      => $field,
-        full_field => $full_field,
-        operator   => $operator,
-        value      => $string_value,
-        all_values => $value,
-        joins      => [],
+        chart_id     => $chart_id,
+        sequence     => $chart_id,
+        field        => $field,
+        full_field   => $full_field,
+        operator     => $operator,
+        value        => $string_value,
+        all_values   => $value,
+        joins        => [],
+        bugs_table   => 'bugs',
+        table_suffix => '',
     );
+    $clause->update_search_args(\%search_args);
+
     $search_args{quoted} = $self->_quote_unless_numeric(\%search_args);
     # This should add a "term" selement to %search_args.
     $self->do_search_function(\%search_args);
@@ -1705,7 +1713,12 @@ sub _handle_chart {
         field => $field, type => $operator,
         value => $string_value, term => $search_args{term},
     });
-    
+
+    foreach my $join (@{ $search_args{joins} }) {
+        $join->{bugs_table}   = $search_args{bugs_table};
+        $join->{table_suffix} = $search_args{table_suffix};
+    }
+
     $condition->translated(\%search_args);
 }
 
@@ -1861,8 +1874,14 @@ sub _quote_unless_numeric {
 }
 
 sub build_subselect {
-    my ($outer, $inner, $table, $cond) = @_;
-    return "$outer IN (SELECT $inner FROM $table WHERE $cond)";
+    my ($outer, $inner, $table, $cond, $negate) = @_;
+    # Execute subselects immediately to avoid dependent subqueries, which are
+    # large performance hits on MySql
+    my $q = "SELECT DISTINCT $inner FROM $table WHERE $cond";
+    my $dbh = Bugzilla->dbh;
+    my $list = $dbh->selectcol_arrayref($q);
+    return $negate ? "1=1" : "1=2" unless @$list;
+    return $dbh->sql_in($outer, $list, $negate);
 }
 
 # Used by anyexact to get the list of input values. This allows us to
@@ -2659,8 +2678,7 @@ sub _multiselect_term {
     my $term = $args->{term};
     $term .= $args->{_extra_where} || '';
     my $select = $args->{_select_field} || 'bug_id';
-    my $not_sql = $not ? "NOT " : '';
-    return "bugs.bug_id ${not_sql}IN (SELECT $select FROM $table WHERE $term)";
+    return build_subselect("$args->{bugs_table}.bug_id", $select, $table, $term, $not);
 }
 
 ###############################
diff --git a/Bugzilla/Search/Clause.pm b/Bugzilla/Search/Clause.pm
index a068ce5ed..38f6f30be 100644
--- a/Bugzilla/Search/Clause.pm
+++ b/Bugzilla/Search/Clause.pm
@@ -42,6 +42,11 @@ sub children {
     return $self->{children};
 }
 
+sub update_search_args {
+    my ($self, $search_args) = @_;
+    # abstract
+}
+
 sub joiner { return $_[0]->{joiner} }
 
 sub has_translated_conditions {
@@ -83,7 +88,7 @@ sub walk_conditions {
     my ($self, $callback) = @_;
     foreach my $child (@{ $self->children }) {
         if ($child->isa('Bugzilla::Search::Condition')) {
-            $callback->($child);
+            $callback->($self, $child);
         }
         else {
             $child->walk_conditions($callback);
diff --git a/extensions/Example/Extension.pm b/extensions/Example/Extension.pm
index 8eef19a6e..d7b892cfa 100644
--- a/extensions/Example/Extension.pm
+++ b/extensions/Example/Extension.pm
@@ -724,10 +724,12 @@ sub _check_short_desc {
     my $invocant = shift;
     my $value = $invocant->$original(@_);
     if ($value !~ /example/i) {
-        # Uncomment this line to make Bugzilla throw an error every time
+        # Use this line to make Bugzilla throw an error every time
         # you try to file a bug or update a bug without the word "example"
         # in the summary.
-        #ThrowUserError('example_short_desc_invalid');
+        if (0) {
+            ThrowUserError('example_short_desc_invalid');
+        }
     }
     return $value;
 }
diff --git a/js/custom-search.js b/js/custom-search.js
index 73897035d..e5c172d3b 100644
--- a/js/custom-search.js
+++ b/js/custom-search.js
@@ -40,7 +40,7 @@ function custom_search_new_row() {
     var row = document.getElementById('custom_search_last_row');
     var clone = row.cloneNode(true);
     
-    _cs_fix_ids(clone);
+    _cs_fix_row_ids(clone);
    
     // We only want one copy of the buttons, in the new row. So the old
     // ones get deleted.
@@ -55,13 +55,28 @@ function custom_search_new_row() {
     // Always make sure there's only one row with this id.
     row.id = null;
     row.parentNode.appendChild(clone);
+    cs_reconfigure(row);
     fix_query_string(row);
     return clone;
 }
 
+var _cs_source_any_all;
 function custom_search_open_paren() {
     var row = document.getElementById('custom_search_last_row');
 
+    // create a copy of j_top and use that as the source, so we can modify
+    // j_top if required
+    if (!_cs_source_any_all) {
+        var j_top = document.getElementById('j_top');
+        _cs_source_any_all = j_top.cloneNode(true);
+    }
+
+    // find the parent any/all select, and remove the grouped option
+    var structure = _cs_build_structure(row);
+    var old_id = _cs_get_row_id(row);
+    var parent_j = document.getElementById(_cs_get_join(structure, 'f' + old_id));
+    _cs_remove_and_g(parent_j);
+
     // If there's an "Any/All" select in this row, it needs to stay as
     // part of the parent paren set.
     var old_any_all = _remove_any_all(row);
@@ -78,21 +93,20 @@ function custom_search_open_paren() {
     var not_for_paren = new_not[0].cloneNode(true);
 
     // Preserve the values when modifying the row.
-    var id = _cs_fix_ids(row, true);
+    var id = _cs_fix_row_ids(row, true);
     var prev_id = id - 1;
 
     var paren_row = row.cloneNode(false);
     paren_row.id = null;
     paren_row.innerHTML = '(<input type="hidden" name="f' + prev_id
-                        + '" value="OP">';
+                        + '" id="f' + prev_id + '" value="OP">';
     paren_row.insertBefore(not_for_paren, paren_row.firstChild);
     row.parentNode.insertBefore(paren_row, row);
-    
+
     // New paren set needs a new "Any/All" select.
     var any_all_container = document.createElement('div');
     YAHOO.util.Dom.addClass(any_all_container, ANY_ALL_SELECT_CLASS);
-    var j_top = document.getElementById('j_top');
-    var any_all = j_top.cloneNode(true);
+    var any_all = _cs_source_any_all.cloneNode(true);
     any_all.name = 'j' + prev_id;
     any_all.id = any_all.name;
     any_all_container.appendChild(any_all);
@@ -104,6 +118,7 @@ function custom_search_open_paren() {
     YAHOO.util.Dom.setStyle(row, 'margin-left', new_margin + 'em');
     YAHOO.util.Dom.removeClass('cp_container', 'bz_default_hidden');
 
+    cs_reconfigure(any_all_container);
     fix_query_string(any_all_container);
 }
 
@@ -112,7 +127,7 @@ function custom_search_close_paren() {
     
     // We need to up the new row's id by one more, because we're going
     // to insert a "CP" before it.
-    var id = _cs_fix_ids(new_row);
+    var id = _cs_fix_row_ids(new_row);
 
     var margin = YAHOO.util.Dom.getStyle(new_row, 'margin-left');
     var int_match = margin.match(/\d+/);
@@ -122,7 +137,7 @@ function custom_search_close_paren() {
     var paren_row = new_row.cloneNode(false);
     paren_row.id = null;
     paren_row.innerHTML = ')<input type="hidden" name="f' + (id - 1)
-                        + '" value="CP">';
+                        + '" id="f' + (id - 1) + '" value="CP">';
   
     new_row.parentNode.insertBefore(paren_row, new_row);
 
@@ -130,6 +145,7 @@ function custom_search_close_paren() {
         YAHOO.util.Dom.addClass('cp_container', 'bz_default_hidden');
     }
 
+    cs_reconfigure(new_row);
     fix_query_string(new_row);
 }
 
@@ -172,19 +188,17 @@ function redirect_html4_browsers() {
     document.location = url;
 }
 
-function _cs_fix_ids(parent, preserve_values) {
+function _cs_fix_row_ids(row, preserve_values) {
     // Update the label of the checkbox.
-    var label = YAHOO.util.Dom.getElementBy(function() { return true },
-                                            'label', parent);
+    var label = YAHOO.util.Dom.getElementBy(function() { return true }, 'label', row);
     var id_match = label.htmlFor.match(/\d+$/);
     var id = parseInt(id_match[0]) + 1;
     label.htmlFor = label.htmlFor.replace(/\d+$/, id);
 
-    // Sets all the inputs in the parent back to their default
+    // Sets all the inputs in the row back to their default
     // and fixes their id.
     var fields =
-        YAHOO.util.Dom.getElementsByClassName('custom_search_form_field', null,
-                                              parent);
+        YAHOO.util.Dom.getElementsByClassName('custom_search_form_field', null, row);
     for (var i = 0; i < fields.length; i++) {
         var field = fields[i];
 
@@ -196,15 +210,141 @@ function _cs_fix_ids(parent, preserve_values) {
                 field.value = '';
             }
         }
-        
-        // Update the numeric id for the new row.
+
+        // Update the numeric id for the row.
         field.name = field.name.replace(/\d+$/, id);
         field.id = field.name;
     }
-    
+
     return id;
 }
 
+function _cs_build_structure(form_member) {
+    // build a map of the structure of the custom fields
+    var form = YAHOO.util.Dom.getAncestorByTagName(form_member, 'form');
+    var last_id = _get_last_cs_row_id(form);
+    var structure = [ 'j_top' ];
+    var nested = [ structure ];
+    for (var id = 1; id <= last_id; id++) {
+        var f = form['f' + id];
+        if (!f || !f.parentNode.parentNode) continue;
+
+        if (f.value == 'OP') {
+            var j = [ 'j' + id ];
+            nested[nested.length - 1].push(j);
+            nested.push(j);
+            continue;
+        } else if (f.value == 'CP') {
+            nested.pop();
+            continue;
+        } else {
+            nested[nested.length - 1].push('f' + id);
+        }
+    }
+    return structure;
+}
+
+function cs_reconfigure(form_member) {
+    var structure = _cs_build_structure(form_member);
+    _cs_add_listeners(structure);
+    _cs_trigger_j_listeners(structure);
+    fix_query_string(form_member);
+
+    var j = _cs_get_join(structure, 'f' + _get_last_cs_row_id());
+    document.getElementById('op_button').disabled = document.getElementById(j).value == 'AND_G';
+}
+
+function _cs_add_listeners(parents) {
+    for (var i = 0, l = parents.length; i < l; i++) {
+        if (typeof(parents[i]) == 'object') {
+            // nested
+            _cs_add_listeners(parents[i]);
+        } else if (i == 0) {
+            // joiner
+            YAHOO.util.Event.removeListener(parents[i], 'change', _cs_j_change);
+            YAHOO.util.Event.addListener(parents[i], 'change', _cs_j_change, parents);
+        } else {
+            // field
+            YAHOO.util.Event.removeListener(parents[i], 'change', _cs_f_change);
+            YAHOO.util.Event.addListener(parents[i], 'change', _cs_f_change, parents);
+        }
+    }
+}
+
+function _cs_trigger_j_listeners(fields) {
+    var has_children = false;
+    for (var i = 0, l = fields.length; i < l; i++) {
+        if (typeof(fields[i]) == 'undefined') {
+            continue;
+        } else if (typeof(fields[i]) == 'object') {
+            // nested
+            _cs_trigger_j_listeners(fields[i]);
+            has_children = true;
+        } else if (i == 0) {
+            _cs_j_change(undefined, fields);
+        }
+    }
+    if (has_children) {
+        _cs_remove_and_g(document.getElementById(fields[0]));
+    }
+}
+
+function _cs_get_join(parents, field) {
+    for (var i = 0, l = parents.length; i < l; i++) {
+        if (typeof(parents[i]) == 'object') {
+            // nested
+            var result = _cs_get_join(parents[i], field);
+            if (result) return result;
+        } else if (parents[i] == field) {
+            return parents[0];
+        }
+    }
+    return false;
+}
+
+function _cs_remove_and_g(join_field) {
+    var index = bz_optionIndex(join_field, 'AND_G');
+    join_field.options[index] = null;
+    join_field.options[bz_optionIndex(join_field, 'AND')].innerHTML = cs_and_label;
+    join_field.options[bz_optionIndex(join_field, 'OR')].innerHTML = cs_or_label;
+}
+
+function _cs_j_change(evt, fields, field) {
+    var j = document.getElementById(fields[0]);
+    if (j && j.value == 'AND_G') {
+        for (var i = 1, l = fields.length; i < l; i++) {
+            if (typeof(fields[i]) == 'object') continue;
+            if (!field) {
+                field = document.getElementById(fields[i]).value;
+            } else {
+                document.getElementById(fields[i]).value = field;
+            }
+        }
+        if (evt) {
+            fix_query_string(j);
+        }
+        if ('f' + _get_last_cs_row_id() == fields[fields.length - 1]) {
+            document.getElementById('op_button').style.display = 'none';
+        }
+    } else {
+        document.getElementById('op_button').style.display = '';
+    }
+}
+
+function _cs_f_change(evt, args) {
+    var field = YAHOO.util.Event.getTarget(evt);
+    _cs_j_change(evt, args, field.value);
+}
+
+function _get_last_cs_row_id() {
+    return _cs_get_row_id('custom_search_last_row');
+}
+
+function _cs_get_row_id(row) {
+    var label = YAHOO.util.Dom.getElementBy(function() { return true }, 'label', row);
+    return parseInt(label.htmlFor.match(/\d+$/)[0]);
+}
+
 function _remove_any_all(parent) {
     var any_all = YAHOO.util.Dom.getElementsByClassName(
         ANY_ALL_SELECT_CLASS, null, parent);
diff --git a/t/Support/Files.pm b/t/Support/Files.pm
index 31a0058db..2898fdd3f 100644
--- a/t/Support/Files.pm
+++ b/t/Support/Files.pm
@@ -34,6 +34,14 @@ my @extension_paths = map { $_->package_dir } @{ Bugzilla->extensions };
 find(sub { push(@files, $File::Find::name) if $_ =~ /\.pm$/;}, 'Bugzilla', @extension_paths);
 push(@files, 'extensions/create.pl');
 
+my @extensions = glob('extensions/*');
+foreach my $extension (@extensions) {
+    # Skip disabled extensions
+    next if -e "$extension/disabled";
+
+    find(sub { push(@files, $File::Find::name) if $_ =~ /\.pm$/;}, $extension);
+}
+
 sub isTestingFile {
     my ($file) = @_;
     my $exclude;
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index c2b2ceb28..fca219401 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1575,6 +1575,17 @@
     and the "matches" search can only be used with the "content"
     field.
 
+  [% ELSIF error == "search_grouped_field_invalid" %]
+    [% terms.Bugzilla %] does not support using the 
+    "[%+ field_descs.$field FILTER html %]" ([% field FILTER html %])
+    field with grouped search conditions.
+
+  [% ELSIF error == "search_grouped_invalid_nesting" %]
+    You cannot nest clauses within grouped search conditions.
+
+  [% ELSIF error == "search_grouped_field_mismatch" %]
+    All conditions under a groups search must use the same field.
+
   [% ELSIF error == "search_field_operator_invalid" %]
     [% terms.Bugzilla %] does not support using the 
     "[%+ field_descs.$field FILTER html %]" ([% field FILTER html %])
diff --git a/template/en/default/search/boolean-charts.html.tmpl b/template/en/default/search/boolean-charts.html.tmpl
index 878589cea..d87db1c6a 100644
--- a/template/en/default/search/boolean-charts.html.tmpl
+++ b/template/en/default/search/boolean-charts.html.tmpl
@@ -78,6 +78,10 @@
   <script type="text/javascript" src="[% 'js/history.js/native.history.js' FILTER mtime %]"></script>
   <script type="text/javascript">
     redirect_html4_browsers();
+    [%# These are alternative labels for the AND and OR options in and_all_select %]
+    var cs_and_label = 'Match ALL of the following:';
+    var cs_or_label  = 'Match ANY of the following:';
+    cs_reconfigure('custom_search_last_row');
   </script>
 </div>
 
@@ -134,7 +138,8 @@
       (
       [% indent_level = indent_level + 1 %]
     [% ELSIF condition.f == "CP" %]
-      <input type="hidden" name="f[% cond_num FILTER html %]" value="CP">
+      <input type="hidden" name="f[% cond_num FILTER html %]"
+             id="f[% cond_num FILTER html %]" value="CP">
       )
     [% ELSE %]
       <select name="f[% cond_num FILTER html %]" title="Field"
@@ -178,9 +183,11 @@
   <div class="any_all_select">
     <select name="[% name FILTER html %]" id="[% name FILTER html %]"
             onchange="fix_query_string(this)">
-      <option value="AND">Match ALL of the following:</option>
+      <option value="AND">Match ALL of the following separately:</option>
       <option value="OR" [% ' selected="selected"' IF selected == "OR" %]>
-        Match ANY of the following:</option>
+        Match ANY of the following separately:</option>
+      <option value="AND_G" [% ' selected' IF selected == "AND_G" %]>
+        Match ALL of the following against the same field:</option>
     </select>
     [% IF with_advanced_link %]
       <a id="custom_search_advanced_controller"