[Bug 15087] New: make dt_from_string return undef for '0000-00-00'
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Bug ID: 15087 Summary: make dt_from_string return undef for '0000-00-00' Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: bgkriegel@gmail.com QA Contact: testopia@bugs.koha-community.org Current master, define circulation rule All, All any number of days, but left 'Hard due date' EMPTY. Try to checkout something to a patron, you get Can't locate object method "truncate" via package "-1" (perhaps you forgot to load "-1"?) at .... Problem comes from GetHardDueDate, it returns return (dt_from_string($rule->{hardduedate}, 'iso'),$rule->{hardduedatecompare}); But dt_from_string returns without value in case of '0000-00-00' date, return if $date_string and $date_string =~ m|^0000-0| which evaluates to an empty list in this case, making the final return value for GetHardDueDate ( -1 ) instead of ( undef, -1 ) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 --- Comment #1 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 44181 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44181&action=edit Bug 15087: make dt_from_string return undef for '0000-00-00' To test: 1) Reproduce problem in description 2) Apply the patch 3) Try again, no more problems I don't know if this could have side effects -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |veron@veron.ch Patch complexity|--- |Trivial patch Assignee|gmcharlt@gmail.com |bgkriegel@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hector.hecaxmmx@gmail.com, | |jonathan.druart@bugs.koha-c | |ommunity.org Version|unspecified |master --- Comment #2 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Hi Bernardo I'll add here Marc and Jonathan, since they have more experience with Koha::DateUtils. I feel scared when you mention that can have side effects. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14870 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14870 --- Comment #3 from Marc Véron <veron@veron.ch> --- At other places we did return ( eval { dt_from_string($rule->{hardduedate}, 'iso'),$rule->{hardduedatecompare}); }; ...to avoid such errors. I propose to do it here as well and postpone the change to dt_from_string until the C4::Dates removal is finished. It is in C4/Circulation.pm Line 1498 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=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Bernardo Gonzalez Kriegel from comment #0)
Current master, define circulation rule All, All any number of days, but left 'Hard due date' EMPTY.
Try to checkout something to a patron, you get
Can't locate object method "truncate" via package "-1" (perhaps you forgot to load "-1"?) at ....
Outch!
Problem comes from GetHardDueDate, it returns
return (dt_from_string($rule->{hardduedate}, 'iso'),$rule->{hardduedatecompare});
Yep.
But dt_from_string returns without value in case of '0000-00-00' date,
It usually return undef when not called in a list context. (In reply to Bernardo Gonzalez Kriegel from comment #1)
Created attachment 44181 [details] [review] Bug 15087: make dt_from_string return undef for '0000-00-00'
No! :) It is usually a bad idea to explicitly return undef, and you get a perlcritic warning.
I don't know if this could have side effects
That's why it's preferable not to change the return of the subroutine but the caller(s?). (In reply to Marc Véron from comment #3)
At other places we did
return ( eval { dt_from_string($rule->{hardduedate}, 'iso'),$rule->{hardduedatecompare}); };
It's a weird hack :) dt_from_string does not return an error.
I propose to do it here as well and postpone the change to dt_from_string until the C4::Dates removal is finished.
We cannot wait for a fix here as it makes the application explodes. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Bernardo, I don't reproduce the problem you described. My issuingrules table is empty. Please detail. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44181|0 |1 is obsolete| | --- Comment #6 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 44194 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44194&action=edit Bug 15087: Fix GetHardDueDate return value This patch fixes GetHardDueDate return value in case of empty ('0000-00-00') hard due date. To test: 1) Reproduce problem in description 2) Apply the patch 3) Try again, no more problems -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff Summary|make dt_from_string return |Fix GetHardDueDate return |undef for '0000-00-00' |value --- Comment #7 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to Jonathan Druart from comment #5)
Bernardo, I don't reproduce the problem you described. My issuingrules table is empty. Please detail.
Problem arises when you create a rule mysql kohadev -e "select categorycode,itemtype,hardduedate from issuingrules" +--------------+----------+-------------+ | categorycode | itemtype | hardduedate | +--------------+----------+-------------+ | * | * | 0000-00-00 | +--------------+----------+-------------+ Changed patch and bug description -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 --- Comment #8 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to Jonathan Druart from comment #4)
(In reply to Bernardo Gonzalez Kriegel from comment #0)
Current master, define circulation rule All, All any number of days, but left 'Hard due date' EMPTY.
Try to checkout something to a patron, you get
Can't locate object method "truncate" via package "-1" (perhaps you forgot to load "-1"?) at ....
Outch!
The problem was discovered by Hector. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|bgkriegel@gmail.com |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Bernardo, ok thanks, I am on it! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44194|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 44197 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44197&action=edit Bug 15087: hardduedate should not be inserted as 0000-00-00 but NULL This has been introduced by bug 14949, recently pushed. So I don't think we need to update the DB, only devs will face this problem. The hardduedate was inserted as 0000-00-00 because was en empty string. To be inserted as NULL, the value should be undef. Test plan: 0/ Don't apply this patch 1/ Edit a circ rules and don't fill the hard due date 2/ Look at the DB, the value is 0000-00-00 3/ Apply this patch and confirm that the value is now NULL -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|14870 |14949 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- To restore the behavior that existed before bug 14949, I think this is the way to go. Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14870 [Bug 14870] Delete C4/Dates.pm from System http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14949 [Bug 14949] Remove C4::Dates from admin/smart-rules.pl and admin/categorie.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- And this is critical -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> 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=15087 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44197|0 |1 is obsolete| | --- Comment #13 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 44198 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44198&action=edit [SIGNED-OFF] Bug 15087: hardduedate should not be inserted as 0000-00-00 but NULL This has been introduced by bug 14949, recently pushed. So I don't think we need to update the DB, only devs will face this problem. The hardduedate was inserted as 0000-00-00 because was en empty string. To be inserted as NULL, the value should be undef. Test plan: 0/ Don't apply this patch 1/ Edit a circ rules and don't fill the hard due date 2/ Look at the DB, the value is 0000-00-00 3/ Apply this patch and confirm that the value is now NULL Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Confirmed +--------------+----------+-------------+ | categorycode | itemtype | hardduedate | +--------------+----------+-------------+ | * | * | NULL | +--------------+----------+-------------+ No koha-qa errors -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Fix GetHardDueDate return |hardduedate should not be |value |inserted as 0000-00-00 but | |NULL --- Comment #14 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Signed and changed description... again :) (In reply to Jonathan Druart from comment #11)
To restore the behavior that existed before bug 14949, I think this is the way to go.
Agreed Signed and changed summary... again :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Kyle M Hall <kyle@bywatersolutions.com> 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=15087 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44198|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 44244 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44244&action=edit [PASSED QA] Bug 15087: hardduedate should not be inserted as 0000-00-00 but NULL This has been introduced by bug 14949, recently pushed. So I don't think we need to update the DB, only devs will face this problem. The hardduedate was inserted as 0000-00-00 because was en empty string. To be inserted as NULL, the value should be undef. Test plan: 0/ Don't apply this patch 1/ Edit a circ rules and don't fill the hard due date 2/ Look at the DB, the value is 0000-00-00 3/ Apply this patch and confirm that the value is now NULL Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Confirmed +--------------+----------+-------------+ | categorycode | itemtype | hardduedate | +--------------+----------+-------------+ | * | * | NULL | +--------------+----------+-------------+ No koha-qa errors Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org