From a556c40ff9e207f4c96c3da7fc9a7f2aa2d72037 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti 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/css/opac-detail.css | 9 +++ koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt | 19 ++++-- opac/opac-detail.pl | 77 ++++++++++++++++------ t/db_dependent/Items.t | 6 +- 7 files changed, 96 insertions(+), 26 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 34fe134..bdae692 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
','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..e622253 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. If the user has no branch or the branch has no items, displaying items from the past year. + - pref: OPACmaxItemsOnDetail + class: integer + - - pref: OpacShowLibrariesPulldownMobile choices: yes: Show diff --git a/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css b/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css index ada6ab1..e4bc3fa 100644 --- a/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css +++ b/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css @@ -11,4 +11,13 @@ padding-right: 20px; margin-right: 6px; +} + +#clearFilter { + border: none; + background: none; + color: #006699; +} +#clearFilter:hover { + color : #990033; } \ No newline at end of file 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 59d5688..3e07971 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 () {
- [% 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 ) %] -

This record has many physical items. Click here to view them all. Or use the filter above to limit your selection

- [% ELSE %] - [% INCLUDE items_table items=itemloop tab="holdings" %] +

+ [% IF noBranchItems %] + Limiting items to the most recent received last year.
+ [% ELSE %] + Showing only the most recent items from your branch.
+ [% END %] + This record has many physical items. Use the filter above to limit your selection. +

[% END %] + [% INCLUDE items_table items=itemloop tab="holdings" %] + [% IF holds_count.defined || priority %]
[% IF holds_count.defined %] @@ -1638,9 +1645,9 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
Limit issues [% IF filter %] -
+ - +
[% END %]
diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 74fdf84..7c83222 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) { @@ -732,13 +778,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 59b8416..30e639b 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