Bug 33164 - Return claim message shows intermittently when BlockReturnOfLostItems enabled
Summary: Return claim message shows intermittently when BlockReturnOfLostItems enabled
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Aleisha Amohia
QA Contact: Emily Lamancusa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-08 03:41 UTC by Aleisha Amohia
Modified: 2023-11-14 15:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.06,22.11.12


Attachments
Bug 33164: Ensure return claim message shows if lost item return blocked (1.49 KB, patch)
2023-03-08 03:57 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33164: Do not prevent processing of all error messages (1.46 KB, patch)
2023-10-23 22:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33164: Ensure return claim message shows if lost item return blocked (1.54 KB, patch)
2023-10-27 16:27 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 33164: Do not prevent processing of all error messages (1.51 KB, patch)
2023-10-27 16:27 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 33164: Ensure return claim message shows if lost item return blocked (1.62 KB, patch)
2023-11-09 18:15 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 33164: Do not prevent processing of all error messages (1.58 KB, patch)
2023-11-09 18:15 UTC, Emily Lamancusa
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2023-03-08 03:41:04 UTC
To reproduce:

1. Set BlockReturnOfLostItems system preference to Block
2. Check out an item to a patron
3. Go to the patrons page and claim a return on the item
4. Check in the item and keep checking in the item. Notice the message "Item is lost, cannot be checked in." shows every time, but the return claim message only shows sometimes.

This is because BlockReturnOfLostItems interrupts the return process and may prevent the return claim message from coming through. This is a problem since this message is the only way to resolve a return claim, and the message may not show when the item is returned.
Comment 1 Aleisha Amohia 2023-03-08 03:57:19 UTC
Created attachment 147890 [details] [review]
Bug 33164: Ensure return claim message shows if lost item return blocked

This fix ensures that a return claim confirmation always shows, even if
the return of lost items has been blocked by BlockReturnOfLostItems
being enabled.

To test:

1. Set BlockReturnOfLostItems system preference to Block
2. Check out an item to a patron
3. Go to the patrons page and claim a return on the item
4. Check in the item and keep checking in the item. Notice the message
"Item is lost, cannot be checked in." shows every time, but the return
claim message only shows sometimes.

5. Apply the patch and restart services

6. Check in the item and keep checking in the item. Confirm both the
lost message shows and the return claim message shows every time.
7. Confirm the return of the item is still blocked.

Sponsored-by: Pymble Ladies' College
Comment 2 Phil Ringnalda 2023-03-20 17:45:04 UTC
Wouldn't the correct fix be to instead move the processing of a ReturnClaims message up out of the errmsgloop section, up around Wrongbranch? Otherwise, someone else is going to show up with a workflow that involves withdrawing claimed returns and wanting to resolve them at checkin.

If we believed in comments, the comment for the "# Error messages" loop would be something like "Process messages that only need to be stuffed into @errmsgloop, and which don't mind being randomly kept or ignored if something sets $exit_required_p". Since ReturnClaims doesn't match either of those, it's just in the wrong place (other than having to still have an empty elsif, to stay out of the "unknown error code" path).
Comment 3 Phil Ringnalda 2023-06-09 20:29:19 UTC
If the (admittedly very silly) behavior of $exit_required_p isn't worth keeping for BlockReturnOfLostItems then it should just be removed completely. Otherwise, either ReturnClaims processing should move out of the "# Error messages" loop, or the $exit_required_p functionality should be rewritten to sort the waslost and/or withdrawn messages to the bottom of the list.
Comment 4 Aleisha Amohia 2023-10-23 22:30:29 UTC
Created attachment 157710 [details] [review]
Bug 33164: Do not prevent processing of all error messages

This follow-up patch removes the use of $exit_required_p completely. The sysprefs BlockReturnOfLostItems and BlockReturnOfWithdrawnItems are handled in other scripts, and should not prevent the processing of all returns error messages when enabled.
Comment 5 Phil Ringnalda 2023-10-27 16:27:23 UTC
Created attachment 157993 [details] [review]
Bug 33164: Ensure return claim message shows if lost item return blocked

This fix ensures that a return claim confirmation always shows, even if
the return of lost items has been blocked by BlockReturnOfLostItems
being enabled.

To test:

1. Set BlockReturnOfLostItems system preference to Block
2. Check out an item to a patron
3. Go to the patrons page and claim a return on the item
4. Check in the item and keep checking in the item. Notice the message
"Item is lost, cannot be checked in." shows every time, but the return
claim message only shows sometimes.

5. Apply the patch and restart services

6. Check in the item and keep checking in the item. Confirm both the
lost message shows and the return claim message shows every time.
7. Confirm the return of the item is still blocked.

Sponsored-by: Pymble Ladies' College
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 6 Phil Ringnalda 2023-10-27 16:27:26 UTC
Created attachment 157994 [details] [review]
Bug 33164: Do not prevent processing of all error messages

This follow-up patch removes the use of $exit_required_p completely. The sysprefs BlockReturnOfLostItems and BlockReturnOfWithdrawnItems are handled in other scripts, and should not prevent the processing of all returns error messages when enabled.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 7 Emily Lamancusa 2023-11-09 18:15:40 UTC
Created attachment 158760 [details] [review]
Bug 33164: Ensure return claim message shows if lost item return blocked

This fix ensures that a return claim confirmation always shows, even if
the return of lost items has been blocked by BlockReturnOfLostItems
being enabled.

To test:

1. Set BlockReturnOfLostItems system preference to Block
2. Check out an item to a patron
3. Go to the patrons page and claim a return on the item
4. Check in the item and keep checking in the item. Notice the message
"Item is lost, cannot be checked in." shows every time, but the return
claim message only shows sometimes.

5. Apply the patch and restart services

6. Check in the item and keep checking in the item. Confirm both the
lost message shows and the return claim message shows every time.
7. Confirm the return of the item is still blocked.

Sponsored-by: Pymble Ladies' College
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 8 Emily Lamancusa 2023-11-09 18:15:42 UTC
Created attachment 158761 [details] [review]
Bug 33164: Do not prevent processing of all error messages

This follow-up patch removes the use of $exit_required_p completely. The sysprefs BlockReturnOfLostItems and BlockReturnOfWithdrawnItems are handled in other scripts, and should not prevent the processing of all returns error messages when enabled.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 9 Tomás Cohen Arazi 2023-11-10 14:02:15 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 10 Fridolin Somers 2023-11-11 07:38:07 UTC
Pushed to 23.05.x for 23.05.06
Comment 11 Pedro Amorim 2023-11-14 15:02:03 UTC
Nice work everyone!

Pushed to 22.11.x for next release