Bugzilla – Attachment 60713 Details for
Bug 18169
Date like 2999 should not be used arbitrarily
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18169: Make 'before' param non mandatory for Koha::Patrons->anonymise_issue_history
Bug-18169-Make-before-param-non-mandatory-for-Koha.patch (text/plain), 8.04 KB, created by
Marcel de Rooy
on 2017-02-27 13:52:49 UTC
(
hide
)
Description:
Bug 18169: Make 'before' param non mandatory for Koha::Patrons->anonymise_issue_history
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2017-02-27 13:52:49 UTC
Size:
8.04 KB
patch
obsolete
>From e0b65bf159019b0618cf6c4bd66836156f35bb4a Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 24 Feb 2017 13:26:29 +0100 >Subject: [PATCH] Bug 18169: Make 'before' param non mandatory for > Koha::Patrons->anonymise_issue_history >Content-Type: text/plain; charset=utf-8 > >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> >--- > Koha/Patrons.pm | 19 +++++++++++++----- > opac/opac-privacy.pl | 4 +--- > t/db_dependent/Koha/Patrons.t | 46 +++++++++++++++++++++++++++++++++---------- > 3 files changed, 51 insertions(+), 18 deletions(-) > >diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm >index 068c4fb..6548f02 100644 >--- a/Koha/Patrons.pm >+++ b/Koha/Patrons.pm >@@ -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 } ); > } >diff --git a/opac/opac-privacy.pl b/opac/opac-privacy.pl >index 622b9f5..ff0d328 100755 >--- a/opac/opac-privacy.pl >+++ b/opac/opac-privacy.pl >@@ -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 ); >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 06d4d86..7fac24e 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -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; > }; > >-- >2.1.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=60713">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18169
:
60655
|
60712
| 60713 |
60714