[Bug 29018] New: Deleting patrons from REST API doesn't do any checks or move to deletedborrowers
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Bug ID: 29018 Summary: Deleting patrons from REST API doesn't do any checks or move to deletedborrowers Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: katrin.fischer@bsz-bw.de When deleting a patron via the REST API it doesn't block on - has a guarantee - has checkouts - has pending fines and it doesn't move the patron to deletedborrowers :( It looks like the code around deleting patrons could do with a little work: Patrons batch deletes appear to use: https://git.koha-community.org/Koha-community/Koha/src/commit/42ce8c29960dc8... Deleting a single patron from the account in staff has a lot of additional checks in the .pl https://git.koha-community.org/Koha-community/Koha/src/commit/9d6d641d1f8b77... And then there are delete and move_to_deleted in Koha/Patron* -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I believe the API should not allow deletes on the same "hard reasons" as the GUI and take care of holds etc. silently as if it was confirmed in staff. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #2 from Fridolin Somers <fridolin.somers@biblibre.com> --- Arg this is bad. We can see there is only one check from Bug 14708 Maybe we need to create like for Koha::Item : safe_to_delete() safe_delete() Or throw more exceptions. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It's really bad but didn't get much attention yet. At the moment I would almost lean to deactivate that route as it might leave you with inconsistent data. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #3)
It's really bad but didn't get much attention yet. At the moment I would almost lean to deactivate that route as it might leave you with inconsistent data.
I'll fix it first thing in the morning next Monday :-) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 128762 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128762&action=edit Bug 29018: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 128763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128763&action=edit Bug 29018: Make DELETE /patrons/:patron_id check things When the route was implemented, the checks were overlooked. This patch adds checks for: - Guarantees - Debts - Current checkouts Any of those will block deletion, as it should. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Tests fail, the route misses checks 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The three conditions prevent deletion! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29741 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29741 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29741 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29741 [Bug 29741] Add Koha::Patron->safe_to_delete -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Fridolin Somers from comment #2)
Arg this is bad.
We can see there is only one check from Bug 14708
Maybe we need to create like for Koha::Item : safe_to_delete() safe_delete()
Done safe_to_delete on bug 29741 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128762|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 128812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128812&action=edit Bug 29018: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128763|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 128813 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128813&action=edit Bug 29018: Make DELETE /patrons/:patron_id check things When the route was implemented, the checks were overlooked. This patch adds checks for: - Guarantees - Debts - Current checkouts Any of those will block deletion, as it should. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Tests fail, the route misses checks 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The three conditions prevent deletion! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |These fixes the REST API release notes| |route for deleting patrons | |so that it now checks for | |guarantees, debts, and | |current checkouts. If any | |of these checks fail, the | |patron is not deleted. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28020 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28020 [Bug 28020] Error responses should return a code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128812|0 |1 is obsolete| | Attachment #128813|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 128959 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128959&action=edit Bug 29018: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 128960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128960&action=edit Bug 29018: Make DELETE /patrons/:patron_id check things When the route was implemented, the checks were overlooked. This patch adds checks for: - Guarantees - Debts - Current checkouts Any of those will block deletion, as it should. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Tests fail, the route misses checks 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The three conditions prevent deletion! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #12 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am late here, but thx a lot for fixing it! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Pushed to master |Pushed to stable Version(s)|22.05.00 |22.05.00,21.11.02 released in| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129282 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129282&action=edit [21.05.x] Bug 29018: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129283 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129283&action=edit [21.05.x] Bug 29018: Make DELETE /patrons/:patron_id check things When the route was implemented, the checks were overlooked. This patch adds checks for: - Guarantees - Debts - Current checkouts Any of those will block deletion, as it should. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Tests fail, the route misses checks 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The three conditions prevent deletion! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- The last patches should apply to 20.11 as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.05.00,21.11.02 |22.05.00,21.11.02,21.05.08 released in| | CC| |andrew@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable --- Comment #18 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Pushed to oldoldstable CC| |victor@tuxayo.net Version(s)|22.05.00,21.11.02,21.05.08 |22.05.00,21.11.02,21.05.08, released in| |20.11.15 --- Comment #19 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported: Pushed to 20.11.x branch for 20.11.15 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|These fixes the REST API |This fixes the REST API release notes|route for deleting patrons |route for deleting patrons |so that it now checks for |so that it now checks for |guarantees, debts, and |guarantees, debts, and |current checkouts. If any |current checkouts. If any |of these checks fail, the |of these checks fail, the |patron is not deleted. |patron is not deleted. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 wainuiwitikapark@catalyst.net.nz changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #20 from wainuiwitikapark@catalyst.net.nz --- Do I need to backport this to 19.11.x? I probably won't unless there is a need for it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29018 --- Comment #21 from wainuiwitikapark@catalyst.net.nz --- Not backported to 19.11.x. Please request it if you need it. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org