[Bug 14903] New: Remove C4::Dates from circ/circulation.pl
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Bug ID: 14903 Summary: Remove C4::Dates from circ/circulation.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 Patch to remove deprectated C4::Dates from circ/circulation.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #1 from Marc Véron <veron@veron.ch> --- Created attachment 42895 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42895&action=edit Bug 14903 - Remove C4::Dates from circ/circulation.pl Patch to remove deprectated C4::Dates from circ/circulation.pl To test: - Apply patch - Go to the checkout site (Home > Circulation > Checkouts) - Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture') -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|gmcharlt@gmail.com |veron@veron.ch Patch complexity|--- |Trivial patch Blocks| |14870 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=14903 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion CC| |hector.hecaxmmx@gmail.com --- Comment #2 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Before applying the patch I search and found the followin lines: 169 if ($duedatespec =~ C4::Dates->regexp('syspref')) { 178 our $todaysdate = C4::Dates->new->output('iso'); I do not know if there should be these lines! If the intention in bug 14870 is to delete C4::Dates, this will launch an error or warning when the script will executed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #3 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- (In reply to Héctor Eduardo Castro Avalos from comment #2)
Before applying the patch I search and found the followin lines:
169 if ($duedatespec =~ C4::Dates->regexp('syspref')) { 178 our $todaysdate = C4::Dates->new->output('iso');
I do not know if there should be these lines!
If the intention in bug 14870 is to delete C4::Dates, this will launch an error or warning when the script will executed.
After applying the patch these lines continued there. The patch doesn't touch it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42895|0 |1 is obsolete| | --- Comment #4 from Marc Véron <veron@veron.ch> --- Created attachment 42989 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42989&action=edit Bug 14903 - Remove C4::Dates from circ/circulation.pl Patch to remove deprectated C4::Dates from circ/circulation.pl To test: - Apply patch - Go to the checkout site (Home > Circulation > Checkouts) - Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture') - Verify that due date works as before (with syspref 'SpecifyDueDate' on and off (Amended following comment #3) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42989|0 |1 is obsolete| | --- Comment #5 from Marc Véron <veron@veron.ch> --- Comment on attachment 42989 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42989 Bug 14903 - Remove C4::Dates from circ/circulation.pl New patch will follow with C4/Dates.pm included. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #6 from Marc Véron <veron@veron.ch> --- Created attachment 43303 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43303&action=edit Bug 14903 - Remove C4::Dates from circ/circulation.pl (and more) Patch to remove deprectated C4::Dates from: - circ/circulation.pl - C4/Circulation.pm - koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt To test: - Apply patch - Go to the checkout site (Home > Circulation > Checkouts) - Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture') - Search for regressions - prove -v t/db_dependent/Circulation.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |koha.sekjal@gmail.com --- Comment #7 from Marc Véron <veron@veron.ch> --- *** Bug 7899 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #8 from Marc Véron <veron@veron.ch> --- Still applies / auto-merging -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Mirko Tietgen <mirko@abunchofthings.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mirko@abunchofthings.net Status|Needs Signoff |In Discussion --- Comment #9 from Mirko Tietgen <mirko@abunchofthings.net> --- Hi Marc, it works mostly well for me. Among other things I tried changing the date format in between and using wrong preset dates in the URL like circulation.pl?borrowernumber=51&duedatespec=10%2F30%2F2015+23%3A59 While the old version shows warnings or even breaks ("30" in the "month" position leads to a software error because it is not between 1 and 12), with your patch the wrong format passed by URL is displayed in the calendar, but no warnings or errors appear and the correct date is used when checking out. (In reply to Marc Véron from comment #6)
- Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture')
That breaks it for me:
Template process failed: undef error - KohaDates: filter not found at /home/mirko/koha/C4/Templates.pm line 121.
As you seem to have tested this, is that taken care of by one of the other patches by any chance? I would sign off if it has a "depends on" that works. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #10 from Mirko Tietgen <mirko@abunchofthings.net> --- There is bug 14985 labelled as "part one". Maybe C4/Templates.pm is in a part two not yet on BZ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #11 from Marc Véron <veron@veron.ch> --- Hi Mirko, Thank you for the comments and for testing. Sorry, I have no time at teh moment to fined out what's wrong here; maybe I can come back to that on Sunday evening. I think the circulation files are the most sensible ones,maybe it is a good idea to fix them in one of the very last steps of this loooong journey. That said I really appreciate every sign-off and QA on the other C4::Dates Bugs. BTW: If anybody wants to take one of the C4::Dates bugs to help to get them through, I'll be very grateful. Marc -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #12 from Marc Véron <veron@veron.ch> --- (In reply to Mirko Tietgen from comment #9) (...)
....using wrong preset dates in the URL like circulation.pl?borrowernumber=51&duedatespec=10%2F30%2F2015+23%3A59
While the old version shows warnings or even breaks ("30" in the "month" position leads to a software error because it is not between 1 and 12), with your patch the wrong format passed by URL is displayed in the calendar, but no warnings or errors appear and the correct date is used when checking out. (...)
MV: Found, will be fixed in amended patch: Date will not be displayed if params do not result in a correct date.
(In reply to Marc Véron from comment #6)
- Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture')
That breaks it for me:
Template process failed: undef error - KohaDates: filter not found at /home/mirko/koha/C4/Templates.pm line 121.
(...) MV: Found, was due to a typo, will be fixed in amended patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43303|0 |1 is obsolete| | --- Comment #13 from Marc Véron <veron@veron.ch> --- Created attachment 43958 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43958&action=edit Bug 14903 - Remove C4::Dates from circ/circulation.pl (and more) Patch to remove deprectated C4::Dates from: - circ/circulation.pl - C4/Circulation.pm - koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt To test: - Apply patch - Go to the checkout site (Home > Circulation > Checkouts) - Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture') - Search for regressions - prove -v t/db_dependent/Circulation.t (Amended following comment #9 / #11 25.10.2015 /mv) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Mirko Tietgen <mirko@abunchofthings.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43958|0 |1 is obsolete| | --- Comment #14 from Mirko Tietgen <mirko@abunchofthings.net> --- Created attachment 44072 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44072&action=edit Bug 14903 - Remove C4::Dates from circ/circulation.pl (and more) Patch to remove deprectated C4::Dates from: - circ/circulation.pl - C4/Circulation.pm - koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt To test: - Apply patch - Go to the checkout site (Home > Circulation > Checkouts) - Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture') - Search for regressions - prove -v t/db_dependent/Circulation.t (Amended following comment #9 / #11 25.10.2015 /mv) Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Mirko Tietgen <mirko@abunchofthings.net> 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=14903 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 44072 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44072 Bug 14903 - Remove C4::Dates from circ/circulation.pl (and more) Review of attachment 44072: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14903&attachment=44072) ----------------------------------------------------------------- ::: circ/circulation.pl @@ +174,4 @@
my $stickyduedate = $query->param('stickyduedate') || $session->param('stickyduedate'); my $duedatespec = $query->param('duedatespec') || $session->param('stickyduedate'); +$duedatespec = eval { output_pref( { dt => dt_from_string( $duedatespec ), dateformat => 'iso' }); };
No, only if duedatespec if given. Otherwise "Automatic due date" is always filled with now. @@ +203,4 @@
$datedue = output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' }); $datedue .= ' 23:59:00'; } elsif( $duedatespec_allow ) { + if ($datedue) {
That's not the same code as before ($dateduespec vs $datedue), please double check. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #16 from Marc Véron <veron@veron.ch> --- Created attachment 44096 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44096&action=edit Bug 14903 - (followup)Remove C4::Dates from circ/circulation.pl Follow up to fix QA issues from comment #15 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #17 from Marc Véron <veron@veron.ch> --- Jonathan, is it OK to switch back to Signed-off with the followup? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marc Véron from comment #17)
Jonathan, is it OK to switch back to Signed-off with the followup?
Yes because the following is still wrong: $template->param( IMPOSSIBLE=>1, INVALID_DATE=>$datedue ) Test plan: Set and force an invalid date for the due date and confirm you get a warning with the invalid date displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44096|0 |1 is obsolete| | --- Comment #19 from Marc Véron <veron@veron.ch> --- Created attachment 44131 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44131&action=edit Bug 14903 - (followup)Remove C4::Dates from circ/circulation.pl Follow up to fix QA issues from comment #15 ...and amended for comment #18 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #20 from Marc Véron <veron@veron.ch> --- Hi Jonathan, I amended the patch. However, I can not trigger the message. The datepicker does not allow to enter wrong dates (field is readonly). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=14903 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44072|0 |1 is obsolete| | Attachment #44131|0 |1 is obsolete| | --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 44137 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44137&action=edit Bug 14903 - Remove C4::Dates from circ/circulation.pl (and more) Patch to remove deprectated C4::Dates from: - circ/circulation.pl - C4/Circulation.pm - koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt To test: - Apply patch - Go to the checkout site (Home > Circulation > Checkouts) - Verify that data displays properly (including for users with a card that expires in the near future, see syspref 'NotifyBorrowerDeparture') - Search for regressions - prove -v t/db_dependent/Circulation.t (Amended following comment #9 / #11 25.10.2015 /mv) Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 44138 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44138&action=edit Bug 14903 - (followup)Remove C4::Dates from circ/circulation.pl Follow up to fix QA issues from comment #15 ...and amended for comment #18 Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14903 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Marc! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org