|
Description
Kyle M Hall (khall)
2014-01-30 15:18:31 UTC
Created attachment 24891 [details] [review] Bug 11642 - Batch patron deletion/anonymization should not allow permanent destruction of patron data The Batch patron deletion/anonymization tool gives the option of destroying patron data or moving it to the "trash". I see know reason why we allow deletion of patrons without moving them to deletedborrowers first. We don't allow this kind of behavior for records or items, so why patrons? Furthermore it seems far to easy to mis-use this tool by accident, further exacerbating the situation. Test Plan: 1) Apply this patch 2) Verify you can no longer permanently delete patrons with the batch patron deletion/anonymization tool, instead they will always be moved to deletedborrowers. Records and items aren't interesting to the FBI in the same way patron data is. I can see that as a valid reason for deleting them completely. Perhaps a better response is to do a "Are you sure you want to delete permanently .. there is no way to get them back?" Confirmation prompt is a better way than disabling this. I agree with Chris - there are good reasons to offer a way to permanently delete patron information from the interface. Maybe we should also have a way to clean the deleted_borrowers table regularly on some conditions or at least delete the personal information in those records anonymizing them. Additionally, in EU countries - ability to permanently remove patron personal data from computer systems may be actually required by law, as mandated by EU Data Protection Directive 95/46/EC (to be replaced in 2016 by even more strict version). Aka: "Right to be Forgotten" (= personal data has to be deleted when the individual withdraws consent or the data is no longer necessary and there is no legitimate reason for an organization to keep it). AFAIK, interpretations, implementations and practical enforcement of this directive widely varies by country, but still - it will be better to have such functionality in the system, just to be on the safe side ;). But I agree it's perhaps too easy to destroy patron data by mistake using this tool. Maybe "Move these patrons to the trash" should be default option instead of "Permanently delete these patrons"? I've seen serious data loss because of bugs/misuse in the batch patron tools. I think it would be much better for all interactive Koha patron deletes to move them to deleted, and then have cleanup_database.pl delete them permanently. That way we have the most flexibility and safety possible. (In reply to Chris Cormack from comment #2) > Records and items aren't interesting to the FBI in the same way patron data > is. > I can see that as a valid reason for deleting them completely. > > Perhaps a better response is to do a "Are you sure you want to delete > permanently .. there is no way to get them back?" > > Confirmation prompt is a better way than disabling this. Created attachment 90381 [details] [review] Bug 11642: Add confirmation and tooltips to batch deletion tool Just trying to revive this one - this doesn't go as far as Kyle suggests, but at least adds a warn. I think we could do a second bug to add a permission to permanently delete to make the tool a bit safer generally? Also, we should probably add a check so you can't delete your own record (the account that is signed in) with this tool? I like the idea and discussion here. I am not quite ready to sign this of, some things I have thought about testing: Asking for confirmation - YES! The message itself appears a bit non-standard in wording and formatting. Maybe it could be a bit improved there. I'd also add a confirmation message in any case - we are deleting data. The hints on the radio boxes are a nice idea, but maybe too hidden. Also I'd be careful about the 'recoverable'. Not all of the deleted data is, even if you could (only with SQL) recover some of it. Adding Owen for a GUI opinion. Created attachment 90457 [details] [review] Bug 11642: (follow-up) Batch patron deletion/anonymization should not allow permanent destruction of patron data This patch makes some template changes to try to make this interface more consistent with other areas of Koha and to make warnings more clear. I have also changed the phrase "deletion/anonymization" to "deletion and anonymization." To test, apply the patch and test the patron deletion and anonymization process and confirm that the interface is clear and works well. Created attachment 90496 [details] [review] Bug 11642: change / to and in related tools menu Created attachment 90497 [details] [review] Bug 11642: Add confirmation and tooltips to batch deletion tool Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 90498 [details] [review] Bug 11642: (follow-up) Batch patron deletion/anonymization should not allow permanent destruction of patron data This patch makes some template changes to try to make this interface more consistent with other areas of Koha and to make warnings more clear. I have also changed the phrase "deletion/anonymization" to "deletion and anonymization." To test, apply the patch and test the patron deletion and anonymization process and confirm that the interface is clear and works well. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 90499 [details] [review] Bug 11642: change / to and in related tools menu Signed-off-by: Mark Tompsett <mtompset@hotmail.com> I like this much better, just one more thing for later, do we not want to rephrase this? 6 patrons have been successfully moved to trash Created attachment 90556 [details] [review] Bug 11642: Add confirmation and tooltips to batch deletion tool Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 90557 [details] [review] Bug 11642: (follow-up) Batch patron deletion/anonymization should not allow permanent destruction of patron data This patch makes some template changes to try to make this interface more consistent with other areas of Koha and to make warnings more clear. I have also changed the phrase "deletion/anonymization" to "deletion and anonymization." To test, apply the patch and test the patron deletion and anonymization process and confirm that the interface is clear and works well. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 90558 [details] [review] Bug 11642: change / to and in related tools menu Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Ok, sorry to be nit-picky, but this will still allow for permanent desctruction of data... so we need to change the bug title and patch title please. Feel free to change to 'passed QA' after. Created attachment 91511 [details] [review] Bug 11642: Add confirmation and tooltips to batch deletion tool Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 91512 [details] [review] Bug 11642: (follow-up) Improve Batch patron deletion and anonymization GUI to make consequences clearer This patch makes some template changes to try to make this interface more consistent with other areas of Koha and to make warnings more clear. I have also changed the phrase "deletion/anonymization" to "deletion and anonymization." To test, apply the patch and test the patron deletion and anonymization process and confirm that the interface is clear and works well. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 91513 [details] [review] Bug 11642: change / to and in related tools menu Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> (In reply to Katrin Fischer from comment #18) > Ok, sorry to be nit-picky, but this will still allow for permanent > desctruction of data... so we need to change the bug title and patch title > please. I've made those changes, while not perfect, I think a little closer to the truth of what these patches do. Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.03 backported to 18.11.x for 18.11.09 |