|
Description
Martin Renvoize (ashimema)
2021-11-18 13:58:25 UTC
Created attachment 127802 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Quick test plan for checking both that current behavior is unchanged and that new behavior works. Let me know if I'm missing something! 1 - set ClaimReturnedLostValue syspref to enable claims 2 - set ClaimReturnedChargeFee to "ask if a lost fee should be charged" 3 - check out an item, mark claimed, choose to charge a fee, confirm your fee charged 4 - check out an item, mark claimed, choose NOT to charge a fee, confirm your fee did not charge 5 - check a claimed item in 6 - confirm you see the new Resolve button and can use it to successfully resolve a claim 7 - resolve your other claim without checking the item in 8 - check your item in and confirm you don't see the Resolve button, since this claim is already resolved 9 - set ClaimReturnedWarningThreshold to 1, confirm your claims are correctly counted toward this threshold When I check in an item with an unresolved claim, I see the new Resolve button and it opens up a modal with which to resolve the claim, but I get an API error when I click the Resolve Claim button in the modal. The button spins indefinitely and the browser console says "None of these routes could generate a response for your GET request for /api/v1/return_claims/1/resolve, maybe you need to add a new one?" I'm not sure if that's an issue with the patch or with how I tested it. After applying the patch I restarted services and cleared by browser cache. Did I miss a step there? Ha.. this highlights a bug in the existing code I already fixed but hadn't realised affected here too. I've now just added it as a dependancy Hold off on testing until I mark back to NSO.. just trying out the fix here first to make sure it's not more involved. Created attachment 127805 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Created attachment 127806 [details] [review] Bug 29519: (follow-up) Use 'item' relationship This patch adds the missing 'item' relationship in Checkouts::ReturnClaim and then uses it from the resolve method. This improve the reliability of the resolution code so it works when the item has already been checked in (without having to check Old::Checkouts). That should be better now :) Created attachment 127807 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Created attachment 127808 [details] [review] Bug 29519: (follow-up) Use 'item' relationship This patch adds the missing 'item' relationship in Checkouts::ReturnClaim and then uses it from the resolve method. This improve the reliability of the resolution code so it works when the item has already been checked in (without having to check Old::Checkouts). Created attachment 127814 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 127815 [details] [review] Bug 29519: (follow-up) Use 'item' relationship This patch adds the missing 'item' relationship in Checkouts::ReturnClaim and then uses it from the resolve method. This improve the reliability of the resolution code so it works when the item has already been checked in (without having to check Old::Checkouts). Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Damn.. t/db_dependant/Koha/Checkouts/ReturnClaim.pm is failing.. can't for the life of me work out what the particular test is testing for (Why it's using builder to test for a allow null?) or why TestBuilder itself doesn't like a constraint drop... (Tests fails before and after running the included DB update).. Any help at the QA stage gratefully appreciated. Created attachment 127945 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Tests added to dependancy bug and item relation patch moved there.. Over to QA :) Created attachment 129700 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Just a rebase to keep it applying. Created attachment 130338 [details] [review] Bug 29519: Allow resolution of claims return at checkin Add a 'Resolve' button in the alert dialogue that is displayed when a lost item with a return claim is checked in. The button will trigger the usual resolution modal allowing the user to pick their resolution. This patch splits the resolution modal out of checkouts.js and checkouts-table.inc so it can be used outside of the checkouts table. We then reload it, optionally based upon the presence of the claims preference, where needed. This has the added benefit that it saves a little bit of page load data in cases where the feature is not enabled. Test plan 1. As we alter the file locations of the resolution handling code we need to test that normal claims functionality continue to work as expected. 2. Test the new functoinality by checking in an item that has been claimed as returned (but not yet resolved). The dialogue box should now contain a 'resolve' button next to each claimant and clicking upon it should trigger the resolution modal where the librarian can subsequently pick the resolution and submit it. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I like it. Maybe the interaction between marking the return claim as resolved and the previous checkin modal could be revisited at some point (i.e. it still lets you choose the 'Resolve' button). Not a blocker IMHO. Great Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.03 I'd love to backport this to 21.05. It applies cleanly, but the API call hangs when I try to resolve a claim using the modal. I get a 500 error wit this message: None of these routes could generate a response for your GET request for /api/v1/return_claims/4/resolve, maybe you need to add a new one? Any help would be appreciated. |