From 8149275912260bb01a0f7f04e8bb12ed8a43c9f2 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Mon, 28 Oct 2013 13:18:35 +0200 Subject: [PATCH] Bug 11129 - Filtering Items based on type in opac-detail.pl Currently all Items are always shown on the opac-detail.pl. This is difficult when the amount of Items grows substantially large. Implemented a filter to limit the Items shown (and SELECTed from the DB) based on some typical filters, locationbranch, volume, number, issue, fromDate, toDate. Also streamlined how Serials-data is pulled from the DB to prevent substantial extra work in C4::Items::GetItemsInfo(). C4::Items::GetItemsInfo() extended to support various filters. All modifications: Item filter shown when there are over 50 items (lotsofitems-flag). Filter fields can be changed based on the Biblio type (isSerial-flag). -Volume + Issue + Number available only for serials. Can syspref if Issue-field is used in serial records. Can syspref a regexp to parse the Volume + Number + Issue from the enumeration or chronology field. FromDate and ToDate filter the serial.publisheddate when dealing with serials otherwise target the items.timestamp -column. C4::Items::GetItemsInfo() simplified to include the serial data in the BIG SQL. This makes filtering by publisheddate much more faster. User input validated using HTML5 Business layer validations as well. Unit tests: Serials enumeration and chronology filtering Items date and branch filtering Sponsored by the Joensuu Regional Library --- C4/Items.pm | 131 ++++++++++++++-- installer/data/mysql/sysprefs.sql | 2 + .../prog/en/modules/admin/preferences/serials.pref | 12 ++ koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css | 41 +++++ koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt | 174 ++++++++++++++++++--- opac/opac-detail.pl | 114 +++++++++++++- t/db_dependent/Items.t | 35 ++++- t/db_dependent/Serials.t | 88 ++++++++++- 8 files changed, 553 insertions(+), 44 deletions(-) create mode 100644 koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css diff --git a/C4/Items.pm b/C4/Items.pm index 67d8736..1178daf 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -25,6 +25,7 @@ use Carp; use C4::Context; use C4::Koha; use C4::Biblio; +use C4::SQLHelper; use C4::Dates qw/format_date format_date_in_iso/; use MARC::Record; use C4::ClassSource; @@ -1169,6 +1170,7 @@ sub GetItemsByBiblioitemnumber { =head2 GetItemsInfo @results = GetItemsInfo($biblionumber); + @results = GetItemsInfo($biblionumber, $filter); Returns information about items with the given biblionumber. @@ -1206,12 +1208,70 @@ If this is set, it is set to C. =back +=item C<$filter> + +A reference-to-hash. Valid filters are: +$filter->{branch} = 'CPL'; #Branchcode of the library whose items should only be displayed. +$filter->{volume} = '2013'; #The volume of the item from items.enumchron aka. "Numbering formula". +$filter->{number} = '11'; #The number or issue of the item from items.enumchron aka. "Numbering formula". +$filter->{fromDate} = '01/01/2013'; #Filters only serial issues by the serialitems.publisheddate + #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 + +Filters are expected to be validated! If a filter is not defined, that filter is not present in the $filter-HASH + =cut sub GetItemsInfo { - my ( $biblionumber ) = @_; + my ( $biblionumber, $filter ) = @_; my $dbh = C4::Context->dbh; + + #Prepare the filter + my $filterEnumchron = 0; + my $enumchronSQLRegexp; + if (defined $filter && ref $filter eq 'HASH') { + + #Items enumchron can be filtered by volume or number or both. + #Because the format of enumchron + #For performance reasons regexp's need to be as simple as possible. + ## It is entirely possible to search with just volume or just number or just issue. + ## We don't know which filters are used so it is safer and more efficient to just + ## prepare the enumeration parsing SQL every time. + $enumchronSQLRegexp = C4::Context->preference('NumberingFormulaParsingRegexp'); + + if (exists $filter->{volume}) { + $filterEnumchron = 1; + $enumchronSQLRegexp =~ s/volume/$filter->{volume}/; + } + else { + $enumchronSQLRegexp =~ s/volume/[0-9]*/; + } + if (exists $filter->{number}) { + $filterEnumchron = 1; + $enumchronSQLRegexp =~ s/number/$filter->{number}/; + } + else { + $enumchronSQLRegexp =~ s/number/[0-9]*/; + } + if (exists $filter->{issue}) { + $filterEnumchron = 1; + $enumchronSQLRegexp =~ s/issue/$filter->{issue}/; + } + else { + $enumchronSQLRegexp =~ s/issue/[0-9]*/; + } + } + #If we know that this item is a serial, we can better optimize our big SQL. + # This is especially useful when we want to filter based on the publication date. + # SELECTing a huge blob of serials just to remove unnecessary ones will be really sloooow. + my $search = C4::SQLHelper::SearchInTable("biblio",{biblionumber => $biblionumber}, undef, undef, ['serial'], undef, "exact"); + my $serial = $search->[0]->{serial}; + # note biblioitems.* must be avoided to prevent large marc and marcxml fields from killing performance. + #Because it is uncertain how many parameters the SQL query needs, we need to build the parameters dynamically + # This is because we cannot predict what filters our users use. + my $queryParams = [$biblionumber]; my $query = " SELECT items.*, biblio.*, @@ -1231,7 +1291,13 @@ sub GetItemsInfo { itemtypes.notforloan as notforloan_per_itemtype, holding.branchurl, holding.branchname, - holding.opac_info as branch_opac_info + holding.opac_info as branch_opac_info "; + if ($serial) { + $query .= ", + serial.serialseq, + serial.publisheddate "; + } + $query .= " FROM items LEFT JOIN branches AS holding ON items.holdingbranch = holding.branchcode LEFT JOIN branches AS home ON items.homebranch=home.branchcode @@ -1239,19 +1305,55 @@ sub GetItemsInfo { LEFT JOIN biblioitems ON biblioitems.biblioitemnumber = items.biblioitemnumber LEFT JOIN itemtypes ON itemtypes.itemtype = " . (C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype'); - $query .= " WHERE items.biblionumber = ? ORDER BY home.branchname, items.enumchron, LPAD( items.copynumber, 8, '0' ), items.dateaccessioned DESC" ; + + if ($serial) { + $query .= " + LEFT JOIN serialitems ON serialitems.itemnumber = items.itemnumber + LEFT JOIN serial ON serialitems.serialid = serial.serialid "; + } + + $query .= " WHERE items.biblionumber = ? "; + + if (exists $filter->{branch}) { + $query .= " AND items.holdingbranch = ?"; + push @$queryParams, $filter->{branch}; + } + if ($filterEnumchron) { + $query .= " AND items.enumchron REGEXP ?"; + push @$queryParams, $enumchronSQLRegexp; + } + if (exists $filter->{fromDate}) { + if ($serial) { + $query .= " AND serial.publisheddate >= ?"; + } + else { + $query .= " AND items.timestamp >= ?"; + } + push @$queryParams, $filter->{fromDate}; + } + if (exists $filter->{toDate}) { + if ($serial) { + $query .= " AND serial.publisheddate <= ?"; + } + else { + $query .= " AND items.timestamp <= ?"; + } + push @$queryParams, $filter->{toDate}; + } + + $query .= "ORDER BY home.branchname, items.enumchron, LPAD( items.copynumber, 8, '0' ), items.dateaccessioned DESC" ; my $sth = $dbh->prepare($query); - $sth->execute($biblionumber); + $sth->execute(@$queryParams); my $i = 0; my @results; - my $serial; my $isth = $dbh->prepare( "SELECT issues.*,borrowers.cardnumber,borrowers.surname,borrowers.firstname,borrowers.branchcode as bcode FROM issues LEFT JOIN borrowers ON issues.borrowernumber=borrowers.borrowernumber WHERE itemnumber = ?" ); - my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=? "); + + while ( my $data = $sth->fetchrow_hashref ) { my $datedue = ''; $isth->execute( $data->{'itemnumber'} ); @@ -1262,17 +1364,12 @@ sub GetItemsInfo { $data->{firstname} = $idata->{firstname}; $data->{lastreneweddate} = $idata->{lastreneweddate}; $datedue = $idata->{'date_due'}; - if (C4::Context->preference("IndependentBranches")){ - my $userenv = C4::Context->userenv; - if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) { - $data->{'NOTSAMEBRANCH'} = 1 if ($idata->{'bcode'} ne $userenv->{branch}); - } - } - } - if ( $data->{'serial'}) { - $ssth->execute($data->{'itemnumber'}) ; - ($data->{'serialseq'} , $data->{'publisheddate'}) = $ssth->fetchrow_array(); - $serial = 1; + if (C4::Context->preference("IndependentBranches")){ + my $userenv = C4::Context->userenv; + if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) { + $data->{'NOTSAMEBRANCH'} = 1 if ($idata->{'bcode'} ne $userenv->{branch}); + } + } } #get branch information..... my $bsth = $dbh->prepare( diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 71513ce..165ed01 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -108,6 +108,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('ExtendedPatronAttributes','0',NULL,'Use extended patron IDs and attributes','YesNo'), ('FacetLabelTruncationLength','20',NULL,'Specify the facet max length in OPAC','Integer'), ('FilterBeforeOverdueReport','0','','Do not run overdue report until filter selected','YesNo'), +('FilterSerialsByIssue','0',NULL,'Use issue-field when filtering serial issues in addition to the volume- and number-fields? This relates to NumberingFormulaParsingRegexp system preference.','YesNo'), ('FineNotifyAtCheckin','0',NULL,'If ON notify librarians of overdue fines on the items they are checking in.','YesNo'), ('finesCalendar','noFinesWhenClosed','ignoreCalendar|noFinesWhenClosed','Specify whether to use the Calendar in calculating duedates and fines','Choice'), ('FinesIncludeGracePeriod','1',NULL,'If enabled, fines calculations will include the grace period.','YesNo'), @@ -190,6 +191,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('NovelistSelectPassword',NULL,NULL,'Enable Novelist user Profile','free'), ('NovelistSelectProfile',NULL,NULL,'Novelist Select user Password','free'), ('NovelistSelectView','tab','tab|above|below|right','Where to display Novelist Select content','Choice'), +('NumberingFormulaParsingRegexp','','','Explanation','free') ('numReturnedItemsToShow','20',NULL,'Number of returned items to show on the check-in page','Integer'), ('numSearchResults','20',NULL,'Specify the maximum number of results to display on a page of results','Integer'), ('numSearchRSSResults','50',NULL,'Specify the maximum number of results to display on a RSS page of results','Integer'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref index db727b6..0d675d6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref @@ -50,3 +50,15 @@ Serials: - - List of fields which must not be rewritten when a subscription is duplicated (Separated by pipe |) - pref: SubscriptionDuplicateDroppedInput + - + - "When dealing with a large number of Serial items, there is a need to filter the a large number of serial issues in OPAC and staff client Numbering formula" + - pref: NumberingFormulaParsingRegexp + class: long + - for lists of browsable letters. This should be a space separated list of uppercase letters. + - + - pref: FilterSerialsByIssue + choices: + yes: "Use" + no: "Don't use" + - issue-field when filtering serial issues in addition to the volume- and number-fields. This relates to NumberingFormulaParsingRegexp system preference. + \ No newline at end of file diff --git a/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css b/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css new file mode 100644 index 0000000..13a6a05 --- /dev/null +++ b/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css @@ -0,0 +1,41 @@ +/* Lots of stuff copied from opac.css since it shouldn't be modified. The button elements should be generalized for maintainability reasons! */ + +#filterIssuesButton { + z-index: 1001; /* Make sure this element is always over the #filterIssuesFormContainer */ + background-repeat: no-repeat; + -webkit-border-radius: 5px; + -moz-border-radius: 5px; + border-radius: 5px; + text-decoration : none; + cursor : pointer; + font-weight : bold; + padding : .3em .7em; + + background : #151515; + background: url("../../images/desc.gif"),-moz-linear-gradient(top, #eeeeee 0%, #e0e0e0 50%, #d9d9d9 100%); /* FF3.6+ */ + background: url("../../images/desc.gif"),-webkit-gradient(linear, left top, left bottom, color-stop(0%,#eeeeee), color-stop(50%,#e0e0e0), color-stop(100%,#d9d9d9)); /* Chrome,Safari4+ */ + background: url("../../images/desc.gif"),-webkit-linear-gradient(top, #eeeeee 0%,#e0e0e0 50%,#d9d9d9 100%); /* Chrome10+,Safari5.1+ */ + background: url("../../images/desc.gif"),-o-linear-gradient(top, #eeeeee 0%,#e0e0e0 50%,#d9d9d9 100%); /* Opera 11.10+ */ + background: url("../../images/desc.gif"),-ms-linear-gradient(top, #eeeeee 0%,#e0e0e0 50%,#d9d9d9 100%); /* IE10+ */ + background: url("../../images/desc.gif"),linear-gradient(top, #eeeeee 0%,#e0e0e0 50%,#d9d9d9 100%); /* W3C */ + filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#eeeeee', endColorstr='#d9d9d9',GradientType=0 ); /* IE6-9 */ + background-position: center right; + background-repeat: no-repeat; + border: 1px solid #c3c3c3; + + padding-right: 20px; + margin-right: 6px; +} + + + +/* IE 6 & 7 don't do multiple backgrounds, so remove extra padding */ +* html #filterIssuesButton, +*+html #filterIssuesButton { + padding-right : .7em; +} + +/* IE 8 doesn't do multiple backgrounds, so remove extra padding */ +#filterIssuesButton { + padding-right: .7em\0/; +} \ 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 e5d9c3c..cb76413 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt @@ -19,6 +19,7 @@ [% INCLUDE 'doc-head-open.inc' %][% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog › Details for: [% title |html %][% FOREACH subtitl IN subtitle %], [% subtitl.subfield |html %][% END %] [% INCLUDE 'doc-head-close.inc' %] +[% INCLUDE 'calendar.inc' %] [% INCLUDE 'datatables.inc' %] [% IF ( SocialNetworks ) %]