Currently CanBookBeIssued handles the logic of what to do with statistical patron checkouts and a call to AddReturn. ( see Bug 27992 ) Maybe that logic would be better handled elsewhere? But where? Koha::Patron->statistical_checkout? Also see Bug 35840
I think the AddReturn should be moved into circ/circulation.pl after the call to CanBookBeIssued when CanBookBeIssued returns a 'stat' response.. I also think that perhaps the actual recording of the state should be dropped from CanBookBeIssued and only take place within AddReturn.
(In reply to Martin Renvoize from comment #1) > I think the AddReturn should be moved into circ/circulation.pl after the > call to CanBookBeIssued when CanBookBeIssued returns a 'stat' response.. I > also think that perhaps the actual recording of the state should be dropped > from CanBookBeIssued and only take place within AddReturn. Thank you, I agree.
Created attachment 162616 [details] [review] Bug 35950: Move AddReturn call into circulation.pl
Created attachment 162617 [details] [review] Bug 35950: Move AddReturn call into circulation.pl There should be no change in beahavior. Following the test plan from Bug 35840. 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.
Created attachment 162618 [details] [review] Bug 35950: Adjust tests
Created attachment 162686 [details] [review] Bug 35950: Move AddReturn call into circulation.pl There should be no change in beahavior. Following the test plan from Bug 35840. 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>
Created attachment 162687 [details] [review] Bug 35950: Adjust tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Created attachment 162966 [details] [review] Bug 35950: Move AddReturn call into circulation.pl There should be no change in beahavior. Following the test plan from Bug 35840. 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>
Created attachment 162967 [details] [review] Bug 35950: Adjust tests Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks Lucas, all looks good here, Passing QA
Looking at this I was wondering, if moving from .pm to .pl is really the right move here concerning the API. Digging a little deeper, I think we are using AddIssue without CanBookBeIssued in the API, so this would not cause a change. I'll push, but leaving this note here in case I missed something in my line of thinking! :)
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.04
There are a lot of call on CanBookBeIssued() : ILS-DI, SIP, SCO ... Any need to also deal with AddReturn() ? Especially SCO may be used for local circulation no ?