Bug 24152 - Add the ability to purge pseudonymized data
Summary: Add the ability to purge pseudonymized data
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 19008 24151
Blocks: 24153
  Show dependency treegraph
 
Reported: 2019-12-03 15:48 UTC by Jonathan Druart
Modified: 2020-11-30 21:52 UTC (History)
7 users (show)

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


Attachments
Bug 24152: Add method Koha::Objects->filter_by_last_update (5.03 KB, patch)
2019-12-03 18:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add a warning about the delete of statistics's table entries (1.27 KB, patch)
2019-12-03 18:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add the ability to purge pseudonymized tables (10.59 KB, patch)
2019-12-03 18:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add method Koha::Objects->filter_by_last_update (5.03 KB, patch)
2020-01-11 18:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add a warning about the delete of statistics's table entries (1.27 KB, patch)
2020-01-11 18:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add the ability to purge pseudonymized tables (9.53 KB, patch)
2020-01-11 18:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add method Koha::Objects->filter_by_last_update (5.17 KB, patch)
2020-03-20 16:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add a warning about the delete of statistics's table entries (1.33 KB, patch)
2020-03-20 16:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add the ability to purge pseudonymized tables (9.64 KB, patch)
2020-03-20 16:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add method Koha::Objects->filter_by_last_update (5.20 KB, patch)
2020-06-12 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add a warning about the delete of statistics's table entries (1.33 KB, patch)
2020-06-12 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add the ability to purge pseudonymized tables (9.68 KB, patch)
2020-06-12 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24152: Add method Koha::Objects->filter_by_last_update (5.30 KB, patch)
2020-06-19 08:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24152: Add a warning about the delete of statistics's table entries (1.43 KB, patch)
2020-06-19 08:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24152: Add the ability to purge pseudonymized tables (9.78 KB, patch)
2020-06-19 08:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24152: (QA follow-up) Add tests for alternative from and to pars (1.41 KB, patch)
2020-06-19 08:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24152: Allow purge with 0 day (1.36 KB, patch)
2020-06-23 10:03 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2019-12-03 15:48:17 UTC

    
Comment 1 Jonathan Druart 2019-12-03 18:12:42 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2019-12-03 18:12:45 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2019-12-03 18:12:48 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize 2020-01-03 11:29:05 UTC
I'd like to see tests for the new `filter_by_last_update` method added to Koha::Objects (and really, I'd love to see that in it's own mini bug for fast QA and push in the future.. smaller bugs in a tree means others can benefit from such nice little additions more quickly)
Comment 5 Jonathan Druart 2020-01-11 18:14:10 UTC
Created attachment 97253 [details] [review]
Bug 24152: Add method Koha::Objects->filter_by_last_update
Comment 6 Jonathan Druart 2020-01-11 18:14:13 UTC
Created attachment 97254 [details] [review]
Bug 24152: Add a warning about the delete of statistics's table entries
Comment 7 Jonathan Druart 2020-01-11 18:14:16 UTC
Created attachment 97255 [details] [review]
Bug 24152: Add the ability to purge pseudonymized tables

anonymized_* tables have been added by bug 24151, this patch adds the
usual way to purge data from them.

The cleanup_database.pl script has been adjusted to take new parameters
that will help to delete pseudonymized data.

Test plan:
Call the scrip with the new parameter to remove pseudonymized data
* --pseudo-transactions DAYS will remove entries from pseudonymized_transactions older
than DAYS day
* --pseudo-transactions can be used without the parameter DAYS but with
-- pseudo-transactions-from and/or --pseudo-transactions-to instead, to provide a
range of date

You can use the patch from bug 24153 to make the tests easier, data will
not be deleted if the new --confirm flag is not passed.

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 8 Koha Team University Lyon 3 2020-03-20 14:31:10 UTC
I've tested for Association KohaLa.
The data in the tables pseudonymized_transactions and pseudonymized_borrower_attributes are correctly deleted when using --pseudo-transactions flag for cleanup_database.pl
I tried it with a number of days and also with the choice of dates :
--pseudo-transactions --pseudo-transactions-from YYYY-MM-DD --pseudo-transactions-to YYYY-MM-DD 
and both work correctly.

Thanks!

Sonia BOUIS
Comment 9 Jonathan Druart 2020-03-20 16:02:54 UTC
Created attachment 101297 [details] [review]
Bug 24152: Add method Koha::Objects->filter_by_last_update

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 10 Jonathan Druart 2020-03-20 16:02:57 UTC
Created attachment 101298 [details] [review]
Bug 24152: Add a warning about the delete of statistics's table entries

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 11 Jonathan Druart 2020-03-20 16:03:01 UTC
Created attachment 101299 [details] [review]
Bug 24152: Add the ability to purge pseudonymized tables

anonymized_* tables have been added by bug 24151, this patch adds the
usual way to purge data from them.

The cleanup_database.pl script has been adjusted to take new parameters
that will help to delete pseudonymized data.

Test plan:
Call the scrip with the new parameter to remove pseudonymized data
* --pseudo-transactions DAYS will remove entries from pseudonymized_transactions older
than DAYS day
* --pseudo-transactions can be used without the parameter DAYS but with
-- pseudo-transactions-from and/or --pseudo-transactions-to instead, to provide a
range of date

You can use the patch from bug 24153 to make the tests easier, data will
not be deleted if the new --confirm flag is not passed.

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 12 Julian Maurice 2020-06-12 13:45:04 UTC
Changing status to BLOCKED because of dependency on bug 24151, which does not apply at the moment.
Comment 13 Jonathan Druart 2020-06-12 14:08:58 UTC
Created attachment 105817 [details] [review]
Bug 24152: Add method Koha::Objects->filter_by_last_update

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 14 Jonathan Druart 2020-06-12 14:09:02 UTC
Created attachment 105818 [details] [review]
Bug 24152: Add a warning about the delete of statistics's table entries

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 15 Jonathan Druart 2020-06-12 14:09:05 UTC
Created attachment 105819 [details] [review]
Bug 24152: Add the ability to purge pseudonymized tables

anonymized_* tables have been added by bug 24151, this patch adds the
usual way to purge data from them.

The cleanup_database.pl script has been adjusted to take new parameters
that will help to delete pseudonymized data.

Test plan:
Call the scrip with the new parameter to remove pseudonymized data
* --pseudo-transactions DAYS will remove entries from pseudonymized_transactions older
than DAYS day
* --pseudo-transactions can be used without the parameter DAYS but with
-- pseudo-transactions-from and/or --pseudo-transactions-to instead, to provide a
range of date

You can use the patch from bug 24153 to make the tests easier, data will
not be deleted if the new --confirm flag is not passed.

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 16 Marcel de Rooy 2020-06-19 07:38:58 UTC
QAing
Comment 17 Marcel de Rooy 2020-06-19 08:05:47 UTC
Created attachment 106051 [details] [review]
Bug 24152: Add method Koha::Objects->filter_by_last_update

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2020-06-19 08:05:51 UTC
Created attachment 106052 [details] [review]
Bug 24152: Add a warning about the delete of statistics's table entries

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Marcel de Rooy 2020-06-19 08:05:55 UTC
Created attachment 106053 [details] [review]
Bug 24152: Add the ability to purge pseudonymized tables

anonymized_* tables have been added by bug 24151, this patch adds the
usual way to purge data from them.

The cleanup_database.pl script has been adjusted to take new parameters
that will help to delete pseudonymized data.

Test plan:
Call the scrip with the new parameter to remove pseudonymized data
* --pseudo-transactions DAYS will remove entries from pseudonymized_transactions older
than DAYS day
* --pseudo-transactions can be used without the parameter DAYS but with
-- pseudo-transactions-from and/or --pseudo-transactions-to instead, to provide a
range of date

You can use the patch from bug 24153 to make the tests easier, data will
not be deleted if the new --confirm flag is not passed.

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 20 Marcel de Rooy 2020-06-19 08:06:00 UTC
Created attachment 106054 [details] [review]
Bug 24152: (QA follow-up) Add tests for alternative from and to pars

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2020-06-19 08:06:41 UTC
Looks good to me, minor points:

+    || defined $pPseudoTransactions
 ) {
     print "You did not specify any cleanup work for the script to do.\n\n";
=> Looks like you forgot the alternative parameters here with from and to ? Follow-up added.

Use of uninitialized value $pPseudoTransactions in concatenation (.) or string at misc/cronjobs/cleanup_database.pl line 462.
Purging pseudonymized transactions older than  days.
=> Since we have three pars, removed the one parameter from the message.

Looks like it is not possible to purge with days 0 ? Is the time stripped in the dtf parsing?
Comment 22 Jonathan Druart 2020-06-23 10:00:05 UTC
(In reply to Marcel de Rooy from comment #21)
> Looks good to me, minor points:
> 
> +    || defined $pPseudoTransactions
>  ) {
>      print "You did not specify any cleanup work for the script to do.\n\n";
> => Looks like you forgot the alternative parameters here with from and to ?
> Follow-up added.
> 
> Use of uninitialized value $pPseudoTransactions in concatenation (.) or
> string at misc/cronjobs/cleanup_database.pl line 462.
> Purging pseudonymized transactions older than  days.
> => Since we have three pars, removed the one parameter from the message.
> 
> Looks like it is not possible to purge with days 0 ? Is the time stripped in
> the dtf parsing?

I think it's because of the follow-up, before it was "if defined $pPseudoTransactions".
I will add another follow-up, can you double check?
Comment 23 Jonathan Druart 2020-06-23 10:03:32 UTC
Created attachment 106196 [details] [review]
Bug 24152: Allow purge with 0 day
Comment 24 Jonathan Druart 2020-06-23 10:03:57 UTC
Well no, it was not the follow-up :)
Comment 25 Marcel de Rooy 2020-06-24 12:40:48 UTC
(In reply to Jonathan Druart from comment #23)
> Well no, it was not the follow-up :)

-            ( $pPseudoTransactions     ? ( days => $pPseudoTransactions     ) : () ),
-            ( $pPseudoTransactionsFrom ? ( from => $pPseudoTransactionsFrom ) : () ),
-            ( $pPseudoTransactionsTo   ? ( to   => $pPseudoTransactionsTo   ) : () ),
+            ( defined $pPseudoTransactions  ? ( days => $pPseudoTransactions     ) : () ),
+            ( $pPseudoTransactionsFrom      ? ( from => $pPseudoTransactionsFrom ) : () ),
+            ( $pPseudoTransactionsTo        ? ( to   => $pPseudoTransactionsTo   ) : () ),


Well I did not test, but the change with defined explains it for me.
Comment 26 Marcel de Rooy 2020-07-02 07:51:01 UTC
(In reply to Marcel de Rooy from comment #25)
> (In reply to Jonathan Druart from comment #23)
> > Well no, it was not the follow-up :)
> 
> -            ( $pPseudoTransactions     ? ( days => $pPseudoTransactions    
> ) : () ),
> -            ( $pPseudoTransactionsFrom ? ( from => $pPseudoTransactionsFrom
> ) : () ),
> -            ( $pPseudoTransactionsTo   ? ( to   => $pPseudoTransactionsTo  
> ) : () ),
> +            ( defined $pPseudoTransactions  ? ( days =>
> $pPseudoTransactions     ) : () ),
> +            ( $pPseudoTransactionsFrom      ? ( from =>
> $pPseudoTransactionsFrom ) : () ),
> +            ( $pPseudoTransactionsTo        ? ( to   =>
> $pPseudoTransactionsTo   ) : () ),
> 
> 
> Well I did not test, but the change with defined explains it for me.

Hmm. I tried it now but filter_last_update works with < and truncates to days. So what actually happens, is that I am purging records before today. Which is a bit counterintuitive.
Comment 27 Jonathan Druart 2020-07-20 12:21:55 UTC
Indeed, the script does not allow to remove data from today. It is the current behaviour for other data this script purges. I am going to open a separate bug report.
Comment 28 Jonathan Druart 2020-07-20 12:24:58 UTC
(In reply to Jonathan Druart from comment #27)
> Indeed, the script does not allow to remove data from today. It is the
> current behaviour for other data this script purges. I am going to open a
> separate bug report.

See bug 26026.
Comment 29 Jonathan Druart 2020-07-20 15:48:20 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 30 Lucas Gass 2020-07-25 18:10:44 UTC
Not backporting enhancement to 20.05.x series.