[Bug 12801] New: Koha::Calendar is caching holidays for the wrong branch!
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Bug ID: 12801 Summary: Koha::Calendar is caching holidays for the wrong branch! Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org If you try to use Koha::Calendar twice, the second instance will use the first instances special holidays! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 31052 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31052&action=edit Bug 12801 - Unit Test -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 31053 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31053&action=edit Bug 12801 - Remove caching This patch removes the special holiday caching which fixes the bug, but causes a different unit test to fail. I do not no if the failing unit test is valid. If it is, this patch will cause a regression. If it is not valid, the test should be removed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Is there a place where 2 instances are created with different branches? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11112 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- The single_holidays has been cached by bug 11112. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #3)
Is there a place where 2 instances are created with different branches?
I discovered this while attempting to use is_holiday in a new feature which would use an instance for each branch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- I am convinced we should keep the cache. You will get performance issues if you remove it. Maybe another way to fix this could be to cache using a hash: branchcode_1 => $single_holidays_for_branch_1 branchcode_2 => $single_holidays_for_branch_2 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Architecture, internals, |Holidays |and plumbing | Assignee|gmcharlt@gmail.com |koha-bugs@lists.koha-commun | |ity.org -- 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=12801 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Would you be able to submit a replacement for my second patch? (In reply to Jonathan Druart from comment #6)
I am convinced we should keep the cache. You will get performance issues if you remove it. Maybe another way to fix this could be to cache using a hash: branchcode_1 => $single_holidays_for_branch_1 branchcode_2 => $single_holidays_for_branch_2
-- 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=12801 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 31056 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31056&action=edit [ALT] Bug 12801: Use the branchcode to cache holidays -- 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=12801 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Kyle M Hall from comment #7)
Would you be able to submit a replacement for my second patch?
Something like that? Note: I didn't test this patch! -- 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=12801 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31053|0 |1 is obsolete| | Attachment #31056|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 31075 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31075&action=edit [SIGNED-OFF] Bug 12801: Use the branchcode to cache holidays 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=12801 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Looks good. Thanks! (In reply to Jonathan Druart from comment #9)
(In reply to Kyle M Hall from comment #7)
Would you be able to submit a replacement for my second patch?
Something like that? Note: I didn't test this patch!
-- 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=12801 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=12801 Kyle M Hall <kyle@bywatersolutions.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.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12803 -- 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=12801 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de 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=12801 Brendan Gallagher <brendan@bywatersolutions.com> 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=12801 --- Comment #12 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 32514 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32514&action=edit Bug 10126: (qa followup) fix tests It seems the removal of global variables changes the behaviour of Test::MockModule, and it now expects the namespace in front of the statically called method. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31052|0 |1 is obsolete| | --- Comment #13 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 32515 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32515&action=edit Bug 12801 - Unit Test Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 --- Comment #14 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 32516 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32516&action=edit Bug 12801: Use the branchcode to cache holidays Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31075|0 |1 is obsolete| | CC| |brendan@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32514|0 |1 is obsolete| | CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Kyle and Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |olli-antti.kivilahti@jns.fi --- Comment #16 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Thanks for fixing this! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org