From e0598beab61e494251141b7ac6c6bcba8afceb66 Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Thu, 1 Oct 2015 15:13:20 +0200
Subject: [PATCH] Bug 11139: Add support for multiple values in
 AdvancedSearchTypes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

AdvancedSearchTypes syspref can be multivalued, opac-topissues.pl
have to support that.

AdvancedSearchTypes can have one or more of the following values:
'itemtypes', 'ccode', 'loc'.  But this patch adds support only for
itemtypes and ccode.  AdvancedSearchTypes can still contain 'loc' but it
will change nothing to the behaviour of opac-topissues.pl.

This patch also removes duplicate code in template (the "Refine your
search" form was written in two different places)
This introduces a small behaviour change: when there is no results,
instead of having the form in the center of the page, it remains in the
left sidebar and we have a "No results" message in the main area.

Depends on bug 14788

Tested on top of 14788. Works as expected.
Signed-off-by: Marc VĂ©ron <veron@veron.ch>
---
 Koha/Template/Plugin/ItemTypes.pm                  |    6 ++
 .../bootstrap/en/includes/opac-topissues.inc       |   49 ++++++++----
 .../bootstrap/en/modules/opac-topissues.tt         |   81 ++------------------
 opac/opac-topissues.pl                             |   61 ++++-----------
 4 files changed, 65 insertions(+), 132 deletions(-)

diff --git a/Koha/Template/Plugin/ItemTypes.pm b/Koha/Template/Plugin/ItemTypes.pm
index 800e151..91e8c4e 100644
--- a/Koha/Template/Plugin/ItemTypes.pm
+++ b/Koha/Template/Plugin/ItemTypes.pm
@@ -35,4 +35,10 @@ sub GetDescription {
 
 }
 
+sub Get {
+    my @itemtypes = @{ GetItemTypes(style => 'array') };
+    @itemtypes = sort { $a->{description} cmp $b->{description} } @itemtypes;
+    return \@itemtypes;
+}
+
 1;
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc
index 4d6ca89..0a211d5 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc
@@ -1,3 +1,5 @@
+[% USE Koha %]
+
 <div id="search-facets">
     <form method="get" action="/cgi-bin/koha/opac-topissues.pl">
     <h4><a href="#" class="menu-collapse-toggle">Refine your search</a></h4>
@@ -27,23 +29,42 @@
                                 [% END %]
                             </select></li>
 
-                <li><label for="itemtype">Limit to: </label>
-                            <select name="itemtype" id="itemtype">
-                                [% IF ( ccodesearch ) %]
-                                    <option value="">All collections</option>
+                [% AdvancedSearchTypes = Koha.Preference('AdvancedSearchTypes').split('\|') %]
+                [% IF AdvancedSearchTypes.grep('^itemtypes$').size %]
+                    <li>
+                        <label for="itemtype">Limit to: </label>
+                        <select name="itemtype" id="itemtype">
+                            <option value="">All item types</option>
+                            [% FOREACH itemtype IN ItemTypes.Get() %]
+                                [% IF itemtype.itemtype == selected_itemtype %]
+                                    <option value="[% itemtype.itemtype %]" selected="selected">
                                 [% ELSE %]
-                                    <option value="">All item types</option>
+                                    <option value="[% itemtype.itemtype %]">
                                 [% END %]
-                                [% FOREACH itemtypeloo IN itemtypeloop %]
-                                    [% IF ( itemtypeloo.selected ) %]
-                                        <option value="[% itemtypeloo.value %]" selected="selected">
-                                    [% ELSE %]
-                                        <option value="[% itemtypeloo.value %]">
-                                    [% END %]
-                                        [% itemtypeloo.description %]
-                                    </option>
+                                    [% itemtype.description %]
+                                </option>
+                            [% END %]
+                        </select>
+                    </li>
+                [% END %]
+
+                [% IF AdvancedSearchTypes.grep('^ccode$').size %]
+                    <li>
+                        <label for="ccode">Limit to:</label>
+                        <select name="ccode" id="ccode">
+                            <option value="">All collections</option>
+                            [% FOREACH ccode IN AuthorisedValues.Get('CCODE', selected_ccode, 1) %]
+                                [% IF ccode.selected %]
+                                    <option value="[% ccode.authorised_value %]" selected="selected">
+                                [% ELSE %]
+                                    <option value="[% ccode.authorised_value %]">
                                 [% END %]
-                            </select></li>
+                                    [% ccode.lib %]
+                                </option>
+                            [% END %]
+                        </select>
+                    </li>
+                [% END %]
 
                 <li><label for="timeLimit">Acquired in the last:</label>
                             <select name="timeLimit" id="timeLimit">
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
index acd67a1..672e241 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
@@ -22,20 +22,12 @@
         <div class="container-fluid">
             <div class="row-fluid">
                 <div class="span2">
-                    [% IF ( results ) %]
-                        <div id="usertopissues">
-                            [% INCLUDE 'opac-topissues.inc' %]
-                            [% IF ( OpacNav || OpacNavBottom ) %]
-                                [% INCLUDE 'navigation.inc' %]
-                            [% END %]
-                        </div>
-                    [% ELSE %]
+                    <div id="usertopissues">
+                        [% INCLUDE 'opac-topissues.inc' %]
                         [% IF ( OpacNav || OpacNavBottom ) %]
-                            <div id="leftmenus">
-                                [% INCLUDE 'navigation.inc' %]
-                            </div>
+                            [% INCLUDE 'navigation.inc' %]
                         [% END %]
-                    [% END %]
+                    </div>
                 </div>
                 <div class="span10">
                     <div id="topissues" class="maincontent">
@@ -44,8 +36,8 @@
                             <table id="topissuest" class="table table-bordered table-striped">
                                 <caption>
                                     The [% limit %] most checked-out
-                                    [% IF ( itemtype ) %]
-                                        [% itemtype %]
+                                    [% IF selected_itemtype %]
+                                        [% ItemTypes.GetDescription(selected_itemtype) %]
                                     [% END %]
                                     [% IF ( branch ) %]
                                     at
@@ -93,65 +85,8 @@
                                 </tbody>
                             </table>
                         [% ELSE %]
-                            <form method="post" action="/cgi-bin/koha/opac-topissues.pl">
-                                <fieldset class="rows">
-                                <legend>See the most popular titles</legend>
-                                    <ol>
-                                        <li><label for="limit">Show the top </label> <select name="limit" id="limit">
-                                                    [% IF ( limit == 10 ) %]<option value ="10" selected="selected">10 titles</option>[% ELSE %]<option value="10">10 titles</option>[% END %]
-                                                    [% IF ( limit == 15 ) %]<option value ="15" selected="selected">15 titles</option>[% ELSE %]<option value="15">15 titles</option>[% END %]
-                                                    [% IF ( limit == 20 ) %]<option value ="20" selected="selected">20 titles</option>[% ELSE %]<option value="20">20 titles</option>[% END %]
-                                                    [% IF ( limit == 30 ) %]<option value ="30" selected="selected">30 titles</option>[% ELSE %]<option value="30">30 titles</option>[% END %]
-                                                    [% IF ( limit == 40 ) %]<option value ="40" selected="selected">40 titles</option>[% ELSE %]<option value="40">40 titles</option>[% END %]
-                                                    [% IF ( limit == 50 ) %]<option value ="50" selected="selected">50 titles</option>[% ELSE %]<option value="50">50 titles</option>[% END %]
-                                                    [% IF ( limit == 100 ) %]<option value ="100" selected="selected">100 titles</option>[% ELSE %]<option value="100">100 titles</option>[% END %]
-                                                </select>
-                                        </li>
-                                        <li>
-                                            <label for="branch">From: </label>
-                                            <select name="branch" id="branch">
-                                            <option value="">All libraries</option>
-                                            [% FOREACH branchloo IN branchloop %]
-                                                [% IF ( branchloo.selected ) %]
-                                                    <option value="[% branchloo.value %]" selected="selected">[% branchloo.branchname %]</option>
-                                                [% ELSE %]
-                                                    <option value="[% branchloo.value %]">[% branchloo.branchname %]</option>
-                                                [% END %]
-                                            [% END %]
-                                            </select>
-                                        </li>
-                                        <li>
-                                            <label for="itemtype">Limit to: </label>
-                                            <select name="itemtype" id="itemtype">
-                                                [% IF ( ccodesearch ) %]
-                                                    <option value="">All collections</option>
-                                                [% ELSE %]
-                                                    <option value="">All item types</option>
-                                                [% END %]
-                                                [% FOREACH itemtypeloo IN itemtypeloop %]
-                                                    [% IF ( itemtypeloo.selected ) %]<option value="[% itemtypeloo.value %]" selected="selected">[% ELSE %]<option value="[% itemtypeloo.value %]">[% END %]
-                                                        [% itemtypeloo.description %]
-                                                    </option>
-                                                [% END %]
-                                            </select>
-                                        </li>
-                                        <li>
-                                            <label for="timeLimit">of the last:</label>
-                                            <select name="timeLimit" id="timeLimit">
-                                                [% IF ( timeLimit == 3 ) %]<option value="3" selected="selected">3 months</option>[% ELSE %]<option value="3">3 months</option>[% END %]
-                                                 [% IF ( timeLimit == 6 ) %]<option value="6" selected="selected">6 months</option>[% ELSE %]<option value="6">6 months</option>[% END %]
-                                                 [% IF ( timeLimit == 12 ) %]<option value="12" selected="selected">12 months</option>[% ELSE %]<option value="12">12 months</option>[% END %]
-                                                 [% IF ( timeLimit == 999 ) %]<option value="999" selected="selected">No limit</option>[% ELSE %]<option value="999">No limit</option>[% END %]
-                                            </select>
-                                        </li>
-                                    </ol>
-                                    <input type="hidden" name="do_it" value="1" />
-                                </fieldset>
-                                <fieldset class="action">
-                                    <input type="submit" class="btn" value="Submit" />
-                                </fieldset>
-                            </form>
-                       [% END # / IF results %]
+                            No results, try to change filters.
+                        [% END # / IF results %]
                     </div> <!-- / #topissues -->
                 </div> <!-- / .span10 -->
             </div> <!-- / .row-fluid -->
diff --git a/opac/opac-topissues.pl b/opac/opac-topissues.pl
index 772ec03..8e4e6db 100755
--- a/opac/opac-topissues.pl
+++ b/opac/opac-topissues.pl
@@ -73,7 +73,7 @@ if (!$do_it && C4::Context->userenv && C4::Context->userenv->{'branch'} ) {
 my $itemtype = $input->param('itemtype') || '';
 my $timeLimit = $input->param('timeLimit') || 3;
 my $advanced_search_types = C4::Context->preference('AdvancedSearchTypes');
-
+my @advanced_search_types = split /\|/, $advanced_search_types;
 
 my $params = {
     count => $limit,
@@ -81,53 +81,24 @@ my $params = {
     newness => $timeLimit < 999 ? $timeLimit * 30 : undef,
 };
 
-if($advanced_search_types eq 'ccode'){
-    $params->{ccode} = $itemtype;
-    $template->param(ccodesearch => 1);
-} else {
-    $params->{itemtype} = $itemtype;
-    $template->param(itemtypesearch => 1);
+@advanced_search_types = grep /^(ccode|itemtypes)$/, @advanced_search_types;
+foreach my $type (@advanced_search_types) {
+    if ($type eq 'itemtypes') {
+        $type = 'itemtype';
+    }
+    $params->{$type} = $input->param($type);
+    $template->param('selected_' . $type => $input->param($type));
 }
 
 my @results = GetTopIssues($params);
 
-$template->param(do_it => 1,
-                limit => $limit,
-                branch => $branches->{$branch}->{branchname},
-                itemtype => $itemtypes->{$itemtype}->{description},
-                timeLimit => $timeLimit,
-                results => \@results,
-                );
-
-$template->param( branchloop => GetBranchesLoop($branch));
-
-# the index parameter is different for item-level itemtypes
-my $itype_or_itemtype = (C4::Context->preference("item-level_itypes"))?'itype':'itemtype';
-$itemtypes = GetItemTypes;
-my @itemtypesloop;
-if (!$advanced_search_types or $advanced_search_types eq 'itemtypes') {
-        foreach my $thisitemtype ( sort {$itemtypes->{$a}->{'description'} cmp $itemtypes->{$b}->{'description'} } keys %$itemtypes ) {
-        my %row =( value => $thisitemtype,
-                   description => $itemtypes->{$thisitemtype}->{'description'},
-                   selected    => $thisitemtype eq $itemtype,
-            );
-        push @itemtypesloop, \%row;
-        }
-} else {
-    my $advsearchtypes = GetAuthorisedValues($advanced_search_types, '', 'opac');
-        for my $thisitemtype (@$advsearchtypes) {
-                my $selected;
-            $selected = 1 if $thisitemtype->{authorised_value} eq $itemtype;
-                my %row =( value => $thisitemtype->{authorised_value},
-                selected    => $thisitemtype eq $itemtype,
-                description => $thisitemtype->{'lib'},
-            );
-                push @itemtypesloop, \%row;
-        }
-}
-
 $template->param(
-                 itemtypeloop =>\@itemtypesloop,
-                );
-output_html_with_http_headers $input, $cookie, $template->output;
+    limit => $limit,
+    branch => $branches->{$branch}->{branchname},
+    timeLimit => $timeLimit,
+    results => \@results,
+);
 
+$template->param(branchloop => GetBranchesLoop($branch));
+
+output_html_with_http_headers $input, $cookie, $template->output;
-- 
1.7.10.4