Bugzilla – Attachment 157116 Details for
Bug 23260
Anonymize (remove) patron data from items_last_borrower
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23260: (follow-up) Tidy
Bug-23260-follow-up-Tidy.patch (text/plain), 15.06 KB, created by
Nick Clemens
on 2023-10-13 15:32:26 UTC
(
hide
)
Description:
Bug 23260: (follow-up) Tidy
Filename:
MIME Type:
Creator:
Nick Clemens
Created:
2023-10-13 15:32:26 UTC
Size:
15.06 KB
patch
obsolete
>From b97c38b527e5584c7de6e1343942ca7baf6c5eef Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Fri, 13 Oct 2023 15:23:35 +0000 >Subject: [PATCH] Bug 23260: (follow-up) Tidy > >--- > Koha/Patrons.pm | 66 ++++++++++----------- > misc/cronjobs/anonymise_last_borrowers.pl | 9 +-- > t/db_dependent/Koha/Patrons.t | 72 +++++++++++++---------- > 3 files changed, 77 insertions(+), 70 deletions(-) > >diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm >index d2fd2980fb..f596b55944 100644 >--- a/Koha/Patrons.pm >+++ b/Koha/Patrons.pm >@@ -159,6 +159,7 @@ 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 ] } ); >@@ -179,18 +180,17 @@ sub anonymise_issue_history { > # 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 $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) } ) >- : () >- ) >- } >+ { >+ ( >+ $older_than_date >+ ? ( returndate => { '<' => $dtf->format_datetime($older_than_date) } ) >+ : () >+ ) >+ } > ); > my $anonymous_patron = C4::Context->preference('AnonymousPatron') || undef; > $nb_rows += $old_issues_to_anonymise->update( { 'old_issues.borrowernumber' => $anonymous_patron } ); >@@ -198,7 +198,6 @@ sub anonymise_issue_history { > return $nb_rows; > } > >- > =head3 anonymise_last_borrowers > > Koha::Patrons->search->anonymise_last_borrowers(); >@@ -213,19 +212,21 @@ sub anonymise_last_borrowers { > > return unless C4::Context->preference("AnonymiseLastBorrower"); > my $days = C4::Context->preference("AnonymiseLastBorrowerDays") || 0; >- my ($year,$month,$day) = Today(); >- my ($newyear,$newmonth,$newday) = Add_Delta_Days ($year,$month,$day,(-1)*$days); >- my $older_than_date = dt_from_string(sprintf "%4d-%02d-%02d",$newyear,$newmonth,$newday); >+ my ( $year, $month, $day ) = Today(); >+ my ( $newyear, $newmonth, $newday ) = Add_Delta_Days( $year, $month, $day, (-1) * $days ); >+ my $older_than_date = dt_from_string( sprintf "%4d-%02d-%02d", $newyear, $newmonth, $newday ); > > my $anonymous_patron = C4::Context->preference('AnonymousPatron') || undef; > > my $dtf = Koha::Database->new->schema->storage->datetime_parser; >- my $rs = $self->_resultset->search( >- { created_on => { '<' => $dtf->format_datetime($older_than_date), }, >- 'items_last_borrowers.borrowernumber' => { 'not' => undef }, # Keep forever >+ my $rs = $self->_resultset->search( >+ { >+ created_on => { '<' => $dtf->format_datetime($older_than_date), }, >+ 'items_last_borrowers.borrowernumber' => { 'not' => undef }, # Keep forever > ( $anonymous_patron ? ( 'items_last_borrowers.borrowernumber' => { '!=' => $anonymous_patron } ) : () ), > }, >- { join => ["items_last_borrowers"], >+ { >+ join => ["items_last_borrowers"], > distinct => 1, > } > ); >@@ -233,24 +234,21 @@ sub anonymise_last_borrowers { > > my $nb_rows = 0; > while ( my $patron = $patrons->next ) { >- 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 $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'] } > ); >- $nb_rows += $last_borrowers_to_anonymise->update( { 'items_last_borrower.borrowernumber' => $anonymous_patron } ); >+ $nb_rows += >+ $last_borrowers_to_anonymise->update( { 'items_last_borrower.borrowernumber' => $anonymous_patron } ); > } > > return $nb_rows; >diff --git a/misc/cronjobs/anonymise_last_borrowers.pl b/misc/cronjobs/anonymise_last_borrowers.pl >index 7f73794196..6eb47976cb 100755 >--- a/misc/cronjobs/anonymise_last_borrowers.pl >+++ b/misc/cronjobs/anonymise_last_borrowers.pl >@@ -1,4 +1,3 @@ >- > #!/usr/bin/perl > > # Copyright 2011, ByWater Solutions. >@@ -36,7 +35,6 @@ use Koha::Patrons; > use Getopt::Long; > use C4::Log; > >- > sub usage { > print STDERR <<USAGE; > Usage: $0 [-h|--help] >@@ -51,8 +49,8 @@ USAGE > my ( $help, $verbose ); > > GetOptions( >- 'h|help' => \$help, >- 'v|verbose' => \$verbose, >+ 'h|help' => \$help, >+ 'v|verbose' => \$verbose, > ) || usage(1); > > if ($help) { >@@ -61,8 +59,7 @@ if ($help) { > > my $pref = C4::Context->preference("AnonymiseLastBorrower"); > >- >-if( !$pref ) { >+if ( !$pref ) { > print "Preference 'AnonymiseLastBorrower' must be enabled to anonymise item's last borrower\n\n"; > usage(1); > } >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 299b76f681..943845451b 100755 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -1289,14 +1289,14 @@ subtest 'anonymise items_last_borrower' => sub { > > my $anonymous = $builder->build_object( { class => 'Koha::Patrons', }, ); > >- t::lib::Mocks::mock_preference( 'AnonymousPatron', $anonymous->borrowernumber ); >- t::lib::Mocks::mock_preference('StoreLastBorrower', 1); >- t::lib::Mocks::mock_preference('AnonymiseLastBorrower', 1); >+ t::lib::Mocks::mock_preference( 'AnonymousPatron', $anonymous->borrowernumber ); >+ t::lib::Mocks::mock_preference( 'StoreLastBorrower', 1 ); >+ t::lib::Mocks::mock_preference( 'AnonymiseLastBorrower', 1 ); > > subtest 'anonymise items_last_borrower by days' => sub { > plan tests => 4; > >- t::lib::Mocks::mock_preference('AnonymiseLastBorrowerDays', 5); >+ t::lib::Mocks::mock_preference( 'AnonymiseLastBorrowerDays', 5 ); > > my $patron = $builder->build_object( > { >@@ -1304,7 +1304,7 @@ subtest 'anonymise items_last_borrower' => sub { > value => { privacy => 1, } > } > ); >- my $item_1 = $builder->build_sample_item; >+ my $item_1 = $builder->build_sample_item; > my $issue_1 = $builder->build_object( > { > class => 'Koha::Checkouts', >@@ -1314,7 +1314,7 @@ subtest 'anonymise items_last_borrower' => sub { > }, > } > ); >- my $item_2 = $builder->build_sample_item; >+ my $item_2 = $builder->build_sample_item; > my $issue_2 = $builder->build_object( > { > class => 'Koha::Checkouts', >@@ -1326,29 +1326,35 @@ subtest 'anonymise items_last_borrower' => sub { > ); > > my $dbh = C4::Context->dbh; >- my ( $returned_1 ) = C4::Circulation::AddReturn( $item_1->barcode, undef, undef, dt_from_string ); >- my ( $returned_2 ) = C4::Circulation::AddReturn( $item_2->barcode, undef, undef, dt_from_string ); >+ my ($returned_1) = C4::Circulation::AddReturn( $item_1->barcode, undef, undef, dt_from_string ); >+ my ($returned_2) = C4::Circulation::AddReturn( $item_2->barcode, undef, undef, dt_from_string ); > is( $returned_1 && $returned_2, 1, 'The items should have been returned' ); > my $stu = $dbh->prepare(q|UPDATE items_last_borrower set created_on = ? where itemnumber = ?|); >- $stu->execute(DateTime->today(time_zone => C4::Context->tz())->add( days => -6 ), $item_2->itemnumber); >+ $stu->execute( DateTime->today( time_zone => C4::Context->tz() )->add( days => -6 ), $item_2->itemnumber ); > > my $nb_rows = Koha::Patrons->anonymise_last_borrowers(); >- is( $nb_rows, 1, 'anonymise_issue_history should have returned the number of last borrowers anonymised'); >+ is( $nb_rows, 1, 'anonymise_issue_history should have returned the number of last borrowers anonymised' ); > > my $sth = $dbh->prepare(q|SELECT borrowernumber FROM items_last_borrower where itemnumber = ?|); >- $sth->execute($item_1->itemnumber); >+ $sth->execute( $item_1->itemnumber ); > my ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array; >- is( $borrowernumber_used_to_anonymised, $patron->borrowernumber, 'Last borrower younger than 5 days are not anonymised' ); >- $sth->execute($item_2->itemnumber); >+ is( >+ $borrowernumber_used_to_anonymised, $patron->borrowernumber, >+ 'Last borrower younger than 5 days are not anonymised' >+ ); >+ $sth->execute( $item_2->itemnumber ); > ($borrowernumber_used_to_anonymised) = $sth->fetchrow_array; >- is( $borrowernumber_used_to_anonymised, $anonymous->borrowernumber, 'Last borrower older than 5 days are anonymised' ); >+ is( >+ $borrowernumber_used_to_anonymised, $anonymous->borrowernumber, >+ 'Last borrower older than 5 days are anonymised' >+ ); > > }; > > subtest 'withrawn, lost and damaged items' => sub { > plan tests => 6; > >- t::lib::Mocks::mock_preference('AnonymiseLastBorrowerDays', 0); >+ t::lib::Mocks::mock_preference( 'AnonymiseLastBorrowerDays', 0 ); > > my $patron = $builder->build_object( > { >@@ -1356,7 +1362,7 @@ subtest 'anonymise items_last_borrower' => sub { > value => { privacy => 1, } > } > ); >- my $item_1 = $builder->build_sample_item; >+ my $item_1 = $builder->build_sample_item; > my $issue_1 = $builder->build_object( > { > class => 'Koha::Checkouts', >@@ -1366,7 +1372,7 @@ subtest 'anonymise items_last_borrower' => sub { > }, > } > ); >- my $item_2 = $builder->build_sample_item; >+ my $item_2 = $builder->build_sample_item; > my $issue_2 = $builder->build_object( > { > class => 'Koha::Checkouts', >@@ -1376,7 +1382,7 @@ subtest 'anonymise items_last_borrower' => sub { > }, > } > ); >- my $item_3 = $builder->build_sample_item; >+ my $item_3 = $builder->build_sample_item; > my $issue_3 = $builder->build_object( > { > class => 'Koha::Checkouts', >@@ -1386,7 +1392,7 @@ subtest 'anonymise items_last_borrower' => sub { > }, > } > ); >- my $item_4 = $builder->build_sample_item; >+ my $item_4 = $builder->build_sample_item; > my $issue_4 = $builder->build_object( > { > class => 'Koha::Checkouts', >@@ -1397,10 +1403,10 @@ subtest 'anonymise items_last_borrower' => sub { > } > ); > >- my ( $returned_1 ) = C4::Circulation::AddReturn( $item_1->barcode, undef, undef, dt_from_string('2010-10-11') ); >- my ( $returned_2 ) = C4::Circulation::AddReturn( $item_2->barcode, undef, undef, dt_from_string('2010-10-11') ); >- my ( $returned_3 ) = C4::Circulation::AddReturn( $item_3->barcode, undef, undef, dt_from_string('2010-10-11') ); >- my ( $returned_4 ) = C4::Circulation::AddReturn( $item_4->barcode, undef, undef, dt_from_string('2010-10-11') ); >+ my ($returned_1) = C4::Circulation::AddReturn( $item_1->barcode, undef, undef, dt_from_string('2010-10-11') ); >+ my ($returned_2) = C4::Circulation::AddReturn( $item_2->barcode, undef, undef, dt_from_string('2010-10-11') ); >+ my ($returned_3) = C4::Circulation::AddReturn( $item_3->barcode, undef, undef, dt_from_string('2010-10-11') ); >+ my ($returned_4) = 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; >@@ -1408,24 +1414,30 @@ subtest 'anonymise items_last_borrower' => sub { > > my $dbh = C4::Context->dbh; > my $stu = $dbh->prepare(q|UPDATE items_last_borrower set created_on = ? where itemnumber in (?, ?, ?, ?)|); >- $stu->execute(DateTime->today(time_zone => C4::Context->tz())->add( days => -1 ), $item_1->itemnumber,$item_2->itemnumber,$item_3->itemnumber,$item_4->itemnumber); >+ $stu->execute( >+ DateTime->today( time_zone => C4::Context->tz() )->add( days => -1 ), $item_1->itemnumber, >+ $item_2->itemnumber, $item_3->itemnumber, $item_4->itemnumber >+ ); > > my $nb_rows = Koha::Patrons->anonymise_last_borrowers(); >- is( $nb_rows, 1, 'anonymise_issue_history should have returned the number of last borrowers anonymised'); >+ is( $nb_rows, 1, 'anonymise_issue_history should have returned the number of last borrowers anonymised' ); > > my $sth = $dbh->prepare(q|SELECT borrowernumber FROM items_last_borrower where itemnumber = ?|); >- $sth->execute($item_1->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); >+ $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); >+ $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); >+ $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' ); >+ is( >+ $borrowernumber_used_to_anonymised, $anonymous->borrowernumber, >+ 'not withdrawn, lost or damaged items are anonymised' >+ ); > > }; > >-- >2.30.2
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 23260
:
92863
|
92864
|
93052
|
94858
|
94859
|
95033
|
95457
|
95458
|
95459
|
95460
|
95651
|
95698
|
95699
|
95700
|
95701
|
95769
|
95770
|
95771
|
95772
|
96791
|
97841
|
97842
|
98044
|
98053
|
98054
|
98064
|
98282
|
98349
|
98352
|
112734
|
112735
|
112736
|
112737
|
112738
|
112739
|
112740
|
112741
|
112742
|
112743
|
117974
|
117975
|
117976
|
117977
|
117978
|
117979
|
117980
|
117981
|
117982
|
117983
|
120522
|
120523
|
120524
|
120525
|
120526
|
120527
|
120528
|
120529
|
120530
|
120531
|
122963
|
122964
|
122965
|
122966
|
122967
|
122968
|
122969
|
122970
|
122971
|
122972
|
145175
|
145176
|
145177
|
145178
|
145179
|
145180
|
145181
|
145182
|
145183
|
145184
|
157106
|
157107
|
157108
|
157109
|
157110
|
157111
|
157112
|
157113
|
157114
|
157115
|
157116
|
159291
|
159292
|
159293
|
159294
|
159298
|
159299
|
159300
|
159301
|
164129
|
164130
|
164131
|
164132
|
164133