Bug 29519 - Add option to resolve a return claim at checkin
Summary: Add option to resolve a return claim at checkin
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on: 29495
Blocks: 31085
  Show dependency treegraph
 
Reported: 2021-11-18 13:58 UTC by Martin Renvoize (ashimema)
Modified: 2024-11-07 16:08 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds the option to resolve a return claim upon the next check-in of the item.
Version(s) released in:
22.05.00,21.11.03
Circulation function:


Attachments
Bug 29519: Allow resolution of claims return at checkin (14.90 KB, patch)
2021-11-18 14:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: Allow resolution of claims return at checkin (14.90 KB, patch)
2021-11-18 15:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: (follow-up) Use 'item' relationship (2.09 KB, patch)
2021-11-18 15:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: Allow resolution of claims return at checkin (14.90 KB, patch)
2021-11-18 15:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: (follow-up) Use 'item' relationship (2.09 KB, patch)
2021-11-18 15:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: Allow resolution of claims return at checkin (14.96 KB, patch)
2021-11-18 16:27 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29519: (follow-up) Use 'item' relationship (2.16 KB, patch)
2021-11-18 16:27 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29519: Allow resolution of claims return at checkin (14.96 KB, patch)
2021-11-22 18:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: Allow resolution of claims return at checkin (14.95 KB, patch)
2022-01-21 15:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29519: Allow resolution of claims return at checkin (15.00 KB, patch)
2022-02-08 18:48 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2021-11-18 13:58:25 UTC
Currently, upon checking in an item that has a return claim associated with it, we highlight this to the librarian but we don't give any option to resolve the claim there and then (but we do mark the item as found in the background).

We should give the librarian the option to resolve the claim.
Comment 1 Martin Renvoize (ashimema) 2021-11-18 14:30:08 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.
Comment 2 Andrew Fuerste-Henry 2021-11-18 15:13:23 UTC
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
Comment 3 Andrew Fuerste-Henry 2021-11-18 15:21:39 UTC
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?
Comment 4 Martin Renvoize (ashimema) 2021-11-18 15:40:16 UTC
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
Comment 5 Martin Renvoize (ashimema) 2021-11-18 15:40:48 UTC
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.
Comment 6 Martin Renvoize (ashimema) 2021-11-18 15:48:45 UTC
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.
Comment 7 Martin Renvoize (ashimema) 2021-11-18 15:48:49 UTC
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).
Comment 8 Martin Renvoize (ashimema) 2021-11-18 15:49:16 UTC
That should be better now :)
Comment 9 Martin Renvoize (ashimema) 2021-11-18 15:54:50 UTC
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.
Comment 10 Martin Renvoize (ashimema) 2021-11-18 15:54:54 UTC
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).
Comment 11 Andrew Fuerste-Henry 2021-11-18 16:27:06 UTC
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>
Comment 12 Andrew Fuerste-Henry 2021-11-18 16:27:09 UTC
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>
Comment 13 Martin Renvoize (ashimema) 2021-11-19 07:58:53 UTC
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.
Comment 14 Martin Renvoize (ashimema) 2021-11-22 18:03:47 UTC
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>
Comment 15 Martin Renvoize (ashimema) 2021-11-22 18:04:41 UTC
Tests added to dependancy bug and item relation patch moved there..

Over to QA :)
Comment 16 Martin Renvoize (ashimema) 2022-01-21 15:28:35 UTC
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>
Comment 17 Martin Renvoize (ashimema) 2022-01-21 15:28:55 UTC
Just a rebase to keep it applying.
Comment 18 Tomás Cohen Arazi (tcohen) 2022-02-08 18:48:06 UTC
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>
Comment 19 Tomás Cohen Arazi (tcohen) 2022-02-08 18:49:46 UTC
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
Comment 20 Fridolin Somers 2022-02-15 07:42:36 UTC
Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑
Comment 21 Kyle M Hall (khall) 2022-02-17 12:08:57 UTC
Pushed to 21.11.x for 21.11.03
Comment 22 Andrew Fuerste-Henry 2022-02-17 20:43:47 UTC
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.