From 7075b18d5f21092a6d9cb52b638e98f7c06d3961 Mon Sep 17 00:00:00 2001 From: "A. Sassmannshausen" Date: Thu, 24 Apr 2014 13:57:07 +0000 Subject: [PATCH] Bug 6906 - show 'Borrower has previously... New module: provide granular means to configure warnings about items that have been issued to a particular borrower before, according to their loan history. - Global syspref ('CheckPrevIssue'), set to 'No' by default, allows - users to enable this feature library wide. - Per category pref allow libraries to create overrides per category, falling back on the global setting by default. - Per borrower pref allows switching the functionality on at the level of borrowers. Fall-back to category settings by default. * Koha/Borrowers/CheckPrevIssue.pm: new module. * C4/Circulation.pm (CanBookBeIssued): introduce CheckPrevIssue check. * koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt: Add 'CHECKPREVISSUE' confirmation message. * installer/data/mysql/kohastructure.sql: modify structure of 'categories', 'borrowers'. * installer/data/mysql/sysprefs.sql: add 'CheckPrevIssue' (exposed in patrons.pref). * installer/data/mysql/updatedatabase.pl: provide upgrade path. * admin/categorie.pl: add 'checkprevissue' to sql queries; pass it to template. * .../admin/categorie.tt: .../members/memberentrygen.tt: .../members/moremember.tt: add content. * CheckPrevIssue.t: new file with unit tests. Test plan: - Apply patch. - Run db updates from updatedatabase.pl manually. - Verify 'CheckPrevIssue' is visible in Patrons sysprefs and can be switched 'on' and 'off'. + Issue previously issued items to a borrower, checking the message appears as expected. - Verify 'Check previous loans' setting appears on the borrower category pages and can be modified per borrower category. + Issue previously issued items to a borrower, checking the message appears as expected (This setting should override the default setting). - Verify 'Check previous loans' setting appears on individual borrower pages and can be modified. + Issue previously issued items to a borrower, checking the message appears as expected (This setting should override the category setting and the default setting). --- C4/Circulation.pm | 13 ++ Koha/Borrower/CheckPrevIssue.pm | 126 ++++++++++++++++++++ admin/categorie.pl | 14 ++- installer/data/mysql/kohastructure.sql | 2 + installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 9 ++ .../prog/en/modules/admin/categorie.tt | 37 ++++++ .../prog/en/modules/admin/preferences/patrons.pref | 6 + .../prog/en/modules/circ/circulation.tt | 4 + .../prog/en/modules/members/memberentrygen.tt | 19 ++- .../prog/en/modules/members/moremember.tt | 9 ++ t/CheckPrevIssue.t | 114 ++++++++++++++++++ 12 files changed, 348 insertions(+), 6 deletions(-) create mode 100644 Koha/Borrower/CheckPrevIssue.pm create mode 100755 t/CheckPrevIssue.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index dc7334a..3ea37e3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -48,6 +48,7 @@ use Data::Dumper; use Koha::DateUtils; use Koha::Calendar; use Koha::Borrower::Debarments; +use Koha::Borrower::CheckPrevIssue qw( WantsCheckPrevIssue CheckPrevIssue ); use Carp; use Date::Calc qw( Today @@ -848,6 +849,18 @@ sub CanBookBeIssued { } } + # If patron uses checkPrevIssue or inherits it, check for previous + # issue of item to patron. + my $checkPrevIssueOverride = WantsCheckPrevIssue( $borrower ); + if ( ( $checkPrevIssueOverride eq 'yes' ) + or ( $checkPrevIssueOverride eq 'inherit' + and C4::Context->preference("checkPrevIssue") ) ) + { + $needsconfirmation{PREVISSUE} = 1 + if CheckPrevIssue( $borrower->{borrowernumber}, + $item->{biblionumber} ); + } + # # ITEM CHECKING # diff --git a/Koha/Borrower/CheckPrevIssue.pm b/Koha/Borrower/CheckPrevIssue.pm new file mode 100644 index 0000000..40276cf --- /dev/null +++ b/Koha/Borrower/CheckPrevIssue.pm @@ -0,0 +1,126 @@ +package Koha::Borrower::CheckPrevIssue; + +# This file is part of Koha. +# +# Copyright 2014 PTFS Europe +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use C4::Context; + +use parent qw( Exporter ); + +our @EXPORT = qw( + WantsCheckPrevIssue + CheckPrevIssue +); + +=head1 Koha::Borrower::CheckPrevIssue + +Koha::Borrower::Debarments - Manage Previous Issue preferences & searches. + +=head2 WantsCheckPrevIssue + + ($CheckPrevIssueOverride) = WantsCheckPrevIssue( $borrower ); + +Returns 'yes', 'no' or 'inherit' depending on whether the patron or +patron category should be reminded when items to be loaned have +already been loaned to this borrower. + +=cut + +sub WantsCheckPrevIssue { + my ( $borrower ) = @_; + my $CheckPrevIssueByBrw = $borrower->{checkprevissue}; + if ( $CheckPrevIssueByBrw eq 'inherit' ) { + return _WantsCheckPrevIssueByCat( $borrower->{borrowernumber} ); + } else { + return $CheckPrevIssueByBrw; + } +} + +=head2 _WantsCheckPrevIssueByCat + + ($CheckPrevIssueByCatOverride) = _WantsCheckPrevIssueByCat( $borrowernumber ); + +Returns 'yes', 'no' or 'inherit' depending on whether the patron +in this category should be reminded when items to be loaned have already been +loaned to this borrower. + +=cut + +sub _WantsCheckPrevIssueByCat { + my ( $borrowernumber ) = @_; + my $dbh = C4::Context->dbh; + my $query = ' +SELECT categories.checkprevissue +FROM borrowers +LEFT JOIN categories ON borrowers.categorycode = categories.categorycode +WHERE borrowers.borrowernumber = ? +'; + my $sth; + if ($borrowernumber) { + $sth = $dbh->prepare($query); + $sth->execute($borrowernumber); + } else { + return; + } + return ${$sth->fetchrow_arrayref()}[0]; +} + +=head2 CheckPrevIssue + + ($PrevIssue) = CheckPrevIssue( $borrowernumber, $biblionumber ); + +Return 1 if $BIBLIONUMBER has previously been issued to +$BORROWERNUMBER, 0 otherwise. + +=cut + +sub CheckPrevIssue { + my ( $borrowernumber, $biblionumber ) = @_; + my $dbh = C4::Context->dbh; + my $previssue = 0; + my $query_items = 'select itemnumber from items where biblionumber=?'; + my $sth_items = $dbh->prepare($query_items); + $sth_items->execute($biblionumber); + + my $query_issues = ' +select count(itemnumber) from old_issues +where borrowernumber=? and itemnumber=? +'; + my $sth_issues = $dbh->prepare($query_issues); + + while ( my @row = $sth_items->fetchrow_array() ) { + $sth_issues->execute( $borrowernumber, $row[0] ); + while ( my @matches = $sth_issues->fetchrow_array() ) { + if ( $matches[0] > 0 ) { + $previssue = 1; + last; + } + } + last if $previssue; + } + return $previssue; +} + +1; + +=head2 AUTHOR + +Alex Sassmannshausen + +=cut diff --git a/admin/categorie.pl b/admin/categorie.pl index 1b4ad85..699eb59 100755 --- a/admin/categorie.pl +++ b/admin/categorie.pl @@ -159,6 +159,7 @@ if ( $op eq 'add_form' ) { $data->{'BlockExpiredPatronOpacActions'}, TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification"), + checkprevissue => $data->{'checkprevissue'}, ); if ( C4::Context->preference('EnhancedMessagingPreferences') ) { @@ -199,7 +200,8 @@ elsif ( $op eq 'add_validate' ) { overduenoticerequired=?, category_type=?, BlockExpiredPatronOpacActions=?, - default_privacy=? + default_privacy=?, + checkprevissue=? WHERE categorycode=?" ); $sth->execute( @@ -210,7 +212,7 @@ elsif ( $op eq 'add_validate' ) { 'reservefee', 'hidelostitems', 'overduenoticerequired', 'category_type', 'block_expired', 'default_privacy', - 'categorycode' + 'checkprevissue', 'categorycode' ) ); my @branches = $input->param("branches"); @@ -246,9 +248,10 @@ elsif ( $op eq 'add_validate' ) { overduenoticerequired, category_type, BlockExpiredPatronOpacActions, - default_privacy + default_privacy, + checkprevissue ) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)" ); + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?)" ); $sth->execute( map { $input->param($_) } ( 'categorycode', 'description', @@ -257,7 +260,7 @@ elsif ( $op eq 'add_validate' ) { 'enrolmentfee', 'reservefee', 'hidelostitems', 'overduenoticerequired', 'category_type', 'block_expired', - 'default_privacy', + 'default_privacy', 'checkprevissue' ) ); $sth->finish; @@ -354,6 +357,7 @@ else { # DEFAULT enrolmentfee => sprintf( "%.2f", $results->[$i]{'enrolmentfee'} || 0 ), "type_" . $results->[$i]{'category_type'} => 1, + checkprevissue => $results->[$i]{'checkprevissue'}, ); if ( C4::Context->preference('EnhancedMessagingPreferences') ) { diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index b46dc6a..6357df2 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -265,6 +265,7 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons `altcontactphone` varchar(50) default NULL, -- the phone number for the alternate contact for the patron/borrower `smsalertnumber` varchar(50) default NULL, -- the mobile phone number where the patron/borrower would like to receive notices (if SNS turned on) `privacy` integer(11) DEFAULT '1' NOT NULL, -- patron/borrower's privacy settings related to their reading history + `checkprevissue` varchar(7) NOT NULL default 'inherit', -- produce a warning for this borrower if this item has previously been issued to this borrower if 'yes', not if 'no', defer to category setting if 'inherit'. UNIQUE KEY `cardnumber` (`cardnumber`), PRIMARY KEY `borrowernumber` (`borrowernumber`), KEY `categorycode` (`categorycode`), @@ -468,6 +469,7 @@ CREATE TABLE `categories` ( -- this table shows information related to Koha patr `category_type` varchar(1) NOT NULL default 'A', -- type of Koha patron (Adult, Child, Professional, Organizational, Statistical, Staff) `BlockExpiredPatronOpacActions` tinyint(1) NOT NULL default '-1', -- wheither or not a patron of this category can renew books or place holds once their card has expired. 0 means they can, 1 means they cannot, -1 means use syspref BlockExpiredPatronOpacActions `default_privacy` ENUM( 'default', 'never', 'forever' ) NOT NULL DEFAULT 'default', -- Default privacy setting for this patron category + `checkprevissue` varchar(7) NOT NULL default 'inherit', -- produce a warning for this borrower category if this item has previously been issued to this borrower if 'yes', not if 'no', defer to syspref setting if 'inherit'. PRIMARY KEY (`categorycode`), UNIQUE KEY `categorycode` (`categorycode`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 03a3f1e..93e0ae8 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -82,6 +82,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('CatalogModuleRelink','0',NULL,'If OFF the linker will never replace the authids that are set in the cataloging module.','YesNo'), ('CataloguingLog','1',NULL,'If ON, log edit/create/delete actions on bibliographic data. WARNING: this feature is very resource consuming.','YesNo'), ('checkdigit','none','none|katipo','If ON, enable checks on patron cardnumber: none or \"Katipo\" style checks','Choice'), +('CheckPrevIssue','0','','By default, for every item issued, should we warn if the patron has borrowed that item in the past?','YesNo'), ('CircAutocompl','1',NULL,'If ON, autocompletion is enabled for the Circulation input','YesNo'), ('CircAutoPrintQuickSlip','qslip',NULL,'Choose what should happen when an empty barcode field is submitted in circulation: Display a print quick slip window, Display a print slip window or Clear the screen.','Choice'), ('CircControl','ItemHomeLibrary','PickupLibrary|PatronLibrary|ItemHomeLibrary','Specify the agency that controls the circulation and fines policy','Choice'), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 3f1af57..b69fbb0 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8603,6 +8603,15 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.17.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do("INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES('CheckPrevIssue','0','','By default, for every item issued, should we warn if the patron has borrowed that item in the past?','YesNo')"); + $dbh->do("ALTER TABLE categories ADD (`checkprevissue` varchar(7) NOT NULL default 'inherit')"); + $dbh->do("ALTER TABLE borrowers ADD (`checkprevissue` varchar(7) NOT NULL default 'inherit')"); + print "Upgrade to $DBversion done (Bug 6906: show 'Borrower has previously issued \$ITEM' alert on checkout)\n"; + SetVersion ($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt index d03309a..e0f370b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt @@ -146,6 +146,22 @@
  • years
  • years
  • +
  • +
  • + [% IF ( checkprevissue == 'yes' ) %] + + + + [% ELSIF (checkprevissue == 'no' ) %] + + + + [% ELSE %] + + + + [% END %] + +
  • + [% UNLESS nodateenrolled && noopacnote && noborrowernotes %]
    diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index cf4cd8c..5b25775 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -350,6 +350,15 @@ function validate1(date) { [% IF ( borrowernotes ) %]
  • Circulation note: [% borrowernotes %]
  • [% END %] [% IF ( opacnote ) %]
  • OPAC note:[% opacnote %]
  • [% END %] +
  • Check previous loans: + [% IF ( checkprevissue == 'yes' ) %] + Yes + [% ELSIF ( checkprevissue == 'no' ) %] + No + [% ELSE %] + Inherited + [% END %] +
  • diff --git a/t/CheckPrevIssue.t b/t/CheckPrevIssue.t new file mode 100755 index 0000000..1cc67c6 --- /dev/null +++ b/t/CheckPrevIssue.t @@ -0,0 +1,114 @@ +#!/usr/bin/perl +use strict; +use warnings; + +use C4::Context; +use Test::More tests => 5; +use Test::MockModule; +use DBD::Mock; + +use_ok('Koha::Borrower::CheckPrevIssue'); + +use Koha::Borrower::CheckPrevIssue qw( WantsCheckPrevIssue CheckPrevIssue ); + +# Setup mock db +my $module_context = new Test::MockModule('C4::Context'); +$module_context->mock( + '_new_dbh', + sub { + my $dbh = DBI->connect( 'DBI:Mock:', '', '' ) + || die "Cannot create handle: $DBI::errstr\n"; + return $dbh; + } +); + +my $dbh = C4::Context->dbh(); + +# convenience variables +my $name; +# mock_add_resultset vars +my ( $sql, $sql2, @bound_params, @keys, @values, %result ) = ( ); +# mock_history_verification vars +my ( $history, $params, $query ) = ( ); + +sub clean_vars { + ( $name, $sql, $sql2, @bound_params, @keys, @values, %result, + $history, $params, $query ) = ( ); + $dbh->{mock_clear_history} = 1; +} + +# Tests +## WantsCheckPrevIssue +$name = 'WantsCheckPrevIssue'; +$sql = ' +SELECT categories.checkprevissue +FROM borrowers +LEFT JOIN categories ON borrowers.categorycode = categories.categorycode +WHERE borrowers.borrowernumber = ? +'; + +# Test if, as brw inherits, result is simply passed from +# _WantsCheckPrevIssueByCat back through WantsCheckPrevIssue. +my %brw = ( + checkprevissue => 'inherit', + borrowernumber => '101', +); + +$dbh->{mock_add_resultset} = + { + sql => $sql, + bound_params => [ '101' ], + results => [ [ 'categories.checkprevissue' ], [ 'pass_thru' ] ], + }; + +is( WantsCheckPrevIssue(\%brw), 'pass_thru', + $name . ": Return value \"pass_thru\"." ); + +# Test if, if brw does not inherit, WantsCheckPrevIssue simply returns +# its contents. +%brw = ( + checkprevissue => 'brw_pass_thru', + borrowernumber => '101', +); + +is( WantsCheckPrevIssue(\%brw), 'brw_pass_thru', + $name . ": Return value \"$brw{checkprevissue}\"." ); + +clean_vars(); + +## CheckPrevIssue +$name = 'CheckPrevIssue'; +$sql = 'select itemnumber from items where biblionumber=?'; +$sql2 = ' +select count(itemnumber) from old_issues +where borrowernumber=? and itemnumber=? +'; +@keys = qw< borrowernumber biblionumber itemnumber >; +@values = qw< 101 3576 5043 >; + +# 1) Prepop items with itemnumber for result +$dbh->{mock_add_resultset} = { + sql => $sql, + bound_params => $keys[1], + results => [ [ ( $keys[2] ) ], [ ( $values[2] ) ] ], + }; +# 2) Test if never issued before (expect 0) +is( CheckPrevIssue( $keys[0], $keys[1] ), 0, + $name . ': Return value "no matches".' ); +# 3) Prepop old_issues with itemnumber and borrowernumber +$dbh->{mock_add_resultset} = { + sql => $sql2, + bound_params => [ $keys[0], $keys[2] ], + results => [ + [ ( $keys[0], $keys[2] ) ], + [ ( $values[0], $values[2] ) ], + [ ( $values[0], $values[2] ) ], + [ ( $values[0], $values[2] ) ], + [ ( $values[0], $values[2] ) ], + ], + }; +# 4) Test if issued before (e.g. 7 times — expect 1) +is( CheckPrevIssue( $keys[0], $keys[1] ), 1, + $name . ': Return value "> 0 matches".' ); + +clean_vars(); -- 1.7.10.4