http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8418 --- Comment #1 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 10769 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10769&action=edit Bug 8418 - Koha::Calendar is_holiday ignores repeating holidays This is simple fix since day_month_closed_days hash has wrong arguments order. -- 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=8418 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10770|0 |1 is obsolete| | --- Comment #3 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 10773 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10773&action=edit Bug 8418 - fix dt->truncate( to => 'day' ) -- 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=8418 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Blocks| |8110, 8345 --- Comment #5 from Dobrica Pavlinusic <dpavlin@rot13.org> --- This bug duplicates some parts of following bugs which are not signed off: Bug 8110 - Fines accruing on closed days Bug 8348 - useDaysMode syspref does not apply It modifies only Koha::Calendar code, making is simper and faster (saving one SQL query as side-effect) and basically brings back Koha behaviour before we decided to move C4::Calendar into Koha::Calendar and broke stuff. -- 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=8418 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Koha::Calendar is_holiday |Koha::Calendar is_holiday |ignores repeating holidays |ignores all user data -- 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=8418 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=8251 -- 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=8418 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10773|0 |1 is obsolete| | --- Comment #7 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 10811 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10811&action=edit Bug 8418 - fix dt->truncate( to => 'day' ) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- 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=8418 --- Comment #9 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 10838 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10838&action=edit Bug 8418 - correctly mock test data -- 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=8418 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Dobrica, I am a bit worried about Chris' question:

Also are we certain everywhere is specifying holidays as day month, not month >day as the test and code was before this change?

So maybe he patches are breaking things in other places now? Is there a way to check/make sure? -- 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=8418 --- Comment #13 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Rest of the code doesn't use (In reply to comment #11)

Hi Dobrica,

So maybe he patches are breaking things in other places now? Is there a way to check/make sure?

I did that before I started modifing code. git grep day_month_closed_days is quite useful to get all places in code without need to write elaborate grep statements. To be honest, I switched to git grep from my own hand-crafted greps in this bug, so I'm sharing my excitement here :-) According to our coding guidelines, other code shouldn't access values directly in Koha namespace anyway, which is probably only reason why we are mocking test data within module itself (which was strange to me at first, and is probably reason why I broke it). I cleaned up two patches which to include correct mocking of test data. -- 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=8418 --- Comment #15 from Dobrica Pavlinusic <dpavlin@rot13.org> --- It shouldn't. This module respect them :-) -- 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=8418 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10839|0 |1 is obsolete| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 11026 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11026&action=edit Bug 8418 - Koha::Calendar is_holiday ignores repeating holidays This is simple fix since day_month_closed_days hash has wrong arguments order and it's not used outside Koha::Calenar code. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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=8418 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |koha.sekjal@gmail.com --- Comment #18 from Ian Walls <koha.sekjal@gmail.com> --- Patch 1: correct 'to' value of truncate function is 'day', not 'days', but the patch no longer applies. Patch 2: looks to be removing 'singleholiday' and 'exceptionholiday' references in favour of 'specialholiday'. Also, patch does not apply anymore. Patch 3: swaps around hash from 'month' => 'day' to 'day' => 'month', which seems backwards as far as granularity goes. I remember us switching this for a reason... was that reason half-baked? Or did we just not switch it in enough places? -- 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=8418 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11026|0 |1 is obsolete| | --- Comment #20 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 11611 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11611&action=edit Proposed Patch The repeating holidays bug is still there. It should be corrected if the structure is setup correctly on load from the database as the TEST bersion of the object does. This fixes it in a single place unlike the previous which addressed it in the access method and meant that the test mock had to change to conform -- 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=8418 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11025|0 |1 is obsolete| | -- 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=8418 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off --- Comment #22 from Chris Cormack <chris@bigballofwax.co.nz> --- perl t/Calendar.t 2012-08-15 11:28:50 chrisc pts/26 1..21 ok 1 - use Koha::Calendar; ok 2 - use C4::Calendar; ok 3 - Calendar class returned isa Koha::Calendar ok 4 - Sunday is a closed day ok 5 - Monday is not a closed day ok 6 - month/day closed day test ok 7 - special closed day test ok 8 - open day test ok 9 - addDate skips closed Sunday ok 10 - Negative call to addDate ok 11 - days_between calculates correctly ok 12 - Simple Single Day Add (Calendar) ok 13 - Add 7 days Calendar mode ok 14 - Add 7 days Datedue mode ok 15 - Add 7 days Days mode ok 16 - test larger intervals ok 17 - test positive intervals ok 18 - test parameter order not relevant ok 19 - days_between calculates correctly ok 20 - holiday correctly recognized ok 21 - multiple holidays correctly recognized -- 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=8418 L. Bareño <leonardobareno@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |leonardobareno@gmail.com --- Comment #24 from L. Bareño <leonardobareno@gmail.com> --- 1) Current code in ./Koha/Calendar.pm is ignoring single holidays (those whose special_holidays.isexception == 0) (line numbers and code from master at this message's date) ./Koha/Calendar.pm line 68: - $special->execute( $branch, 1 ); + $special->execute( $branch, 0 ); 2) exception_holidays are not holidays, and (please you people tell me if the following interpretation I'm doing is correct) these must disable any other repeatable or single holiday. lines 141-165 (suggested code): sub is_holiday { my ( $self, $dt ) = @_; if ( $self->{exception_holidays}->contains($dt) ) { return 0; } my $dow = $dt->day_of_week; if ( $dow == 7 ) { $dow = 0; } if ( $self->{weekly_closed_days}->[$dow] == 1 ) { return 1; } $dt->truncate( to => 'day' ); my $day = $dt->day; my $month = $dt->month; if ( exists $self->{day_month_closed_days}->{$month}->{$day} ) { return 1; } if ( $self->{single_holidays}->contains($dt) ) { return 1; } # damn have to go to work after all return 0; } -- 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=8418 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com QA Contact| |mtj@kohaaloha.com --- Comment #26 from Mason James <mtj@kohaaloha.com> ---

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

patch looks good, passing QA... $ koha-qa.pl testing 1 commit(s) (applied to commit 60790d0) * 1230d06 Bug 8418: load repeating holidays hash correctly Koha/Calendar.pm * Koha/Calendar.pm OK -- 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=8418 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master Version|master |rel_3_8 --- Comment #27 from Paul Poulain <paul.poulain@biblibre.com> --- Patch pushed to master -- 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=8418 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- *** Bug 6398 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.