@@ -, +, @@ respected --- 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 --- a/C4/Circulation.pm +++ a/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 = ?"); --- a/installer/data/mysql/updatedatabase.pl +++ a/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. --- a/t/db_dependent/Circulation/MarkIssueReturned.t +++ a/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, ); --