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.
Created attachment 60655 [details] [review] 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
Question: You adjust the test, but you do not call search_patrons_to_anonymise without the before parameter ??
Created attachment 60712 [details] [review] Bug 18169: Add test without before parameter for search_patrons_to_anonymise
(In reply to Marcel de Rooy from comment #2) > Question: You adjust the test, but you do not call > search_patrons_to_anonymise without the before parameter ?? Nope because the first patch does not modify the behavior for search_patrons_to_anonymise but anonymise_issue_history. This is more a follow-up for 16966, but indeed it makes sense to have a test for this case.
Created attachment 60713 [details] [review] 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>
Created attachment 60714 [details] [review] Bug 18169: Add test without before parameter for search_patrons_to_anonymise Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Jonathan Druart from comment #4) > This is more a follow-up for 16966, but indeed it makes sense to have a test > for this case. Taking the liberty to pass qa directly on this patch set, as we could indeed have handled both patches on the other report as well..
Pushed to master for 17.05, thanks Jonathan!
This won't get ported back to 16.11.x as it is an enhancement.