When a staff deletes another staff member's staff account and that particular account happens to have created return_claims, it creates a NULL value in the return_claims.created_by, causing a problem when another staff tries to resolve the claims. On the patron's account Claim tab, when one tries to resolve the claim, it will perpetually load without doing anything. Only way to fix is update the NULL value on the created_by and add a borrowernumber. Is there a way to transfer the claim to a different staff account? Should Koha allow the deletion of an account when there are still return_claims unresolved?
(In reply to Enica Davis from comment #0) > Should Koha allow the deletion of an account when there are still > return_claims unresolved? I think yes, either by transferring to another account or by storing a "dead" borrowernumber like in the reports table.
Could setting to NULL also be an option? I think it might work better with FK constraints than allowing a "dead" one :)
(In reply to Katrin Fischer from comment #2) > Could setting to NULL also be an option? I think it might work better with > FK constraints than allowing a "dead" one :) Yeah I think that would work.
That's the problem: NULL value on return_claims.created_by would cause a problem in the staff client if you try to resolve the claim. The "Resolve" button would perpetually spin. It's a minor issue that the only quick fix is to update the database and assign a borrowernumber to return_claims.created_by. :(
Ok, so we can fix this outside the database and teach it to work with NULL borrowernumber then?
Created attachment 114547 [details] [review] Bug 27004: Prevent claim resolution to fail If the account of the patron who created a claim is removed, then the claim won't be resolvable. Trying to resolve the claim will result in a 500: [2020/12/21 10:05:55] [ERROR] PUT /api/v1/return_claims/1/resolve: unhandled exception (Koha::Exceptions::Checkouts::ReturnClaims::NoCreatedBy)<<[created_by is mandatory]>> Koha::REST::Plugin::Exceptions::__ANON We should not check if created_by exist when we update an existing hold, only when we create it. Test plan: - Login with patron A - Check an item in - Claim returned it - Login with patron B - Delete patron A - Resolve the claim
QA: should not we have a test/exception for resolved_by? We have one for created_by, it would make sense to have the same behaviour for *_by fields.
Created attachment 114573 [details] [review] Bug 27004: Prevent claim resolution to fail If the account of the patron who created a claim is removed, then the claim won't be resolvable. Trying to resolve the claim will result in a 500: [2020/12/21 10:05:55] [ERROR] PUT /api/v1/return_claims/1/resolve: unhandled exception (Koha::Exceptions::Checkouts::ReturnClaims::NoCreatedBy)<<[created_by is mandatory]>> Koha::REST::Plugin::Exceptions::__ANON We should not check if created_by exist when we update an existing hold, only when we create it. Test plan: - Login with patron A - set a value to syspref ClaimReturnedLostValue - Check an item out - Claim returned it - Login with patron B - Delete patron A - Resolve the claim - ERROR in api-error.log an UI never confirming the resolution - apply patch & restart services - Resolve the claim - it works Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works :) Completed test plan + fixed "Check an item in" => "Check an item out" (maybe it's actually doable with a checkin)
Created attachment 114732 [details] [review] Bug 27004: Regression tests This patch introduces regression tests for the change in Koha::Checkouts::ReturnClaim->store. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Checkouts/ReturnClaim.t => FAIL: Tests fail Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 114733 [details] [review] Bug 27004: Prevent claim resolution to fail If the account of the patron who created a claim is removed, then the claim won't be resolvable. Trying to resolve the claim will result in a 500: [2020/12/21 10:05:55] [ERROR] PUT /api/v1/return_claims/1/resolve: unhandled exception (Koha::Exceptions::Checkouts::ReturnClaims::NoCreatedBy)<<[created_by is mandatory]>> Koha::REST::Plugin::Exceptions::__ANON We should not check if created_by exist when we update an existing hold, only when we create it. Test plan: - Login with patron A - set a value to syspref ClaimReturnedLostValue - Check an item out - Claim returned it - Login with patron B - Delete patron A - Resolve the claim - ERROR in api-error.log an UI never confirming the resolution - apply patch & restart services - Resolve the claim - it works Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.01
Pushed to 20.05.x for 20.05.07
Backported: Pushed to 19.11.x branch for 19.11.14