[Bug 25850] New: CalcDateDue freezes with 'useDaysMode' set to 'Daysweek' and the due date lands on a sunday.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Bug ID: 25850 Summary: CalcDateDue freezes with 'useDaysMode' set to 'Daysweek' and the due date lands on a sunday. Change sponsored?: --- Product: Koha Version: 19.11 Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: andreas.jonsson@kreablo.se QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com
From the DateTime manual page:
$dt->day_of_week() Returns the day of the week as a number, from 1..7, with 1 being Monday and 7 being Sunday. Also available as "$dt->wday()" and "$dt->dow()". But the method "get_push_amt" assumes that day_of_week is numbered 0..6 with 0 being Sunday. (Also, there is a funny equality test of a boolean value and the number 1.) The result is that for Sundays, $self->{weekly_closed_days}->[$dow] is undefined and its negation is thus true, which results in a push_amt of seven days, which lands on the next Sunday and the process repeats. sub get_push_amt { my ( $self, $base_date) = @_; my $dow = $base_date->day_of_week; return ( # We're using Dayweek useDaysMode option $self->{days_mode} eq 'Dayweek' && # It's not a permanently closed day !$self->{weekly_closed_days}->[$dow] == 1 ) ? 7 : 1; } -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|CalcDateDue freezes with |CalcDateDue freezes with |'useDaysMode' set to |'useDaysMode' set to |'Daysweek' and the due date |'Dayweek' and the due date |lands on a sunday. |lands on a sunday. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #1 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 106207 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106207&action=edit Bug 25850: Adapt day of week in Koha::Calendar::get_push_amt to 0..6 with 0 being Sunday. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #2 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Test plan 1. Go to tools -> calendar and make sure Sundays are holidays by adding a holiday on a Sunday and selecting "Holiday repeated every same day of the week" and "Copy to all libraries" and save. 2. Go to administration -> system preferences and select "Use the calendar to push the due date to the next open matching weekday for weekly loan periods, or the next open day otherwise" for the system preference "useDaysMode" and save the system preferences. 3. Put the below code in a file name test.pl and execute it using the command "sudo koha-shell -c 'perl test.pl' kohadev" use Koha::Calendar; my $calendar = Koha::Calendar->new( branchcode => 'CPL' ); $dt = DateTime->new( year => 2020, month => 06, day => 21 ); print "This is a sunday: " . $dt->day_of_week . "\n"; my $ndt = $calendar->next_open_days($dt, 0); print "This is a monday: " . $ndt->day_of_week . "\n"; 4. Without the patch applied, this script will freeze after printing "This is a sunday ...". Abort using ctrl-c. 5. Apply patch and run the script again. The test script will now complete. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |15260 CC| |martin.renvoize@ptfs-europe | |.com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15260 [Bug 15260] Option for extended loan with useDaysMode -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I like the modulo approach you've used, but I wonder if perhaps it's a tiny bit clearer if you use the same approach as the 'is_holiday' method uses a few lines beneath your fix? Also.. I'd love to see a unit test added to prevent future regression. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106346 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106346&action=edit Bug 25850: Add tests for weekday holidays -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106347 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106347&action=edit Bug 25850: WIP - Add Unit Test -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Added the beginnings of a test there.. first one is unrelated but I wrote it whilst working out options to test each weekday holiday option.. second patch I've not managed to get failing yet.. bear with. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106348 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106348&action=edit Bug 25850: Add Unit Test -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106347|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- That's better. So.. before I signoff I'd be interested in opinions on my above question regarding readability and making it match other blocks that do the same logical fix. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106346|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106381 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106381&action=edit Bug 25850: Add tests for weekday holidays Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106382&action=edit Bug 25850: Regression Test This patch adds a unit test to test for an infinite loop as highlighted by the bug. Test plan 1/ Run the test before applying the fix 2/ The test should fail for 'Sundays' 3/ Apply the subsquent patch 4/ Re-run the test 5/ It should now pass Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106207|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106383 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106383&action=edit Bug 25850: Adapt day of week in Koha::Calendar::get_push_amt to 0..6 with 0 being Sunday. 1. Go to tools -> calendar and make sure Sundays are holidays by adding a holiday on a Sunday and selecting "Holiday repeated every same day of the week" and "Copy to all libraries" and save. 2. Go to administration -> system preferences and select "Use the calendar to push the due date to the next open matching weekday for weekly loan periods, or the next open day otherwise" for the system preference "useDaysMode" and save the system preferences. 3. Put the below code in a file name test.pl and execute it using the command "sudo koha-shell -c 'perl test.pl' kohadev" use Koha::Calendar; my $calendar = Koha::Calendar->new( branchcode => 'CPL' ); $dt = DateTime->new( year => 2020, month => 06, day => 21 ); print "This is a sunday: " . $dt->day_of_week . "\n"; my $ndt = $calendar->next_open_days($dt, 0); print "This is a monday: " . $ndt->day_of_week . "\n"; 4. Without the patch applied, this script will freeze after printing "This is a sunday ...". Abort using ctrl-c. 5. Apply patch and run the script again. The test script will now complete. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106348|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25723 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|CalcDateDue freezes with |CalcDateDue freezes with |'useDaysMode' set to |'useDaysMode' set to |'Dayweek' and the due date |'Dayweek' and the due date |lands on a sunday. |lands on a Sunday -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I did a quick benchmark on '`Modulo` Vs `comparison + assignment`'. Rate Modulo Assign Modulo 23693859/s -- -22% Assign 30513086/s 29% -- So.. it looks like the test + assign is the quicker operation... I'll add a followup here to make that conversion to match the logic here to the logic in the is_holiday routine. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106384&action=edit Bug 25850: (QA follow-up) Match logic in is_holiday -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|19.11 |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Nick Clemens <nick@bywatersolutions.com> 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=25850 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106381|0 |1 is obsolete| | --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 106626 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106626&action=edit Bug 25850: Add tests for weekday holidays Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106382|0 |1 is obsolete| | --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 106627 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106627&action=edit Bug 25850: Regression Test This patch adds a unit test to test for an infinite loop as highlighted by the bug. Test plan 1/ Run the test before applying the fix 2/ The test should fail for 'Sundays' 3/ Apply the subsquent patch 4/ Re-run the test 5/ It should now pass Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106383|0 |1 is obsolete| | --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 106628 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106628&action=edit Bug 25850: Adapt day of week in Koha::Calendar::get_push_amt to 0..6 with 0 being Sunday. 1. Go to tools -> calendar and make sure Sundays are holidays by adding a holiday on a Sunday and selecting "Holiday repeated every same day of the week" and "Copy to all libraries" and save. 2. Go to administration -> system preferences and select "Use the calendar to push the due date to the next open matching weekday for weekly loan periods, or the next open day otherwise" for the system preference "useDaysMode" and save the system preferences. 3. Put the below code in a file name test.pl and execute it using the command "sudo koha-shell -c 'perl test.pl' kohadev" use Koha::Calendar; my $calendar = Koha::Calendar->new( branchcode => 'CPL' ); $dt = DateTime->new( year => 2020, month => 06, day => 21 ); print "This is a sunday: " . $dt->day_of_week . "\n"; my $ndt = $calendar->next_open_days($dt, 0); print "This is a monday: " . $ndt->day_of_week . "\n"; 4. Without the patch applied, this script will freeze after printing "This is a sunday ...". Abort using ctrl-c. 5. Apply patch and run the script again. The test script will now complete. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106384|0 |1 is obsolete| | --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 106629 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106629&action=edit Bug 25850: (QA follow-up) Match logic in is_holiday Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 106630 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106630&action=edit Bug 25850: (QA follow-up) use dt_from_string Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Who signed the| |20.11.00 patch off| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to master for 20.11, thanks everyone. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.11.00, 20.05.03 released in| | CC| |lucas@bywatersolutions.com --- Comment #20 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 20.05.x for 20.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |aleisha@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00, 20.05.03 |20.11.00, 20.05.03, released in| |19.11.09 Status|Pushed to stable |Pushed to oldstable --- Comment #21 from Aleisha Amohia <aleisha@catalyst.net.nz> --- backported to 19.11.x for 19.11.09 t/db_dependent/Calendar.t needed an additional patch to mock the useDaysMode syspref, otherwise the test would fail if useDaysMode wasn't set to 'calendar'. @19.05.x rmaint: You will need my additional patch as well because Koha::Calendar->_init checks the syspref. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #22 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Thanks for the notice Aleisha. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 --- Comment #23 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Can't backport to 19.05.x, the test uses next_open_days which isn't in 19.05 If someone needs this for 19.05.x, please send a patch or code extract that would allow the test to be backported. And thus ensuring that the fix works. kohadev-koha@891563ed356b:/kohadevbox/koha$ prove t/db_dependent/Calendar.t t/db_dependent/Calendar.t .. 1/8 # No tests run! # Failed test 'No tests run for subtest "weekday holidays"' # at t/db_dependent/Calendar.t line 183. # Looks like you failed 1 test of 1. # Failed test 'get_push_amt' # at t/db_dependent/Calendar.t line 184. Can't locate object method "next_open_days" via package "Koha::Calendar" at t/db_dependent/Calendar.t line 165. ...propagated at t/db_dependent/Calendar.t line 169. # Looks like your test exited with 11 just after 8. t/db_dependent/Calendar.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 1/8 subtests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27835 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25850 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27835 | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org