If a patron chooses to never keep privacy we should immediately remove their history, rather than waiting on the anonymization cronjob
Created attachment 148186 [details] [review] Bug 33229: Clear patron reading history when privacy is set to never This patch adds a check to patron store routine. When privacy is changed to 'Never' we should immediately clear their history To test: 1 - Set system preferences: AnonymousPatron - to a valid borrowernumber OPACPrivacy - Allow 2 - Checkout some items to a patron with priacy set to default, return them 3 - Log in to opac as patron 4 - Confirm you can see your history 5 - Change history to Never 6 - Confirm history still viewable 7 - Apply patch, restart all 8 - Set privacy to Default 9 - Confirm you can still see history, not deleted when not set as Never 10 - Set privacy to 'Never' 11 - Confirm history is anonymized
Created attachment 148197 [details] [review] Bug 33229: Clear patron reading history when privacy is set to never This patch adds a check to patron store routine. When privacy is changed to 'Never' we should immediately clear their history To test: 1 - Set system preferences: AnonymousPatron - to a valid borrowernumber OPACPrivacy - Allow 2 - Checkout some items to a patron with priacy set to default, return them 3 - Log in to opac as patron 4 - Confirm you can see your history 5 - Change history to Never 6 - Confirm history still viewable 7 - Apply patch, restart all 8 - Set privacy to Default 9 - Confirm you can still see history, not deleted when not set as Never 10 - Set privacy to 'Never' 11 - Confirm history is anonymized Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Works great, thanks for the patch.
Created attachment 148546 [details] [review] Bug 33229: Clear patron reading history when privacy is set to never This patch adds a check to patron store routine. When privacy is changed to 'Never' we should immediately clear their history To test: 1 - Set system preferences: AnonymousPatron - to a valid borrowernumber OPACPrivacy - Allow 2 - Checkout some items to a patron with priacy set to default, return them 3 - Log in to opac as patron 4 - Confirm you can see your history 5 - Change history to Never 6 - Confirm history still viewable 7 - Apply patch, restart all 8 - Set privacy to Default 9 - Confirm you can still see history, not deleted when not set as Never 10 - Set privacy to 'Never' 11 - Confirm history is anonymized Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 148547 [details] [review] Bug 33229: Remove unecessary txn We don't need a transaction here. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nick, I don't understand why you need a txn here. Do you agree with removing it?
(In reply to Jonathan Druart from comment #6) > Nick, I don't understand why you need a txn here. Do you agree with removing > it? My assumption was that I copied it from somewhere, but I don't see that. Possibly I thought we didn't want partial deletion? But if we are trying to anonymize i suppose removing some is better than none
(In reply to Nick Clemens from comment #7) > (In reply to Jonathan Druart from comment #6) > > Nick, I don't understand why you need a txn here. Do you agree with removing > > it? > > My assumption was that I copied it from somewhere, but I don't see that. > > Possibly I thought we didn't want partial deletion? But if we are trying to > anonymize i suppose removing some is better than none It's done in a single query 104 return $self->update( { borrowernumber => $anonymous_id }, { no_triggers => 1 } ); So it's either all or none. Anyway, thanks for you reply!
Pushed to master for 23.05. Nice work everyone, thanks!
Hi Nick, git bisect (not me!) blames you for the failing test on master: https://jenkins.koha-community.org/job/Koha_Master/2479/testReport/junit/(root)/t_db_dependent_Koha_Hold_t/fill___tests__1624_/ I had a look, but couldn't figure it out: # Failed test 'fill() tests' # at t/db_dependent/Koha/Hold.t line 307. Exception 'Koha::Exceptions::SysPref::NotSet' thrown 'Required syspref is not set' with syspref => AnonymousPatron # Looks like your test exited with 255 just after 2. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 10/11 subtests
This broke tests. Please review the consequences of this new exception thrown, on the UI as well. kohadev-koha@kohadevbox:koha(master)$ prove t/db_dependent/Koha/Hold.t t/db_dependent/Koha/Hold.t .. 1/11 # Looks like you planned 5 tests but ran 2. # Failed test 'anonymization behavior tests' # at t/db_dependent/Koha/Hold.t line 265. # Looks like you planned 13 tests but ran 12. # Looks like you failed 1 test of 12 run. t/db_dependent/Koha/Hold.t .. 2/11 # Failed test 'fill() tests' # at t/db_dependent/Koha/Hold.t line 307. Exception 'Koha::Exceptions::SysPref::NotSet' thrown 'Required syspref is not set' with syspref => AnonymousPatron # Looks like your test exited with 255 just after 2. t/db_dependent/Koha/Hold.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 10/11 subtests Test Summary Report ------------------- t/db_dependent/Koha/Hold.t (Wstat: 65280 Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 255 Parse errors: Bad plan. You planned 11 tests but ran 2. Files=1, Tests=2, 3 wallclock secs ( 0.02 usr 0.00 sys + 2.91 cusr 0.55 csys = 3.48 CPU) Result: FAIL
Created attachment 149753 [details] [review] Bug 33229: (follow-up) Adjust tests to set anonymous patron before setting patron privacy to 2
Follow-up pushed. Thanks Nick for the quick fix.
Nice work everyone! Pushed to stable for 22.11.x
Doesn't apply clean to 22.05.x, no backport.