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.
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
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).
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.
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.
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>
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>
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>
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>
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.06
Nice work everyone! Pushed to 22.11.x for next release