Bug 38246 - If using automatic return claim resolution on checkout, each checkout will overwrite the previous resolution
Summary: If using automatic return claim resolution on checkout, each checkout will ov...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Emily Lamancusa (emlam)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords: rel_24_05_candidate
Depends on: 27753
Blocks:
  Show dependency treegraph
 
Reported: 2024-10-23 19:31 UTC by Emily Lamancusa (emlam)
Modified: 2025-04-18 19:52 UTC (History)
8 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.08
Circulation function:


Attachments
Bug 38246: Only check unresolved claims (1.79 KB, patch)
2024-10-23 20:01 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 38246: Perltidy (18.36 KB, patch)
2024-10-23 20:01 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 38246: Only check unresolved claims (1.85 KB, patch)
2024-10-24 05:14 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 38246: Perltidy (18.42 KB, patch)
2024-10-24 05:14 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 38246: Only check unresolved claims (1.91 KB, patch)
2024-10-28 10:35 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38246: Perltidy (18.48 KB, patch)
2024-10-28 10:35 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38246: [24.05.x] Only check unresolved claims (17.67 KB, patch)
2024-12-16 20:22 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emily Lamancusa (emlam) 2024-10-23 19:31:31 UTC
The checkout page searches for all return claims rather than only unresolved ones, so if an item being checked out has a return claim that was previously resolved, it will resolve it again and overwrite the previous resolution.

Fortunately, any notes on the return claim are a separate functionality, so this will not erase the notes - it only updates the resolution timestamp and sets the resolution code (which is an authorized value and cannot contain free text).
Comment 1 Emily Lamancusa (emlam) 2024-10-23 20:01:31 UTC
Created attachment 173238 [details] [review]
Bug 38246: Only check unresolved claims

To test:
1. Set up sysprefs:
   - Set any non-empty value for ClaimReturnedLostValue
   - Set AutoClaimReturnStatusOnCheckout to "found in library"
   - Set AutoClaimReturnStatusOnCheckin to "returned by patron"
2. Check out an item to a patron
3. On the patron's account page, click the "Claim Return" button next to
   that checkout
4. Check the item in. Note the message that the claim was auto-resolved.
5. Return to the patron's account page and open the Claims Returned tab
--> Note that the claim is resolved and hidden
6. Click the "Show all 1 claims" link to view the hidden claim. Make a
   note of the resolution code and timestamp.
7. Check the item out again
--> Note the message saying that the claim was resolved, even though it
    was already resolved previously
8. View the return claim again
--> Note that the timestamp and resolution code have changed
9. Apply patch and restart_all
10. Repeat steps 2-8 with a new item
--> Note you do not get a claim resolved message on checkout this time
--> Note that the timestamp and resolution code remain the same before
    and after the checkout
Comment 2 Emily Lamancusa (emlam) 2024-10-23 20:01:32 UTC
Created attachment 173239 [details] [review]
Bug 38246: Perltidy

QA tool failed because the previous patch added one line to a large
block of code that was incorrectly indented. Fixed the indentation
separately on this patch.
Comment 3 Phil Ringnalda 2024-10-24 05:14:37 UTC
Created attachment 173248 [details] [review]
Bug 38246: Only check unresolved claims

To test:
1. Set up sysprefs:
   - Set any non-empty value for ClaimReturnedLostValue
   - Set AutoClaimReturnStatusOnCheckout to "found in library"
   - Set AutoClaimReturnStatusOnCheckin to "returned by patron"
2. Check out an item to a patron
3. On the patron's account page, click the "Claim Return" button next to
   that checkout
4. Check the item in. Note the message that the claim was auto-resolved.
5. Return to the patron's account page and open the Claims Returned tab
--> Note that the claim is resolved and hidden
6. Click the "Show all 1 claims" link to view the hidden claim. Make a
   note of the resolution code and timestamp.
7. Check the item out again
--> Note the message saying that the claim was resolved, even though it
    was already resolved previously
8. View the return claim again
--> Note that the timestamp and resolution code have changed
9. Apply patch and restart_all
10. Repeat steps 2-8 with a new item
--> Note you do not get a claim resolved message on checkout this time
--> Note that the timestamp and resolution code remain the same before
    and after the checkout

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 4 Phil Ringnalda 2024-10-24 05:14:40 UTC
Created attachment 173249 [details] [review]
Bug 38246: Perltidy

QA tool failed because the previous patch added one line to a large
block of code that was incorrectly indented. Fixed the indentation
separately on this patch.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 5 Martin Renvoize (ashimema) 2024-10-28 10:35:04 UTC
Created attachment 173497 [details] [review]
Bug 38246: Only check unresolved claims

To test:
1. Set up sysprefs:
   - Set any non-empty value for ClaimReturnedLostValue
   - Set AutoClaimReturnStatusOnCheckout to "found in library"
   - Set AutoClaimReturnStatusOnCheckin to "returned by patron"
2. Check out an item to a patron
3. On the patron's account page, click the "Claim Return" button next to
   that checkout
4. Check the item in. Note the message that the claim was auto-resolved.
5. Return to the patron's account page and open the Claims Returned tab
--> Note that the claim is resolved and hidden
6. Click the "Show all 1 claims" link to view the hidden claim. Make a
   note of the resolution code and timestamp.
7. Check the item out again
--> Note the message saying that the claim was resolved, even though it
    was already resolved previously
8. View the return claim again
--> Note that the timestamp and resolution code have changed
9. Apply patch and restart_all
10. Repeat steps 2-8 with a new item
--> Note you do not get a claim resolved message on checkout this time
--> Note that the timestamp and resolution code remain the same before
    and after the checkout

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2024-10-28 10:35:06 UTC
Created attachment 173498 [details] [review]
Bug 38246: Perltidy

QA tool failed because the previous patch added one line to a large
block of code that was incorrectly indented. Fixed the indentation
separately on this patch.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Katrin Fischer 2024-10-29 07:54:40 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 8 Lucas Gass (lukeg) 2024-11-22 21:22:02 UTC
Doesn't clean apply to 24.05.x, rebase if needed.
Comment 9 Emily Lamancusa (emlam) 2024-11-25 21:15:04 UTC
(In reply to Lucas Gass (lukeg) from comment #8)
> Doesn't clean apply to 24.05.x, rebase if needed.

I do reproduce the bug on 24.05, so yes, it should be backported. The first patch applies cleanly for me, and the second patch is just a big whitespace perltidy of existing code. I'll rebase the perltidy if it's necessary, but it would also be fine to just pick the first patch for backport if you're okay with that.
Comment 10 Emily Lamancusa (emlam) 2024-12-16 20:22:04 UTC
Created attachment 175578 [details] [review]
Bug 38246: [24.05.x] Only check unresolved claims

To test:
1. Set up sysprefs:
   - Set any non-empty value for ClaimReturnedLostValue
   - Set AutoClaimReturnStatusOnCheckout to "found in library"
   - Set AutoClaimReturnStatusOnCheckin to "returned by patron"
2. Check out an item to a patron
3. On the patron's account page, click the "Claim Return" button next to
   that checkout
4. Check the item in. Note the message that the claim was auto-resolved.
5. Return to the patron's account page and open the Claims Returned tab
--> Note that the claim is resolved and hidden
6. Click the "Show all 1 claims" link to view the hidden claim. Make a
   note of the resolution code and timestamp.
7. Check the item out again
--> Note the message saying that the claim was resolved, even though it
    was already resolved previously
8. View the return claim again
--> Note that the timestamp and resolution code have changed
9. Apply patch and restart_all
10. Repeat steps 2-8 with a new item
--> Note you do not get a claim resolved message on checkout this time
--> Note that the timestamp and resolution code remain the same before
    and after the checkout

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Emily Lamancusa (emlam) 2024-12-16 20:23:25 UTC
Reopened to add rebased patch for 24.05.x backport
Comment 12 Alex Buckley 2025-02-21 02:38:02 UTC
Hi Emily, 

I have applied your third patch to 24.05.x and the test plan worked as expected.

Could you please clarify for us, should we only be applying and backporting the third patch?

Thanks,
Alex
Comment 13 Emily Lamancusa (emlam) 2025-02-21 14:39:59 UTC
(In reply to Alex Buckley from comment #12)
> Hi Emily, 
> 
> I have applied your third patch to 24.05.x and the test plan worked as
> expected.
> 
> Could you please clarify for us, should we only be applying and backporting
> the third patch?
> 
> Thanks,
> Alex

Yes, just backport the third patch - it's a rebased equivalent of the first two, since they didn't apply cleanly to 24.05.x. (I can't remember now why I decided to combine them into one patch when doing the rebase ^^") Thank you for backporting!
Comment 14 Alex Buckley 2025-02-24 01:30:21 UTC
(In reply to Emily Lamancusa (emlam) from comment #13)
> (In reply to Alex Buckley from comment #12)
> > Hi Emily, 
> > 
> > I have applied your third patch to 24.05.x and the test plan worked as
> > expected.
> > 
> > Could you please clarify for us, should we only be applying and backporting
> > the third patch?
> > 
> > Thanks,
> > Alex
> 
> Yes, just backport the third patch - it's a rebased equivalent of the first
> two, since they didn't apply cleanly to 24.05.x. (I can't remember now why I
> decided to combine them into one patch when doing the rebase ^^") Thank you
> for backporting!

Hi Emily, 

Thank you for that!

We probably won't be able to include it in the 24.05.07 release, due to time constraints - but it's on the list for backporting in the 24.05.08 release!
Comment 15 Alex Buckley 2025-03-20 04:49:05 UTC
Nice work everyone!

Pushed to 24.05.x for 24.05.08
Comment 16 Fridolin Somers 2025-03-20 15:27:40 UTC
Depends on Bug 27753 not in 23.11.x
Comment 17 Caroline Cyr La Rose 2025-04-18 19:52:24 UTC
Big fix, nothing to add/edit in the manual.