From c59a5c3929ac7b7eff272a69c024c7e4794751e2 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 9 Apr 2015 13:07:05 +0200 Subject: [PATCH] [SIGNED-OFF] Bug 9942: Make Koha fails if privacy is not respected If a patron has requested anonymity on returning items and the system is not correctly configured (AnonymousPatron no set or set to an inexistent patron), the application should take it into account and not fail quietly. This patch is quite radical: the script will die loudly if the privacy is not respected. To be care of the bad "Software error", some checks are done in the updatedatabase to be sure the admin will be warned is something is wrong in the configuration. Test plan: 1/ Test the updatedatabase entry: a. Turn on OPACPrivacy and set AnonymousPatron to an existing patron => You will get a warning b. Turn on OPACPrivacy and set AnonymousPatron to 0 or '' => You will get a warning c. Turn on OPACPrivacy and set the privacy to 2 (Never) for at least 1 patron Turn off OPACPrivacy => You will get a warning d. In all other cases you will get no error 2/ Test the interface a. Turn on OPACPrivacy and set the privacy to 2 (Never) for a patron b. Now you can turn off OPACPrivacy or keep it on, behavior should be the same c. check an item out the patron d. Check the item in using the check out table => fail e. Check the item in using the Check in tab => fail (not gracefully). Note that the software error could appear on other pages too. Signed-off-by: Bernardo Gonzalez Kriegel Updatedatabase works as described On staff, if don't have correct settings for anonymity it's impossible to check-in (with OPACPrivacy on) No errors --- C4/Circulation.pm | 28 +++++++++--- installer/data/mysql/updatedatabase.pl | 34 +++++++++++++++ t/db_dependent/Circulation/MarkIssueReturned.t | 55 ++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 t/db_dependent/Circulation/MarkIssueReturned.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d825d81..89dbdcc 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1899,8 +1899,18 @@ sub AddReturn { } } - MarkIssueReturned( $borrowernumber, $item->{'itemnumber'}, - $circControlBranch, $return_date, $borrower->{'privacy'} ); + eval { + MarkIssueReturned( $borrowernumber, $item->{'itemnumber'}, + $circControlBranch, $return_date, $borrower->{'privacy'} ); + }; + if ( $@ ) { + $messages->{'Wrongbranch'} = { + Wrongbranch => $branch, + Rightbranch => $message + }; + carp $@; + return ( 0, { WasReturned => 0 }, $issue, $borrower ); + } # FIXME is the "= 1" right? This could be the borrower hash. $messages->{'WasReturned'} = 1; @@ -2075,6 +2085,16 @@ routine in C. sub MarkIssueReturned { my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_; + my $anonymouspatron; + if ( $privacy == 2 ) { + # The default of 0 will not work due to foreign key constraints + # The anonymisation will fail if AnonymousPatron is not a valid entry + # We need to check if the anonymous patron exist, Koha will fail loudly if it does not + # Note that a warning should appear on the about page (System information tab). + $anonymouspatron = C4::Context->preference('AnonymousPatron'); + die "Fatal error: the patron ($borrowernumber) has requested a privacy on returning item but the AnonymousPatron pref is not set correctly" + unless C4::Members::GetMember( borrowernumber => $anonymouspatron ); + } my $dbh = C4::Context->dbh; my $query = 'UPDATE issues SET returndate='; my @bind; @@ -2100,10 +2120,6 @@ sub MarkIssueReturned { $sth_copy->execute($borrowernumber, $itemnumber); # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber if ( $privacy == 2) { - # The default of 0 does not work due to foreign key constraints - # The anonymisation will fail quietly if AnonymousPatron is not a valid entry - # FIXME the above is unacceptable - bug 9942 relates - my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0; my $sth_ano = $dbh->prepare("UPDATE old_issues SET borrowernumber=? WHERE borrowernumber = ? AND itemnumber = ?"); diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index ee76d7d..b1a017d 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -10595,6 +10595,40 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.21.00.XXX"; +if ( CheckVersion($DBversion) ) { + my $msg; + if ( C4::Context->preference('OPACPrivacy') ) { + if ( my $anonymous_patron = C4::Context->preference('AnonymousPatron') ) { + my $anonymous_patron_exists = $dbh->selectcol_arrayref(q| + SELECT COUNT(*) + FROM borrowers + WHERE borrowernumber=? + |, {}, $anonymous_patron); + unless ( $anonymous_patron_exists->[0] ) { + $msg = "Configuration WARNING: OPACPrivacy is set but AnonymousPatron is not linked to an existing patron"; + } + } + else { + $msg = "Configuration WARNING: OPACPrivacy is set but AnonymousPatron is not"; + } + } + else { + my $patrons_have_required_anonymity = $dbh->selectcol_arrayref(q| + SELECT COUNT(*) + FROM borrowers + WHERE privacy = 2 + |, {} ); + if ( $patrons_have_required_anonymity->[0] ) { + $msg = "Configuration WARNING: OPACPrivacy is not set but $patrons_have_required_anonymity->[0] patrons have required anonymity (perhaps in a previous configuration). You should fix that asap."; + } + } + + $msg //= "Privacy is correctly set"; + print "Upgrade to $DBversion done (Bug 9942: $msg)\n"; + SetVersion ($DBversion); +} + # DEVELOPER PROCESS, search for anything to execute in the db_update directory # SEE bug 13068 # if there is anything in the atomicupdate, read and execute it. diff --git a/t/db_dependent/Circulation/MarkIssueReturned.t b/t/db_dependent/Circulation/MarkIssueReturned.t new file mode 100644 index 0000000..1409fc7 --- /dev/null +++ b/t/db_dependent/Circulation/MarkIssueReturned.t @@ -0,0 +1,55 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# 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 Test::More tests => 2; +use Test::Warn; + +use C4::Branch; +use C4::Circulation; +use C4::Members; +use t::lib::Mocks; + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +t::lib::Mocks::mock_preference('AnonymousPatron', ''); + +my $branchcode = 'B'; +ModBranch({ add => 1, branchcode => $branchcode, branchname => 'Branch' }); + +my $categorycode = 'C'; +$dbh->do("INSERT INTO categories(categorycode) VALUES(?)", undef, $categorycode); + +my %item_branch_infos = ( + homebranch => $branchcode, + holdingbranch => $branchcode, +); + +my $borrowernumber = AddMember( categorycode => $categorycode, branchcode => $branchcode ); + +eval { C4::Circulation::MarkIssueReturned( $borrowernumber, 'itemnumber', 'dropbox_branch', 'returndate', 2 ) }; +like ( $@, qr, ); + +my $anonymous_borrowernumber = AddMember( categorycode => $categorycode, branchcode => $branchcode ); +t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous_borrowernumber); +# The next call will raise an error, because data are not correctly set +$dbh->{PrintError} = 0; +eval { C4::Circulation::MarkIssueReturned( $borrowernumber, 'itemnumber', 'dropbox_branch', 'returndate', 2 ) }; +unlike ( $@, qr, ); -- 1.7.9.5