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: | Circulation | Assignee: | 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
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. > 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.
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." 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> (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? 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'.
> 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?
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). 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> 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> Created attachment 173051 [details] [review] Bug 37505: (QA follow-up) Adjust tests Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.06x for upcoming 24.05.06 Pushed to 23.11.x for 23.11.11 |