From a6f641cb4bd0bebf08b1675110348aa619d66774 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... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ('CheckPrevIssueByDefault'), 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 'CheckPrevIssueByDefault' (exposed in patrons.pref). * installer/data/mysql/updatedatabase.pl: provide upgrade path. * admin/categorie.pl: add 'checkprevissuebycategory' 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 updateds from updatedatabase.pl manually (I have not updated the database version yet — not sure whether I should have done?) - Optionally, run Unit Tests (t/CheckPrevIssue.t) - Verify 'CheckPrevIssueByDefault' 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 | 116 ++++++++++++++++++++ admin/categorie.pl | 15 ++- installer/data/mysql/kohastructure.sql | 4 +- installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 10 ++ .../prog/en/modules/admin/categorie.tt | 31 ++++++ .../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 | 107 ++++++++++++++++++ 12 files changed, 327 insertions(+), 8 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 bd9e1cc..6e0fbf0 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 @@ -841,6 +842,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("checkPrevIssueByDefault") ) ) + { + $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..7942668 --- /dev/null +++ b/Koha/Borrower/CheckPrevIssue.pm @@ -0,0 +1,116 @@ +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 $CheckPrevIssueByBorrower = $borrower->{checkprevissuebyborrower}; + if ( $CheckPrevIssueByBorrower eq 'inherit' ) { + return _WantsCheckPrevIssueByCategory( $borrower->{borrowernumber} ); + } else { + return $CheckPrevIssueByBorrower; + } +} + +=head2 _WantsCheckPrevIssueByCategory + + ($CheckPrevIssueByCatOverride) = _WantsCheckPrevIssueByCategory( $borrowernumber ); + +Returns 'yes', 'no' or 'inherit' depending on whether the patron +category should be reminded when items to be loaned have already been +loaned to this borrower. + +=cut + +sub _WantsCheckPrevIssueByCategory { + my ( $borrowernumber ) = @_; + my $dbh = C4::Context->dbh; + my $query; + my $sth; + if ($borrowernumber) { + $sth = $dbh->prepare("SELECT categories.checkprevissuebycategory FROM borrowers LEFT JOIN categories ON borrowers.categorycode=categories.categorycode WHERE borrowers.borrowernumber=?"); + $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 $sth = + $dbh->prepare('select itemnumber from items where biblionumber=?'); + $sth->execute($biblionumber); + + my $query = $dbh->prepare( +'select count(itemnumber) from old_issues where borrowernumber=? and itemnumber=?' + ); + while ( my @row = $sth->fetchrow_array() ) { + $query->execute( $borrowernumber, $row[0] ); + while ( my @matches = $query->fetchrow_array() ) { + if ( $matches[0] > 0 ) { + $previssue = 1; + } + } + } + return $previssue; +} + +1; + +=head2 AUTHOR + +Alex Sassmannshausen + +=cut diff --git a/admin/categorie.pl b/admin/categorie.pl index 6ea7b5a..07d2db3 100755 --- a/admin/categorie.pl +++ b/admin/categorie.pl @@ -96,7 +96,7 @@ if ($op eq 'add_form') { my @selected_branches; if ($categorycode) { my $dbh = C4::Context->dbh; - my $sth=$dbh->prepare("select categorycode,description,enrolmentperiod,enrolmentperioddate,upperagelimit,dateofbirthrequired,enrolmentfee,issuelimit,reservefee,hidelostitems,overduenoticerequired,category_type from categories where categorycode=?"); + my $sth=$dbh->prepare("select categorycode,description,enrolmentperiod,enrolmentperioddate,upperagelimit,dateofbirthrequired,enrolmentfee,issuelimit,reservefee,hidelostitems,overduenoticerequired,category_type,checkprevissuebycategory from categories where categorycode=?"); $sth->execute($categorycode); $data=$sth->fetchrow_hashref; @@ -139,6 +139,7 @@ if ($op eq 'add_form') { TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification"), "type_".$data->{'category_type'} => 1, branches_loop => \@branches_loop, + checkprevissue => $data->{'checkprevissuebycategory'}, ); if (C4::Context->preference('EnhancedMessagingPreferences')) { C4::Form::MessagingPreferences::set_form_values({ categorycode => $categorycode } , $template); @@ -155,8 +156,8 @@ if ($op eq 'add_form') { } if ($is_a_modif) { - my $sth=$dbh->prepare("UPDATE categories SET description=?,enrolmentperiod=?, enrolmentperioddate=?,upperagelimit=?,dateofbirthrequired=?,enrolmentfee=?,reservefee=?,hidelostitems=?,overduenoticerequired=?,category_type=? WHERE categorycode=?"); - $sth->execute(map { $input->param($_) } ('description','enrolmentperiod','enrolmentperioddate','upperagelimit','dateofbirthrequired','enrolmentfee','reservefee','hidelostitems','overduenoticerequired','category_type','categorycode')); + my $sth=$dbh->prepare("UPDATE categories SET description=?,enrolmentperiod=?, enrolmentperioddate=?,upperagelimit=?,dateofbirthrequired=?,enrolmentfee=?,reservefee=?,hidelostitems=?,overduenoticerequired=?,category_type=?,checkprevissuebycategory=? WHERE categorycode=?"); + $sth->execute(map { $input->param($_) } ('description','enrolmentperiod','enrolmentperioddate','upperagelimit','dateofbirthrequired','enrolmentfee','reservefee','hidelostitems','overduenoticerequired','category_type','checkprevissue','categorycode')); my @branches = $input->param("branches"); if ( @branches ) { $sth = $dbh->prepare("DELETE FROM categories_branches WHERE categorycode = ?"); @@ -175,8 +176,8 @@ if ($op eq 'add_form') { } $sth->finish; } else { - my $sth=$dbh->prepare("INSERT INTO categories (categorycode,description,enrolmentperiod,enrolmentperioddate,upperagelimit,dateofbirthrequired,enrolmentfee,reservefee,hidelostitems,overduenoticerequired,category_type) values (?,?,?,?,?,?,?,?,?,?,?)"); - $sth->execute(map { $input->param($_) } ('categorycode','description','enrolmentperiod','enrolmentperioddate','upperagelimit','dateofbirthrequired','enrolmentfee','reservefee','hidelostitems','overduenoticerequired','category_type')); + my $sth=$dbh->prepare("INSERT INTO categories (categorycode,description,enrolmentperiod,enrolmentperioddate,upperagelimit,dateofbirthrequired,enrolmentfee,reservefee,hidelostitems,overduenoticerequired,category_type,checkprevissuebycategory) values (?,?,?,?,?,?,?,?,?,?,?,?)"); + $sth->execute(map { $input->param($_) } ('categorycode','description','enrolmentperiod','enrolmentperioddate','upperagelimit','dateofbirthrequired','enrolmentfee','reservefee','hidelostitems','overduenoticerequired','category_type','checkprevissue')); $sth->finish; } if (C4::Context->preference('EnhancedMessagingPreferences')) { @@ -199,7 +200,7 @@ if ($op eq 'add_form') { $sth->finish; $template->param(total => $total->{'total'}); - my $sth2=$dbh->prepare("select categorycode,description,enrolmentperiod,enrolmentperioddate,upperagelimit,dateofbirthrequired,enrolmentfee,issuelimit,reservefee,hidelostitems,overduenoticerequired,category_type from categories where categorycode=?"); + my $sth2=$dbh->prepare("select categorycode,description,enrolmentperiod,enrolmentperioddate,upperagelimit,dateofbirthrequired,enrolmentfee,issuelimit,reservefee,hidelostitems,overduenoticerequired,category_type,checkprevissuebycategory from categories where categorycode=?"); $sth2->execute($categorycode); my $data=$sth2->fetchrow_hashref; $sth2->finish; @@ -221,6 +222,7 @@ if ($op eq 'add_form') { reservefee => sprintf("%.2f",$data->{'reservefee'} || 0), hidelostitems => $data->{'hidelostitems'}, category_type => $data->{'category_type'}, + checkprevissue => $data->{'checkprevissuebycategory'}, ); # END $OP eq DELETE_CONFIRM ################## DELETE_CONFIRMED ################################## @@ -268,6 +270,7 @@ if ($op eq 'add_form') { category_type => $results->[$i]{'category_type'}, "type_".$results->[$i]{'category_type'} => 1, branches => \@selected_branches, + checkprevissue => $results->[$i]{'checkprevissuebycategory'}, ); if (C4::Context->preference('EnhancedMessagingPreferences')) { my $brief_prefs = _get_brief_messaging_prefs($results->[$i]{'categorycode'}); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index ea4f944..0a076ab 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 + `checkprevissuebyborrower` 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`), @@ -465,7 +466,8 @@ CREATE TABLE `categories` ( -- this table shows information related to Koha patr `issuelimit` smallint(6) default NULL, -- unused in Koha `reservefee` decimal(28,6) default NULL, -- cost to place holds `hidelostitems` tinyint(1) NOT NULL default '0', -- are lost items shown to this category (1 for yes, 0 for no) - `category_type` varchar(1) NOT NULL default 'A', -- type of Koha patron (Adult, Child, Professional, Organizational, Statistical, Staff) + `category_type` varchar(1) NOT NULL default 'A', -- type of Koha patron (Adult, Child, Professional, Organizational, Statistical, Staff), + `checkprevissuebycategory` 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 category 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 05a0031..3d95567 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -79,6 +79,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'), +('CheckPrevIssueByDefault','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 8917081..2fda934 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8202,6 +8202,16 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +# FIXME: change $DBversion +$DBversion = "3.15.00.036"; +if ( CheckVersion($DBversion) ) { + $dbh->do("INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES('CheckPrevIssueByDefault','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 (`checkprevissuebycategory` varchar(7) NOT NULL default 'inherit')"); + $dbh->do("ALTER TABLE borrowers ADD (`checkprevissuebyborrower` 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 c32094e..31f0844 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 ( checkprevissuebyborrower == 'yes' ) %] + + + + [% ELSIF (checkprevissuebyborrower == '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 9cd6a90..248b0b7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -384,6 +384,15 @@ function validate1(date) { [% IF ( borrowernotes ) %]
  • Circulation note: [% borrowernotes %]
  • [% END %] [% IF ( opacnote ) %]
  • OPAC note:[% opacnote %]
  • [% END %] +
  • Check previous loans: + [% IF ( checkprevissuebyborrower == 'yes' ) %] + Yes + [% ELSIF ( checkprevissuebyborrower == 'no' ) %] + No + [% ELSE %] + Inherited + [% END %] +
  • diff --git a/t/CheckPrevIssue.t b/t/CheckPrevIssue.t new file mode 100755 index 0000000..59e179d --- /dev/null +++ b/t/CheckPrevIssue.t @@ -0,0 +1,107 @@ +#!/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.checkprevissuebycategory FROM borrowers LEFT JOIN categories ON borrowers.categorycode=categories.categorycode WHERE borrowers.borrowernumber=?'; + +# Test if, as brw inherits, result is simply passed from +# _WantsCheckPrevIssueByCategory back through WantsCheckPrevIssue. +my %brw = ( + checkprevissuebyborrower => 'inherit', + borrowernumber => '101', +); + +$dbh->{mock_add_resultset} = + { + sql => $sql, + bound_params => [ '101' ], + results => [ [ 'categories.checkprevissuebycategory' ], [ '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 = ( + checkprevissuebyborrower => 'brw_pass_thru', + borrowernumber => '101', +); + +is( WantsCheckPrevIssue(\%brw), 'brw_pass_thru', + $name . ": Return value \"$brw{checkprevissuebyborrower}\"." ); + +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