big ol' commit making advanced search work, albeit with issues
authorsenator <lebbeous@esilibrary.com>
Mon, 21 Feb 2011 19:42:38 +0000 (14:42 -0500)
committersenator <lebbeous@esilibrary.com>
Mon, 21 Feb 2011 19:42:38 +0000 (14:42 -0500)
most obvious issue is that advanced search leads to a result page that
renders with a basic search form filled out in a silly-looking way

I'm thinking it'd be best when showing results from an advanced search
to *not* show that basic search form, but instead have a hidden version
of the advanced search form, prepopulated, revealable by a button
labeled "refine your search" or something.

That way we'd not have to worry about reversing query parser syntax back
into broken-out widget values. make sense?

13 files changed:
Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Search.pm
Open-ILS/web/css/skin/default/opac/semiauto.css
Open-ILS/web/images/adv_row_close_btn.png [new file with mode: 0644]
Open-ILS/web/js/opac/simple.js [new file with mode: 0644]
Open-ILS/web/templates/default/opac/parts/advanced/global_row.tt2
Open-ILS/web/templates/default/opac/parts/advanced/search.tt2
Open-ILS/web/templates/default/opac/parts/base.tt2
Open-ILS/web/templates/default/opac/parts/format_selector.tt2
Open-ILS/web/templates/default/opac/parts/header.tt2
Open-ILS/web/templates/default/opac/parts/result/table.tt2
Open-ILS/web/templates/default/opac/parts/searchbar.tt2
Open-ILS/web/templates/default/opac/parts/stypes_selector.tt2
Open-ILS/web/templates/default/opac/results.tt2

index 2d826cd..e294aab 100644 (file)
@@ -8,6 +8,85 @@ use OpenILS::Application::AppUtils;
 my $U = 'OpenILS::Application::AppUtils';
 
 
+sub _prepare_biblio_search_basics {
+    my ($cgi) = @_;
+
+    my %parts;
+    my @part_names = qw/class contains query/;
+    $parts{$_} = [ $cgi->param($_) ] for (@part_names);
+
+    my @chunks = ();
+    for (my $i = 0; $i < scalar @{$parts{'class'}}; $i++) {
+        my ($class, $contains, $query) = map { $parts{$_}->[$i] } @part_names;
+
+        push(@chunks, $class . ':') unless $class eq 'keyword' and $i == 0;
+
+        # This stuff probably will need refined or rethought to better handle
+        # the weird things Real Users will surely type in.
+        if ($contains eq 'nocontains') {
+            $query =~ s/"//g;
+            $query = ('"' . $query . '"') if index $query, ' ';
+            $query = '-' . $query;
+        } elsif ($contains eq 'exact') {
+            $query =~ s/"//g;
+            $query = ('"' . $query . '"') if index $query, ' ';
+        }
+        push @chunks, $query;
+    }
+
+    return join(' ', @chunks);
+}
+
+sub _prepare_biblio_search {
+    my ($cgi, $ctx) = @_;
+
+    my $query = _prepare_biblio_search_basics($cgi);
+    my $args = {};
+
+    $args->{'org_unit'} = $cgi->param('loc') || $ctx->{aou_tree}->()->id;
+    $args->{'depth'} = defined $cgi->param('depth') ?
+        $cgi->param('depth') :
+        $ctx->{find_aou}->($args->{'org_unit'})->ou_type->depth;
+
+    if ($cgi->param('available')) {
+        $query = '#available ' . $query;
+    }
+
+    if ($cgi->param('format')) {
+        $args->{'format'} = join('', $cgi->param('format'));
+    }
+
+    if ($cgi->param('lang')) {
+        # XXX TODO find out how to build query with multiple langs, if that
+        # even needs to be a feature of adv search.
+        $query .= ' lang:' . $cgi->param('lang');
+    }
+
+    if ($cgi->param('audience')) {
+        $query .= ' audience(' . $cgi->param('audience') . ')';
+    }
+
+    if (defined $cgi->param('sort')) {
+        my $sort = $cgi->param('sort');
+        my $sort_order = $cgi->param('sort_order');
+        $query .= " sort($sort)";
+        $query .= '#' . $sort_order if $sort_order and $sort ne 'rel';
+    }
+
+    if ($cgi->param('pubyear_how') && $cgi->param('pubyear1')) {
+        if ($cgi->param('pubyear_how') eq 'between') {
+            $query .= ' between(' . $cgi->param('pubyear1');
+            $query .= ',' .  $cgi->param('pubyear2') if $cgi->param('pubyear2');
+            $query .= ')';
+        } else {
+            $query .= ' ' . $cgi->param('pubyear_how') .
+                '(' . $cgi->param('pubyear1') . ')';
+        }
+    }
+
+    return ($args, $query);
+}
+
 # context additions: 
 #   page_size
 #   hit_count
@@ -20,25 +99,19 @@ sub load_rresults {
 
     $ctx->{page} = 'rresult';
     my $page = $cgi->param('page') || 0;
-    my $item_type = $cgi->param('item_type');
     my $facet = $cgi->param('facet');
-    my $query = $cgi->param('query');
-    my $search_class = $cgi->param('class');
     my $limit = $cgi->param('limit') || 10; # TODO user settings
 
-    my $loc = $cgi->param('loc') || $ctx->{aou_tree}->()->id;
-    my $depth = defined $cgi->param('depth') ? 
-        $cgi->param('depth') : $ctx->{find_aou}->($loc)->ou_type->depth;
+    my ($args, $query) = _prepare_biblio_search($cgi, $ctx);
 
-    my $args = {
-        limit => $limit, offset => $page * $limit,
-        org_unit => $loc, depth => $depth, $item_type ? (item_type => [$item_type]) : ()
-    };
+    # Stuff these into the TT context so that templates can use them in redrawing forms
+    $ctx->{processed_search_query} = $query;
+    $ctx->{processed_search_args} = $args;
+
+    $args->{'limit'} = $limit;
+    $args->{'offset'} = $page * $limit;
 
     $query = "$query $facet" if $facet; # TODO
-    # XXX Since open-ils.search is a public service, it is responsible for
-    # being wary of injection/bad input, not us, right?
-    $query = $search_class . ':' . $query if $search_class;
 
     my $results;
 
index 550669f..68edb5e 100644 (file)
 .opac-auto-149 { position: relative; top: 5px; left: 25px; }
 #util_print_btn { position: relative; top: 5px; left: 30px; }
 .opac-auto-151 { position: relative; top: 75px; }
-.opac-auto-152 { position: relative; top: -9px; }
+#adv_reset { position: relative; top: -9px; }
 .opac-auto-153 { position: relative; z-index: 100; }
 .text-center { text-align: center; }
 .opac-auto-156 { text-align: center; font-weight: bold; }
-.opac-auto-157 { text-align: center; margin-top: 20px; width: 400px; }
+#adv_quick_search_sidebar { text-align: center; margin-top: 20px; width: 400px; }
 .opac-auto-158 { text-align: center; margin-top: 6px; margin-bottom: 6px }
 .opac-auto-159 { text-align: center; padding: 20px; width: 100% }
 .opac-auto-160 { text-align: center; padding-bottom: 8px; }
diff --git a/Open-ILS/web/images/adv_row_close_btn.png b/Open-ILS/web/images/adv_row_close_btn.png
new file mode 100644 (file)
index 0000000..edccf37
Binary files /dev/null and b/Open-ILS/web/images/adv_row_close_btn.png differ
diff --git a/Open-ILS/web/js/opac/simple.js b/Open-ILS/web/js/opac/simple.js
new file mode 100644 (file)
index 0000000..7124d03
--- /dev/null
@@ -0,0 +1,29 @@
+/* Keep this dead simple. No dojo. Call nothing via onload. */
+function $(s) { return document.getElementById(s); }
+function removeClass(node, cls) {
+    if (!node || !node.className) return;
+    node.className =
+        node.className.replace(new RegExp("\\b" + cls + "\\b", "g"), "");
+}
+function addClass(node, cls) {
+    if (!node) return;
+    removeClass(node, cls);
+    if (!node.className) node.className = cls;
+    else node.className += ' ' + cls;
+}
+function unHideMe(node) { removeClass(node, "hide_me"); }
+function hideMe(node) { addClass(node, "hide_me"); }
+
+var _search_row_template;
+function addSearchRow() {
+    if (!_search_row_template) {
+        t = $("adv_global_row").cloneNode(true);
+        t.id = null;
+        _search_row_template = t;
+    }
+
+    $("adv_global_tbody").insertBefore(
+        _search_row_template.cloneNode(true),
+        $("adv_global_addrow")
+    );
+}
index 941cf7c..6d65c68 100644 (file)
@@ -1,4 +1,4 @@
-<tr id='adv_global_trow' type='input'>
+<tr id="adv_global_row" type='input'>
     <td align='left' width='100%' nowrap='nowrap'>
         <!-- select the search class -->
         <span class="opac-auto-078">
             <option value='exact'>[% l("Matches Exactly") %]</option>
         </select>
         <!-- search term -->
-        <input type='text' size='18' name='term' style='margin-right: 3px;' />
+        <input type='text' size='18' name='query' style='margin-right: 3px;' />
         <!-- Remove this row -->
+        <a href="javascript:;"
+            style="position:relative;top:1px; vertical-align: middle;"
+            title="[% l('Remove row') %]" alt="[% l('Remove row') %]"
+            onclick='var row = this.parentNode.parentNode;var tbody = row.parentNode; if( tbody.getElementsByTagName("tr").length > 2 ) row.parentNode.removeChild(row);'><img src="[% ctx.media_prefix %]/images/adv_row_close_btn.png" /></a>
     </td>
 </tr>
index c4fde3a..0fb4ab1 100644 (file)
@@ -1,3 +1,4 @@
+<form id="adv_search_form" action="[% ctx.opac_root %]/results" method="GET">
 <table id='adv_global_search' class='data_grid data_grid_center' width='100%'>
     <tr style='border-bottom: none;'>
         <!-- Contains the user-addable(?) rows to define search class, containment and text -->
@@ -17,8 +18,7 @@
                     <!-- add a new row -->
                     <tr id='adv_global_addrow'>
                         <td align='left' style="padding-top:7px;">
-                            <a href="javascript:;" id="myopac_new_global_row" onclick='advAddGblRow();'>Add Search Row</a>
-                            <button class="hide_me">[% l("Submit Search") %]</button><!-- XXX TODO make a real form -->
+                            <a href="javascript:;" id="myopac_new_global_row" onclick='addSearchRow();'>Add Search Row</a>
                         </td>
                     </tr>
                 </tbody>
             </td>
             <td valign='top'>
                 <strong>[% l("Language") %]</strong><br />
-                <select multiple='multiple' size='4' id='adv_global_lang'>
+                <select multiple='multiple' size='4' name="lang" id='adv_global_lang'>
                     [% INCLUDE "default/opac/parts/item_lang_options.tt2" %]
                 </select>
             </td>
             <td valign='top'>
                 <strong>[% l("Audience") %]</strong><br />
-                <select multiple='multiple' size='3' id='adv_global_audience' class='hide_me'></select>
-                <select multiple='multiple' size='3' id='adv_global_audience_basic'>
+                <!-- XXX this used to be multiple, but when would that be
+                useful? -->
+                <select size='3' name="audience" id='adv_global_audience_basic'>
                     <option value='abcj'>[% l("Juvenile") %]</option>
                     <option value='d'>[% l("General") %]</option>
                     <option value='e'>[% l("Adult") %]</option>
@@ -73,7 +74,7 @@
                             <tbody>
                                 <tr>
                                     <td align=''>
-                                        <select id='adv_global_sort_by' onchange='__setsortsel();'>
+                                        <select id='adv_global_sort_by' name="sort" onchange="$('adv_global_sort_dir').disabled = !Boolean(this.selectedIndex);">
                                             <option value='rel'>[% l("Relevance") %]</option>
                                             <option value='title'>[% l("Title") %]</option>
                                             <option value='author'>[% l("Author") %]</option>
                                 </tr>
                                 <tr>
                                     <td>
-                                        <select id='adv_global_sort_dir' disabled='disabled'>
+                                        <select id='adv_global_sort_dir' name="sort_order" disabled='disabled'>
                                             <option value='asc'>[% l("Ascending / A to Z") %]</option>
                                             <option value='desc'>[% l("Descending / Z to A") %]</option>
                                         </select>
                                     </td>
                            <!-- force the enable/disable sort dir code to run -->
-                           <script language='javascript' type='text/javascript'>__setsortsel();</script>
                                 </tr>
                                 <tr>
                                     <td align='center' class="hide_me">
                             [% PROCESS "default/opac/parts/org_selector.tt2";
                                 PROCESS build_org_selector name='loc' value=loc %]
                             <div style="position:relative;top:7px;">
-                                <input type='checkbox'
-                                id='opac.result.limit2avail'/>
+                                <input type='checkbox' name="available" value="1"
+                                    id='opac.result.limit2avail'/>
                                 <label style="position:relative;top:-2px;"
                                     for='opac.result.limit2avail'>
                                     [% l("Limit to Available") %]</label>
                         </td>
                         <td valign='top'>
                             <strong>[% l("Publication Year") %]</strong><br />
-                            <select id='adv_global_pub_date_type' onchange='
-                                if($("adv_global_pub_date_type").selectedIndex == 3)
+                            <select id='adv_global_pub_date_type' name='pubyear_how' onchange='
+                                if(this.selectedIndex == 3)
                                     unHideMe($("adv_global_pub_date_2_span"));
                                 else
                                     hideMe($("adv_global_pub_date_2_span"));'>
-                                <option value='equals' selected='selected'>[% l("Is") %]</option>
+                                <option value='between' selected='selected'>[% l("Is") %]</option><!-- sic -->
                                 <option value='before'>[% l("Before") %]</option>
                                 <option value='after'>[% l("After") %]</option>
                                 <option value='between'>[% l("Between") %]</option>
                             </select>    
                             <div style='margin-top:5px;'>
-                                <input id='adv_global_pub_date_1' type='text' size='4' maxlength='4'/>
+                                <input id='adv_global_pub_date_1' name='pubyear1' type='text' size='4' maxlength='4'/>
                                 <span id='adv_global_pub_date_2_span' class='hide_me'>
-                                   [% l("and") %] <input id='adv_global_pub_date_2' type='text' size='4' maxlength='4'/>
+                                   [% l("and") %] <input name='pubyear2' id='adv_global_pub_date_2' type='text' size='4' maxlength='4'/>
                                 </span>
                             </div>
                         </td>
             </table>
         </td>
     </tr>
-
     <tr class='border_4_2'>
         <td align="left" colspan='2'>
-            <!-- XXX TODO make a real form, and make this a real submitter -->
-        <img src="[% ctx.media_prefix %]/images/search_btn.gif" alt="[% l('Search') %]"  class='pointer' />
-        &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
-        <!-- XXX TODO make a real form, and make this a real resetter -->
-        <a href="javascript:;" style="position: relative; top: -9px;">Reset Form</a>
+            <input type="image" src="[% ctx.media_prefix %]/images/search_btn.gif"
+            alt="[% l('Search') %]" class='pointer' />
+            &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
+            <a href="javascript:document.getElementById('adv_search_form').reset();"
+                id="adv_reset">[% l('Reset Form') %]</a>
         </td>
     </tr>
 </table>
-
-
-<div id='adv_quick_search_sidebar' class='sidebar_chunk hide_me' style='text-align:center; margin-top: 20px; width:400px;'> 
+</form>
+<div id='adv_quick_search_sidebar' class='sidebar_chunk hide_me'> 
     <div id='adv_quick_search'>
         <div class='adv_quick_search color_4'><b>[% l("Quick Search") %]</b></div>
         <div style='margin-top: 8px;'>
index 2ff73d5..69fddc1 100644 (file)
@@ -11,6 +11,7 @@
             href="[% ctx.media_prefix %]/css/skin/default/opac/style.css" />
         <title>[% l('Catalog - King County Library - [_1]', ctx.page_title) %]</title>
         <link rel="unapi-server" type="application/xml" title="unAPI" href="/opac/extras/unapi" />
+        <script type="text/javascript" src="[% ctx.media_prefix %]/js/opac/simple.js"></script>
         [% BLOCK html_head; END; # provide a default that can be overridden -%]
         [%- PROCESS html_head -%]
     </head>
index d7c755f..a93a4ad 100644 (file)
@@ -1,10 +1,10 @@
-[%  name = name || "item_type";
+[%  name = name || "format";
     id = id || "format_selector" %]
 <select id='[% id %]' name='[% name %]'[%
     multiple ? ' multiple="multiple"' : '';
     size ? (' size="' _ size _ '"') : ''; %]>
     <option value=''>[% l("All Formats") %]</option>
-[% FOR o IN item_types %]
+[% FOR o IN formats %]
     <option value='[% o.code %]'[% value == o.code ? ' selected="selected"' : ''%]>[% o.name %]</option>
 [%- END %]
 <!--
index c4a9ce1..866ce2d 100644 (file)
@@ -4,7 +4,7 @@
     USE EGI18N;
     SET DATE_FORMAT = l('%m/%d/%Y');
 
-    item_types = [  # XXX KCLS-specific
+    formats = [  # XXX KCLS-specific
         {'code' => 'a', 'name' => 'Book', 'image' => 'media_book.jpg'},
         {'code' => 'i', 'name' => 'Book on cassette', 'image' => 'media_bookoncasset.jpg'},
         {'code' => 'n', 'name' => 'Book on CD', 'image' => 'media_bookoncd.jpg'},
         {'code' => 's', 'name' => 'Slide set', 'image' => 'media_slide.jpg'}
     ];
 
-    icon_by_mattype = {};
-    FOR o IN item_types;
+    icon_by_format = {};
+    FOR o IN formats;
         code = o.code;
-        icon_by_mattype.$code = o.image;
+        icon_by_format.$code = o.image;
     END;
 
 -%]
index 58d4f63..b08dabe 100644 (file)
@@ -1,13 +1,8 @@
 [%  PROCESS "default/opac/parts/marc_misc.tt2";
 
-    q = query | url;
-    np_link = '?query=' _ q;
-    IF loc;
-        np_link = np_link _ "&loc=" _ loc;
-    END;
-    IF depth or depth == 0;
-        np_link = np_link _ "&depth=" _ depth;
-    END;
+    q = query_string | url;
+    np_link = '?' _ q;
+
     ctx.result_start = 1 + ctx.page_size * page;
     ctx.result_stop = 1 + ctx.page_size * (page + 1);
     IF ctx.result_stop > ctx.hit_count; ctx.result_stop = ctx.hit_count; END;
index aeceb34..e4d157d 100644 (file)
@@ -17,7 +17,7 @@
             <td>
                 <div id="search_box_wrapper">
                     <!-- Note: when common browsers support HTML5 placeholder text, we can remove the JS -->
-                    <input type="text" id="search_box" name="query" value="[% query || l("Search Keyword") | html %]"
+                    <input type="text" id="search_box" name="query" value="[% ctx.processed_search_query || l("Search Keyword") | html %]"
                         onfocus="if (this.value=='[% l("Search Keyword") %]'){this.value='';this.style.color='#000';}"
                         onblur="if (this.value==''){this.value='[% l("Search Keyword") %]';this.style.color='#999';}" />
                     <input name='page' type='hidden' value="0" />
@@ -35,7 +35,7 @@
         </tr>
         <tr>
             <td>
-                [% INCLUDE "default/opac/parts/format_selector.tt2" value=CGI.param("item_type") %]
+                [% INCLUDE "default/opac/parts/format_selector.tt2" value=CGI.param("format") %]
             </td>
             <td>
                 <span id='depth_selector_span'>
index f3e30ad..69389e9 100644 (file)
@@ -1,12 +1,12 @@
 [%  search_classes = [
-    {value => "", label => l("Keyword")},
+    {value => "keyword", label => l("Keyword")},
     {value => "title", label => l("Title")},
     {value => "author", label => l("Author")},
     {value => "subject", label => l("Subject")},
     {value => "series", label => l("Series")},
-    {value => "cn", label => l("Call Number")}  # XXX or should this be bibcn?
+    {value => "id|bibcn", label => l("Call Number")}
 ] %]
-<select id='search_type_selector' name="class">
+<select name="class">
     [%  search_class = CGI.param('class');
         FOR sc IN search_classes -%]
     <option value='[% sc.value %]'[%
index 3c99292..2bfa7ca 100644 (file)
@@ -5,8 +5,10 @@
     INCLUDE "default/opac/parts/topnav.tt2";
     ctx.page_title = l("Search Results");
 
+    query_string = CGI.query_string |
+        replace(';x=\d+','') | replace(';y=\d+','') | replace(';page=\d*', '') |
+        replace(';', '&');
     page = CGI.param('page') || 0;
-    query = CGI.param('query');
     loc = CGI.param('loc');
     page_count = POSIX.ceil(ctx.hit_count / ctx.page_size);
 %]