Bugzilla – Attachment 17113 Details for
Bug 6506
deleting patron history missing errors
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed.
Bug-6506-When-AnonymousPatron-not-set-deletion-of-.patch (text/plain), 6.87 KB, created by
Mason James
on 2013-04-02 08:46:53 UTC
(
hide
)
Description:
Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed.
Filename:
MIME Type:
Creator:
Mason James
Created:
2013-04-02 08:46:53 UTC
Size:
6.87 KB
patch
obsolete
>From 5c63820469cbd495b90fe5d10e343b41b9ed271c Mon Sep 17 00:00:00 2001 >From: Peter Crellan Kelly <peter@catalyst.net.nz> >Date: Thu, 28 Mar 2013 21:00:47 +1300 >Subject: [PATCH] Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed. >Content-Type: text/plain; charset="utf-8" > >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 <mtj@kohaaloha.com> >--- > 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 f3c5808..85c417b 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 ) %] > <div class="dialog message">Your reading history has been deleted.</div> >+ [% ELSIF ( err_history_not_deleted ) %] >+ <div class="dialog error">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.</div> > [% END %] > [% IF ( privacy_updated ) %] >- <div class="dialog message">Your privacy rules have been updated</div> >+ <div class="dialog message">Your privacy rules have been updated.</div> > [% END %] > > <h2>Privacy rule</h2> >diff --git a/misc/cronjobs/batch_anonymise.pl b/misc/cronjobs/batch_anonymise.pl >index 857171b..c2ce0f4 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..e7dfe4b 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
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 6506
:
16955
|
17113
|
17305
|
17317
|
17385
|
17386
|
17387
|
17753
|
17754
|
17755