[Bug 13242] New: Slow checkin for patrons restricted indefinitely
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Bug ID: 13242 Summary: Slow checkin for patrons restricted indefinitely Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |jonathan.druart@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33510 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33510&action=edit Bug 13242: DateUtils should manage separately 9999-* dates On bug 13226, we have seen that my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) For instance if you try to check in items for a patron which is restricted indefinitely (and there is no overdue on the item), the code in C4::Circulation::AddReturn l~1948 my $borrower_debar_dt = dt_from_string( $borrower->{debarred} ); will be called. dt_from_string should not called DateTime::Format::DateParse->parse_datetime with the time zone parameter in this case. Test plan: Create an indefinitely restriction for a patron and check an item in. Without this patch, it should take ~20 sec. With, ~2 or 3 sec. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Depends on| |8231 --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Original code introduced by bug 8231. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33510|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33511 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33511&action=edit Bug 13242: DateUtils should manage separately 9999-* dates On bug 13226, we have seen that my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) For instance if you try to check in items for a patron which is restricted indefinitely (and there is no overdue on the item), the code in C4::Circulation::AddReturn l~1948 my $borrower_debar_dt = dt_from_string( $borrower->{debarred} ); will be called. dt_from_string should not called DateTime::Format::DateParse->parse_datetime with the time zone parameter in this case. Test plan: Create an indefinitely restriction for a patron and check an item in. Without this patch, it should take ~20 sec. With, ~2 or 3 sec. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #4 from Frédéric Demians <frederic@tamil.fr> --- It seems to come from here: https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo... Why is it necessary to use timezone for DateTime conversion process in the first place? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=13250 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Frédéric Demians from comment #4)
It seems to come from here:
https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo...
Why is it necessary to use timezone for DateTime conversion process in the first place?
Salut Frédéric, I don't understand, what is "the first place"? we use DateTime::Format::DateParse->parse_datetime, not DateTime->new (maybe it calls it, I didn't deep into the code). All what I know is that it is useless to specify timezone for 9999-12-31 :) Note that $ENV{TZ} is not set on my local instance. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #6 from Frédéric Demians <frederic@tamil.fr> ---
I don't understand, what is "the first place"?
I mean: Is it really necessary to use timezone in dt_from_string function? Otherwise, I fully agree that your patch solve the bug (ugly one). I would add a FIXME comment in the code to point to bug number, in order to avoid that someone else reintroduce the bug in the future. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Frédéric Demians from comment #6)
I don't understand, what is "the first place"?
I mean: Is it really necessary to use timezone in dt_from_string function?
I don't know.
Otherwise, I fully agree that your patch solve the bug (ugly one). I would add a FIXME comment in the code to point to bug number, in order to avoid that someone else reintroduce the bug in the future.
Maybe a test based on localtime_before - localtime_after should be < 3 sec. But it's not really clean, it could depend on the load of the machine. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #8 from Frédéric Demians <frederic@tamil.fr> ---
Maybe a test based on localtime_before - localtime_after should be < 3 sec. But it's not really clean, it could depend on the load of the machine.
Very good suggestion. I'd try to add a test to t/DateUtils.t before signing your patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #9 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33560 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33560&action=edit Bug 13242 Add a UT to t/DateUtils.t for testing DateTime bug A bug in DateTime slow down drastically date parsing when the dates are in the far distant future: https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo... This UT tests this situation. TO TEST: - Apply the patch containing the UT - prove -v t/DateUtils.t - You see that parsing a 9999-01-01 that take forever (ie more than 1s) - Apply Jonathan patch - prove -v t/DateUtils.t - No more complain. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Frédéric Demians <frederic@tamil.fr> 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=13242 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33511|0 |1 is obsolete| | --- Comment #10 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33561 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33561&action=edit Bug 13242: DateUtils should manage separately 9999-* dates On bug 13226, we have seen that my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) For instance if you try to check in items for a patron which is restricted indefinitely (and there is no overdue on the item), the code in C4::Circulation::AddReturn l~1948 my $borrower_debar_dt = dt_from_string( $borrower->{debarred} ); will be called. dt_from_string should not called DateTime::Format::DateParse->parse_datetime with the time zone parameter in this case. Test plan: Create an indefinitely restriction for a patron and check an item in. Without this patch, it should take ~20 sec. With, ~2 or 3 sec. Signed-off-by: Frederic Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 33560 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33560 Bug 13242 Add a UT to t/DateUtils.t for testing DateTime bug Review of attachment 33560: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13242&attachment=33560) ----------------------------------------------------------------- ::: t/DateUtils.t @@ +7,2 @@
use Test::MockModule; +use Time::HiRes qw/ gettimeofday /;
Time::HiRes seems to be a new dependency. Is not it possible to do the same without adding it? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #12 from Frédéric Demians <frederic@tamil.fr> ---
Time::HiRes seems to be a new dependency. Is not it possible to do the same without adding it?
I see Time::HiRes in core dependencies: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Installer/PerlD... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #13 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Frédéric Demians from comment #12)
Time::HiRes seems to be a new dependency. Is not it possible to do the same without adding it?
I see Time::HiRes in core dependencies:
http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Installer/ PerlDependencies.pm;h=09be2cda3e3ccc84777d1e50b9d2054d778d6a0f;hb=HEAD#l490
Oops yes, sorry. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33560|0 |1 is obsolete| | Attachment #33561|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33564 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33564&action=edit Bug 13242: Add a UT to t/DateUtils.t for testing DateTime bug A bug in DateTime slow down drastically date parsing when the dates are in the far distant future: https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo... This UT tests this situation. TO TEST: - Apply the patch containing the UT - prove -v t/DateUtils.t - You see that parsing a 9999-01-01 that take forever (ie more than 1s) - Apply Jonathan patch - prove -v t/DateUtils.t - No more complain. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33565 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33565&action=edit Bug 13242: DateUtils should manage separately 9999-* dates On bug 13226, we have seen that my $date = DateTime->new(year => 9999, month => 12, day => 31); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31' ); => OK DateTime::Format::DateParse->parse_datetime( '9999-12-31', 'America/Los_Angeles' ); => KO (~20sec on my laptop) For instance if you try to check in items for a patron which is restricted indefinitely (and there is no overdue on the item), the code in C4::Circulation::AddReturn l~1948 my $borrower_debar_dt = dt_from_string( $borrower->{debarred} ); will be called. dt_from_string should not called DateTime::Format::DateParse->parse_datetime with the time zone parameter in this case. Test plan: Create an indefinitely restriction for a patron and check an item in. Without this patch, it should take ~20 sec. With, ~2 or 3 sec. Signed-off-by: Frederic Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- I added my sign off on your patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #17 from Marc Véron <veron@veron.ch> --- I tested with and without patch and had the same problem (about 20 sec for checking in an item for an undefinitly restricted patron. With patch, I played around with the following line: return DateTime::Format::DateParse->parse_datetime($date_string); Same result without and with time zone parameter (e.g. 'floating' or '+0000'). I also tested and had the same result using the following code (with and without time_zone variants) return DateTime->new ( year=> 9999, month=> 12, day => 31, hour => 0, minute => 0, second => 0, time_zone => '+0000', ); The only thing that reduced time was to change the year e.g. to 2999 (for both methods). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #18 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Marc Véron from comment #17)
I tested with and without patch and had the same problem (about 20 sec for checking in an item for an undefinitly restricted patron.
Did you test with bug 13226 (pushed in master 1 hour ago)? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #19 from Marc Véron <veron@veron.ch> --- No, I did not. Sorry, I can not test for the moment, just got a Windows 8.1 Update and that crashed my VM. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #20 from Marc Véron <veron@veron.ch> --- OK, deinstalled Windows update and re-installed VM... Tested again: If I check in the item using the button "Renew or return checked items" below the checkout table it is now < 5 sec => OK If I check in the item using the Check in form at top of the patrons checkout page (circulation.pl), it is about 30 sec. => not OK If I check in the item using the Check in form at top of other pages page (e.g. mainpage.pl) , it is < 5 sec. => OK -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I see the same behaviour MARC noted - using the checkin tab on top of the patrons page is still slow. So is the normal check-in page - should this be fixed? Checking in from the patron account is ok, as tested for the earlier other patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #22 from Frédéric Demians <frederic@tamil.fr> --- (In reply to Katrin Fischer from comment #21)
I see the same behaviour MARC noted - using the checkin tab on top of the patrons page is still slow. So is the normal check-in page - should this be fixed?
Checking in from the patron account is ok, as tested for the earlier other patch.
I can also see the slowness... I only occurs when checking in a non-overdue item. If the item is overdue, it's OK. - a debarred borrower - an item that isn't overdue - check-in from the circ/circulation-home.pl page -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #23 from Frédéric Demians <frederic@tamil.fr> --- I will send a re-factored patch: - complete Jonathan fix to dt_from_string() with an equivalent fix to output_pref(). - separate the fix patch and the UT patch to facilitate testing and signing-off - new sign-off required -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33564|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=13242 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33565|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=13242 --- Comment #24 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33589 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33589&action=edit Bug 13242: Add a UT to t/DateUtils.t for testing DateTime bug A bug in DateTime slow down drastically date parsing when the dates are in the far distant future: https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo... This UT tests this situation which affects Koha::DateUtils function dt_from_string() and output_pref(). TO TEST: - Apply the patch containing the UT - prove -v t/DateUtils.t - You see that parsing a 9999-01-01 that take forever (ie more than 1s) - Apply the patch containing the fix - prove -v t/DateUtils.t - No more complain. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #25 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33590 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33590&action=edit Bug 13242: Fix DateUtils for 'infinite' dates (ie year 9999) TEST PLAN: - Method 1--with UT - Use the UT associated to this bug, without applying this patch, and then after applying this patch - Method 2--using Koha - Without this patch - Find a borrower with several checkouts that are not overdue. - Debarred the borrower - Go on circ/circulation-home.pl page - Select Check in tab, and do a check in - It required more than 20s to display the return.pl page - Apply the patch, and repeat previous steps => return.pl is immediately displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33590|0 |1 is obsolete| | --- Comment #26 from Marc Véron <veron@veron.ch> --- Created attachment 33591 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33591&action=edit Bug 13242: Fix DateUtils for 'infinite' dates (ie year 9999) TEST PLAN: - Method 1--with UT - Use the UT associated to this bug, without applying this patch, and then after applying this patch - Method 2--using Koha - Without this patch - Find a borrower with several checkouts that are not overdue. - Debarred the borrower - Go on circ/circulation-home.pl page - Select Check in tab, and do a check in - It required more than 20s to display the return.pl page - Apply the patch, and repeat previous steps => return.pl is immediately displayed. Followed method 2. Time problem no longer exists. 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=13242 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33589|0 |1 is obsolete| | --- Comment #27 from Marc Véron <veron@veron.ch> --- Created attachment 33592 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33592&action=edit Bug 13242: Add a UT to t/DateUtils.t for testing DateTime bug A bug in DateTime slow down drastically date parsing when the dates are in the far distant future: https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo... This UT tests this situation which affects Koha::DateUtils function dt_from_string() and output_pref(). TO TEST: - Apply the patch containing the UT - prove -v t/DateUtils.t - You see that parsing a 9999-01-01 that take forever (ie more than 1s) - Apply the patch containing the fix - prove -v t/DateUtils.t - No more complain. Followed test plan. Test behaves as expected. 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=13242 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=13242 --- Comment #28 from Marc Véron <veron@veron.ch> --- Note: While testing, I did not like the check in message for a user with indefinite restriction and changed it in Bug 13261 It was: Reminder: Patron was earlier restricted until 9999-12-31 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33596 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33596&action=edit [PASSED QA] Bug 13242: Fix DateUtils for 'infinite' dates (ie year 9999) TEST PLAN: - Method 1--with UT - Use the UT associated to this bug, without applying this patch, and then after applying this patch - Method 2--using Koha - Without this patch - Find a borrower with several checkouts that are not overdue. - Debarred the borrower - Go on circ/circulation-home.pl page - Select Check in tab, and do a check in - It required more than 20s to display the return.pl page - Apply the patch, and repeat previous steps => return.pl is immediately displayed. Followed method 2. Time problem no longer exists. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #30 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33597 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33597&action=edit [PASSED QA] Bug 13242: Add a UT to t/DateUtils.t for testing DateTime bug A bug in DateTime slow down drastically date parsing when the dates are in the far distant future: https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slo... This UT tests this situation which affects Koha::DateUtils function dt_from_string() and output_pref(). TO TEST: - Apply the patch containing the UT - prove -v t/DateUtils.t - You see that parsing a 9999-01-01 that take forever (ie more than 1s) - Apply the patch containing the fix - prove -v t/DateUtils.t - No more complain. Followed test plan. Test behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Works as described - check-ins are now much faster. Passes tests and QA script. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=13242 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33592|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=13242 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33591|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=13242 --- Comment #31 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 33596 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33596 [PASSED QA] Bug 13242: Fix DateUtils for 'infinite' dates (ie year 9999) Review of attachment 33596: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13242&attachment=33596) ----------------------------------------------------------------- ::: Koha/DateUtils.pm @@ +128,5 @@
return unless defined $dt;
+ # FIXME: see bug 13242 => no TZ for dates 'infinite' + $dt->set_time_zone( C4::Context->tz ) if $dt->ymd !~ /^9999/;
Fredéric, did you locate where output_pref is called with "9999-12-31"? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #32 from Frédéric Demians <frederic@tamil.fr> ---
Fredéric, did you locate where output_pref is called with "9999-12-31"?
circ/returns.tt, line 423: [% IF ( errmsgloo.prevdebarred ) %] <p class="problem"><b>Reminder: </b>Patron was earlier restricted until [% errmsgloo.prevdebarred | $KohaDates %]</p> [% END %] The "| $KohaDate" directive calls the KohaDate TT plugin. Tracking this kind of issue is awful. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #33 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Frédéric Demians from comment #32) Good catch! That means we need a specific alert message in this case. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #34 from Marc Véron <veron@veron.ch> --- Regarding alert message: Bug 13261 - Better check in message for patrons with indefinite restriction Signed off (first part), re-wording of message waiting for Sign-off Marc -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #35 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Frédéric! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #36 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33945 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33945&action=edit Bug 13242: Remove warning if no parameter given Without any parameter, dt_from_string should not raise a warning message. Test plan: Verify that the test file t/DateUtils.t displays a warning: Use of uninitialized value $date_string in pattern match (m//) at Koha/DateUtils.pm line 58 if the change in dt_from_string is not applied (manually edit the file). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Followup pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com, | |sophie.meynieux@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 --- Comment #38 from Sophie MEYNIEUX <sophie.meynieux@biblibre.com> --- Would it be possible to have it in 3.16.x ? Thanx -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_3_16_candidate --- Comment #39 from Mason James <mtj@kohaaloha.com> --- (In reply to Sophie MEYNIEUX from comment #38)
Would it be possible to have it in 3.16.x ? Thanx
oops, i missed this bug i'll take a look for the next 3.16 release -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #40 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Found checks like: if ( $dt->ymd !~ /^9999/ ) { $dt_params{year} < 9999 Note that if you would pick year 3000 or 9000, it still would take forever to let DateTime calculate things in a named timezone. If the date is more than a few days/weeks in the future, I suppose that we already could jump to floating or UTC instead of the named timezone? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13242 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14494 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org