From 2ccce4c8dd1a6df4c34621ca237d8a2e2d5d98a9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 22 Jul 2016 17:25:24 +0100 Subject: [PATCH] Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed to return the patrons with an issue history older than a given date. It would make more sense to return a list of Koha::Patrons. On the way, the code from AnonymiseIssueHistory will be moved as well to anonymise_issue_history. Note that these 2 subroutines are strongly linked: one is used to know the number of patrons we will anonymise the history, the other one is used to anonymise the issues history. The problem is that the first one is not used to do the action, but only for displayed purpose. In some cases, these 2 values can differ, which could be confusing. Case 1: The logged in librarian is not superlibrarian and IndependentBranches is set: if 2+ patrons from different libraries match the date parameter, the interface will display "Checkout history for 2 patrons will be anonymized", when actually only 1 will be. Case 2: If 2+ patrons match the date parameter but one of them has his privacy set to forever (privacy=0), the same issue will appear. This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons->search_patrons_to_anonymise and from C4::Circulation::AnonymiseIssueHistory to Koha::Patrons->anonymise_issue_history Test plan: 1/ Confirm the 2 issues and make sure they are fixed using the Batch patron anonymization tool (tools/cleanborrowers.pl) 2/ At the OPAC, use the 'Immediate deletion' button to delete all your reading history (regardless the setting of the privacy rule) 3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to anonymise patrons. --- C4/Circulation.pm | 44 ---------------- C4/Members.pm | 46 ----------------- Koha/Patrons.pm | 59 +++++++++++++++++++++- .../prog/en/modules/tools/cleanborrowers.tt | 4 +- misc/cronjobs/batch_anonymise.pl | 7 ++- opac/opac-privacy.pl | 13 +---- tools/cleanborrowers.pl | 15 +++--- 7 files changed, 72 insertions(+), 116 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 9b95bbf..e67495a 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -97,7 +97,6 @@ BEGIN { &GetBranchItemRule &GetBiblioIssues &GetOpenIssue - &AnonymiseIssueHistory &CheckIfIssuedToPatron &IsItemIssued GetTopIssues @@ -3444,49 +3443,6 @@ sub DeleteTransfer { return $sth->execute($itemnumber); } -=head2 AnonymiseIssueHistory - - ($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>. - -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 and a value that evaluates to true if an error occurred deleting the history. - -=cut - -sub AnonymiseIssueHistory { - my $date = shift; - my $borrowernumber = shift; - my $dbh = C4::Context->dbh; - my $query = " - UPDATE old_issues - SET borrowernumber = ? - WHERE returndate < ? - AND borrowernumber IS NOT NULL - "; - - # The default of 0 does not work due to foreign key constraints - # The anonymisation should not fail quietly if AnonymousPatron is not a valid entry - # Set it to undef (NULL) - my $anonymouspatron = C4::Context->preference('AnonymousPatron') || undef; - my @bind_params = ($anonymouspatron, $date); - if (defined $borrowernumber) { - $query .= " AND borrowernumber = ?"; - push @bind_params, $borrowernumber; - } else { - $query .= " AND (SELECT privacy FROM borrowers WHERE borrowers.borrowernumber=old_issues.borrowernumber) <> 0"; - } - 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, $anonymisation_err); -} - =head2 SendCirculationAlert Send out a C or C alert using the messaging system. diff --git a/C4/Members.pm b/C4/Members.pm index 63e3f36..a0a421c 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -86,7 +86,6 @@ BEGIN { &GetBorrowersToExpunge &GetBorrowersWhoHaveNeverBorrowed - &GetBorrowersWithIssuesHistoryOlderThan &GetExpiryDate &GetUpcomingMembershipExpires @@ -1815,51 +1814,6 @@ sub GetBorrowersWhoHaveNeverBorrowed { return \@results; } -=head2 GetBorrowersWithIssuesHistoryOlderThan - - $results = &GetBorrowersWithIssuesHistoryOlderThan($date) - -this function get all borrowers who has an issue history older than I<$date> given on input arg. - -I<$result> is a ref to an array which all elements are a hashref. -This hashref is containt the number of time this borrowers has borrowed before I<$date> and the borrowernumber. - -=cut - -sub GetBorrowersWithIssuesHistoryOlderThan { - my $dbh = C4::Context->dbh; - my $date = shift ||POSIX::strftime("%Y-%m-%d",localtime()); - my $filterbranch = shift || - ((C4::Context->preference('IndependentBranches') - && C4::Context->userenv - && !C4::Context->IsSuperLibrarian() - && C4::Context->userenv->{branch}) - ? C4::Context->userenv->{branch} - : ""); - my $query = " - SELECT count(borrowernumber) as n,borrowernumber - FROM old_issues - WHERE returndate < ? - AND borrowernumber IS NOT NULL - "; - my @query_params; - push @query_params, $date; - if ($filterbranch){ - $query.=" AND branchcode = ?"; - push @query_params, $filterbranch; - } - $query.=" GROUP BY borrowernumber "; - warn $query if $debug; - my $sth = $dbh->prepare($query); - $sth->execute(@query_params); - my @results; - - while ( my $data = $sth->fetchrow_hashref ) { - push @results, $data; - } - return \@results; -} - =head2 IssueSlip IssueSlip($branchcode, $borrowernumber, $quickslip) diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index e001b84..7566e61 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -1,6 +1,7 @@ package Koha::Patrons; -# Copyright ByWater Solutions 2014 +# Copyright 2014 ByWater Solutions +# Copyright 2016 Koha Development Team # # This file is part of Koha. # @@ -22,6 +23,7 @@ use Modern::Perl; use Carp; use Koha::Database; +use Koha::DateUtils; use Koha::Patron; @@ -37,6 +39,61 @@ Koha::Patron - Koha Patron Object class =cut +=head3 search_patrons_to_anonymise + + my $patrons = Koha::Patrons->search_patrons_to_anonymise( $date ); + +This method returns all patrons who has an issue history older than a given date. + +=cut + +sub search_patrons_to_anonymise { + my ( $class, $older_than_date ) = @_; + $older_than_date = $older_than_date ? dt_from_string($older_than_date) : dt_from_string; + my $library = + ( C4::Context->preference('IndependentBranches') && C4::Context->userenv && !C4::Context->IsSuperLibrarian() && C4::Context->userenv->{branch} ) + ? C4::Context->userenv->{branch} + : undef; + + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + my $rs = $class->search( + { returndate => { '<' => $dtf->format_datetime($older_than_date), }, + 'old_issues.borrowernumber' => { 'not' => undef }, + privacy => { '<>' => 0 }, # Keep forever + ( $library ? ( 'old_issues.branchcode' => $library ) : () ), + }, + { join => ["old_issues"], + group_by => 'borrowernumber' + } + ); + return Koha::Patrons->_new_from_dbic($rs); +} + +=head3 anonymise_issue_history + + Koha::Patrons->search->anonymise_issue_history( $older_than_date ); + +Anonymise issue history (old_issues) for all patrons older than the given date. +To make sure all the conditions are met, the caller has the responsability to +call search_patrons_to_anonymise to filter the Koha::Patrons set + +=cut + +sub anonymise_issue_history { + my ( $self, $older_than_date ) = @_; + + return unless $older_than_date; + $older_than_date = dt_from_string $older_than_date; + + # The default of 0 does not work due to foreign key constraints + # The anonymisation should not fail quietly if AnonymousPatron is not a valid entry + # Set it to undef (NULL) + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + my $old_issues_to_anonymise = $self->search_related( 'old_issues', { returndate => { '<' => $dtf->format_datetime($older_than_date) } } ); + my $anonymous_patron = C4::Context->preference('AnonymousPatron') || undef; + $old_issues_to_anonymise->update( { 'old_issues.borrowernumber' => $anonymous_patron } ); +} + =head3 type =cut diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt index 9e68141..4ce84eb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt @@ -187,8 +187,8 @@

No patron records have been removed

[% END %] [% END %] - [% IF ( do_anonym ) %] -

All checkouts older than [% last_issue_date | $KohaDates %] have been anonymized

+ [% IF do_anonym %] +

All checkouts ([% do_anonym %]) older than [% last_issue_date | $KohaDates %] have been anonymized

[% ELSE %]

No patron records have been anonymized

[% END %] diff --git a/misc/cronjobs/batch_anonymise.pl b/misc/cronjobs/batch_anonymise.pl index c90842f..7197603 100755 --- a/misc/cronjobs/batch_anonymise.pl +++ b/misc/cronjobs/batch_anonymise.pl @@ -30,7 +30,7 @@ BEGIN { } use C4::Context; -use C4::Circulation; +use Koha::Patrons; use Date::Calc qw( Today Add_Delta_Days @@ -74,8 +74,7 @@ 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, $err_history_not_deleted) = AnonymiseIssueHistory($formatdate); -carp "Anonymisation of reading history failed." if ($err_history_not_deleted); -$verbose and print "$rows checkouts anonymised.\n"; +my $rows = Koha::Patrons->search_patrons_to_anonymise( $formatdate )->anonymise_issue_history( $formatdate ); +$verbose and print int($rows) . " checkouts anonymised.\n"; exit(0); diff --git a/opac/opac-privacy.pl b/opac/opac-privacy.pl index 969c46a..5658521 100755 --- a/opac/opac-privacy.pl +++ b/opac/opac-privacy.pl @@ -21,7 +21,6 @@ use CGI qw ( -utf8 ); use C4::Auth; # checkauth, getborrowernumber. use C4::Context; -use C4::Circulation; use C4::Members; use C4::Output; use Koha::Patrons; @@ -62,16 +61,8 @@ elsif ( $op eq "delete_record" ) { # delete all reading records for items returned # uses a hardcoded date ridiculously far in the future - my ( $rows, $err_history_not_deleted ) = - AnonymiseIssueHistory( '2999-12-12', $borrowernumber ); - - # confirm the user the deletion has been done - if ( !$err_history_not_deleted ) { - $template->param( 'deleted' => 1 ); - } - else { - $template->param( 'err_history_not_deleted' => 1 ); - } + my $rows = Koha::Patrons->search({ borrowernumber => $borrowernumber })->anonymise_issue_history( '2999-12-12' ); + $template->param( 'deleted' => $rows ); } # get borrower privacy .... diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl index d108d53..1ba6f71 100755 --- a/tools/cleanborrowers.pl +++ b/tools/cleanborrowers.pl @@ -38,9 +38,9 @@ use CGI qw ( -utf8 ); use C4::Auth; use C4::Output; use C4::Members; # GetBorrowersWhoHavexxxBorrowed. -use C4::Circulation; # AnonymiseIssueHistory. use Koha::DateUtils qw( dt_from_string output_pref ); use Date::Calc qw/Today Add_Delta_YM/; +use Koha::Patrons; use Koha::List::Patron; my $cgi = new CGI; @@ -95,14 +95,13 @@ if ( $step == 2 ) { } _skip_borrowers_with_nonzero_balance($patrons_to_delete); - my $members_to_anonymize; - if ( $checkboxes{issue} ) { - $members_to_anonymize = GetBorrowersWithIssuesHistoryOlderThan($last_issue_date); - } + my $patrons_to_anonymize = $checkboxes{issue} + ? Koha::Patrons->search_patrons_to_anonymise( $last_issue_date ) + : undef; $template->param( patrons_to_delete => $patrons_to_delete, - patrons_to_anonymize => $members_to_anonymize, + patrons_to_anonymize => $patrons_to_anonymize, patron_list_id => $patron_list_id, ); } @@ -141,9 +140,9 @@ elsif ( $step == 3 ) { # Anonymising all members if ($do_anonym) { #FIXME: anonymisation errors are not handled - ($totalAno,my $anonymisation_error) = AnonymiseIssueHistory($last_issue_date); + my $rows = Koha::Patrons->search_patrons_to_anonymise( $last_issue_date )->anonymise_issue_history( $last_issue_date ); $template->param( - do_anonym => '1', + do_anonym => $rows, ); } -- 2.8.1