From 4c80d4c8d5549ce5bfb3a2b13b893862448fa890 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 3 Jan 2020 12:35:33 +0000 Subject: [PATCH] Bug 23260: Inline code to anonymise_issue_history This alternative patch reduces the complexity of the change by simply updateing the existing anonymise_issue_history method to include anonymisation of the items_last_borrowers table. To test: 1) apply this patch and the previous one 2) perl installer/data/mysql/updatedatabase.pl 3) set StoreLastBorrower preference to Allow. 4) set AnonymousPatron preference to a valid patron id 5) Create a Check out followed by a Check in. CHECK => a row should appear in items_last_borrower table with the borrower and the item number. 6) In mysql, update created_on of items_last_borrower and returndate of old_issues to two days earlier. 7) perl misc/cronjobs/batch_anonymise.pl --days 1 SUCCESS => borrower number in items_last_borrower changed to AnonymousPatron id 8) repeat steps 5 and 6 9) set AnonymousPatron preference to 0 10) perl misc/cronjobs/batch_anonymise.pl --days 1 SUCCESS => borrower number in items_last_borrower changed to null 11) prove t/db_dependent/Koha/Patrons.t 12) Sign off Signed-off-by: Agustin Moyano --- Koha/Patrons.pm | 61 ++++++++++++++++++ t/db_dependent/Koha/Patrons.t | 116 ++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+) diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 92d3918363..e64c394fe8 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -158,6 +158,67 @@ sub search_patrons_to_anonymise { return Koha::Patrons->_new_from_dbic($rs); } +=head3 anonymise_issue_history + + Koha::Patrons->search->anonymise_issue_history( { [ before => $older_than_date ] } ); + +Anonymise issue history (old_issues and items_last_borrowers) for all issues older +than the given date (optional). + +To make sure all the conditions are met, the caller has the responsibility to +call search_patrons_to_anonymise to filter the Koha::Patrons set. + +=cut + +sub anonymise_issue_history { + my ( $self, $params ) = @_; + + my $older_than_date = $params->{before}; + + $older_than_date = dt_from_string $older_than_date if $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 $nb_rows = 0; + while ( my $patron = $self->next ) { + my $old_issues_to_anonymise = $patron->old_checkouts->search( + { + ( + $older_than_date + ? ( returndate => + { '<' => $dtf->format_datetime($older_than_date) } ) + : () + ) + } + ); + + my $last_borrowers_to_anonymise = + $patron->_result->items_last_borrowers->search( + { + ( + $older_than_date + ? ( created_on => + { '<' => $dtf->format_datetime($older_than_date) } ) + : (), + "itemnumber.damaged" => 0, + "itemnumber.itemlost" => 0, + "itemnumber.withdrawn" => 0, + ) + }, + { + join => ['itemnumber'] + } + ); + + my $anonymous_patron = C4::Context->preference('AnonymousPatron') || undef; + $nb_rows += $old_issues_to_anonymise->update( { 'old_issues.borrowernumber' => $anonymous_patron } ); + $nb_rows += $last_borrowers_to_anonymise->update( { 'items_last_borrower.borrowernumber' => $anonymous_patron } ); + } + return $nb_rows; +} + =head3 delete Koha::Patrons->search({ some filters here })->delete({ move => 1 }); diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 24d4f7a811..e25468b0b1 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1284,6 +1284,122 @@ subtest 'search_patrons_to_anonymise' => sub { t::lib::Mocks::mock_preference('IndependentBranches', 0); }; +subtest 'anonymise items_last_borrower' => sub { + plan tests => 1; + + # TODO create a subroutine in t::lib::Mocks + my $anonymous = $builder->build( { source => 'Borrower', }, ); + + t::lib::Mocks::mock_preference( 'AnonymousPatron', $anonymous->{borrowernumber} ); + t::lib::Mocks::mock_preference('StoreLastBorrower', 1); + + subtest 'withrawn, lost and damaged items' => sub { + plan tests => 5; + + my $patron = $builder->build( + { source => 'Borrower', + value => { privacy => 1, } + } + ); + my $item_1 = $builder->build_object( + { class => 'Koha::Items', + value => { + itemlost => 0, + withdrawn => 0, + damaged => 0, + }, + } + ); + my $issue_1 = $builder->build( + { source => 'Issue', + value => { + borrowernumber => $patron->{borrowernumber}, + itemnumber => $item_1->itemnumber, + }, + } + ); + my $item_2 = $builder->build_object( + { class => 'Koha::Items', + value => { + itemlost => 0, + withdrawn => 0, + damaged => 0, + }, + } + ); + my $issue_2 = $builder->build( + { source => 'Issue', + value => { + borrowernumber => $patron->{borrowernumber}, + itemnumber => $item_2->itemnumber, + }, + } + ); + my $item_3 = $builder->build_object( + { class => 'Koha::Items', + value => { + itemlost => 0, + withdrawn => 0, + damaged => 0, + }, + } + ); + my $issue_3 = $builder->build( + { source => 'Issue', + value => { + borrowernumber => $patron->{borrowernumber}, + itemnumber => $item_3->itemnumber, + }, + } + ); + my $item_4 = $builder->build_object( + { class => 'Koha::Items', + value => { + itemlost => 0, + withdrawn => 0, + damaged => 0, + }, + } + ); + my $issue_4 = $builder->build( + { source => 'Issue', + value => { + borrowernumber => $patron->{borrowernumber}, + itemnumber => $item_4->itemnumber, + }, + } + ); + + my ( $returned_1, undef, undef ) = C4::Circulation::AddReturn( $item_1->barcode, undef, undef, dt_from_string('2010-10-11') ); + my ( $returned_2, undef, undef ) = C4::Circulation::AddReturn( $item_2->barcode, undef, undef, dt_from_string('2010-10-11') ); + my ( $returned_3, undef, undef ) = C4::Circulation::AddReturn( $item_3->barcode, undef, undef, dt_from_string('2010-10-11') ); + my ( $returned_4, undef, undef ) = C4::Circulation::AddReturn( $item_4->barcode, undef, undef, dt_from_string('2010-10-11') ); + is( $returned_1 && $returned_2 && $returned_3 && $returned_4, 1, 'The items should have been returned' ); + $item_1->withdrawn(1)->store; + $item_2->itemlost(1)->store; + $item_3->damaged(1)->store; + + Koha::Patrons->search_patrons_to_anonymise( { before => '2010-10-12' } )->anonymise_issue_history(); + + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare(q|SELECT borrowernumber FROM items_last_borrower where itemnumber = ?|); + $sth->execute($item_1->itemnumber); + my ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array; + is( $borrowernumber_used_to_anonymised, $patron->{borrowernumber}, 'withrawn items should not be anonymised' ); + $sth->execute($item_2->itemnumber); + ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array; + is( $borrowernumber_used_to_anonymised, $patron->{borrowernumber}, 'lost items should not be anonymised' ); + $sth->execute($item_3->itemnumber); + ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array; + is( $borrowernumber_used_to_anonymised, $patron->{borrowernumber}, 'damaged items should not be anonymised' ); + $sth->execute($item_4->itemnumber); + ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array; + is( $borrowernumber_used_to_anonymised, $anonymous->{borrowernumber}, 'not withdrawn, lost or damaged items are anonymised' ); + + }; + +}; + subtest 'libraries_where_can_see_patrons + can_see_patron_infos + search_limited' => sub { plan tests => 3; -- 2.30.2