Bug 8738 - DateTime object being compared to a date string in circulation
Summary: DateTime object being compared to a date string in circulation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low major (vote)
Assignee: Bugs List
QA Contact: Mason James
URL: http://intranet/cgi-bin/koha/circ/cir...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-07 14:13 UTC by MJ Ray (software.coop)
Modified: 2013-12-05 20:05 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
circ/circulation.pl: force issuedate to a string to fix bug 8738 (1004 bytes, patch)
2012-09-07 14:18 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
[SIGNED-OFF] circ/circulation.pl: force issuedate to a string to fix bug 8738 (1.19 KB, patch)
2012-09-26 11:36 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description MJ Ray (software.coop) 2012-09-07 14:13:37 UTC
A DateTime object can only be compared to another DateTime object (DateTime=HASH(0x54110d8), 0). at /home/coop/koha/unstable/src/circ/circulation.pl line 346
 at /usr/lib/perl5/DateTime.pm line 1801
	DateTime::_compare('DateTime=HASH(0x54110d8)', 0, 0) called at /usr/lib/perl5/DateTime.pm line 1780
	DateTime::compare('DateTime=HASH(0x54110d8)', 0) called at /usr/lib/perl5/DateTime.pm line 1762
	DateTime::_compare_overload('DateTime=HASH(0x54110d8)', 0, '') called at /home/coop/koha/unstable/src/circ/circulation.pl line 346
Comment 1 MJ Ray (software.coop) 2012-09-07 14:14:16 UTC
This is happening when I try to circulate a book.
Comment 2 MJ Ray (software.coop) 2012-09-07 14:18:47 UTC Comment hidden (obsolete)
Comment 3 MJ Ray (software.coop) 2012-09-07 15:07:36 UTC
Tested as working on local development installation. I can now circulate books again.

I apologise for the incorrect line number in the earlier backtrace. It seems I had some development code (analytic item types, bug 7716) in the tree at the time.
Comment 4 Owen Leonard 2012-09-14 13:25:16 UTC
See a lot of errors in the log from circulation.pl, but not this one. Does this only happen with a certain version or under certain circumstances?
Comment 5 Kyle M Hall 2012-09-21 12:23:10 UTC
(In reply to comment #4)
> See a lot of errors in the log from circulation.pl, but not this one. Does
> this only happen with a certain version or under certain circumstances?

I must concur with Owen. I'm not seeing this error in master.
Comment 6 MJ Ray (software.coop) 2012-09-24 10:43:49 UTC
Well, it's great that it works for you, but it's erroring for some libraries.

I suspect this might only present itself on upgrades where the circulation history meets some combination of conditions, but I have met it three times on different servers at different libraries so far: one we installed, one we upgraded and one we only support after they met this.

In theory, it looks like a type error to me.  Can you see how that circ/circulation.pl line http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=circ/circulation.pl;h=f15d4cea94e67e74b6c87ccbef50d0c572b658d1;hb=HEAD#l469 isn't comparing a DateTime to a string, or explain why that isn't a bug?

In the first comparison in that line:

$it->{'issuedate'} is a DateTime returned by C4::Members::GetPendingIssues(...) since the addition of hourly loans. http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Members.pm;h=b2f45b955bf3ff3ba57457ca6ec2130fb38021ec;hb=HEAD#l1039

$todaysdate is a string returned by C4::Dates->new->output('iso') http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Dates.pm;h=4fd08aeb6c4e94a744652b321202d313cd531eb5;hb=HEAD#l184

At best, if this fix doesn't break your test copies in practice and you can't see a theoretical problem with it, could we include it, please?
Comment 7 Kyle M Hall 2012-09-24 11:40:48 UTC
MJ, can you please provide us with a scenario that will produce the error you describe in comment 1? You tell us to prove this is not a bug. That seems a bit backwards, don't you think? I just want to be able to reproduce your error, and prove this patch fixes it with no new errors or bad behavior.

Kyle

(In reply to comment #6)
> Well, it's great that it works for you, but it's erroring for some libraries.
> 
> I suspect this might only present itself on upgrades where the circulation
> history meets some combination of conditions, but I have met it three times
> on different servers at different libraries so far: one we installed, one we
> upgraded and one we only support after they met this.
> 
> In theory, it looks like a type error to me.  Can you see how that
> circ/circulation.pl line
> http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=circ/circulation.
> pl;h=f15d4cea94e67e74b6c87ccbef50d0c572b658d1;hb=HEAD#l469 isn't comparing a
> DateTime to a string, or explain why that isn't a bug?
> 
> In the first comparison in that line:
> 
> $it->{'issuedate'} is a DateTime returned by
> C4::Members::GetPendingIssues(...) since the addition of hourly loans.
> http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Members.pm;
> h=b2f45b955bf3ff3ba57457ca6ec2130fb38021ec;hb=HEAD#l1039
> 
> $todaysdate is a string returned by C4::Dates->new->output('iso')
> http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Dates.pm;
> h=4fd08aeb6c4e94a744652b321202d313cd531eb5;hb=HEAD#l184
> 
> At best, if this fix doesn't break your test copies in practice and you
> can't see a theoretical problem with it, could we include it, please?
Comment 8 MJ Ray (software.coop) 2012-09-25 10:45:41 UTC
For completeness, here's the correct error message when the analytic
items enh isn't installed:

Software error:
A DateTime object can only be compared to another DateTime object (DateTime=HASH(0xa2d8aa8), 2012-08-31). at /usr/share/koha/intranet/cgi-bin/circ/circulation.pl line 464

I think the scenario is as follows:

a. install a post-hourly-loans koha-common (3.8.4 was where I met it)
on Ubuntu 10.04 LTS;

b. circulate some books.

And no, I don't think it's "backwards" to study the code and question
how that ever doesn't error!

Looking into that key question further, I found the changelog for
DateTime.pm suggests only versions >= 0.54 allow comparing a DateTime
to a string, but koha-common doesn't require any version
(interestingly C4::Installer::PerlDependencies::PERL_DEPS says 0.58,
so there's arguably a bug there) and Ubuntu 10.04 LTS (which is on at
least two of the servers I've seen this bug on) only has version 0.52.
So there are two obvious fixes:

1. apply this patch;

2. add a libdatetime-perl version requirement to koha-common which
effectively excludes Ubuntu 10.04 Long Term Support from them, or
would need us to provide packages for it.

(This seems like an old chestnut - racing ahead with adoption of new
versions of dependencies without considering what is actually
available on stable server distributions - with a new twist, in that
koha-common's version requirements don't match what's in
C4::Installer::PerlDepenencies.)
Comment 9 Kyle M Hall 2012-09-26 11:36:14 UTC
Created attachment 12522 [details] [review]
[SIGNED-OFF] circ/circulation.pl: force issuedate to a string to fix bug 8738

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Installed DataTime version 0.53, verified the error occurs.
Applied patch, verified patch removes the error.
Comment 10 Kyle M Hall 2012-09-26 11:40:33 UTC
Thanks for the extra info. I was able to recreate your error and verify the patch fixes it.
Comment 11 Mason James 2012-10-04 00:37:43 UTC
(In reply to comment #9)
> Created attachment 12522 [details] [review]
> [SIGNED-OFF] circ/circulation.pl: force issuedate to a string to fix bug 8738
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
> 
> Installed DataTime version 0.53, verified the error occurs.
> Applied patch, verified patch removes the error.


i've verified this too, passing QA...

$ koha-qa.pl -c 1

testing 1 commit(s) (applied to commit 6c18204)
 * 8bae944 circ/circulation.pl: force issuedate to a string to fix bug 8738
      circ/circulation.pl

* circ/circulation.pl                                                      OK
Comment 12 Paul Poulain 2012-10-05 15:38:34 UTC
Patch pushed to master
Comment 13 Chris Cormack 2013-04-20 23:31:35 UTC
Released in 3.10.0