This patch removes C4::Dates from circ/pendingreserves.pl
Created attachment 42946 [details] [review] Remove C4::Dates from circ/pendingreserves.pl This patch removes C4::Dates from circ/pendingreserves.pl To test: Apply patch Go to Home > Circulation > Holds to pull Verify that the the filter and the list behave as before.
Created attachment 42959 [details] [review] [SIGNED-OFF]Remove C4::Dates from circ/pendingreserves.pl This patch removes C4::Dates from circ/pendingreserves.pl To test: Apply patch Go to Home > Circulation > Holds to pull Verify that the the filter and the list behave as before. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised. C4::Dates is deprecated according to Coding Guidelines Wiki.
Created attachment 42992 [details] [review] Bug 14918: Remove C4::Dates from circ/pendingreserves.pl This patch uses Koha::DateUtils instead of C4::Dates and Date::Calc. It also sent DateTime objects to the template, which use the TT plugin to display them. If an invalid dates is entered, the default date is picked. Test plan: 1/ Go to Home > Circulation > Holds to pull 2/ Verify that the the filter and the list behave as before 3/ Enter invalid dates and confirm that you do not get a software error
I suggest this counter-patch, the badly formatted dates are caught and don't make Koha explodes with a software error.
Created attachment 43003 [details] [review] [Signed-off] Bug 14918: Remove C4::Dates from circ/pendingreserves.pl This patch uses Koha::DateUtils instead of C4::Dates and Date::Calc. It also sent DateTime objects to the template, which use the TT plugin to display them. If an invalid dates is entered, the default date is picked. Test plan: 1/ Go to Home > Circulation > Holds to pull 2/ Verify that the the filter and the list behave as before 3/ Enter invalid dates and confirm that you do not get a software error Counter patch works as expected and is a much cleaner solution. Signed-off-by: Marc Véron <veron@veron.ch>
- my $pastdate= sprintf("%-04.4d-%-02.2d-%02.2d", Add_Delta_YMD($year, $month, $day, 0, 0, -C4::Context->preference('HoldsToPullStartDate')||TWO_DAYS_AGO )); - $startdate = format_date($pastdate); + $startdate = $today - DateTime::Duration->new( days => C4::Context->preference('HoldsToPullStartDate') || 2 ); Replacing a constant by a hardcoded value is not an improvement.
(In reply to Marcel de Rooy from comment #6) > - my $pastdate= sprintf("%-04.4d-%-02.2d-%02.2d", Add_Delta_YMD($year, > $month, $day, 0, 0, > -C4::Context->preference('HoldsToPullStartDate')||TWO_DAYS_AGO )); > - $startdate = format_date($pastdate); > + $startdate = $today - DateTime::Duration->new( days => > C4::Context->preference('HoldsToPullStartDate') || 2 ); > > > Replacing a constant by a hardcoded value is not an improvement. A constant named TWO_DAYS_AGO does not make really sense... And is only used once. Feel free to use TWO_DAYS instead, if you prefer.
I hope that is no blocker :-)
(In reply to Marc Véron from comment #8) > I hope that is no blocker :-) No. I will fix it next week.
Created attachment 43796 [details] [review] Bug 14918: Remove C4::Dates from circ/pendingreserves.pl This patch uses Koha::DateUtils instead of C4::Dates and Date::Calc. It also sent DateTime objects to the template, which use the TT plugin to display them. If an invalid dates is entered, the default date is picked. Test plan: 1/ Go to Home > Circulation > Holds to pull 2/ Verify that the the filter and the list behave as before 3/ Enter invalid dates and confirm that you do not get a software error Counter patch works as expected and is a much cleaner solution. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 43797 [details] [review] Bug 14918: [QA Follow-up] Correct enddate and use constant Corrected enddate (subtraction in the revised code must be an addition). Replaced hardcoded 2 days by a constant. Renamed the constant TWO_DAYS to PULL_INTERVAL. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested all three date formats.
Created attachment 43798 [details] [review] Bug 14918: Remove C4::Dates from circ/pendingreserves.pl This patch uses Koha::DateUtils instead of C4::Dates and Date::Calc. It also sent DateTime objects to the template, which use the TT plugin to display them. If an invalid dates is entered, the default date is picked. Test plan: 1/ Go to Home > Circulation > Holds to pull 2/ Verify that the the filter and the list behave as before 3/ Enter invalid dates and confirm that you do not get a software error Counter patch works as expected and is a much cleaner solution. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 43799 [details] [review] Bug 14918: [QA Follow-up] Correct enddate and use constant Corrected enddate (subtraction in the revised code must be an addition). Replaced hardcoded 2 days by a constant. Renamed the constant TWO_DAYS to PULL_INTERVAL. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested all three date formats.
Patches pushed to master. Thanks Jonathan and Marcel!