From c152484d3ff1a8fa9bc09ed51dde3a5892ba814c Mon Sep 17 00:00:00 2001 From: Peter Crellan Kelly Date: Thu, 28 Mar 2013 21:00:47 +1300 Subject: [PATCH] Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed. Remedied by: - in Circulation.pm changing AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows - consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value, and fail if there is an error - consequential change to tools/cleanborrowers.pl although this still fails silently (raised as bug 9944) - update of opac-privacy.pl to check return value and pass on error - update of opac-privacy.tt to display error if appropriate Note bug 9942 remains unfixed, which is a similar issue upon issue return. To test: 1. OPAC - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - attempt to delete user history - observe error - check history - still there - change anonymous patron to a valid user - attempt to delete user history - observe success message - check history - gone 2. cleanborrowers.pl - test it functions as before. bug 9944 has been raised for it continuing to silently fail. 3. batch_anonymise.pl - enable privacy mode (preference OpacPrivacy) - leave anonymous patron set to zero (preference AnonymousPatron) - run script (I use --days -1 for testing) - script should fail with a Carp message - change anonymous patron to a valid user - run script as before - script returns quietly - check history - gone Signed-off-by: Mason James Signed-off-by: Kyle M Hall --- C4/Circulation.pm | 8 +++++--- .../opac-tmpl/prog/en/modules/opac-privacy.tt | 4 +++- misc/cronjobs/batch_anonymise.pl | 4 +++- opac/opac-privacy.pl | 8 ++++++-- tools/cleanborrowers.pl | 3 ++- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 3fc124f..1de40e8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1967,6 +1967,7 @@ sub MarkIssueReturned { 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 = ? @@ -2821,7 +2822,7 @@ sub DeleteTransfer { =head2 AnonymiseIssueHistory - $rows = AnonymiseIssueHistory($date,$borrowernumber) + ($rows,$err_history_not_deleted) = AnonymiseIssueHistory($date,$borrowernumber) This function write NULL instead of C<$borrowernumber> given on input arg into the table issues. if C<$borrowernumber> is not set, it will delete the issue history for all borrower older than C<$date>. @@ -2829,7 +2830,7 @@ if C<$borrowernumber> is not set, it will delete the issue history for all borro If c<$borrowernumber> is set, it will delete issue history for only that borrower, regardless of their opac privacy setting (force delete). -return the number of affected rows. +return the number of affected rows and a value that evaluates to true if an error occurred deleting the history. =cut @@ -2856,8 +2857,9 @@ sub AnonymiseIssueHistory { } my $sth = $dbh->prepare($query); $sth->execute(@bind_params); + my $anonymisation_err = $dbh->err; my $rows_affected = $sth->rows; ### doublecheck row count return function - return $rows_affected; + return ($rows_affected, $anonymisation_err); } =head2 SendCirculationAlert diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-privacy.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-privacy.tt index 5245fdf..233222b 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-privacy.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-privacy.tt @@ -13,9 +13,11 @@ [% IF ( deleted ) %]
Your reading history has been deleted.
+ [% ELSIF ( err_history_not_deleted ) %] +
The deletion of your reading history failed, because there is a problem with the configuration of this feature. Please help to fix the system by informing your library of this error.
[% END %] [% IF ( privacy_updated ) %] -
Your privacy rules have been updated
+
Your privacy rules have been updated.
[% END %]

Privacy rule

diff --git a/misc/cronjobs/batch_anonymise.pl b/misc/cronjobs/batch_anonymise.pl index 857171b..48a58c6 100755 --- a/misc/cronjobs/batch_anonymise.pl +++ b/misc/cronjobs/batch_anonymise.pl @@ -19,6 +19,7 @@ use strict; use warnings; +use Carp; BEGIN { @@ -70,7 +71,8 @@ my ($newyear,$newmonth,$newday) = Add_Delta_Days ($year,$month,$day,(-1)*$days); my $formatdate = sprintf "%4d-%02d-%02d",$newyear,$newmonth,$newday; $verbose and print "Checkouts before $formatdate will be anonymised.\n"; -my $rows = AnonymiseIssueHistory($formatdate); +my ($rows, $err_history_not_deleted) = AnonymiseIssueHistory($formatdate); +carp "Anonymisation of reading history failed." if ($err_history_not_deleted); $verbose and print "$rows checkouts anonymised.\n"; exit(0); diff --git a/opac/opac-privacy.pl b/opac/opac-privacy.pl index bbfb8fc..1be8a8f 100755 --- a/opac/opac-privacy.pl +++ b/opac/opac-privacy.pl @@ -51,9 +51,13 @@ if ($op eq "update_privacy") if ($op eq "delete_record") { # delete all reading records for items returned # uses a hardcoded date ridiculously far in the future - AnonymiseIssueHistory('2999-12-12',$borrowernumber); + my ($rows,$err_history_not_deleted) = AnonymiseIssueHistory('2999-12-12',$borrowernumber); # confirm the user the deletion has been done - $template->param('deleted' => 1); + if (! $err_history_not_deleted) { + $template->param('deleted' => 1); + } else { + $template->param('err_history_not_deleted' => 1); + } } # get borrower privacy .... my ( $borr ) = GetMemberDetails( $borrowernumber ); diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl index 65ce813..0d48b99 100755 --- a/tools/cleanborrowers.pl +++ b/tools/cleanborrowers.pl @@ -145,7 +145,8 @@ if ( $params->{'step3'} ) { # Anonymising all members if ($do_anonym) { - $totalAno = AnonymiseIssueHistory($filterdate2); + #FIXME: anonymisation errors are not handled + ($totalAno,my $anonymisation_error) = AnonymiseIssueHistory($filterdate2); $template->param( filterdate1 => $filterdate2, do_anonym => '1', -- 1.7.2.5