From c27bb933636006dd2414474bbcf513fe8f377464 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 Formatted using perltidy and Perl::Critic Sponsored by the Joensuu Regional Library --- C4/Items.pm | 131 +++++++- installer/data/mysql/sysprefs.sql | 2 + .../prog/en/modules/admin/preferences/serials.pref | 14 + koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css | 14 + 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 | 372 ++++++++++++++------- 8 files changed, 694 insertions(+), 162 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 073d6f7..52b8517 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; @@ -1186,6 +1187,7 @@ sub GetItemsByBiblioitemnumber { =head2 GetItemsInfo @results = GetItemsInfo($biblionumber); + @results = GetItemsInfo($biblionumber, $filter); Returns information about items with the given biblionumber. @@ -1223,12 +1225,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.*, @@ -1248,7 +1308,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 @@ -1256,19 +1322,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'} ); @@ -1279,17 +1381,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 6438483..864a892 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -110,6 +110,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'), @@ -192,6 +193,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..49d37ba 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,17 @@ Serials: - - List of fields which must not be rewritten when a subscription is duplicated (Separated by pipe |) - pref: SubscriptionDuplicateDroppedInput + - + - pref: NumberingFormulaParsingRegexp + class: long + - "When dealing with potentially thousands of Serial items, there is a need to limit the display of a large number of serial issues in OPAC. This preference controls how the enumeration of a serial issue should be parsed." + - "It must be a MySQL compliant regular expression, with values volume, number, issue representing those enumerants." + - 'Example: ^[^0-9]*volume[^0-9]*number[^0-9]*$ would work with enumerations like "Vol 2013, No 11" or "2013 :11", but not with enumerations like "No 11, Vol 2013" or "1.a Vol 2013, No 11".' + - It is recommended to not mix different enumeration schemes inside a Koha installation. It is not possible to have one branch enumerate like "issue, volume, number" and other as "volume, number" or even "volume, number, issue". + - Use the recommeded value, or if you are using the FilterSerialsByIssue -system preference, use this ^[^0-9]*volume[^0-9]*number[^0-9]*issue[^0-9]*$ + - + - 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. 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..d58453a --- /dev/null +++ b/koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css @@ -0,0 +1,14 @@ +/* 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 */ + + padding : .3em .7em; + + background-image: url("../../images/desc.gif"); /* FF3.6+ */ + background-position: center right; + background-repeat: no-repeat; + + padding-right: 20px; + margin-right: 6px; +} \ 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 05c4010..d1b8e94 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 ) %]