[Bug 35840] New: Local use is double-counted when using both RecordLocalUseOnCheckin and statistical patrons
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Bug ID: 35840 Summary: Local use is double-counted when using both RecordLocalUseOnCheckin and statistical patrons Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: andrewfh@dubcolib.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com To recreate: - enable RecordLocalUseOnReturn - create a statistical patron category and a patron in it - check an item out to your statistical patron - count local use entries with a report: SELECT * FROM statistics WHERE type='localuse' AND itemnumber= (your item) - see that your one checkout has created 2 entries in statistics It seems plausible that a single library might want to use these two features concurrently to count different things, but doing so creates an inflated count. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #1 from Magnus Enger <magnus@libriotech.no> --- *** Bug 35839 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Summary|Local use is double-counted |Local use is double-counted |when using both |when using both |RecordLocalUseOnCheckin and |RecordLocalUseOnReturn and |statistical patrons |statistical patrons -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27992 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27992 [Bug 27992] When recording local use with statistical patron items are not checked in -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #2 from Lucas Gass <lucas@bywatersolutions.com> --- This is due to Bug 27992, which adds a call to AddReturn -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #3 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 161212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161212&action=edit Bug 35840: Only AddReturn if item is onloan To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Assignee|koha-bugs@lists.koha-commun |lucas@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Biblibre Sandboxes <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161212|0 |1 is obsolete| | --- Comment #4 from Biblibre Sandboxes <sandboxes@biblibre.com> --- Created attachment 161217 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161217&action=edit Bug 35840: Only AddReturn if item is onloan To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov Status|Signed Off |Failed QA --- Comment #5 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Hmmm...looking at the patch from 27992, all of the checking for holds, recalls, return claims, and lost or withdrawn statuses is based on the return values from AddReturn. So avoiding the call when the item isn't checked out would undo a lot of the functionality for 27992. (I did a quick test on master and confirmed this.) Wouldn't it be better to skip logging the local use in CanBookBeIssued if RecordLocalUseOnReturn is enabled (probably with a comment in the code that it is being skipped because AddReturn will log the local use in that case)? Either way, this is also going to need a unit test. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161217|0 |1 is obsolete| | --- Comment #6 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 161223 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161223&action=edit Bug 35840: Fix Local use being double-counted in CanBookBeIssued To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #7 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Emily Lamancusa from comment #5)
Hmmm...looking at the patch from 27992, all of the checking for holds, recalls, return claims, and lost or withdrawn statuses is based on the return values from AddReturn. So avoiding the call when the item isn't checked out would undo a lot of the functionality for 27992. (I did a quick test on master and confirmed this.) Wouldn't it be better to skip logging the local use in CanBookBeIssued if RecordLocalUseOnReturn is enabled (probably with a comment in the code that it is being skipped because AddReturn will log the local use in that case)?
Either way, this is also going to need a unit test. :)
Maybe this? If you like this better I'll write a unit test. Let me know. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #8 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Works for me! Just one tweak to line 2476: - }) unless $localuse; + }) unless ( $localuse && $stat_type eq 'localuse' ); Otherwise, when there is an actual return, it skips logging that, too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #9 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Emily Lamancusa from comment #8)
Works for me! Just one tweak to line 2476:
- }) unless $localuse; + }) unless ( $localuse && $stat_type eq 'localuse' );
Otherwise, when there is an actual return, it skips logging that, too.
Are you saying it should add 2 entries to the stats table if a checked out item is checked out to the stats patron? 1 for localuse and 1 for a return? Seems like it should be one or the other, not both. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #10 from Lucas Gass <lucas@bywatersolutions.com> ---
Are you saying it should add 2 entries to the stats table if a checked out item is checked out to the stats patron? 1 for localuse and 1 for a return? Seems like it should be one or the other, not both.
Im 2nd guessing now. I'm thinking that if it actually does a return it should be logged as such but NOT logged as a localuse. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #11 from Lucas Gass <lucas@bywatersolutions.com> --- This one needs to go 'In discussion' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |16122 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16122 [Bug 16122] Item record needs to keep local use count -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #12 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Lucas Gass from comment #10)
Are you saying it should add 2 entries to the stats table if a checked out item is checked out to the stats patron? 1 for localuse and 1 for a return? Seems like it should be one or the other, not both.
Im 2nd guessing now. I'm thinking that if it actually does a return it should be logged as such but NOT logged as a localuse.
In this scenario a book had been checked out to a patron but for whatever reason is left behind at the library. Staff finds it and checks it in. That should count as a return and not local use. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #13 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- (In reply to Lucas Gass from comment #9)
Are you saying it should add 2 entries to the stats table if a checked out item is checked out to the stats patron? 1 for localuse and 1 for a return? Seems like it should be one or the other, not both.
That's a good question... At the very least, we shouldn't change the existing behavior that a statistical patron records a localuse no matter what, imo, because that's the primary purpose of a statistical patron. For MCPL, I think we would want both - we use both pieces of data to track library usage in different ways, and in this scenario, we would generally assume that the item really was returned by one patron and later used locally by another. (It's pretty common for items to get accidentally skipped when a staff member is scanning a pile of returns, and get put back on the shelf without actually getting checked in.) But I'm sure there are other libraries that use those stats in very different ways, and may have different opinions - I agree we need to get more input on this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #14 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- I tend to agree with Emily's assumption: an item found to be already checked out when scanned for a local use via statistical patron was likely returned by one patron, not properly checked in, and then later used by another patron. In that case, it would be correct to record both a return *and* a local use. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #15 from Lucas Gass <lucas@bywatersolutions.com> --- So maybe, in an effort to move this forward and stop the double counting of localuse, we keep current existing behavior and stop the double increment of localuse? That seems like the kind of thing that could be a problem and inflate stats. We could move the discussion about localuse + return / just return to another bug? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #16 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- (In reply to Lucas Gass from comment #15)
So maybe, in an effort to move this forward and stop the double counting of localuse, we keep current existing behavior and stop the double increment of localuse? That seems like the kind of thing that could be a problem and inflate stats.
We could move the discussion about localuse + return / just return to another bug?
This sounds like a good path to me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35860 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35860 [Bug 35860] Checking out an item that to stats patron that is already checked out results in 2 entries in the stats table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #17 from Lucas Gass <lucas@bywatersolutions.com> --- Filed Bug 35860 to continue the discussion. Setting this one back to FQA and will have a patch soon! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #18 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 161374 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161374&action=edit Bug 35840: (QA follow-up) Make sure return and localuse is recorded and unit tests To test: prove t/db_dependent/Circulation.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161223|0 |1 is obsolete| | --- Comment #19 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 161442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161442&action=edit Bug 35840: Fix Local use being double-counted in CanBookBeIssued To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161374|0 |1 is obsolete| | --- Comment #20 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 161443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161443&action=edit Bug 35840: (QA follow-up) Make sure return and localuse is recorded and unit tests To test: prove t/db_dependent/Circulation.t Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org --- Comment #21 from Brendan Lawlor <blawlor@clamsnet.org> --- Tested, works as described and it makes sense too. Now I want to learn how these tests work... prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. ok All tests successful. Files=1, Tests=68, 27 wallclock secs ( 0.08 usr 0.00 sys + 20.93 cusr 2.38 csys = 23.39 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Confusing material. Why did we add AddReturn to CanBookBeIssued in the first place? It feels not good. Consider this out of scope :) Would be helpful to have the module changes in one patch and the test in another. Now they are spread. Could be squashed? Please add POD changes for the new parameter of AddReturn. The name $localuse is not clear enough imo. You want to skip localuse, right? The unit test is not completely clear too. The results of various searches on item3 could overlap or so? Could you make more explicit what really happens for both patrons? Needs a bit of attention still. Thanks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #23 from Lucas Gass <lucas@bywatersolutions.com> ---
Why did we add AddReturn to CanBookBeIssued in the first place? It feels not good.
I added it here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27992 Can you propose a better way to do this? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161442|0 |1 is obsolete| | --- Comment #24 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 161612 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161612&action=edit Bug 35840: Add option to AddReturn to skip recording of localuse To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161612|0 |1 is obsolete| | --- Comment #25 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 161613 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161613&action=edit Bug 35840: Add option to AddReturn to skip recording of localuse To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161443|0 |1 is obsolete| | --- Comment #26 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 161614 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161614&action=edit Bug 35840: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Lucas Gass from comment #23)
Why did we add AddReturn to CanBookBeIssued in the first place? It feels not good.
I added it here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27992
Can you propose a better way to do this?
That is no simple question. Depends on the whole context of why (and when) we are doing it, and the use cases for it. But it feels that it should not be there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #28 from Lucas Gass <lucas@bywatersolutions.com> ---
That is no simple question. Depends on the whole context of why (and when) we are doing it, and the use cases for it. But it feels that it should not be there.
In CanBookBeIssued there is: if ( $patron->category->category_type eq 'X' ) where we do some things in the borrower is a statistical patron. This is mostly for updating the stats table 'C4::Stats::UpdateStats'. As of Bug 27992, this also includes a call to AddReturn to make sure anything checked out to a stats patron is returned. Maybe we should open a new bug report to discuss the overall architecture? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #29 from Lucas Gass <lucas@bywatersolutions.com> ---
Maybe we should open a new bug report to discuss the overall architecture?
I have opened Bug 35950. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161613|0 |1 is obsolete| | --- Comment #30 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- Created attachment 161732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161732&action=edit Bug 35840: Add option to AddReturn to skip recording of localuse To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161614|0 |1 is obsolete| | --- Comment #31 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- Created attachment 161733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161733&action=edit Bug 35840: Add unit tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35950 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35950 [Bug 35950] Move the handling of statistics patron logic out of CanBookBeIssued -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Whilst I agree that having AddReturn embedding inside CanBookBeIssued is wrong, it is at least contained to 'statistical patrons'. I think we should allow this through QA, but I really want to see the logic moved out of CanBookBeIssued and into the controllers calling it really.. are you intending to work on bug 25950 at all Lucas? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|35950 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35950 [Bug 35950] Move the handling of statistics patron logic out of CanBookBeIssued -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35950 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35950 [Bug 35950] Move the handling of statistics patron logic out of CanBookBeIssued -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #33 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #32)
Whilst I agree that having AddReturn embedding inside CanBookBeIssued is wrong, it is at least contained to 'statistical patrons'.
I think we should allow this through QA, but I really want to see the logic moved out of CanBookBeIssued and into the controllers calling it really.. are you intending to work on bug 25950 at all Lucas?
Martin, assuming you mean Bug 35950 I am happy to take that up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|35860 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35860 [Bug 35860] Checking out an item that to stats patron that is already checked out results in 2 entries in the stats table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|m.de.rooy@rijksmuseum.nl |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161732|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162963&action=edit Bug 35840: Add option to AddReturn to skip recording of localuse To test: 1. APPLY PATCH, restart_all 2. Turn on RecordLocalUseOnReturn 3. Create a Statistical patron. 4. Check an item out to a regular patron. 5. Check the item out to a Statistical patron. 6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return. 7. Try checking out an item to the Stats patron that is NOT checked out. 8. You should only see 1 entry, localuse, in the statistics table. Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161733|0 |1 is obsolete| | --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162964 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162964&action=edit Bug 35840: Add unit tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All working well and tested. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #37 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162965 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162965&action=edit Bug 35840: (QA follow-up) Tidy and critic fix -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- At some point, we should make this nicer and easier to understand what's going on: &AddReturn( $barcode, $branch [,$exemptfine] [,$returndate] [,$skip_localuse ] ); AddReturn( $item_object->barcode, C4::Context->userenv->{'branch'}, undef, undef, 1 ) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #40 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Katrin Fischer from comment #38)
At some point, we should make this nicer and easier to understand what's going on:
&AddReturn( $barcode, $branch [,$exemptfine] [,$returndate] [,$skip_localuse ] );
AddReturn( $item_object->barcode, C4::Context->userenv->{'branch'}, undef, undef, 1 )
+1 We should use a %params hash -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.05.00 |24.05.00,23.11.04 released in| | Status|Pushed to master |Pushed to stable --- Comment #41 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 --- Comment #42 from Lucas Gass <lucas@bywatersolutions.com> --- Failing unit tests on 23.05.x: not ok 29 - CanBookBeIssued + Statistic patrons "X" # Failed test 'CanBookBeIssued + Statistic patrons "X"' # at /kohadevbox/koha/t/db_dependent/Circulation.t line 2358. DBIx::Class::ResultSource::_minimal_valueset_satisfying_constraint(): Unable to satisfy requested constraint 'primary', missing values for column(s): 'borrowernumber' at /kohadevbox/koha/Koha/Objects.pm line 96 # Looks like your test exited with 11 just after 29. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 40/68 subtests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |phil@chetcolibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35840 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org