All dates can now be displayed through a Template::Toolkit plugin. All existing code must be updated. Dates can/should/must now be displayed by: * adding [% USE KohaDates %] at the beginning of the template * use [% mydate | KohaDates %] for mydate date variable * remove .pl code that update date format before sending the string to the template. This entry is an omnibus, as many patches will be necessary. Note: the plugin has been introduced in Bug 929
Just a little change it needs to be [% mydate | $KohaDates %]
Created attachment 8130 [details] [review] Bug 7444: Update circ/circulation.pl to use KohaDates TT Plugin Removes all date formatting done in circ/circulation, and sets any/all date outputs to iso format. The formatting of dates is now handled in the template using the KohaDates plugin. This allows use to remove a dependency on C4/Dates.pm from the script. This patch also removes the usage of C4/Overdues/CheckBorrowerDebarred. Since borrowers.debarred is returned by GetMemberDetails, it doesn't make sense to include another module to run another MySQL query just to fetch the date again. To Test: 1. View the checkout page for a patron with materials checked out today and in days past, as well as items on reserve. 2. View a patron who is nearing their expiry date 3. Renew a patron 4. Renew a material from the checkout page using the renewal date selector 5. Attempt to checkout a book to a patron that is on hold for someone else 6. Attempt to checkout a book that is currently checked out 7. View a debarred patron In all cases, any dates displayed should be in syspref format
Created attachment 8153 [details] [review] Bug 7444: Update circ/circulation.pl to use KohaDates TT Plugin Removes all date formatting done in circ/circulation, and sets any/all date outputs to iso format. The formatting of dates is now handled in the template using the KohaDates plugin. This allows use to remove a dependency on C4/Dates.pm from the script. This patch also removes the usage of C4/Overdues/CheckBorrowerDebarred. Since borrowers.debarred is returned by GetMemberDetails, it doesn't make sense to include another module to run another MySQL query just to fetch the date again. To Test: 1. View the checkout page for a patron with materials checked out today and in days past, as well as items on reserve. 2. View a patron who is nearing their expiry date 3. Renew a patron 4. Renew a material from the checkout page using the renewal date selector 5. Attempt to checkout a book to a patron that is on hold for someone else 6. Attempt to checkout a book that is currently checked out 7. View a debarred patron In all cases, any dates displayed should be in syspref format Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Corrected issue display of dates in warnings.
QA comment: The patch says: @@ -909,6 +910,7 @@ sub CanBookBeIssued { $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'}; $needsconfirmation{'resbranchname'} = $branchname; $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'}); +$needsconfirmation{'resreservedateiso'} = $res->{'reservedate'}; Why don't you remove the resreservedate and his "format_date" sub ? shouldn't it be -$needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'}); +$needsconfirmation{'resreservedateiso'} = $res->{'reservedate'};
Until KohaDates has replaced pre-formatted dates everywhere in Koha, we need to pass the unformatted date for use with the filter, and the formatted date, for use where it hasn't changed yet.
QA comment: same comment as Paul, for me, if it's not currently used, it's useless to keep this line. Passed QA
After investigating & thinking of it again, I reject this patch. Here is my explanation: The patch add a reservedateiso hash entry, and keep the reservedate. It means we add some code that may be useless, or will become. Instead of improving code quality, it lower it. So, I request - $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'}); + $needsconfirmation{'resreservedate'} = $res->{'reservedate'}; (ie = don't introduce a new variable reservedateiso, and a checking that there is no side effect) Note that I think there should not be a lot of side effect, as this sub is used in circulation only, so the updated patch would probably be safe. (But it has to be checked, it's just my feeling)
Switching bug back to original, un-signed off patch that does what Paul requests, and returning to "Needs signoff" status. I have expressed my concern about the broken display caused by redefining $needsconfirmation{'resreservedate'}, but as the consensus is that the variable should be redefined anyway, a new signoff is required.
Needs re-basing against master... taking a look now.
Created attachment 10108 [details] [review] Bug 7444: [REBASED] Update circ/circulation.pl to use KohaDates TT Plugin
Created attachment 10114 [details] [review] 10108: Bug 7444: [SIGNED] Update circ/circulation.pl to use KohaDates TT Plugin Tested as outline in Ian's comment. All working as expected, taking note of the syspref.
Related Note: Under "Details" tab for patron record, the checkouts summary at the bottom of the page displays as a Date Time (i.e is followed by 00:00) This is not consistent with how the table is displayed elsewhere..
QA comment: cleaning code patch only, no more reason to object !
This patch has stripped the hour/minute portion of the date due out on circulation.pl
This patch appears to have removed the time from the list of checkouts - the time is necessary for hourly loans ....
Created attachment 10279 [details] timestamp This is the timestamp that was there in 3.8.1 and needs to be put back ...
Reverted from 3.8.x, so needs to be fixed before the 3.10.0 release
(In reply to comment #17) > Reverted from 3.8.x, so needs to be fixed before the 3.10.0 release Patch also reverted from master. Switching status to failed QA.