C4::Circulation::CanbookBeIssued does some unnecessary stuff with dates, opening bug to get rid of that extra code and make it simpler and more efficient :)
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