Attachment 22510 Details for Bug 6385 – Bug 6385 - All items should optionally be displayed by default in the OPAC

[patch] Bug 6385 - All items should optionally be displayed by default in the OPAC

Bug-6385---All-items-should-optionally-be-displaye.patch (text/plain), 12.53 KB, created by Olli-Antti Kivilahti on 2013-10-29 08:59:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Olli-Antti Kivilahti

Created: 2013-10-29 08:59:45 UTC

Size: 12.53 KB

patch

obsolete

>From cbc605e5d88b99d5b69075d770c2cbb86bd49249 Mon Sep 17 00:00:00 2001
>From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
>Date: Mon, 28 Oct 2013 15:59:00 +0200
>Subject: [PATCH] Bug 6385 - All items should optionally be displayed by
> default in the OPAC
>
>Added a system preference "OPACmaxItemsOnDetail" to set a threshold after which only results from the
>users branch are displayed, or Items from the past year if homebranch cannot be defined.
>
>This tightly couples with bug 11129 which provides the filtering mechanism to display smaller result sets.
>Also added lots of complexity to define the warning messages based on is the filter used to override the
>"OPACmaxItemsOnDetail".
>
>Added a new "limit"-filter to C4::Items::GetItemsInfo() which adds the LIMIT-clause to the big SQL.
>Added a unit test for the filter to the Items.t
>
>Fixed a use-case where a user has no items in his branch, and the limited items set is emtpy.
>Now using items from past year if none available in the users branch.
>---
> C4/Items.pm                                        |  6 ++
> installer/data/mysql/sysprefs.sql                  |  1 +
> .../prog/en/modules/admin/preferences/opac.pref    |  4 ++
> koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt | 17 +++--
> opac/opac-detail.pl                                | 77 ++++++++++++++++------
> t/db_dependent/Items.t                             |  6 +-
> 6 files changed, 86 insertions(+), 25 deletions(-)
>
>diff --git a/C4/Items.pm b/C4/Items.pm
>index 1178daf..dc6b44c 100644
>--- a/C4/Items.pm
>+++ b/C4/Items.pm
>@@ -1218,6 +1218,7 @@ $filter->{fromDate} = '01/01/2013'; #Filters only serial issues by the serialite
>                                           #The starting date in C4::Context->preference('dateformat') format
> $filter->{toDate} = '31/12/2014';   #Filters only serial issues by the serialitems.publisheddate
>                                           #The ending date in C4::Context->preference('dateformat') format
>+$filter->{limit} = 15;              #Limits the returning set to this many items
>                                           
> Filters are expected to be validated! If a filter is not defined, that filter is not present in the $filter-HASH
> 
>@@ -1342,6 +1343,11 @@ sub GetItemsInfo {
>     }
>     
>     $query .= "ORDER BY home.branchname, items.enumchron, LPAD( items.copynumber, 8, '0' ), items.dateaccessioned DESC" ;
>+    if (exists $filter->{limit}) {
>+        push @$queryParams, $filter->{limit};
>+        $query .= " LIMIT ?";
>+    }
>+    
>     my $sth = $dbh->prepare($query);
>     $sth->execute(@$queryParams);
>     my $i = 0;
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index 165ed01..4db2935 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -239,6 +239,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('OpacMaintenance','0','','If ON, enables maintenance warning in OPAC','YesNo'),
> ('OpacMainUserBlock','Welcome to Koha...\r\n<hr>','70|10','A user-defined block of HTML  in the main content area of the opac main page','Textarea'),
> ('OpacMainUserBlockMobile','',NULL,'Show the following HTML in its own column on the main page of the OPAC (mobile version):','free'),
>+('OPACmaxItemsOnDetail', '50', NULL, 'On the Bibliographic record details page, show only items from the users branch and prompt the user to filter the items if the record has more then this many items.', 'Integer'),
> ('OPACMobileUserCSS','',NULL,'Include the following CSS for the mobile view on all pages in the OPAC:','free'),
> ('OPACMySummaryHTML','','70|10','Enter the HTML that will appear in a column on the \'my summary\' and \'my reading history\' tabs when a user is logged in to the OPAC. Enter {BIBLIONUMBER}, {TITLE}, {AUTHOR}, or {ISBN} in place of their respective variables in the HTML. Leave blank to disable.','Textarea'),
> ('OpacNav','Important links here.','70|10','Use HTML tags to add navigational links to the left-hand navigational bar in OPAC','Textarea'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>index a75c7ef..28d25c6 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>@@ -188,6 +188,10 @@ OPAC:
>               type: textarea
>               class: code
>         -
>+            - On the Bibliographic record details page, show only this many items from the users branch and prompt the user to filter the items if the record has more than this many items.
>+            - pref: OPACmaxItemsOnDetail
>+              class: integer
>+        -
>             - pref: OpacShowLibrariesPulldownMobile
>               choices:
>                   yes: Show
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
>index cb76413..4c7b36a 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
>@@ -1054,7 +1054,7 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
> 
> <div id="holdings">
>     
>-    [% IF ( lotsofitems ) %]
>+    [% IF ( showFilter ) %]
>         [%# Display the items filtering form used to filter the shown items. See the end of this file! %]
>         [% INCLUDE filter_form tab="holdings" %]
>     [% END %]
>@@ -1062,10 +1062,17 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
> [% IF ( itemloop.size ) %]    
>     
>     [% IF ( lotsofitems ) %]
>-	<p>This record has many physical items. <a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblionumber %]&amp;viewallitems=1#holdings">Click here to view them all.</a> Or use the filter above to limit your selection</p>
>-    [% ELSE %]
>-        [% INCLUDE items_table items=itemloop tab="holdings" %]
>+        <p>
>+            [% IF noBranchItems %]
>+                Limiting items to the most recent received last year.</br>
>+            [% ELSE %]
>+                Showing only the most recent items from your branch.</br>
>+            [% END %]
>+            This record has many physical items. Use the filter above to limit your selection.
>+        </p>
>     [% END %]
>+    [% INCLUDE items_table items=itemloop tab="holdings" %]
>+
>     [% IF holds_count.defined || priority %]
>     <div id="bib_holds">
>         [% IF holds_count.defined %]
>@@ -1638,7 +1645,7 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
>     <div id="filterIssuesParentContainer">
>     <a id="filterIssuesButton" >Limit issues</a>
>     [% IF filter %]
>-        <form id="issuesFilter" method="get" action="/cgi-bin/koha/opac-detail.pl">
>+        <form id="issuesFilterClearer" method="get" action="/cgi-bin/koha/opac-detail.pl">
>             <input type="hidden" name="biblionumber" id="biblionumber" value="[% biblionumber %]"/>
>             <input type="submit" name="clearFilter" value="Clear filter" class="submit"/>
>         </form>
>diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
>index 343bf2e..6656543 100755
>--- a/opac/opac-detail.pl
>+++ b/opac/opac-detail.pl
>@@ -77,14 +77,14 @@ $biblionumber = int($biblionumber);
> ##>> Handling the Serial issue filter parameters from the user
> ##
> # We can filter issues based on these five values.
>-my $filterBranchLimiter = $query->param('filterBranchLimiter') ? $query->param('filterBranchLimiter') : '_ShowAll';
>+my $filterBranchLimiter = $query->param('filterBranchLimiter') ? $query->param('filterBranchLimiter') : undef;
> my $filterVolume = $query->param('filterVolume') ? $query->param('filterVolume') : undef;
> my $filterNumber = $query->param('filterNumber') ? $query->param('filterNumber') : undef;
> my $filterIssue = $query->param('filterIssue') ? $query->param('filterIssue') : undef;
> my $filterFromDate = $query->param('filterFrom') ? $query->param('filterFrom') : undef;
> my $filterToDate = $query->param('filterTo') ? $query->param('filterTo') : undef;
> 
>-my $filter; #a HASH! Collect the filters here, so they can be more conveniently moved around.
>+my $filter = {}; #a HASH! Collect the filters here, so they can be more conveniently moved around.
> 
> #We filter by the branch only if a valid branch is given.
> if (defined $filterBranchLimiter && $filterBranchLimiter ne '_ShowAll') {
>@@ -141,6 +141,57 @@ if (defined $filterToDate && length $filterToDate > 0) {
>     }
> }
> 
>+$template->param( filter => $filter );
>+
>+##
>+##<< Serial issues filter parameters handled! ##
>+##
>+
>+# If there are a lot of items, and the user has not decided
>+# to view them all yet, we first warn him
>+my $itemsCount = GetItemsCount($biblionumber);
>+my $lotsofitems = 0;
>+if ($itemsCount > C4::Context->preference('OPACmaxItemsOnDetail') &&
>+    !($query->param('viewallitems')) ) {
>+    
>+    $lotsofitems = 1;
>+    $template->param('lotsofitems' => $lotsofitems);
>+}
>+
>+my @all_items;
>+if ($lotsofitems) {
>+    #Spare the poor old DB from this possibly excruciatinly huge read operation.
>+    if (! scalar(%$filter)) { #Is the $filter empty?
>+        
>+        #Limit the amount of results to the system preference
>+        $filter->{limit} = C4::Context->preference('OPACmaxItemsOnDetail');
>+        
>+        #Limit display of Items based on the users branch
>+        if (C4::Context->userenv){
>+            $filter->{branch} = C4::Context->userenv->{branch};
>+            $filterBranchLimiter = $filter->{branch}; #Making the branches loop react to this!
>+            
>+            @all_items = GetItemsInfo($biblionumber, $filter);
>+        }
>+        #Or if branch cannot be defined or has no items, then display Items only from the past year
>+        if( ! (scalar(@all_items)) ) {
>+            delete $filter->{branch}; #Remove the branch filter as it gave no results
>+            undef $filterBranchLimiter;
>+            
>+            my $now = DateTime->now();
>+            $now->set_year(  ($now->year() - 1)  );
>+            $filter->{fromDate} = Koha::DateUtils::output_pref({ dt => $now, dateformat => 'iso'});
>+            $template->param('noBranchItems' => 1);
>+            @all_items = GetItemsInfo($biblionumber, $filter);
>+        }
>+        
>+    }
>+}
>+#If we haven't found our items yet, then pull them using whatever we've got.
>+if( ! (scalar(@all_items)) ) {
>+    @all_items = GetItemsInfo($biblionumber, $filter);
>+}
>+
> 
> ##Prepare the custom branches loop containing the _ShowAll entry to show issues from all libraries.
> my $branchloop;
>@@ -153,19 +204,14 @@ else {
>     unshift @$branchloop, { branchcode => '_ShowAll', branchname => 'Show from any library', selected => '0', value => '_ShowAll'};
> }
> $template->param( branchloop => $branchloop );
>-$template->param( filter => $filter ) if defined $filter;
> 
>-##
>-##<< Serial issues filter parameters handled! ##
>-##
>-
>-
>-my @all_items = GetItemsInfo($biblionumber, $filter);
> 
> # Now that the filter is no longer needed, we can reuse it to keep the filter modifications in the UI,
>-#  by reverting the dates to the same format as in the UI layer.
>-$filter->{fromDate} = $filterFromDate;
>-$filter->{toDate} = $filterToDate;
>+#  by reverting the dates and the branch to the same format as in the UI layer.
>+$filter->{fromDate} = $filterFromDate if $filterFromDate;
>+$filter->{toDate} = $filterToDate if $filterToDate;
>+$filter->{branch} = $filterBranchLimiter if $filterBranchLimiter;
>+$template->param('showFilter' => 1) if (scalar(%$filter));
> 
> my @hiddenitems;
> if (scalar @all_items >= 1) {
>@@ -731,13 +777,6 @@ if (scalar(@itemloop) == 0 || scalar(@otheritemloop) == 0) {
>     }
> }
> 
>-# If there is a lot of items, and the user has not decided
>-# to view them all yet, we first warn him
>-# TODO: The limit of 50 could be a syspref
>-my $viewallitems = $query->param('viewallitems');
>-if (scalar(@itemloop) >= 50 && !$viewallitems) {
>-    $template->param('lotsofitems' => 1);
>-}
> 
> ## get notes and subjects from MARC record
> my $dbh              = C4::Context->dbh;
>diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t
>index 1d28525..181be59 100755
>--- a/t/db_dependent/Items.t
>+++ b/t/db_dependent/Items.t
>@@ -145,7 +145,7 @@ subtest 'GetHiddenItemnumbers tests' => sub {
> 
> subtest 'Filter items tests' => sub {
>     
>-    plan tests => 2;
>+    plan tests => 3;
> 
>     # Start transaction
>     $dbh->{AutoCommit} = 0;
>@@ -173,6 +173,10 @@ subtest 'Filter items tests' => sub {
>     # Testing the dates filter
>     my @shouldBeEmpty = C4::Items::GetItemsInfo($biblionumber, {toDate => '01/01/1933'});
>     is( scalar(@shouldBeEmpty), 0, "Filtering by date");
>+    
>+    # Testing the limit filter
>+    @shouldBeEmpty = C4::Items::GetItemsInfo($biblionumber, {limit => 0});
>+    is( scalar(@shouldBeEmpty), 0, "Filtering by limit");
> };
> 
> # Helper method to set up a Biblio.
>-- 
>1.8.1.2

Actions: View | Diff | Splinter Review

Attachments on bug 6385: 22469 | 22471 | 22510 | 22514 | 22570 | 23173 | 23207 | 23208