Bug 25552 - Add missing Claims Returned option to MarkLostItemsAsReturned
Summary: Add missing Claims Returned option to MarkLostItemsAsReturned
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
: 25518 (view as bug list)
Depends on: 14697
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-20 13:30 UTC by Kyle M Hall
Modified: 2021-02-22 21:03 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Marking an item as a return claim checks the system preference MarkLostItemsAsReturned to see if the claim should be removed from the patron record. However, the option for "when marking an item as a return claim" was never added to the system preference, so there was no way to remove a checkout from the patron record when marking the checkout as a return claim. This patch set adds that missing option.
Version(s) released in:
21.05.00,20.11.03,20.05.09


Attachments
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned (1.55 KB, patch)
2020-05-20 13:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned (3.99 KB, patch)
2020-05-27 11:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee (1.26 KB, patch)
2020-06-04 15:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee (1.46 KB, patch)
2020-06-04 15:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Unit tests (3.49 KB, patch)
2020-06-08 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned (4.03 KB, patch)
2020-11-20 14:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee (1.46 KB, patch)
2020-11-20 14:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Unit tests (3.55 KB, patch)
2020-11-20 14:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25552: Unit tests (3.59 KB, patch)
2020-11-20 16:30 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned (4.12 KB, patch)
2021-01-22 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee (1.55 KB, patch)
2021-01-22 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25552: Unit tests (3.64 KB, patch)
2021-01-22 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25552: (QA follow-up) Improve claim_returned POD (1.34 KB, patch)
2021-01-22 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25552: (QA follow-up) Fix spelling mistake (1.12 KB, patch)
2021-01-22 13:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25552: Simplify tests using build_sample_item (1.39 KB, patch)
2021-02-01 16:25 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2020-05-20 13:30:34 UTC
Marking an item as a return claim checks MarkLostItemsAsReturned to see if claim_returned is a value in MarkLostItemsAsReturned. However, this option was never added to MarkLostItemsAsReturned so an a return claim can never be automatically removed from the patron record, even if they wanted such behavior.
Comment 1 Kyle M Hall 2020-05-20 13:33:11 UTC
Created attachment 105146 [details] [review]
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned

Marking an item as a return claim checks MarkLostItemsAsReturned to see if claim_returned is a value in MarkLostItemsAsReturned.
However, this option was never added to MarkLostItemsAsReturned so an a return claim can never be automatically removed from the patron record, even if they wanted such behavior.

Test Plan:
1) Apply this patch
2) Restart all the things
3) Note the new return claims option on MarkLostItemsAsReturned in the system preferences
Comment 2 Katrin Fischer 2020-05-22 11:13:36 UTC
Hi Kyle, I tested this, but it doesn't return the item for me :(

There is no error in the logs - just some debug information:
[Fri May 22 11:02:55 2020] [debug] 201 Created (0.0484s, 20.661/s)
[Fri May 22 11:08:03 2020] [debug] POST "/api/v1/return_claims"
[Fri May 22 11:08:03 2020] [debug] Routing to controller "Koha::REST::V1::Auth" and action "under"
[Fri May 22 11:08:03 2020] [debug] Routing to controller "Koha::REST::V1::ReturnClaims" and action "claim_returned"

I think we also need to update the options in sysprefs.sql and maybe have a database update as well.
Comment 3 Katrin Fischer 2020-05-22 22:55:21 UTC
*** Bug 25518 has been marked as a duplicate of this bug. ***
Comment 4 Kyle M Hall 2020-05-27 11:09:58 UTC
Created attachment 105389 [details] [review]
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned

Marking an item as a return claim checks MarkLostItemsAsReturned to see if claim_returned is a value in MarkLostItemsAsReturned.
However, this option was never added to MarkLostItemsAsReturned so an a return claim can never be automatically removed from the patron record, even if they wanted such behavior.

Test Plan:
1) Apply this patch
2) Restart all the things
3) Note the new return claims option on MarkLostItemsAsReturned in the system preferences
Comment 5 Katrin Fischer 2020-05-28 22:28:35 UTC
This includes the fixes I asked for, but is otherwise exactly the same as the patch that didn't work for me and it still doesn't :(

I believe the test plan to be incomplete - adding a new option alone is not enough, it should work as well.

What I've done:
- checked the new option in the pref
- Marked an item as claim returned
- Lost status was set, but item remains on the account
Comment 6 Kyle M Hall 2020-06-04 15:17:56 UTC
Created attachment 105558 [details] [review]
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee
Comment 7 Kyle M Hall 2020-06-04 15:25:25 UTC
Created attachment 105559 [details] [review]
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee
Comment 8 Katrin Fischer 2020-06-07 12:53:01 UTC
I believe this is still not working right. The item is only returned when I check the 'charge lost fee' check box.

Please explain, when adding a new patch. I think we are also going to need unit tests.
Comment 9 Kyle M Hall 2020-06-08 12:15:00 UTC
(In reply to Katrin Fischer from comment #8)
> I believe this is still not working right. The item is only returned when I
> check the 'charge lost fee' check box.
> 
> Please explain, when adding a new patch. I think we are also going to need
> unit tests.

My followup fixes exactly the issue you are describing. I just re-tested and it's still working for me.
Comment 10 Kyle M Hall 2020-06-08 13:17:00 UTC
Created attachment 105643 [details] [review]
Bug 25552: Unit tests
Comment 11 Katrin Fischer 2020-06-08 14:43:49 UTC
I tested it twice, maybe messed up on git. I will redo the tests... again :)
Comment 12 Kelly McElligott 2020-07-09 21:18:20 UTC
Hi,
It would appear that the item does not remove until the page reloads? I would encourage to reload during this transaction so the staff can see it come off of the account.
Comment 13 Kyle M Hall 2020-07-22 13:52:49 UTC
(In reply to Kelly McElligott from comment #12)
> Hi,
> It would appear that the item does not remove until the page reloads? I
> would encourage to reload during this transaction so the staff can see it
> come off of the account.

Kelly, can you make a quick screencast of this so I'm sure I know what to fix?
Thanks!
Comment 14 Kelly McElligott 2020-08-11 16:18:49 UTC
I believe this is filed as a bug 
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25734

But will send along a screen share!
Comment 15 Kelly McElligott 2020-08-11 16:47:12 UTC
I believe this is filed as a bug 
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25734

Screenshare:
https://drive.google.com/file/d/1xVUqmCKD9f4GfqnLU4heeHmNqgig6hR5/view

Now the claim returned is not processing at all.  I have the system preference MarkLostItemsasReturned set to be removed from patron's account and it is not working.
Comment 16 Martha Fuerst 2020-11-19 18:30:07 UTC
Just making noise on this bug so that it get attention and gets resolved.

When we resolve a claim and a patron pays for the item, we currently have no way to remove the item from a patron's checkouts, as the lost status toggle in the bib "more details" items tab is grayed out, and even deleted the claim doesn't resolve that.

Thanks!

-Marti Fuerst
Huntsville-Madison County Public Library
Comment 17 Kyle M Hall 2020-11-20 14:33:39 UTC
Created attachment 113889 [details] [review]
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned

Marking an item as a return claim checks MarkLostItemsAsReturned to see if claim_returned is a value in MarkLostItemsAsReturned.
However, this option was never added to MarkLostItemsAsReturned so an a return claim can never be automatically removed from the patron record, even if they wanted such behavior.

Test Plan:
1) Apply this patch
2) Restart all the things
3) Note the new return claims option on MarkLostItemsAsReturned in the system preferences
Comment 18 Kyle M Hall 2020-11-20 14:33:51 UTC
Created attachment 113892 [details] [review]
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee
Comment 19 Kyle M Hall 2020-11-20 14:33:54 UTC
Created attachment 113893 [details] [review]
Bug 25552: Unit tests
Comment 20 ByWater Sandboxes 2020-11-20 16:30:03 UTC
Created attachment 113898 [details] [review]
Bug 25552: Unit tests

Signed-off-by: Marti Fyerst <mfuerst@hmcpl.org>
Comment 21 Martin Renvoize 2021-01-22 13:01:39 UTC
Created attachment 115685 [details] [review]
Bug 25552: Add missing Claims Returned option to MarkLostItemsAsReturned

Marking an item as a return claim checks MarkLostItemsAsReturned to see if claim_returned is a value in MarkLostItemsAsReturned.
However, this option was never added to MarkLostItemsAsReturned so an a return claim can never be automatically removed from the patron record, even if they wanted such behavior.

Test Plan:
1) Apply this patch
2) Restart all the things
3) Note the new return claims option on MarkLostItemsAsReturned in the system preferences

Signed-off-by: Marti Fyerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2021-01-22 13:01:43 UTC
Created attachment 115686 [details] [review]
Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee

Signed-off-by: Marti Fyerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2021-01-22 13:01:46 UTC
Created attachment 115687 [details] [review]
Bug 25552: Unit tests

Signed-off-by: Marti Fyerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2021-01-22 13:01:49 UTC
Created attachment 115688 [details] [review]
Bug 25552: (QA follow-up) Improve claim_returned POD

The POD for the Koha::Checkout->claim_returned method was somewhat
lacking; This patch attempts to explain what the code appears to
achieve.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2021-01-22 13:01:53 UTC
Created attachment 115689 [details] [review]
Bug 25552: (QA follow-up) Fix spelling mistake

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2021-01-22 13:16:47 UTC
This passes the QA scripts and works as expected for me..

I've added a bit of detail to the POD as there are unrelated things that came to mind whilst QAing this.. but I think those should be handled on their own bug which I'll open shortly.

Passing QA
Comment 27 Jonathan Druart 2021-02-01 15:14:18 UTC
Please add release notes.
Comment 28 Jonathan Druart 2021-02-01 16:25:14 UTC
Created attachment 116173 [details] [review]
Bug 25552: Simplify tests using build_sample_item
Comment 29 Jonathan Druart 2021-02-01 16:26:19 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 30 Martha Fuerst 2021-02-02 15:37:51 UTC
Any chance of getting this backported?

Thanks!

Marti Fuerst
Huntsville-Madison County Public Library
Comment 31 Daniel Gaghan 2021-02-02 15:40:28 UTC
(In reply to Martha Fuerst from comment #30)
> Any chance of getting this backported?
> 
> Thanks!
> 
> Marti Fuerst
> Huntsville-Madison County Public Library

I second this, I thought it was coming in 20.05 and our circ staff were hoping for it.
Comment 32 Fridolin Somers 2021-02-05 16:29:55 UTC
Pushed to 20.11.x for 20.11.03
Comment 33 Andrew Fuerste-Henry 2021-02-09 21:59:13 UTC
Pushed to 20.05.x for 20.05.09
Comment 34 Victor Grousset/tuxayo 2021-02-11 06:15:44 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.