Description
Kyle M Hall (khall)
2020-05-20 13:30:34 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 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. *** Bug 25518 has been marked as a duplicate of this bug. *** 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 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 Created attachment 105558 [details] [review] Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee Created attachment 105559 [details] [review] Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee 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. (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. Created attachment 105643 [details] [review] Bug 25552: Unit tests I tested it twice, maybe messed up on git. I will redo the tests... again :) 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. (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! 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! 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. 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 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 Created attachment 113892 [details] [review] Bug 25552: Mark issue as returned if MarkLostItemsAsReturned, even if not charging a lost fee Created attachment 113893 [details] [review] Bug 25552: Unit tests Created attachment 113898 [details] [review] Bug 25552: Unit tests Signed-off-by: Marti Fyerst <mfuerst@hmcpl.org> 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> 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> 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> 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> Created attachment 115689 [details] [review] Bug 25552: (QA follow-up) Fix spelling mistake Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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 Please add release notes. Created attachment 116173 [details] [review] Bug 25552: Simplify tests using build_sample_item Pushed to master for 21.05, thanks to everybody involved! Any chance of getting this backported? Thanks! Marti Fuerst Huntsville-Madison County Public Library (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. Pushed to 20.11.x for 20.11.03 Pushed to 20.05.x for 20.05.09 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |