[Bug 14918] New: Remove C4::Dates from circ/pendingreserves.pl
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Bug ID: 14918 Summary: Remove C4::Dates from circ/pendingreserves.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: veron@veron.ch QA Contact: testopia@bugs.koha-community.org This patch removes C4::Dates from circ/pendingreserves.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 --- Comment #1 from Marc Véron <veron@veron.ch> --- Created attachment 42946 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42946&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |veron@veron.ch Blocks| |14870 Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14870 [Bug 14870] Delete C4/Dates.pm from System -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14920 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14920 [Bug 14920] Remove C4::Dates from circ/reserveratios.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|14920 | Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14920 [Bug 14920] Remove C4::Dates from circ/reserveratios.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42946|0 |1 is obsolete| | --- Comment #2 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 42959 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42959&action=edit [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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |hector.hecaxmmx@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42959|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 42992 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42992&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I suggest this counter-patch, the badly formatted dates are caught and don't make Koha explodes with a software error. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 --- Comment #5 from Marc Véron <veron@veron.ch> --- Created attachment 43003 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43003&action=edit [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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42992|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|veron@veron.ch |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- - 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 --- Comment #8 from Marc Véron <veron@veron.ch> --- I hope that is no blocker :-) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marc Véron from comment #8)
I hope that is no blocker :-) No. I will fix it next week.
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43003|0 |1 is obsolete| | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 43796 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43796&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 43797 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43797&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43796|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 43798 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43798&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43797|0 |1 is obsolete| | --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 43799 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43799&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14918 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Jonathan and Marcel! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org