From 8532de99d9b6d1a1ec53e9fb2f6ef7374fab4674 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Tue, 31 May 2011 09:24:17 -0700 Subject: Bug 647649: Change the old "Boolean Charts" UI into the new AND/OR "Custom Search" UI. r=timello, a=mkanat --- Bugzilla/CGI.pm | 15 +- buglist.cgi | 13 -- js/custom-search.js | 156 +++++++++++++++++ query.cgi | 106 ++---------- template/en/default/filterexceptions.pl | 7 +- .../en/default/search/boolean-charts.html.tmpl | 186 ++++++++++++--------- template/en/default/search/form.html.tmpl | 3 +- .../en/default/search/search-specific.html.tmpl | 4 +- template/en/default/search/type-select.html.tmpl | 3 +- 9 files changed, 297 insertions(+), 196 deletions(-) create mode 100644 js/custom-search.js diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index 627b78b58..e0e1c40ba 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -149,9 +149,18 @@ sub clean_search_url { $self->delete("${param}_type"); } - # Boolean Chart stuff is empty if it's "noop" - if ($param =~ /\d-\d-\d/ && defined $self->param($param) - && $self->param($param) eq 'noop') + # Custom Search stuff is empty if it's "noop". We also keep around + # the old Boolean Chart syntax for backwards-compatibility. + if (($param =~ /\d-\d-\d/ || $param =~ /^[[:alpha:]]\d+$/) + && defined $self->param($param) && $self->param($param) eq 'noop') + { + $self->delete($param); + } + + # Any "join" for custom search that's an AND can be removed, because + # that's the default. + if (($param =~ /^j\d+$/ || $param eq 'j_top') + && $self->param($param) eq 'AND') { $self->delete($param); } diff --git a/buglist.cgi b/buglist.cgi index 69c4edaec..7549063a4 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -68,19 +68,6 @@ if (length($buffer) == 0) { ThrowUserError("buglist_parameters_required"); } -# If a parameter starts with cmd-, this means the And or Or button has been -# pressed in the advanced search page with JS turned off. -if (grep { $_ =~ /^cmd\-/ } $cgi->param()) { - my $url = "query.cgi?$buffer#chart"; - print $cgi->redirect(-location => $url); - # Generate and return the UI (HTML page) from the appropriate template. - $vars->{'message'} = "buglist_adding_field"; - $vars->{'url'} = $url; - $template->process("global/message.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - exit; -} - $cgi->redirect_search_url(); # Determine whether this is a quicksearch query. diff --git a/js/custom-search.js b/js/custom-search.js new file mode 100644 index 000000000..3f3ffeef2 --- /dev/null +++ b/js/custom-search.js @@ -0,0 +1,156 @@ +/* The contents of this file are subject to the Mozilla Public + * License Version 1.1 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS + * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or + * implied. See the License for the specific language governing + * rights and limitations under the License. + * + * The Original Code is the Bugzilla Bug Tracking System. + * + * The Initial Developer of the Original Code is BugzillaSource, Inc. + * Portions created by the Initial Developer are Copyright (C) 2011 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Max Kanat-Alexander + */ + +var PAREN_INDENT_EM = 2; + +function custom_search_new_row() { + var row = document.getElementById('custom_search_last_row'); + var clone = row.cloneNode(true); + + _cs_fix_ids(clone); + + // We only want one copy of the buttons, in the new row. So the old + // ones get deleted. + var op_button = document.getElementById('op_button'); + row.removeChild(op_button); + var cp_button = document.getElementById('cp_container'); + row.removeChild(cp_button); + var add_button = document.getElementById('add_button'); + row.removeChild(add_button); + _remove_any_all(clone); + + // Always make sure there's only one row with this id. + row.id = null; + row.parentNode.appendChild(clone); + return clone; +} + +function custom_search_open_paren() { + var row = document.getElementById('custom_search_last_row'); + + // If there's an "Any/All" select in this row, it needs to stay as + // part of the parent paren set. + var any_all = _remove_any_all(row); + if (any_all) { + var any_all_row = row.cloneNode(false); + any_all_row.id = null; + any_all_row.appendChild(any_all); + row.parentNode.insertBefore(any_all_row, row); + } + + // We also need a "Not" checkbox to stay in the parent paren set. + var new_not = YAHOO.util.Dom.getElementsByClassName( + 'custom_search_not_container', null, row); + var not_for_paren = new_not[0].cloneNode(true); + + // Preserve the values when modifying the row. + var id = _cs_fix_ids(row, true); + var prev_id = id - 1; + + var paren_row = row.cloneNode(false); + paren_row.id = null; + paren_row.innerHTML = '('; + 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 j_top = document.getElementById('j_top'); + var any_all_container = j_top.parentNode.cloneNode(true); + var any_all = YAHOO.util.Dom.getElementsBy(function() { return true }, + 'select', any_all_container); + any_all[0].name = 'j' + prev_id; + any_all[0].id = any_all[0].name; + row.insertBefore(any_all_container, row.firstChild); + + var margin = YAHOO.util.Dom.getStyle(row, 'margin-left'); + var int_match = margin.match(/\d+/); + var new_margin = parseInt(int_match[0]) + PAREN_INDENT_EM; + YAHOO.util.Dom.setStyle(row, 'margin-left', new_margin + 'em'); + YAHOO.util.Dom.removeClass('cp_container', 'bz_default_hidden'); +} + +function custom_search_close_paren() { + var new_row = custom_search_new_row(); + + // 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 margin = YAHOO.util.Dom.getStyle(new_row, 'margin-left'); + var int_match = margin.match(/\d+/); + var new_margin = parseInt(int_match[0]) - PAREN_INDENT_EM; + YAHOO.util.Dom.setStyle(new_row, 'margin-left', new_margin + 'em'); + + var paren_row = new_row.cloneNode(false); + paren_row.id = null; + paren_row.innerHTML = ')'; + + new_row.parentNode.insertBefore(paren_row, new_row); + + if (new_margin == 0) { + YAHOO.util.Dom.addClass('cp_container', 'bz_default_hidden'); + } +} + + +function _cs_fix_ids(parent, preserve_values) { + // Update the label of the checkbox. + var label = YAHOO.util.Dom.getElementBy(function() { return true }, + 'label', parent); + 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 + // and fixes their id. + var fields = + YAHOO.util.Dom.getElementsByClassName('custom_search_form_field', null, + parent); + for (var i = 0; i < fields.length; i++) { + var field = fields[i]; + + if (!preserve_values) { + if (field.type == "checkbox") { + field.checked = false; + } + else { + field.value = ''; + } + } + + // Update the numeric id for the new row. + field.name = field.name.replace(/\d+$/, id); + field.id = field.name; + } + + return id; +} + +function _remove_any_all(parent) { + var any_all = YAHOO.util.Dom.getElementsByClassName('any_all_select', null, + parent); + if (any_all[0]) { + parent.removeChild(any_all[0]); + return any_all[0]; + } + return null; +} diff --git a/query.cgi b/query.cgi index 92c7889a6..097701b63 100755 --- a/query.cgi +++ b/query.cgi @@ -75,71 +75,36 @@ local our %default; # Items which are single-valued, the template should only reference [0] # and ignore any multiple values. sub PrefillForm { - my ($buf) = (@_); + my ($buf) = @_; my $cgi = Bugzilla->cgi; $buf = new Bugzilla::CGI($buf); my $foundone = 0; - # Nothing must be undef, otherwise the template complains. - my @list = ("bug_status", "resolution", "assigned_to", - "rep_platform", "priority", "bug_severity", - "classification", "product", "reporter", "op_sys", - "component", "version", "chfield", "chfieldfrom", - "chfieldto", "chfieldvalue", "target_milestone", - "email", "emailtype", "emailreporter", - "emailassigned_to", "emailcc", "emailqa_contact", - "emaillongdesc", "content", - "changedin", "short_desc", "short_desc_type", - "longdesc", "longdesc_type", "bug_file_loc", - "bug_file_loc_type", "status_whiteboard", - "status_whiteboard_type", "bug_id", - "bug_id_type", "keywords", "keywords_type", - "deadlinefrom", "deadlineto", - "x_axis_field", "y_axis_field", "z_axis_field", - "chart_format", "cumulate", "x_labels_vertical", - "category", "subcategory", "name", "newcategory", - "newsubcategory", "public", "frequency"); - # These fields can also have default values. And because there are - # hooks in the advanced search page which let you add fields as - # discrete forms, we also need to retain the operators. - my @custom_fields = Bugzilla->active_custom_fields; - push(@list, map { $_->name } @custom_fields); - push(@list, map { $_->name . '_type'} @custom_fields); - - foreach my $name (@list) { - $default{$name} = []; - } - - # we won't prefill the boolean chart data from this query if - # there are any being submitted via params - my $prefillcharts = (grep(/^field-/, $cgi->param)) ? 0 : 1; - + # Query parameters that don't represent form fields on this page. + my @skip = qw(format query_format list_id columnlist); + # Iterate over the URL parameters foreach my $name ($buf->param()) { + next if grep { $_ eq $name } @skip; + $foundone = 1; my @values = $buf->param($name); - - # If the name begins with the string 'field', 'type', 'value', or - # 'negate', then it is part of the boolean charts. Because - # these are built different than the rest of the form, we need - # to store these as parameters. We also need to indicate that - # we found something so the default query isn't added in if - # all we have are boolean chart items. - if ($name =~ m/^(?:field|type|value|negate)/) { - $cgi->param(-name => $name, -value => $values[0]) if ($prefillcharts); - $foundone = 1; + + # If the name is a single letter followed by numbers, it's part + # of Custom Search. We store these as an array of hashes. + if ($name =~ /^([[:lower:]])(\d+)$/) { + $default{'custom_search'}->[$2]->{$1} = $values[0]; } # If the name ends in a number (which it does for the fields which # are part of the email searching), we use the array # positions to show the defaults for that number field. - elsif ($name =~ m/^(.+)(\d)$/ && defined($default{$1})) { - $foundone = 1; + elsif ($name =~ /^(\w)(\d)$/) { $default{$1}->[$2] = $values[0]; } - elsif (exists $default{$name}) { - $foundone = 1; - push (@{$default{$name}}, @values); + else { + push (@{ $default{$name} }, @values); } } + return $foundone; } @@ -153,10 +118,6 @@ if (!PrefillForm($buffer)) { } } -if (!scalar(@{$default{'chfieldto'}}) || $default{'chfieldto'}->[0] eq "") { - $default{'chfieldto'} = ["Now"]; -} - # if using groups for entry, then we don't want people to see products they # don't have access to. Remove them from the list. my @selectable_products = sort {lc($a->name) cmp lc($b->name)} @@ -240,43 +201,6 @@ if (!Bugzilla->user->is_timetracker) { unshift(@fields, { name => "noop", description => "---" }); $vars->{'fields'} = \@fields; -# Creating new charts - if the cmd-add value is there, we define the field -# value so the code sees it and creates the chart. It will attempt to select -# "xyzzy" as the default, and fail. This is the correct behaviour. -foreach my $cmd (grep(/^cmd-/, $cgi->param)) { - if ($cmd =~ /^cmd-add(\d+)-(\d+)-(\d+)$/) { - $cgi->param(-name => "field$1-$2-$3", -value => "xyzzy"); - } -} - -if (!$cgi->param('field0-0-0')) { - $cgi->param(-name => 'field0-0-0', -value => "xyzzy"); -} - -# Create data structure of boolean chart info. It's an array of arrays of -# arrays - with the inner arrays having three members - field, type and -# value. -my @charts; -for (my $chart = 0; $cgi->param("field$chart-0-0"); $chart++) { - my @rows; - for (my $row = 0; $cgi->param("field$chart-$row-0"); $row++) { - my @cols; - for (my $col = 0; $cgi->param("field$chart-$row-$col"); $col++) { - my $value = $cgi->param("value$chart-$row-$col"); - if (!defined($value)) { - $value = ''; - } - push(@cols, { field => $cgi->param("field$chart-$row-$col"), - type => $cgi->param("type$chart-$row-$col") || 'noop', - value => $value }); - } - push(@rows, \@cols); - } - push(@charts, {'rows' => \@rows, 'negate' => scalar($cgi->param("negate$chart")) }); -} - -$default{'charts'} = \@charts; - # Named queries if ($userid) { $vars->{'namedqueries'} = $dbh->selectcol_arrayref( diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index abc57008c..948baa521 100644 --- a/template/en/default/filterexceptions.pl +++ b/template/en/default/filterexceptions.pl @@ -58,12 +58,7 @@ ], 'search/boolean-charts.html.tmpl' => [ - '"field${chartnum}-${rownum}-${colnum}"', - 'field.name', - '"${chartnum}-${rownum}-${newor}"', - '"${chartnum}-${newand}-0"', - 'newchart', - 'jsmagic', + '"id=\"$id\"" IF id' ], 'search/form.html.tmpl' => [ diff --git a/template/en/default/search/boolean-charts.html.tmpl b/template/en/default/search/boolean-charts.html.tmpl index 90b5c790d..82c779612 100644 --- a/template/en/default/search/boolean-charts.html.tmpl +++ b/template/en/default/search/boolean-charts.html.tmpl @@ -48,96 +48,124 @@ "matches", "notmatches", ] %] - +
▼
Custom Search Didn't find what you're looking for above? This area allows for ANDs, ORs, and other more complex searches. -
-
-[%# Whoever wrote the original version of boolean charts had a seriously twisted mind %] +
+
+ [% SET indent_level = 0 %] + [% FOREACH condition = default.custom_search %] + [% SET cond_num = loop.count - 1 %] + [% PROCESS one_condition with_buttons = 0 %] + [% END %] + [% PROCESS one_condition + with_buttons = 1 + condition = { f => 'noop' } + cond_num = cond_num + 1 %] + + +
-[% jsmagic = "onclick=\"this.form.action='query.cgi#chart'; this.form.method='POST'; return 1;\"" %] -[% FOREACH chart = default.charts %] - [% chartnum = loop.count - 1 %] - - - - - [% FOREACH row = chart.rows %] - [% rownum = loop.count - 1 %] - - [% FOREACH col = row %] - [% colnum = loop.count - 1 %] - - - [% UNLESS loop.last %] - - - - [% ELSE %] - - [% END %] - - [% END %] - + [% IF condition.f == "CP" %] + [% indent_level = indent_level - 1 %] + [% END %] + +
- [% UNLESS loop.last %] -
- - + [% IF previous_condition.f == "OP" %] + [% INCLUDE any_all_select + name = "j" _ (cond_num - 1) id = "j" _ (cond_num - 1) + selected = previous_condition.j %] + [% END %] + + [% IF with_buttons %] + + [% END %] + + [% UNLESS condition.f == "CP" %] + + + + + [% END %] + + [% IF condition.f == "OP" %] + + ( + [% indent_level = indent_level + 1 %] + [% ELSIF condition.f == "CP" %] + + ) [% ELSE %] - - - + + + [% INCLUDE "search/type-select.html.tmpl" + name = "o${cond_num}", class = "custom_search_form_field" + types = types, selected = condition.o %] + + [% END %] - [% END %] -
- - -
- +[% BLOCK one_condition %] + [%# Skip any conditions that don't have a field defined. %] + [% RETURN IF !condition.f %] + + [% IF !top_level_any_shown %] + [% INCLUDE any_all_select + name = "j_top" id = "j_top" selected = default.j_top.0 %] + [% top_level_any_shown = 1 %] + [% END %] - [% INCLUDE "search/type-select.html.tmpl" - name = "type${chartnum}-${rownum}-${colnum}", - types = types, selected = col.type %] - - - Or -
- [% newor = colnum + 1 %] - -
And
- [% newand = rownum + 1; newchart = chartnum + 1 %] - -       - -       -
- [% "
" IF NOT loop.last %] + [% IF with_buttons %] + + + + + [% END %] + + + [% previous_condition = condition %] +[% END %] + +[% BLOCK any_all_select %] +
+ +
[% END %] - \ No newline at end of file diff --git a/template/en/default/search/form.html.tmpl b/template/en/default/search/form.html.tmpl index f3104203c..41e116518 100644 --- a/template/en/default/search/form.html.tmpl +++ b/template/en/default/search/form.html.tmpl @@ -395,7 +395,8 @@ TUI_hide_default('information_query'); and
+ value="[% default.chfieldto.0 || "Now" FILTER html %]" + onchange="updateCalendarFromField(this)"> diff --git a/template/en/default/search/search-specific.html.tmpl b/template/en/default/search/search-specific.html.tmpl index 776554a64..1d7229cf7 100644 --- a/template/en/default/search/search-specific.html.tmpl +++ b/template/en/default/search/search-specific.html.tmpl @@ -75,7 +75,7 @@ for "crash secure SSL flash". [% FOREACH p = user.get_selectable_products(c.id) %] [% IF p.components.size %] [% END %] @@ -85,7 +85,7 @@ for "crash secure SSL flash". [% ELSE %] [% FOREACH p = product %] [% END %] diff --git a/template/en/default/search/type-select.html.tmpl b/template/en/default/search/type-select.html.tmpl index 043c4194a..6da88202e 100644 --- a/template/en/default/search/type-select.html.tmpl +++ b/template/en/default/search/type-select.html.tmpl @@ -20,7 +20,8 @@ [% PROCESS "global/field-descs.none.tmpl" %] - [% FOREACH type = types %]