[Bug 17699] New: DateTime durations are not correctly subtracted
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Bug ID: 17699 Summary: DateTime durations are not correctly subtracted Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 16911
From DateTime::Duration pod: """ For positive durations, the "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. """
Today we are Nov 30, that means $today->subtract( months => 1 ) will return Oct 31, when we usually expect Oct 30 in tests Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16911 [Bug 16911] Koha::Patrons - Move ExtendMemberSubscriptionTo to ->renew_account -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57827 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57827&action=edit Bug 17699: Fix negative duration calculations in tests
From DateTime::Duration pod: """ For positive durations, the "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. """
Today we are Nov 30, that means $today->subtract( months => 1 ) will return Oct 31, when we usually expect Oct 30 in tests Test plan: prove t/db_dependent/Koha/Patrons.t should return green -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17580 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17580 [Bug 17580] Add the Koha::Patron->get_overdues method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I think this is more tricky than that, I don't find a way to make these tests pass whenever they are launch (think about 31 March - 1 month + 1 month) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|17580 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17580 [Bug 17580] Add the Koha::Patron->get_overdues method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com Status|Needs Signoff |In Discussion --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- #!/usr/bin/perl -w use Modern::Perl; use Data::Dumper; use List::MoreUtils qw / none /; use Time::Fake; BEGIN { my @paths = split ':', $ENV{'PERL5LIB'}; if (@paths) { foreach my $path (@paths) { if (none { $_ eq $path } @INC) { push @INC, $path; } } } } use Koha::DateUtils; for my $count (1..366) { Time::Fake->offset(sprintf("-%dd",$count)); my $old_last_month = dt_from_string->add( months => -1 )->truncate( to => 'day' ); my $wrap_last_month = dt_from_string->subtract( months => 1, end_of_month => 'wrap' )->truncate( to => 'day' ); my $preserve_last_month = dt_from_string->subtract( months => 1, end_of_month => 'preserve' )->truncate( to => 'day' ); my $limit_last_month = dt_from_string->subtract( months => 1, end_of_month => 'limit' )->truncate( to => 'day' ); my $this_day = dt_from_string; print sprintf("T:%s O:%s W:%s P:%s L:%s\n",$this_day,$old_last_month,$wrap_last_month,$preserve_last_month,$limit_last_month); } I'm thinking 'limit'for month subtraction. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- I got the values from: http://search.cpan.org/~drolsky/DateTime-1.42/lib/DateTime/Duration.pm#$dur->end_of_month_mode() -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Just redone some tests and indeed limit seems like to be what we want: use Modern::Perl; use Time::Fake; use Koha::DateUtils; for my $date ( '2016-03-31', '2016-11-30' ) { my $dt = dt_from_string( $date, 'iso' ); Time::Fake->offset( $dt->epoch ); say "=== $date ==="; say dt_from_string->subtract( months => 1 )->add( months => 1 ); say dt_from_string->subtract( months => 1, end_of_month => 'limit' )->add( months => 1 ); say dt_from_string->subtract( months => 1, end_of_month => 'wrap' )->add( months => 1 ); say dt_from_string->subtract( months => 1, end_of_month => 'wrap' )->add( months => 1 ); } Returns: === 2016-03-31 === 2016-03-29T00:00:00 2016-03-29T00:00:00 => What we want 2016-04-02T00:00:00 2016-04-02T00:00:00 === 2016-11-30 === 2016-12-01T00:00:00 2016-11-30T00:00:00 => What we want 2016-11-30T00:00:00 2016-11-30T00:00:00 Wondering why 'limit' does not work as default... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #5)
Just redone some tests and indeed limit seems like to be what we want: [SNIP] Wondering why 'limit' does not work as default...
You'd have to ask the developer of the module. :) "For positive durations, the ' "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. ' -- DateTime::Duration perl Perhaps we uncovered a bug in the method? "date vs datetime math If you only care about the date (calendar) portion of a datetime, you should use either delta_md() or delta_days(), not subtract_datetime(). This will give predictable, unsurprising results, free from DST-related complications." -- http://search.cpan.org/~drolsky/DateTime-1.42/lib/DateTime.pm#How_DateTime_M... Perhaps we shouldn't be taking the 'to_days' way of doing the math? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Same problem appears today (March 31) for t/db_dependent/Koha/Patrons.t # Failed test at t/db_dependent/Koha/Patrons.t line 290. # got: '2018-02-28T00:00:00' # expected: '2018-03-03T00:00:00' # Failed test at t/db_dependent/Koha/Patrons.t line 292. # got: '2018-02-28T00:00:00' # expected: '2018-03-03T00:00:00' # Looks like you failed 2 tests of 10. # Failed test 'renew_account' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Same today (July 31) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57827|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65358&action=edit Bug 17699: Add more tests to highlight the problem Add problematic cases to highlight the problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65359 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65359&action=edit Bug 17699: Use limit as end_of_month
From DateTime::Duration pod: "" For positive durations, the "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. """"
We need end_of_month => limit for positive durations as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65360&action=edit Bug 17699: Add test descriptions Test plan: prove -v t/db_dependent/Koha/Patrons.t Subtest: renew_account 1..30 ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 4 - today + 12 months must be 2017-03-31T00:00:00 ok 5 - today + 12 months must be 2017-03-31T00:00:00 ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 7 - today + 12 months must be 2017-03-31T00:00:00 ok 8 - today + 12 months must be 2017-03-31T00:00:00 ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 14 - today + 12 months must be 2017-11-30T00:00:00 ok 15 - today + 12 months must be 2017-11-30T00:00:00 ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 17 - today + 12 months must be 2017-11-30T00:00:00 ok 18 - today + 12 months must be 2017-11-30T00:00:00 ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 24 - today + 12 months must be 2018-07-31T00:00:00 ok 25 - today + 12 months must be 2018-07-31T00:00:00 ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 27 - today + 12 months must be 2018-07-31T00:00:00 ok 28 - today + 12 months must be 2018-07-31T00:00:00 ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Important note: these patches are going to change the existing behaviour (how the dates are calculated), not only fix the tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65358|0 |1 is obsolete| | Attachment #65359|0 |1 is obsolete| | Attachment #65360|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65365&action=edit Bug 17699: Add more tests to highlight the problem Add problematic cases to highlight the problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65366 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65366&action=edit Bug 17699: Use limit as end_of_month
From DateTime::Duration pod: "" For positive durations, the "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. """"
We need end_of_month => limit for positive durations as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65367&action=edit Bug 17699: Add test descriptions Test plan: prove -v t/db_dependent/Koha/Patrons.t Subtest: renew_account 1..30 ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 4 - today + 12 months must be 2017-03-31T00:00:00 ok 5 - today + 12 months must be 2017-03-31T00:00:00 ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 7 - today + 12 months must be 2017-03-31T00:00:00 ok 8 - today + 12 months must be 2017-03-31T00:00:00 ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 14 - today + 12 months must be 2017-11-30T00:00:00 ok 15 - today + 12 months must be 2017-11-30T00:00:00 ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 17 - today + 12 months must be 2017-11-30T00:00:00 ok 18 - today + 12 months must be 2017-11-30T00:00:00 ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 24 - today + 12 months must be 2018-07-31T00:00:00 ok 25 - today + 12 months must be 2018-07-31T00:00:00 ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 27 - today + 12 months must be 2018-07-31T00:00:00 ok 28 - today + 12 months must be 2018-07-31T00:00:00 ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #16 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 65369 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65369&action=edit [SIGNED-OFF] Bug 17699: Add test descriptions Test plan: prove -v t/db_dependent/Koha/Patrons.t Subtest: renew_account 1..30 ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 4 - today + 12 months must be 2017-03-31T00:00:00 ok 5 - today + 12 months must be 2017-03-31T00:00:00 ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 7 - today + 12 months must be 2017-03-31T00:00:00 ok 8 - today + 12 months must be 2017-03-31T00:00:00 ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 14 - today + 12 months must be 2017-11-30T00:00:00 ok 15 - today + 12 months must be 2017-11-30T00:00:00 ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 17 - today + 12 months must be 2017-11-30T00:00:00 ok 18 - today + 12 months must be 2017-11-30T00:00:00 ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 24 - today + 12 months must be 2018-07-31T00:00:00 ok 25 - today + 12 months must be 2018-07-31T00:00:00 ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 27 - today + 12 months must be 2018-07-31T00:00:00 ok 28 - today + 12 months must be 2018-07-31T00:00:00 ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 Signed-off-by: Owen Leonard <oleonard@myacpl.org> All tests successful. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65369|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65365|0 |1 is obsolete| | --- Comment #17 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 65370 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65370&action=edit [SIGNED-OFF] Bug 17699: Add more tests to highlight the problem Add problematic cases to highlight the problem. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #18 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 65371 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65371&action=edit [SIGNED-OFF] Bug 17699: Use limit as end_of_month
From DateTime::Duration pod: "" For positive durations, the "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. """"
We need end_of_month => limit for positive durations as well. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #19 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 65372 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65372&action=edit [SIGNED-OFF] Bug 17699: Add test descriptions Test plan: prove -v t/db_dependent/Koha/Patrons.t Subtest: renew_account 1..30 ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 4 - today + 12 months must be 2017-03-31T00:00:00 ok 5 - today + 12 months must be 2017-03-31T00:00:00 ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 7 - today + 12 months must be 2017-03-31T00:00:00 ok 8 - today + 12 months must be 2017-03-31T00:00:00 ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 14 - today + 12 months must be 2017-11-30T00:00:00 ok 15 - today + 12 months must be 2017-11-30T00:00:00 ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 17 - today + 12 months must be 2017-11-30T00:00:00 ok 18 - today + 12 months must be 2017-11-30T00:00:00 ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 24 - today + 12 months must be 2018-07-31T00:00:00 ok 25 - today + 12 months must be 2018-07-31T00:00:00 ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 27 - today + 12 months must be 2018-07-31T00:00:00 ok 28 - today + 12 months must be 2018-07-31T00:00:00 ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65366|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65367|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Status|Signed Off |Failed QA --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looks good to me, but I am wondering if we really need an additional module to test the behavior of BorrowerRenewalPeriodBase. Does this new dependency really bring us great benefits here? Time::Fake is Debian packaged though. It seems to me that testing the "now" variant could be done too with current time. The other variant "dateexpiry" allows you to set dates so that you can see that the end_of_month limit really works. And if we should use it anyway, I would prefer to see the offset call just before the code it should actually work on and an additional Time::Fake->reset after that block. Changing status to reflect need for feedback. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 65400 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65400&action=edit Bug 17699: Reset time simulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #20)
Looks good to me, but I am wondering if we really need an additional module to test the behavior of BorrowerRenewalPeriodBase. Does this new dependency really bring us great benefits here? Time::Fake is Debian packaged though.
It's already a Koha dependency.
It seems to me that testing the "now" variant could be done too with current time. The other variant "dateexpiry" allows you to set dates so that you can see that the end_of_month limit really works.
I think it is less error prone to use Time::Fake
And if we should use it anyway, I would prefer to see the offset call just before the code it should actually work on and an additional Time::Fake->reset after that block.
I added the ->reset call, but I do not understand your other remark. Where do you want it to be moved? It sounds better to me to have it visible at the beginning of the loop. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #22)
It's already a Koha dependency. So it is indeed. Wrong assumption.. t/Circulation/AgeRestrictionMarkers.t
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #22)
I added the ->reset call, but I do not understand your other remark. Where do you want it to be moved? It sounds better to me to have it visible at the beginning of the loop.
Do you need it in the whole loop? I would say that you only need it somewhere deeper when you calculate the new expiry date for 'now'. But adding the reset is good enough, I suppose. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Finish this one tomorrow -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Just a remark: Date::Calc's Add_Delta_YM does the same as what you are doing now with limit. ($year,$month,$day) = Add_Delta_YM($year,$month,$day, $Dy,$Dm);
From POD: This function does no "wrapping" into the next month if the day happens to lie outside the valid range for the resulting year and month (after adding the year and month offsets). Instead, it simply truncates the day to the last possible day of the resulting month.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65370|0 |1 is obsolete| | Attachment #65371|0 |1 is obsolete| | Attachment #65372|0 |1 is obsolete| | --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65419 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65419&action=edit Bug 17699: Add more tests to highlight the problem Add problematic cases to highlight the problem. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65420 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65420&action=edit Bug 17699: Use limit as end_of_month
From DateTime::Duration pod: "" For positive durations, the "end_of_month" parameter defaults to wrap. For negative durations, the default is "limit". This should match how most people "intuitively" expect datetime math to work. """"
We need end_of_month => limit for positive durations as well. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65421 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65421&action=edit Bug 17699: Add test descriptions Test plan: prove -v t/db_dependent/Koha/Patrons.t Subtest: renew_account 1..30 ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00 ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 4 - today + 12 months must be 2017-03-31T00:00:00 ok 5 - today + 12 months must be 2017-03-31T00:00:00 ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 7 - today + 12 months must be 2017-03-31T00:00:00 ok 8 - today + 12 months must be 2017-03-31T00:00:00 ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00 ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00 ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 14 - today + 12 months must be 2017-11-30T00:00:00 ok 15 - today + 12 months must be 2017-11-30T00:00:00 ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 17 - today + 12 months must be 2017-11-30T00:00:00 ok 18 - today + 12 months must be 2017-11-30T00:00:00 ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00 ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00 ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged ok 24 - today + 12 months must be 2018-07-31T00:00:00 ok 25 - today + 12 months must be 2018-07-31T00:00:00 ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged ok 27 - today + 12 months must be 2018-07-31T00:00:00 ok 28 - today + 12 months must be 2018-07-31T00:00:00 ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00 Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65400|0 |1 is obsolete| | --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65422 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65422&action=edit Bug 17699: Reset time simulation Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #31 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 17.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolin.somers@biblibre.co | |m --- Comment #32 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.05.x, will be in 17.05.05. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- These patches have been pushed to 16.11.x and will be in 16.11.13. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #34 from Mason James <mtj@kohaaloha.com> --- Blocked by Enhancement, skipping for 16.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22254 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22254 [Bug 22254] t/db_dependent/Koha/Patrons.t contains a DateTime math error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17699 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |33107 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33107 [Bug 33107] Expiry date tests failing on March 1st 2023 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org