Bug 18169: Make 'before' param non mandatory for Koha::Patrons->anonymise_issue_history

From opac-privacy.pl:

    # delete all reading records for items returned
    # uses a hardcoded date ridiculously far in the future

    my $rows = eval {
        Koha::Patrons->search({ 'me.borrowernumber' => $borrowernumber })->anonymise_issue_history( { before => '2999-12-12' } );
    };

It sounds better to make this before parameter not mandatory, and remove the condition from the sql query if it is not passed.

Test plan:
1. Anonymise your reading history at the OPAC.
2. Confirm that all your reading history has been anonymised

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Jonathan Druart 2017-02-24 13:26:29 +01:00 committed by Kyle M Hall
parent b6cb18c4a8
commit ff647be07c
3 changed files with 51 additions and 18 deletions

View File

@ -216,9 +216,9 @@ sub search_patrons_to_anonymise {
=head3 anonymise_issue_history
Koha::Patrons->search->anonymise_issue_history( { before => $older_than_date } );
Koha::Patrons->search->anonymise_issue_history( { [ before => $older_than_date ] } );
Anonymise issue history (old_issues) for all patrons older than the given date.
Anonymise issue history (old_issues) for all patrons older than the given date (optional).
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
@ -229,14 +229,23 @@ sub anonymise_issue_history {
my $older_than_date = $params->{before};
return unless $older_than_date;
$older_than_date = dt_from_string $older_than_date;
$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 $old_issues_to_anonymise = $self->search_related( 'old_issues', { returndate => { '<' => $dtf->format_datetime($older_than_date) } } );
my $old_issues_to_anonymise = $self->search_related(
'old_issues',
{
(
$older_than_date
? ( 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 } );
}

View File

@ -60,10 +60,8 @@ if ( $op eq "update_privacy" ) {
elsif ( $op eq "delete_record" ) {
# delete all reading records for items returned
# uses a hardcoded date ridiculously far in the future
my $rows = eval {
Koha::Patrons->search({ 'me.borrowernumber' => $borrowernumber })->anonymise_issue_history( { before => '2999-12-12' } );
Koha::Patrons->search({ 'me.borrowernumber' => $borrowernumber })->anonymise_issue_history;
};
$rows = $@ ? 0 : int($rows);
$template->param( 'deleted' => $rows );

View File

@ -688,7 +688,7 @@ subtest 'search_patrons_to_anonymise & anonymise_issue_history' => sub {
t::lib::Mocks::mock_preference( 'AnonymousPatron', $anonymous->{borrowernumber} );
subtest 'patron privacy is 1 (default)' => sub {
plan tests => 4;
plan tests => 6;
t::lib::Mocks::mock_preference('IndependentBranches', 0);
my $patron = $builder->build(
@ -696,7 +696,7 @@ subtest 'search_patrons_to_anonymise & anonymise_issue_history' => sub {
value => { privacy => 1, }
}
);
my $item = $builder->build(
my $item_1 = $builder->build(
{ source => 'Item',
value => {
itemlost => 0,
@ -704,29 +704,55 @@ subtest 'search_patrons_to_anonymise & anonymise_issue_history' => sub {
},
}
);
my $issue = $builder->build(
my $issue_1 = $builder->build(
{ source => 'Issue',
value => {
borrowernumber => $patron->{borrowernumber},
itemnumber => $item->{itemnumber},
itemnumber => $item_1->{itemnumber},
},
}
);
my $item_2 = $builder->build(
{ source => 'Item',
value => {
itemlost => 0,
withdrawn => 0,
},
}
);
my $issue_2 = $builder->build(
{ source => 'Issue',
value => {
borrowernumber => $patron->{borrowernumber},
itemnumber => $item_2->{itemnumber},
},
}
);
my ( $returned, undef, undef ) = C4::Circulation::AddReturn( $item->{barcode}, undef, undef, undef, '2010-10-10' );
is( $returned, 1, 'The item should have been returned' );
my ( $returned_1, undef, undef ) = C4::Circulation::AddReturn( $item_1->{barcode}, undef, undef, undef, '2010-10-10' );
my ( $returned_2, undef, undef ) = C4::Circulation::AddReturn( $item_2->{barcode}, undef, undef, undef, '2011-11-11' );
is( $returned_1 && $returned_2, 1, 'The items should have been returned' );
my $patrons_to_anonymise = Koha::Patrons->search_patrons_to_anonymise( { before => '2010-10-11' } )->search( { 'me.borrowernumber' => $patron->{borrowernumber} } );
is( ref($patrons_to_anonymise), 'Koha::Patrons', 'search_patrons_to_anonymise should return Koha::Patrons' );
my $rows_affected = Koha::Patrons->search_patrons_to_anonymise( { before => '2010-10-11' } )->anonymise_issue_history( { before => '2010-10-11' } );
my $rows_affected = Koha::Patrons->search_patrons_to_anonymise( { before => '2011-11-12' } )->anonymise_issue_history( { before => '2010-10-11' } );
ok( $rows_affected > 0, 'AnonymiseIssueHistory should affect at least 1 row' );
my $dbh = C4::Context->dbh;
my ($borrowernumber_used_to_anonymised) = $dbh->selectrow_array(q|
SELECT borrowernumber FROM old_issues where itemnumber = ?
|, undef, $item->{itemnumber});
my $sth = $dbh->prepare(q|SELECT borrowernumber FROM old_issues where itemnumber = ?|);
$sth->execute($item_1->{itemnumber});
my ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array;
is( $borrowernumber_used_to_anonymised, $anonymous->{borrowernumber}, 'With privacy=1, the issue should have been anonymised' );
$sth->execute($item_2->{itemnumber});
($borrowernumber_used_to_anonymised) = $sth->fetchrow_array;
is( $borrowernumber_used_to_anonymised, $patron->{borrowernumber}, 'The issue should not have been anonymised, the returned date is later' );
$rows_affected = Koha::Patrons->search_patrons_to_anonymise( { before => '2011-11-12' } )->anonymise_issue_history;
$sth->execute($item_2->{itemnumber});
($borrowernumber_used_to_anonymised) = $sth->fetchrow_array;
is( $borrowernumber_used_to_anonymised, $anonymous->{borrowernumber}, 'The issue should have been anonymised, the returned date is before' );
Koha::Patrons->find( $patron->{borrowernumber})->delete;
};