Bug 37505

Summary: Statistical patrons don't display information about item status if item wasn't checked out
Product: Koha Reporter: Emily Lamancusa (emlam) <emily.lamancusa>
Component: CirculationAssignee: Lucas Gass (lukeg) <lucas>
Status: Pushed to oldoldstable --- QA Contact: Emily Lamancusa (emlam) <emily.lamancusa>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, gmcharlt, kelly, kyle.m.hall, lucas, nick, rcoert
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.06,23.11.11
Circulation function:
Bug Depends on: 27992, 35950    
Bug Blocks: 38062    
Attachments: Bug 37505: Remove onloan check when checking out to stats patron
Bug 37505: Remove onloan check when checking out to stats patron
Bug 37505: Remove onloan check when checking out to stats patron
Bug 37505: (QA follow-up) Don't log return stat if ignore_localuse and no checkout
Bug 37505: (QA follow-up) Adjust tests

Description Emily Lamancusa (emlam) 2024-07-26 20:43:17 UTC
If an item is scanned with a statistical patron, the the page is meant to display an alert if the item has any current checkouts, holds, recalls, return claims, or lost/withdrawn status (as well as automatically ending checkouts and clearing lost statuses).

However, the page is not currently displaying those alerts unless the item has a current checkout, even though the other statuses are unrelated to whether the item is currently checked out or not.
Comment 1 Nick Clemens (kidclamp) 2024-08-27 19:17:52 UTC
It looks like bug 27992 added this information for all stats checkouts, then 35950 changed it to only return the book if checked out.

If we are to check in the book each time to show the messages I think we need to add a way to skip the stats for return for each statistical checkout.
Comment 2 Lucas Gass (lukeg) 2024-08-29 21:01:47 UTC
> If we are to check in the book each time to show the messages I think we
> need to add a way to skip the stats for return for each statistical checkout.

If the item is not checked out, right? If it is checked out we want to record the return and the localuse.
Comment 3 Lucas Gass (lukeg) 2024-08-29 21:55:49 UTC
Created attachment 170888 [details] [review]
Bug 37505: Remove onloan check when checking out to stats patron

To test:
1.  Create a Statistical Patron
2.  Set BlockReturnOfLostItems to Block.
3..  Mark an item as lost
4. Check it out to the Statistical Patron. You should see the message "Item was lost, cannot be returned."

5. Set BlockReturnOfWithdrawnItems to Block.
6.  Mark an item as withdrawn
7. Check it out to the Statistical Patron. You should see the message "Item was withdrawn, cannot be returned."
8. Turn off BlockReturnOfWithdrawnItems, check out the same item to the Statistical Patron. You should see a message "Item was withdrawn."

9. Place an item on hold.
10. Check it out to the Statistical Patron
11. See the message "Item on hold, please checkin."

12. Make sure  ClaimReturnedLostValue is set to a lost value
13. Claim a return, making sure it is not still checked out to the patron
14. Checkit it out to the Statistical Patron.
15. See the message "Item claimed returned, please checkin."

16. Have an item on a regular patron account that has been recalled.
17. Checkit it out to the Statistical Patron.
18. See the message "Item can fill a recall, please checkin."
Comment 4 ByWater Sandboxes 2024-08-30 14:09:28 UTC
Created attachment 170911 [details] [review]
Bug 37505: Remove onloan check when checking out to stats patron

To test:
1.  Create a Statistical Patron
2.  Set BlockReturnOfLostItems to Block.
3..  Mark an item as lost
4. Check it out to the Statistical Patron. You should see the message "Item was lost, cannot be returned."

5. Set BlockReturnOfWithdrawnItems to Block.
6.  Mark an item as withdrawn
7. Check it out to the Statistical Patron. You should see the message "Item was withdrawn, cannot be returned."
8. Turn off BlockReturnOfWithdrawnItems, check out the same item to the Statistical Patron. You should see a message "Item was withdrawn."

9. Place an item on hold.
10. Check it out to the Statistical Patron
11. See the message "Item on hold, please checkin."

12. Make sure  ClaimReturnedLostValue is set to a lost value
13. Claim a return, making sure it is not still checked out to the patron
14. Checkit it out to the Statistical Patron.
15. See the message "Item claimed returned, please checkin."

16. Have an item on a regular patron account that has been recalled.
17. Checkit it out to the Statistical Patron.
18. See the message "Item can fill a recall, please checkin."

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 5 Emily Lamancusa (emlam) 2024-09-26 20:30:55 UTC
(In reply to Nick Clemens (kidclamp) from comment #1)
> It looks like bug 27992 added this information for all stats checkouts, then
> 35950 changed it to only return the book if checked out.
> 
> If we are to check in the book each time to show the messages I think we
> need to add a way to skip the stats for return for each statistical checkout.

This is a good point... I was thinking that the skip_localuse parameter would take care of this, but on testing, a return *and* a localuse are in fact both getting recorded, even if the item was not checked out in the first place.

The problem seems to be coming from these lines in AddReturn:
2168        if ( C4::Context->preference("RecordLocalUseOnReturn") ) {
2169            $localuse_count++;
2170            $item->localuse($localuse_count)->store;
2171            $messages->{'LocalUse'} = 1;
2172            $stat_type = 'localuse';
2173        }

and

2459    C4::Stats::UpdateStats({
...
2469    }) unless ( $skip_localuse && $stat_type eq 'localuse' );


That being said, if RecordLocalUseOnReturn is off, and someone checks in an item that was not checked out, it adds a return stat also. Is that a desired behavior, or can we fix two bugs at once by simply telling AddReturn to never add a 'return' stat if there was no checkout?
Comment 6 Emily Lamancusa (emlam) 2024-09-26 20:36:12 UTC
Oops, I meant to add a bit of actual explanation with the above code snippets, but must have accidentally deleted it.

Basically, $stat_type is 'return' by default, and it will never get changed to 'localuse' if RecordLocalUseOnReturn is off. So if RecordLocalUseOnReturn is off, and an item is checked out to a statistical patron with this patch, then AddReturn will always log a return stat, regardless of whether the item was checked out or not, because $stat_type will never change to 'localuse'.
Comment 7 Lucas Gass (lukeg) 2024-09-27 14:46:02 UTC
> That being said, if RecordLocalUseOnReturn is off, and someone checks in an
> item that was not checked out, it adds a return stat also. Is that a desired
> behavior, or can we fix two bugs at once by simply telling AddReturn to
> never add a 'return' stat if there was no checkout?


I would have to imagine that is not the desired behavior. Should we do like you propose and tell AddReturn to never add a stat if no checkout? And should we do it here or on a separate report?
Comment 8 Emily Lamancusa (emlam) 2024-10-01 20:56:01 UTC
Let's just fix the statistical patron case here. I'll open another bug for the general/checkin case - it seems like a bug to me too, but that's been the behavior for long enough that I don't think we should make the behavior change as a side effect here (especially given that the one opinion expressed so far on https://chat.koha-community.org/koha-community/pl/69snjs3ysjb3mysz7455598xww was against the change).
Comment 9 Emily Lamancusa (emlam) 2024-10-21 14:06:10 UTC
Created attachment 173049 [details] [review]
Bug 37505: Remove onloan check when checking out to stats patron

To test:
1.  Create a Statistical Patron
2.  Set BlockReturnOfLostItems to Block.
3..  Mark an item as lost
4. Check it out to the Statistical Patron. You should see the message "Item was lost, cannot be returned."

5. Set BlockReturnOfWithdrawnItems to Block.
6.  Mark an item as withdrawn
7. Check it out to the Statistical Patron. You should see the message "Item was withdrawn, cannot be returned."
8. Turn off BlockReturnOfWithdrawnItems, check out the same item to the Statistical Patron. You should see a message "Item was withdrawn."

9. Place an item on hold.
10. Check it out to the Statistical Patron
11. See the message "Item on hold, please checkin."

12. Make sure  ClaimReturnedLostValue is set to a lost value
13. Claim a return, making sure it is not still checked out to the patron
14. Checkit it out to the Statistical Patron.
15. See the message "Item claimed returned, please checkin."

16. Have an item on a regular patron account that has been recalled.
17. Checkit it out to the Statistical Patron.
18. See the message "Item can fill a recall, please checkin."

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 10 Emily Lamancusa (emlam) 2024-10-21 14:06:12 UTC
Created attachment 173050 [details] [review]
Bug 37505: (QA follow-up) Don't log return stat if ignore_localuse and no checkout

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 11 Emily Lamancusa (emlam) 2024-10-21 14:06:14 UTC
Created attachment 173051 [details] [review]
Bug 37505: (QA follow-up) Adjust tests

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 12 Katrin Fischer 2024-10-22 10:10:00 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 13 Lucas Gass (lukeg) 2024-11-19 16:06:34 UTC
Backported to 24.05.06x for upcoming 24.05.06
Comment 14 Fridolin Somers 2024-12-13 14:19:25 UTC
Pushed to 23.11.x for 23.11.11