Attachment 23205 Details for Bug 11129 – Bug 11129 - Filtering Items based on type in opac-detail.pl

[patch] Bug 11129 - Filtering Items based on type in opac-detail.pl

Bug-11129---Filtering-Items-based-on-type-in-opac-.patch (text/plain), 34.63 KB, created by Olli-Antti Kivilahti on 2013-11-28 10:53:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Olli-Antti Kivilahti

Created: 2013-11-28 10:53:38 UTC

Size: 34.63 KB

patch

obsolete

>From 78115eaa99856722b84cdb79a101df279958bff3 Mon Sep 17 00:00:00 2001
>From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
>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 <input "number">
>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 |  14 ++
> koha-tmpl/opac-tmpl/prog/en/css/opac-detail.css    |  14 ++
> koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt | 171 ++++++++++++++++++---
> opac/opac-detail.pl                                | 114 +++++++++++++-
> t/db_dependent/Items.t                             |  35 ++++-
> t/db_dependent/Serials.t                           |  84 ++++++++++
> 8 files changed, 523 insertions(+), 42 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<One Order>.
> 
> =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..a95d50f 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 &rsaquo; 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 ) %]
>     <script type="text/javascript" src="https://apis.google.com/js/plusone.js">
>@@ -35,6 +36,7 @@
> [% IF ( bidi ) %]
>   <link rel="stylesheet" type="text/css" href="[% themelang %]/css/right-to-left.css" />
> [% END %]
>+<link rel="stylesheet" type="text/css" href="[% themelang %]/css/opac-detail.css" />
> <script type="text/javascript">
> //<![CDATA[
> 
>@@ -66,7 +68,9 @@
>             $(".highlight_toggle").toggle();
>         }
>     [% END %]
>-
>+// ------------------------------ //
>+//>>> Document READY starts here! //
>+// ------------------------------ //
>      $(document).ready(function() { 
>         $('#bibliodescriptions').tabs();
>         $(".branch-info-tooltip-trigger").tooltip({
>@@ -109,27 +113,30 @@
>         });
> [% END %]
> 
>-            $(".holdingst").dataTable($.extend(true, {}, dataTablesDefaults, {
>-                "aoColumns": [
>-                    [% IF ( item_level_itypes ) %]null,[% END %]
>-                    null,
>-                    [% IF ( itemdata_ccode ) %]null,[% END %]
>+        $(".holdingst").dataTable($.extend(true, {}, dataTablesDefaults, {
>+            "aoColumns": [
>+                [% IF ( item_level_itypes ) %]null,[% END %]
>+                null,
>+                [% IF ( itemdata_ccode ) %]null,[% END %]
>+                null,
>+                [% IF ( itemdata_enumchron ) %]null,[% END %]
>+                [% IF ( itemdata_uri ) %]null,[% END %]
>+                [% IF ( itemdata_copynumber ) %]null,[% END %]
>+                null,
>+                [% IF ( itemdata_itemnotes ) %]null,[% END %]
>+                { "sType": "title-string" },
>+                [% IF ( OPACShowBarcode ) %]null,[% END %]
>+                [% IF holds_count.defined %]
>                     null,
>-                    [% IF ( itemdata_enumchron ) %]null,[% END %]
>-                    [% IF ( itemdata_uri ) %]null,[% END %]
>-                    [% IF ( itemdata_copynumber ) %]null,[% END %]
>+                [% ELSIF show_priority %]
>                     null,
>-                    [% IF ( itemdata_itemnotes ) %]null,[% END %]
>-                    { "sType": "title-string" },
>-                    [% IF ( OPACShowBarcode ) %]null,[% END %]
>-                    [% IF holds_count.defined %]
>-                        null,
>-                    [% ELSIF show_priority %]
>-                        null,
>-                    [% END %]
>-                    [% IF ( ShowCourseReservesHeader ) %]null,[% END %]
>-                ]
>-            }));
>+                [% END %]
>+                [% IF ( ShowCourseReservesHeader ) %]null,[% END %]
>+            ]
>+        }));
>+
>+        //Bind the datepicker
>+        $('.datepicker').datepicker();
> 
>         [% IF ( query_desc ) %][% IF ( OpacHighlightedWords ) %]var query_desc = "[% query_desc |replace("'", "\'") |replace('\n', '\\n') |replace('\r', '\\r') |html %]";
>             q_array = query_desc.split(" ");
>@@ -248,7 +255,15 @@ $(function () {
>     }
> 
> [% END %]
>-});
>+
>+        $('#filterIssuesFormContainer').hide(); /* Making this element unobtrusive for javascript consumers */
>+        $('#filterIssuesButton').click(function() {
>+            $('#filterIssuesFormContainer').toggle();
>+        });
>+    });
>+// --------------------------- //
>+//<<< Document READY ends here //
>+// --------------------------- //
> [% IF ( IDreamBooksReviews || IDreamBooksReadometer ) %]
> function parseIDBJSON( json ) {
>     if(json.total_results > 0 && json.book.rating > 0){
>@@ -1038,9 +1053,15 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
> 
> 
> <div id="holdings">
>+
>+    [% IF ( lotsofholdingsitems ) %]
>+        [%# Display the items filtering form used to filter the shown items. See the end of this file! %]
>+        [% INCLUDE filter_form tab="holdings" %]
>+    [% END %]
>+
> [% IF ( itemloop.size ) %]
>     [% IF ( lotsofholdingsitems ) %]
>-        <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></p>
>+        <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" %]
>     [% END %]
>@@ -1064,7 +1085,12 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
>         <div id="alternateholdings"><span class="holdings_label">Holdings:</span> [% ALTERNATEHOLDING.holding %]</div>
>     [% END %]
>     [% ELSE %]
>-    <div id="noitems">No physical items for this record</div>
>+    <h4 id="noitems">
>+        No physical items for this record.
>+        [% IF filter %]
>+            <br/> Try clearing the filter.
>+        [% END %]
>+    </h4>
>     [% END %]
> [% END %]
> 
>@@ -1604,3 +1630,102 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
> 	    [% END %]</tbody>
> 	</table>
> [% END %][%# end of items_table block %]
>+
>+[% BLOCK filter_form %]
>+    [% IF ( notDefined_NumberingFormulaParsingRegexp ) %]
>+        <div class="dialog alert">
>+            You must define the NumberingFormulaParsingRegexp system preference to filter items by enumeration!
>+        </div>
>+    [% END %]
>+
>+    <div id="filterIssuesParentContainer">
>+    <a id="filterIssuesButton" >Limit issues</a>
>+    [% IF filter %]
>+        <form id="issuesFilter" 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>
>+    [% END %]
>+    <div id="filterIssuesFormContainer">
>+    <form id="issuesFilter" method="get" action="/cgi-bin/koha/opac-detail.pl">
>+        <input type="hidden" name="biblionumber" id="biblionumber" value="[% biblionumber %]"/>
>+        <input type="hidden" name="viewallitems" id="viewallitems" value="1"/>
>+
>+        <fieldset>
>+            <table>
>+                <tr><td>
>+                        <label for="filterBranchLimiter">
>+                            Library:
>+                        </label>
>+                    </td><td>
>+                        <select name="filterBranchLimiter" size="1" id="filterBranchLimiter">
>+                        [%- FOREACH branchloo IN branchloop %]
>+                            [% IF ( branchloo.selected ) -%]
>+                                <option value="[% branchloo.branchcode %]" selected="selected">
>+                            [%- ELSE -%]
>+                                <option value="[% branchloo.branchcode %]">
>+                            [%- END -%]
>+                            [% IF ( branchloo.branchcode ) == '_ShowAll' -%]
>+                                Show from any library</option>
>+                            [%- ELSE -%]
>+                                [% branchloo.branchname %]</option>
>+                            [%- END -%]
>+                        [%- END %]
>+                        </select>
>+                    </td>
>+                </tr>
>+                [% IF isSerial %]
>+                    <tr>
>+                        <td>
>+                            <label for="filterVolume">
>+                                Issue volume:
>+                            </label>
>+                        </td><td>
>+                            <input type="number" id="filterVolume" name="filterVolume" min="0" max="9999" maxlength="4" value="[% filter.volume %]">
>+                        </td>
>+                    </tr><tr>
>+                        <td>
>+                            <label for="filterNumber">
>+                                Issue number:
>+                            </label>
>+                        </td><td>
>+                            <input type="number" id="filterNumber" name="filterNumber" min="0" max="99" maxlength="2" value="[% filter.number %]">
>+                        </td>
>+                    </tr>
>+                    [% IF useFilterIssueInput %]
>+                    <tr>
>+                        <td>
>+                            <label for="filterIssue">
>+                                Issue issue:
>+                            </label>
>+                        </td><td>
>+                            <input type="number" id="filterIssue" name="filterIssue" min="0" max="99" maxlength="2" value="[% filter.issue %]">
>+                        </td>
>+                    </tr>
>+                    [% END %]
>+                [% END %][%# End of IF isSerial %]
>+                <tr>
>+                    <td>
>+                        <label for="filterFrom">
>+                            From date:
>+                        </label>
>+                    </td><td>
>+                        <input type="text" size="10" id="filterFrom" name="filterFrom" value="[% filter.serialFromDate %]" class="datepicker" />
>+                    </td>
>+                </tr><tr>
>+                    <td>
>+                        <label for="filterTo">
>+                            To date:
>+                        </label>
>+                    </td><td>
>+                        <input type="text" size="10" id="filterTo" name="filterTo" value="[% filter.serialToDate %]" class="datepicker" />
>+                    </td>
>+                </tr>
>+            </table>
>+
>+            <input type="submit" name="filterIssues" value="Submit" class="submit"/>
>+        </fieldset>
>+    </form>
>+    </div>
>+    </div>
>+[% END %][%# end of filter_form block %]
>diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
>index 41de4fd..1d1f1c6 100755
>--- a/opac/opac-detail.pl
>+++ b/opac/opac-detail.pl
>@@ -73,7 +73,100 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
> my $biblionumber = $query->param('biblionumber') || $query->param('bib') || 0;
> $biblionumber = int($biblionumber);
> 
>-my @all_items = GetItemsInfo($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 $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.
>+
>+#We filter by the branch only if a valid branch is given.
>+if (defined $filterBranchLimiter && $filterBranchLimiter ne '_ShowAll') {
>+    $filter->{branch} = $filterBranchLimiter;
>+}
>+if (defined $filterVolume && length $filterVolume > 0) {
>+    if (!($filterVolume =~ /\d{1,4}/)) {
>+        print $query->header(); #bad data goddamnit!
>+        print "Invalid volume. Please try again. \n";
>+        exit;
>+    }
>+    else {
>+        $filter->{volume} = $filterVolume;
>+    }
>+}
>+if (defined $filterNumber && length $filterNumber > 0) {
>+    if (!($filterNumber =~ /\d{1,2}/)) {
>+        print $query->header(); #stop spamming bad data!
>+        print "Invalid number. Please try again. \n";
>+        exit;
>+    }
>+    else {
>+        $filter->{number} = $filterNumber;
>+    }
>+}
>+if (defined $filterIssue && length $filterIssue > 0) {
>+    if (!($filterIssue =~ /\d{1,2}/) ) {
>+        print $query->header(); #stop spamming bad data!
>+        print "Invalid issue. Please try again. \n";
>+        exit;
>+    }
>+    else {
>+        $filter->{issue} = $filterIssue;
>+    }
>+}
>+if (defined $filterFromDate && length $filterFromDate > 0) {
>+    if (!($filterFromDate =~ C4::Dates->regexp( C4::Context->preference('dateformat') )) ) {
>+        print $query->header(); #noo not anymore noo!
>+        print "Invalid starting date. Please try again. \n";
>+        exit;
>+    }
>+    else {
>+        $filter->{fromDate} = C4::Dates::format_date_in_iso( $filterFromDate );
>+    }
>+}
>+if (defined $filterToDate && length $filterToDate > 0) {
>+    if (!($filterToDate =~ C4::Dates->regexp( C4::Context->preference('dateformat') )) ) {
>+        print $query->header(); #take your bad data away!
>+        print "Invalid ending date. Please try again. \n";
>+        exit;
>+    }
>+    else {
>+        $filter->{toDate} = C4::Dates::format_date_in_iso( $filterToDate );
>+    }
>+}
>+
>+
>+##Prepare the custom branches loop containing the _ShowAll entry to show issues from all libraries.
>+my $branchloop;
>+if ( $filterBranchLimiter eq '_ShowAll' || !(defined $filterBranchLimiter) ) {
>+    $branchloop = C4::Branch::GetBranchesLoop('0'); #Using '0' to disable reverting to the users home branch
>+    unshift @$branchloop, { branchcode => '_ShowAll', branchname => 'Show from any library', selected => '1', value => '_ShowAll'};
>+}
>+else {
>+    $branchloop = C4::Branch::GetBranchesLoop($filterBranchLimiter);
>+    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;
>+
> my @hiddenitems;
> if (scalar @all_items >= 1) {
>     push @hiddenitems, GetHiddenItemnumbers(@all_items);
>@@ -91,6 +184,19 @@ if ( ! $record ) {
> }
> $template->param( biblionumber => $biblionumber );
> 
>+#Figure out if we are dealing with a serial! This affects the filter fields in UI
>+if (scalar @all_items > 0) {
>+    $template->param( isSerial => $all_items[0]->{serial} );
>+}
>+else {
>+    #It could be that a serial has no items to be displayed. This could be because there are none or the filters filter all items
>+    my $search = C4::SQLHelper::SearchInTable("biblio",{biblionumber => $biblionumber}, undef, undef, ['serial'], undef, "exact");
>+    $template->param( isSerial => $search->[0]->{serial} );
>+}
>+
>+
>+
>+
> # get biblionumbers stored in the cart
> my @cart_list;
> 
>@@ -1068,5 +1174,11 @@ if ( C4::Context->preference('UseCourseReserves') ) {
>         $i->{'course_reserves'} = GetItemCourseReservesInfo( itemnumber => $i->{'itemnumber'} );
>     }
> }
>+## Defining general Serial issue filter related system preferences
>+#Making sure the NumberingFormulaParsingRegexp preference is set!
>+if ( length C4::Context->preference('NumberingFormulaParsingRegexp') < 3 ) {
>+    $template->{VARS}->{notDefined_NumberingFormulaParsingRegexp} = 1;
>+}
>+$template->{VARS}->{useFilterIssueInput} = 1 if (C4::Context->preference('FilterSerialsByIssue'));
> 
> output_html_with_http_headers $query, $cookie, $template->output;
>diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t
>index 5ec8d34..59b8416 100755
>--- a/t/db_dependent/Items.t
>+++ b/t/db_dependent/Items.t
>@@ -21,7 +21,7 @@ use Modern::Perl;
> use MARC::Record;
> use C4::Biblio;
> 
>-use Test::More tests => 3;
>+use Test::More tests => 4;
> 
> BEGIN {
>     use_ok('C4::Items');
>@@ -142,6 +142,39 @@ subtest 'GetHiddenItemnumbers tests' => sub {
>     $dbh->rollback;
> };
> 
>+
>+subtest 'Filter items tests' => sub {
>+
>+    plan tests => 2;
>+
>+    # Start transaction
>+    $dbh->{AutoCommit} = 0;
>+    $dbh->{RaiseError} = 1;
>+
>+    # Create a new biblio
>+    my ($biblionumber, $biblioitemnumber) = get_biblio();
>+
>+    # Add two items
>+    my ($item1_bibnum, $item1_bibitemnum, $item1_itemnumber) = AddItem(
>+            { homebranch => 'CPL',
>+              holdingbranch => 'CPL', },
>+            $biblionumber
>+    );
>+    my ($item2_bibnum, $item2_bibitemnum, $item2_itemnumber) = AddItem(
>+            { homebranch => 'MPL',
>+              holdingbranch => 'MPL', },
>+            $biblionumber
>+    );
>+
>+    # Testing the branch filter
>+    my @shouldBeItem2 = C4::Items::GetItemsInfo($biblionumber, {branch => 'MPL'});
>+    is( $shouldBeItem2[0]->{itemnumber}, $item2_itemnumber, "Filtering by branch");
>+
>+    # Testing the dates filter
>+    my @shouldBeEmpty = C4::Items::GetItemsInfo($biblionumber, {toDate => '01/01/1933'});
>+    is( scalar(@shouldBeEmpty), 0, "Filtering by date");
>+};
>+
> # Helper method to set up a Biblio.
> sub get_biblio {
>     my $bib = MARC::Record->new();
>diff --git a/t/db_dependent/Serials.t b/t/db_dependent/Serials.t
>index e942982..2cc63de 100644
>--- a/t/db_dependent/Serials.t
>+++ b/t/db_dependent/Serials.t
>@@ -14,6 +14,9 @@ use C4::Debug;
> use C4::Bookseller;
> use C4::Biblio;
> use C4::Budgets;
>+use C4::Items;
>+use C4::Biblio;
>+
> use Test::More tests => 35;
> 
> BEGIN {
>@@ -180,4 +183,85 @@ is(C4::Serials::check_routing(), undef, 'test checking route');
> 
> is(C4::Serials::addroutingmember(),undef, 'test adding route member');
> 
>+
>+subtest 'Filter items tests' => sub {
>+
>+    plan tests => 4;
>+
>+
>+    # Start transaction
>+    $dbh->{AutoCommit} = 0;
>+    $dbh->{RaiseError} = 1;
>+
>+    # Create a new biblio
>+    my ($biblionumber, $biblioitemnumber) = get_biblio();
>+
>+    # Add items
>+	my ($item0_bibnum, $item0_bibitemnum, $item0_itemnumber) = AddItem(
>+            { homebranch => 'CPL',
>+              holdingbranch => 'CPL',
>+			  enumchron => 'Vol 2012 : No 1, Issuezz 1'},
>+            $biblionumber
>+    );
>+    my ($item1_bibnum, $item1_bibitemnum, $item1_itemnumber) = AddItem(
>+            { homebranch => 'CPL',
>+              holdingbranch => 'CPL',
>+			  enumchron => 'Vol 2013 : No 11, Issuezz 1'},
>+            $biblionumber
>+    );
>+    my ($item2_bibnum, $item2_bibitemnum, $item2_itemnumber) = AddItem(
>+            { homebranch => 'MPL',
>+              holdingbranch => 'MPL',
>+			  enumchron => 'Vol 2013 : No 11, Issuezz 2'},
>+            $biblionumber
>+    );
>+	my ($item3_bibnum, $item3_bibitemnum, $item3_itemnumber) = AddItem(
>+            { homebranch => 'CPL',
>+              holdingbranch => 'CPL',
>+			  enumchron => 'Vol 2013 : No 12, Issuezz 1'},
>+            $biblionumber
>+    );
>+    my ($item4_bibnum, $item4_bibitemnum, $item4_itemnumber) = AddItem(
>+            { homebranch => 'MPL',
>+              holdingbranch => 'MPL',
>+			  enumchron => 'Vol 2013 : No 12, Issuezz 2'},
>+            $biblionumber
>+    );
>+	my ($item5_bibnum, $item5_bibitemnum, $item5_itemnumber) = AddItem(
>+            { homebranch => 'MPL',
>+              holdingbranch => 'MPL',
>+			  enumchron => 'Vol 2014 : No 12, Issuezz 3'},
>+            $biblionumber
>+    );
>+
>+	C4::Context->set_preference('NumberingFormulaParsingRegexp', '^[^0-9]*volume[^0-9]*number[^0-9]*issue[^0-9]*$');
>+
>+    # Testing the volume filter
>+    my @shouldBe4Items = C4::Items::GetItemsInfo($biblionumber, {volume => '2013'});
>+    is( scalar(@shouldBe4Items), 4, "Filtering by volume");
>+
>+    # Testing the number filter
>+    my @shouldBe3Items = C4::Items::GetItemsInfo($biblionumber, {number => '12'});
>+    is( scalar(@shouldBe3Items), 3, "Filtering by number");
>+
>+	# Testing the issue filter
>+    my @shouldBe2Items = C4::Items::GetItemsInfo($biblionumber, {issue => '2'});
>+    is( scalar(@shouldBe2Items), 2, "Filtering by issue");
>+
>+	# Testing the volume + number + issue filter
>+    my @shouldBeItem4 = C4::Items::GetItemsInfo($biblionumber, {volume => 2013, number => 12, issue => '2'});
>+    is( $shouldBeItem4[0]->{itemnumber}, $item4_itemnumber, "Filtering by volume + number + issue");
>+};
>+
>+# Helper method to set up a Biblio.
>+sub get_biblio {
>+    my $bib = MARC::Record->new();
>+    $bib->append_fields(
>+        MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'),
>+        MARC::Field->new('245', ' ', ' ', a => 'Silence in the library'),
>+    );
>+    my ($bibnum, $bibitemnum) = C4::Biblio::AddBiblio($bib, '');
>+    return ($bibnum, $bibitemnum);
>+}
>+
> $dbh->rollback;
>-- 
>1.8.1.2

Actions: View | Diff | Splinter Review

Attachments on bug 11129: 22464 | 22513 | 22566 | 22569 | 23172 | 23205