| Summary: | Simplify date handling in CanbookBeIssued | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Joonas Kylmälä <joonas.kylmala> |
| Component: | Circulation | Assignee: | Joonas Kylmälä <joonas.kylmala> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | gmcharlt, kyle.m.hall, lucas, martin.renvoize, nick |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
22.11.00
|
|
| Circulation function: | |||
| Attachments: |
Bug 30947: Add test for Illrequest checkout with custom due date
Bug 30947: Simplify CanBookBeIssued date handling Bug 30947: Add test for Illrequest checkout with custom due date Bug 30947: Simplify CanBookBeIssued date handling Bug 30947: Add test for Illrequest checkout with custom due date Bug 30947: Simplify CanBookBeIssued date handling Bug 30947: (follow-up) Remove unnecessary object cloning |
||
|
Description
Joonas Kylmälä
2022-06-11 10:53:06 UTC
Created attachment 135959 [details] [review] Bug 30947: Add test for Illrequest checkout with custom due date To test: 1) prove t/db_dependent/Illrequests.t Created attachment 135960 [details] [review] Bug 30947: Simplify CanBookBeIssued date handling 1) This removes support for passing string dates to CanBookBeIssued. The function didn't publicly even document support for string dates, only DateTime objects. 2) We get a $duedate always at least from CalcDateDue so having $issuingimpossible{INVALID_DATE} = output_pref($duedate); was unneccesary and thus removed. 3) The check "duedate cannot be before now" was needlessly complex: if the due date really cannot be before now we should check seconds too and warn the librarian! Thus the truncation to minutes can be dropped safely. To test: 1) prove t/db_dependent/Circulation.t 2) prove t/db_dependent/Illrequests.t 3) Enable OnSiteCheckouts and disable SpecifyDueDate syspref. Create on-site checkout for any patron and verify the due date is your current date at 23:59, you can check the exact minute with sql: > select * from issues Created attachment 136092 [details] [review] Bug 30947: Add test for Illrequest checkout with custom due date To test: 1) prove t/db_dependent/Illrequests.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 136093 [details] [review] Bug 30947: Simplify CanBookBeIssued date handling 1) This removes support for passing string dates to CanBookBeIssued. The function didn't publicly even document support for string dates, only DateTime objects. 2) We get a $duedate always at least from CalcDateDue so having $issuingimpossible{INVALID_DATE} = output_pref($duedate); was unneccesary and thus removed. 3) The check "duedate cannot be before now" was needlessly complex: if the due date really cannot be before now we should check seconds too and warn the librarian! Thus the truncation to minutes can be dropped safely. To test: 1) prove t/db_dependent/Circulation.t 2) prove t/db_dependent/Illrequests.t 3) Enable OnSiteCheckouts and disable SpecifyDueDate syspref. Create on-site checkout for any patron and verify the due date is your current date at 23:59, you can check the exact minute with sql: > select * from issues Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 136158 [details] [review] Bug 30947: Add test for Illrequest checkout with custom due date To test: 1) prove t/db_dependent/Illrequests.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 136159 [details] [review] Bug 30947: Simplify CanBookBeIssued date handling 1) This removes support for passing string dates to CanBookBeIssued. The function didn't publicly even document support for string dates, only DateTime objects. 2) We get a $duedate always at least from CalcDateDue so having $issuingimpossible{INVALID_DATE} = output_pref($duedate); was unneccesary and thus removed. 3) The check "duedate cannot be before now" was needlessly complex: if the due date really cannot be before now we should check seconds too and warn the librarian! Thus the truncation to minutes can be dropped safely. To test: 1) prove t/db_dependent/Circulation.t 2) prove t/db_dependent/Illrequests.t 3) Enable OnSiteCheckouts and disable SpecifyDueDate syspref. Create on-site checkout for any patron and verify the due date is your current date at 23:59, you can check the exact minute with sql: > select * from issues Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> QA scripts are happy and tests all pass. This is a nice simplification and improvement.. great work. Passing QA Created attachment 136329 [details] [review] Bug 30947: (follow-up) Remove unnecessary object cloning CalcDateDue() works on its own copy of the $startdate parameter so the cloning in the calling end is not necessary. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> We were doing a copy of the $now DateTime object unnecessarily so added a trivial follow-up for that. Pushed to master for 22.11. Nice work everyone, thanks! Not going to backport this enhancement to 22.05.x unless requested |