Bug 29525 - Privacy settings for patrons should also affect holds history
Summary: Privacy settings for patrons should also affect holds history
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 29780 29868 20936
Blocks: 29843 29879
  Show dependency treegraph
 
Reported: 2021-11-19 01:15 UTC by Nick Clemens
Modified: 2022-01-17 21:17 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable (6.14 KB, patch)
2022-01-03 18:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.21 KB, patch)
2022-01-03 18:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: (follow-up) Adapt OPAC text to mention holds (2.66 KB, patch)
2022-01-03 18:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable (5.85 KB, patch)
2022-01-10 13:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.74 KB, patch)
2022-01-10 13:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: (follow-up) Adapt OPAC text to mention holds (2.66 KB, patch)
2022-01-10 13:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable (5.65 KB, patch)
2022-01-10 13:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.55 KB, patch)
2022-01-10 13:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: (follow-up) Adapt OPAC text to mention holds (2.66 KB, patch)
2022-01-10 13:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.53 KB, patch)
2022-01-10 14:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29843: Unit tests (6.59 KB, patch)
2022-01-10 15:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable (5.65 KB, patch)
2022-01-11 19:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.53 KB, patch)
2022-01-11 19:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: (follow-up) Adapt OPAC text to mention holds (2.66 KB, patch)
2022-01-11 19:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->cancel anonymize if required (3.39 KB, patch)
2022-01-12 19:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->cancel anonymize if required (3.89 KB, patch)
2022-01-12 19:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable (5.65 KB, patch)
2022-01-13 17:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.53 KB, patch)
2022-01-13 17:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: (follow-up) Adapt OPAC text to mention holds (2.66 KB, patch)
2022-01-13 17:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->cancel anonymize if required (4.48 KB, patch)
2022-01-13 17:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->fill anonymize if required (4.04 KB, patch)
2022-01-13 17:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable (5.72 KB, patch)
2022-01-13 19:01 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29525: Make batch_anonymise.pl deal with holds (2.59 KB, patch)
2022-01-13 19:02 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29525: (follow-up) Adapt OPAC text to mention holds (2.72 KB, patch)
2022-01-13 19:02 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->cancel anonymize if required (4.55 KB, patch)
2022-01-13 19:02 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->fill anonymize if required (4.11 KB, patch)
2022-01-13 19:02 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->cancel anonymize if required (4.57 KB, patch)
2022-01-17 21:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29525: Make Koha::Hold->fill anonymize if required (4.11 KB, patch)
2022-01-17 21:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-11-19 01:15:19 UTC
Adjust Patron Privacy preferences to include holds history into privacy considerations.   
If patron privacy setting is set to never, never retain holds history (anonymize it).  
If privacy is set to keep, keep holds history for the patron forever. 
If privacy is set to default, keep it until library chooses to anonymize hold history. (add holds history functionality or option to batch_anonymise cronjob)
Comment 1 George Williams (NEKLS) 2021-11-19 14:08:17 UTC
+1
Comment 2 Tomás Cohen Arazi 2022-01-03 18:18:14 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2022-01-03 18:18:19 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2022-01-03 18:18:26 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2022-01-10 13:17:24 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2022-01-10 13:17:29 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2022-01-10 13:17:33 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2022-01-10 13:36:49 UTC
Sorry for the noise, just learnt about Koha::Objects->filter_by_last_update
Comment 9 Tomás Cohen Arazi 2022-01-10 13:37:33 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 2022-01-10 13:37:39 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi 2022-01-10 13:37:45 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2022-01-10 14:28:45 UTC Comment hidden (obsolete)
Comment 13 Tomás Cohen Arazi 2022-01-10 15:17:52 UTC Comment hidden (obsolete)
Comment 14 Tomás Cohen Arazi 2022-01-11 19:18:36 UTC
Created attachment 129321 [details] [review]
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable

This patch adds the 'filter_by_anonymizable' method, and tests for it. A
new DBIC relationship is added as well to the OldReserve schema file.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Old/Holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 15 Tomás Cohen Arazi 2022-01-11 19:18:45 UTC
Created attachment 129322 [details] [review]
Bug 29525: Make batch_anonymise.pl deal with holds

This patch makes the batch_anonymise.pl script handle holds too. It does
so by leveraging on the newly introduced method 'filter_by_anonymizable'
and also 'anonymize'.

To test:
1. Have a patron with two past holds.
2. Make sure they are a few days back:
   $ koha-mysql kohadev
   > UPDATE old_reserves SET timestamp=ADDDATE(NOW(), INTERVAL -4 DAY);
3. Run:
   $ kshell
  k$ perl misc/cronjobs/batch_anonymise.pl --days 2 -v
=> SUCCESS: You see something like:
Checkouts and holds before 2022-01-01 will be anonymised.
0 checkouts anonymised.
2 holds anonymised.
4. Repeat 3
=> SUCCESS: They are already anonymized. You see
Checkouts and holds before 2022-01-01 will be anonymised.
0 checkouts anonymised.
0 holds anonymised.
5. Sign off :-D
Comment 16 Tomás Cohen Arazi 2022-01-11 19:18:53 UTC
Created attachment 129323 [details] [review]
Bug 29525: (follow-up) Adapt OPAC text to mention holds
Comment 17 Andrew Fuerste-Henry 2022-01-12 14:33:07 UTC
We're not fully mirroring circ history behavior. For checkout history, if a patron is set to Never they get anonymized as soon as the item is returned. Here, if a patron is set to Never we're still waiting for the cron to anonymize them. We should anonymize their holds as soon as they move to old_reserves.
Comment 18 Tomás Cohen Arazi 2022-01-12 15:35:06 UTC
(In reply to Andrew Fuerste-Henry from comment #17)
> We're not fully mirroring circ history behavior. For checkout history, if a
> patron is set to Never they get anonymized as soon as the item is returned.
> Here, if a patron is set to Never we're still waiting for the cron to
> anonymize them. We should anonymize their holds as soon as they move to
> old_reserves.

This is getting complex: if we anonymize same-day holds... we are gonna break the max holds per day circ rule calculation!
Comment 19 Tomás Cohen Arazi 2022-01-12 15:49:21 UTC
Ok, the current behavior is that only 'active' holds count towards the max:


    my $today_holds = Koha::Holds->search({
        borrowernumber => $borrowernumber,
        reservedate    => dt_from_string->date
    });

    if (!$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne ''
        && $today_holds->count() >= $holds_per_day )
    {
        return { status => 'tooManyReservesToday', limit => $holds_per_day };
    }

so moving on with this, with no behavior change on the calculation. Phew.

Should the whole feature have an on/off switch? Something like OPACHoldsPrivacy on/off?
Comment 20 Andrew Fuerste-Henry 2022-01-12 16:15:48 UTC
Right now these sysprefs come into play:

OPACPrivacy determines whether or not patrons can access the My Privacy tab on which these settings are changed.

OPACHoldsHistory determines whether or not patrons can see their hold history, if it is kept. (opacreadinghitory, a separate syspref, determines whether or not they can see checkout history).

intranetreadinghistory determines whether or not staff members can see BOTH checkout and hold history on that staff side, if kept.

I don't feel these *need* changes for this bug to move forward, but in the long run I'd like to see a new IntranetHoldsHistory syspref to split staff interface holds visibility out of intranetreadinghistory.

If we want to give patrons the ability to control holds history without controlling checkout history (or vice versa), I'd be inclined to accomplish that by adding options to OPACPrivacy.
Comment 21 Tomás Cohen Arazi 2022-01-12 19:33:55 UTC
Created attachment 129363 [details] [review]
Bug 29525: Make Koha::Hold->cancel anonymize if required
Comment 22 Tomás Cohen Arazi 2022-01-12 19:49:58 UTC
Created attachment 129364 [details] [review]
Bug 29525: Make Koha::Hold->cancel anonymize if required

This patch makes cancelling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, cancelling a hold, noticing it gets anonymized if the
   patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.
Comment 23 Tomás Cohen Arazi 2022-01-12 19:56:31 UTC
Andrew and testers, this follow-up patch covers the 'cancelling a hold' use case.
Comment 24 Tomás Cohen Arazi 2022-01-13 14:58:04 UTC
Ok, bug adds Koha::Hold->fill with tests, bells and whistles.
I will now make it anonymize the same way we did with Koha:Hold->cancel. And we are done  :-D
Comment 25 Tomás Cohen Arazi 2022-01-13 17:49:19 UTC
Created attachment 129424 [details] [review]
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable

This patch adds the 'filter_by_anonymizable' method, and tests for it. A
new DBIC relationship is added as well to the OldReserve schema file.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Old/Holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 26 Tomás Cohen Arazi 2022-01-13 17:49:25 UTC
Created attachment 129425 [details] [review]
Bug 29525: Make batch_anonymise.pl deal with holds

This patch makes the batch_anonymise.pl script handle holds too. It does
so by leveraging on the newly introduced method 'filter_by_anonymizable'
and also 'anonymize'.

To test:
1. Have a patron with two past holds.
2. Make sure they are a few days back:
   $ koha-mysql kohadev
   > UPDATE old_reserves SET timestamp=ADDDATE(NOW(), INTERVAL -4 DAY);
3. Run:
   $ kshell
  k$ perl misc/cronjobs/batch_anonymise.pl --days 2 -v
=> SUCCESS: You see something like:
Checkouts and holds before 2022-01-01 will be anonymised.
0 checkouts anonymised.
2 holds anonymised.
4. Repeat 3
=> SUCCESS: They are already anonymized. You see
Checkouts and holds before 2022-01-01 will be anonymised.
0 checkouts anonymised.
0 holds anonymised.
5. Sign off :-D
Comment 27 Tomás Cohen Arazi 2022-01-13 17:49:31 UTC
Created attachment 129426 [details] [review]
Bug 29525: (follow-up) Adapt OPAC text to mention holds
Comment 28 Tomás Cohen Arazi 2022-01-13 17:49:36 UTC
Created attachment 129427 [details] [review]
Bug 29525: Make Koha::Hold->cancel anonymize if required

This patch makes cancelling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, cancelling a hold, noticing it gets anonymized if the
   patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.
Comment 29 Tomás Cohen Arazi 2022-01-13 17:49:42 UTC
Created attachment 129428 [details] [review]
Bug 29525: Make Koha::Hold->fill anonymize if required

This patch makes filling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, notice it gets anonymized if the patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 30 Andrew Fuerste-Henry 2022-01-13 19:01:57 UTC
Created attachment 129432 [details] [review]
Bug 29525: Add Koha::Old::Holds->filter_by_anonymizable

This patch adds the 'filter_by_anonymizable' method, and tests for it. A
new DBIC relationship is added as well to the OldReserve schema file.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ t/db_dependent/Koha/Old/Holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 31 Andrew Fuerste-Henry 2022-01-13 19:02:00 UTC
Created attachment 129433 [details] [review]
Bug 29525: Make batch_anonymise.pl deal with holds

This patch makes the batch_anonymise.pl script handle holds too. It does
so by leveraging on the newly introduced method 'filter_by_anonymizable'
and also 'anonymize'.

To test:
1. Have a patron with two past holds.
2. Make sure they are a few days back:
   $ koha-mysql kohadev
   > UPDATE old_reserves SET timestamp=ADDDATE(NOW(), INTERVAL -4 DAY);
3. Run:
   $ kshell
  k$ perl misc/cronjobs/batch_anonymise.pl --days 2 -v
=> SUCCESS: You see something like:
Checkouts and holds before 2022-01-01 will be anonymised.
0 checkouts anonymised.
2 holds anonymised.
4. Repeat 3
=> SUCCESS: They are already anonymized. You see
Checkouts and holds before 2022-01-01 will be anonymised.
0 checkouts anonymised.
0 holds anonymised.
5. Sign off :-D

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 32 Andrew Fuerste-Henry 2022-01-13 19:02:04 UTC
Created attachment 129434 [details] [review]
Bug 29525: (follow-up) Adapt OPAC text to mention holds

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 33 Andrew Fuerste-Henry 2022-01-13 19:02:09 UTC
Created attachment 129435 [details] [review]
Bug 29525: Make Koha::Hold->cancel anonymize if required

This patch makes cancelling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, cancelling a hold, noticing it gets anonymized if the
   patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 34 Andrew Fuerste-Henry 2022-01-13 19:02:13 UTC
Created attachment 129436 [details] [review]
Bug 29525: Make Koha::Hold->fill anonymize if required

This patch makes filling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, notice it gets anonymized if the patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 35 Martin Renvoize 2022-01-17 17:17:09 UTC
hmm.. 'Make Koha::Hold->cancel' claims there's nothing to apply here
Comment 36 Tomás Cohen Arazi 2022-01-17 21:17:34 UTC
Created attachment 129544 [details] [review]
Bug 29525: Make Koha::Hold->cancel anonymize if required

This patch makes cancelling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, cancelling a hold, noticing it gets anonymized if the
   patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 37 Tomás Cohen Arazi 2022-01-17 21:17:39 UTC
Created attachment 129545 [details] [review]
Bug 29525: Make Koha::Hold->fill anonymize if required

This patch makes filling a hold anonymize it on the same call, if
settings require it (i.e. if borrowers.privacy is set to 2).

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: The code actually does what it is meant to
3. Try on the UI, notice it gets anonymized if the patron has privacy == always/2.
4. Sign off :-D

Note: AnonymousPatron should be set. Otherwise it would set NULL. But
that's fine, that's what Koha does already.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>