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 )
Created attachment 44181 [details] [review] 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
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.
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
(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] [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.
Bernardo, I don't reproduce the problem you described. My issuingrules table is empty. Please detail.
Created attachment 44194 [details] [review] 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
(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
(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.
Bernardo, ok thanks, I am on it!
Created attachment 44197 [details] [review] 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
To restore the behavior that existed before bug 14949, I think this is the way to go.
And this is critical
Created attachment 44198 [details] [review] [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
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 :)
Created attachment 44244 [details] [review] [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>
Patch pushed to master. Thanks Jonathan!
Created attachment 52003 [details] [review] Bug 15087 - Follow for koha-create Logic in the koha-create was requiring a DEFAULTSQL to trigger. This patch moves it, such that --create-db will be registered all the time. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>