[Bug 38357] New: When adding new holidays Koha sometimes copies same holidays to other librarys
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Bug ID: 38357 Summary: When adding new holidays Koha sometimes copies same holidays to other librarys Change sponsored?: --- Product: Koha Version: 24.05 Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Holidays Assignee: koha-bugs@lists.koha-community.org Reporter: piia.semenoff@ouka.fi QA Contact: testopia@bugs.koha-community.org Created attachment 173980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173980&action=edit Holidays module (calendar) I noticed that when I add unique holiday to a library and repeat adding different holidays to two other librarys, the third library will have random unique holidays even if it should't have. It's easier to explain with pictures so I put them in an PDF and added it as an attachment. But here's what I did: 1. I added one week as unique holidays to Tyrnava library from 2025.03.03.to 2025.03.09. All went well. 2. I added one week as unique holidays to Tupos library from 2025.03.10 to 2025.03.16. All went well. 3. I added one week as unique holidays to Tuira library from 2025.03.17 to 2025.03.23. This didin't go so well: Tuira had unique holidays two weeks, but I added only one week. 4. I added one week as unique holidays to Temmes library from 2025.03.03 to 2025.03.09. This didin't go so well either: After saving Temmes had unique holidays three weeks, but I added only one week. 5. I added one week as unique holidays to Taivalkoski library from 2025.03.24 to 2025.03.30. This time after saving Taivalkoski's holidays, library had unique holidays two weeks (in the beginning of March and in the end of March), but I added only one week. These above are easy to fix because you see them at once, but we had also unique holidays that appeared on the other months (for example christmas-holidays appeared to librarys that are open on christmastime and don't need holiday-markings on their calendar.) so those extra-holidays could be hard to catch. We don't have this problem in Koha version 23.11, but in the version 24.05 it is going to be a problem. -- 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=38357 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emmi.takkinen@koha-suomi.fi --- Comment #1 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Adding my test results to this. I strongly believe this has something to do with plack, since I'm unable to reproduce this on my own test environments where I don't have plack running. We tested this with Piia on our upcoming upgrades test environment and this seems to follow a certain pattern: adding holidays to first and second library works as expected. Then adding holidays to the third one and all the following libraries fail. After restarting plack everything works fine for the first two libraries and then it fails again. Piia also tested this on sandbox and was able reproduce this also there. We had same kind of problem with version 17.05, but it didn't happen again after we upgraded to version 22.11. I'm not sure if this followed same pattern then but symptoms were the same. -- 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=38357 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32048 CC| |jonathan.druart@gmail.com Severity|normal |major Version|24.05 |unspecified -- 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=38357 Jonathan Druart <jonathan.druart@gmail.com> 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=38357 --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 174065 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174065&action=edit Bug 38357: Prevent holidays to be wrongly added This patch resets @holiday_list to an empty array, to prevent holidays to be accumulated in worker's memory. If you added single holidays to library A then others to library B, holidays from A were also added to B. Test plan: Go to /cgi-bin/koha/tools/holidays.pl Define the holidays for Centerville Select a date To: another date (pick only 2 days to ease testing) Set a title "cpl" Tick "Holidays on a range" Save Define the holidays for Fairview Select *another* date To: another date (pick only 2 days to ease testing) Set a title "fpl" Tick "Holidays on a range" Save => Without this patch Fairview has 4 days of holidays instead of 2 => With this patch applied the dates you selected are considered holidays for Fairview -- 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=38357 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@gmail.com |ity.org | --- Comment #3 from Jonathan Druart <jonathan.druart@gmail.com> --- I haven't tried on 23.11 but I didn't find what could have caused this in the git log. -- 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=38357 --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Emmi Takkinen from comment #1)
Adding my test results to this. I strongly believe this has something to do with plack, since I'm unable to reproduce this on my own test environments where I don't have plack running. We tested this with Piia on our upcoming upgrades test environment and this seems to follow a certain pattern: adding holidays to first and second library works as expected. Then adding holidays to the third one and all the following libraries fail. After restarting plack everything works fine for the first two libraries and then it fails again. Piia also tested this on sandbox and was able reproduce this also there.
I get the wrong behaviour for the second library. Can you please try the patch and see if it fixes the problem you described? Maybe there is still something else to fix. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #5 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Jonathan Druart from comment #4)
(In reply to Emmi Takkinen from comment #1)
Adding my test results to this. I strongly believe this has something to do with plack, since I'm unable to reproduce this on my own test environments where I don't have plack running. We tested this with Piia on our upcoming upgrades test environment and this seems to follow a certain pattern: adding holidays to first and second library works as expected. Then adding holidays to the third one and all the following libraries fail. After restarting plack everything works fine for the first two libraries and then it fails again. Piia also tested this on sandbox and was able reproduce this also there.
I get the wrong behaviour for the second library. Can you please try the patch and see if it fixes the problem you described? Maybe there is still something else to fix.
Piia tested this on 24.05.01 and it works! She didn't find any other problems so this probably can be signed off. Thank you for quick fix :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #6 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Tested this also in main and calendar works as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #174065|0 |1 is obsolete| | --- Comment #7 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 174110 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174110&action=edit Bug 38357: Prevent holidays to be wrongly added This patch resets @holiday_list to an empty array, to prevent holidays to be accumulated in worker's memory. If you added single holidays to library A then others to library B, holidays from A were also added to B. Test plan: Go to /cgi-bin/koha/tools/holidays.pl Define the holidays for Centerville Select a date To: another date (pick only 2 days to ease testing) Set a title "cpl" Tick "Holidays on a range" Save Define the holidays for Fairview Select *another* date To: another date (pick only 2 days to ease testing) Set a title "fpl" Tick "Holidays on a range" Save => Without this patch Fairview has 4 days of holidays instead of 2 => With this patch applied the dates you selected are considered holidays for Fairview Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=38357 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #174110|0 |1 is obsolete| | --- Comment #8 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 174435 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174435&action=edit Bug 38357: Prevent holidays to be wrongly added This patch resets @holiday_list to an empty array, to prevent holidays to be accumulated in worker's memory. If you added single holidays to library A then others to library B, holidays from A were also added to B. Test plan: Go to /cgi-bin/koha/tools/holidays.pl Define the holidays for Centerville Select a date To: another date (pick only 2 days to ease testing) Set a title "cpl" Tick "Holidays on a range" Save Define the holidays for Fairview Select *another* date To: another date (pick only 2 days to ease testing) Set a title "fpl" Tick "Holidays on a range" Save => Without this patch Fairview has 4 days of holidays instead of 2 => With this patch applied the dates you selected are considered holidays for Fairview Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |emily.lamancusa@montgomeryc | |ountymd.gov Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.11.00 |24.11.00,24.05.06 released in| | Status|Pushed to main |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #10 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Backported to 24.05.x for upcoming 24.05.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.11.00,24.05.06 |24.11.00,24.05.06,23.11.11 released in| | Status|Pushed to oldstable |Pushed to oldoldstable CC| |fridolin.somers@biblibre.co | |m --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jesse@bywatersolutions.com Resolution|--- |FIXED Status|Pushed to oldoldstable |RESOLVED --- Comment #12 from Jesse Maseto <jesse@bywatersolutions.com> --- Not pushed to LTS. Marked Resolved. If you feel this should be in LTS please reply with your reason. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Is there a reason not to get rid of all the `our` variable definitions? I'm not sure we know exactly why it is being used. We only know those variables persist in memory in Plack mode (and other persistent runtime environments). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jesse Maseto from comment #12)
Not pushed to LTS. Marked Resolved.
If you feel this should be in LTS please reply with your reason.
I feel this should be backported if possible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate Resolution|FIXED |--- Status|RESOLVED |REOPENED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=38357 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.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=38357 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to oldoldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #15 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #13)
Is there a reason not to get rid of all the `our` variable definitions? I'm not sure we know exactly why it is being used.
We only know those variables persist in memory in Plack mode (and other persistent runtime environments).
It has a global scope, and is available from the subroutines. When switching to plack we (lazily) switched some "my" to "our" to just make things worked. It can be fixed, but it's not simply a s/^our/my/g substitution. But yes, it must be done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #16 from Jonathan Druart <jonathan.druart@gmail.com> --- ``` my $global; foo(); foo(); warn $global; sub foo { $global++; } ``` => Variable "$global" is not available Replace with `our` => 2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 --- Comment #17 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #15)
(In reply to Tomás Cohen Arazi (tcohen) from comment #13)
Is there a reason not to get rid of all the `our` variable definitions? I'm not sure we know exactly why it is being used.
We only know those variables persist in memory in Plack mode (and other persistent runtime environments).
It has a global scope, and is available from the subroutines. When switching to plack we (lazily) switched some "my" to "our" to just make things worked.
It can be fixed, but it's not simply a s/^our/my/g substitution.
But yes, it must be done.
Yeah, I know. I asked on Mattermost too, if someone remembered some caching mechanism would need to be re-done (i.e. if this was somehow a feature). I think we all agree this needs to go away. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 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=38357 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldoldstable |Pushed to oldoldoldstable --- Comment #18 from Jesse Maseto <jesse@bywatersolutions.com> --- Pushed to 22.11.x. Will be in next point release. 22.11.25 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38357 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to oldoldoldstable |RESOLVED CC| |wainuiwitikapark@catalyst.n | |et.nz Version(s)|24.11.00,24.05.06,23.11.11 |24.11.00,24.05.06,23.11.11, released in| |22.11.25 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org