Summary: | Move the handling of statistics patron logic out of CanBookBeIssued | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Architecture, internals, and plumbing | Assignee: | Lucas Gass (lukeg) <lucas> |
Status: | RESOLVED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | andrew, fridolin.somers, m.de.rooy, phil |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
24.05.00,23.11.04
|
|
Circulation function: | |||
Bug Depends on: | 35840 | ||
Bug Blocks: | 16122, 37505 | ||
Attachments: |
Bug 35950: Move AddReturn call into circulation.pl
Bug 35950: Move AddReturn call into circulation.pl Bug 35950: Adjust tests Bug 35950: Move AddReturn call into circulation.pl Bug 35950: Adjust tests Bug 35950: Move AddReturn call into circulation.pl Bug 35950: Adjust tests |
Description
Lucas Gass (lukeg)
2024-01-30 15:40:47 UTC
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 ? |