Bugzilla – Attachment 37617 Details for
Bug 9942
Circulation module cannot anonymise if anonymity user is not set up correctly.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9942: Make Koha fails if privacy is not respected
Bug-9942-Make-Koha-fails-if-privacy-is-not-respect.patch (text/plain), 8.40 KB, created by
Jonathan Druart
on 2015-04-09 12:01:37 UTC
(
hide
)
Description:
Bug 9942: Make Koha fails if privacy is not respected
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2015-04-09 12:01:37 UTC
Size:
8.40 KB
patch
obsolete
>From ef6b59f37bb1a19042661c5e906949e46034d063 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Thu, 9 Apr 2015 13:07:05 +0200 >Subject: [PATCH] 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. >--- > C4/Circulation.pm | 28 ++++++++++--- > installer/data/mysql/updatedatabase.pl | 33 ++++++++++++++++ > t/db_dependent/Circulation/MarkIssueReturned.t | 55 ++++++++++++++++++++++++++ > 3 files changed, 110 insertions(+), 6 deletions(-) > create mode 100644 t/db_dependent/Circulation/MarkIssueReturned.t > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index dc828da..6de94b2 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1892,8 +1892,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; >@@ -2071,6 +2081,16 @@ routine in C<C4::Accounts>. > 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; >@@ -2096,10 +2116,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 78e79d3..59ce7b3 100755 >--- a/installer/data/mysql/updatedatabase.pl >+++ b/installer/data/mysql/updatedatabase.pl >@@ -9978,6 +9978,39 @@ if ( CheckVersion($DBversion) ) { > SetVersion($DBversion); > } > >+$DBversion = "3.19.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 >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 <http://www.gnu.org/licenses>. >+ >+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<Fatal error: the patron \(\d+\) .* AnonymousPatron>, ); >+ >+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<Fatal error: the patron \(\d+\) .* AnonymousPatron>, ); >-- >2.1.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9942
:
37617
|
40394
|
40682
|
40683