https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |42206 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42206 [Bug 42206] Add REST endpoint GET /libraries/{library_id}/closed_dates -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196959 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196959&action=edit Bug 42310: Normalize calendar tables Replace the two overloaded tables (repeatable_holidays, special_holidays) with four purpose-specific tables: - library_weekly_closures: weekly closed days (e.g. every Sunday) - library_repeating_closures: annual repeating closures (e.g. Dec 25) - library_single_closures: one-off closures with a DATE column - library_closure_exceptions: open overrides with a DATE column All use library_id FK and follow current community standards. It points to API usage in a short term so it follows the API naming conventions. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196961&action=edit Bug 42310: Refactor Koha::Calendar to use Koha::Calendar::* classes Replace raw DBI queries in _init() and _holidays() with Koha::Calendar::WeeklyClosures, RepeatingClosures, SingleClosures, and Exceptions searches. The internal data structures remain identical (weekly_closed_days array, day_month_closed_days hash, _holidays hash with YYYYMMDD keys) so all consumers (is_holiday, addDuration, next_open_days) continue to work unchanged. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196963&action=edit Bug 42310: (follow-up) Remove DBIC schema files for dropped tables Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196965 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196965&action=edit Bug 42310: (follow-up) Add CRUD methods to Koha::Calendar, delegate C4::Calendar Add to Koha::Calendar: add_weekly_closure, add_repeating_closure, add_single_closure, add_exception, delete_weekly_closure, delete_repeating_closure, delete_single_closure, delete_exception, copy_to, _clear_cache C4::Calendar write methods now delegate to Koha::Calendar instead of using raw DBI. No old table references remain in C4::Calendar. Tests in t/db_dependent/Koha/Calendar.t cover all CRUD operations: create, verify via is_holiday, delete, copy_to (including idempotency). Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 David Nind <david@davidnind.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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196960|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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196962|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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196964|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=42310 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 196978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196978&action=edit Bug 42310: Normalize calendar tables Replace the two overloaded tables (repeatable_holidays, special_holidays) with four purpose-specific tables: - library_weekly_closures: weekly closed days (e.g. every Sunday) - library_repeating_closures: annual repeating closures (e.g. Dec 25) - library_single_closures: one-off closures with a DATE column - library_closure_exceptions: open overrides with a DATE column All use library_id FK and follow current community standards. It points to API usage in a short term so it follows the API naming conventions. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #10 from David Nind <david@davidnind.com> --- Created attachment 196980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196980&action=edit Bug 42310: Refactor Koha::Calendar to use Koha::Calendar::* classes Replace raw DBI queries in _init() and _holidays() with Koha::Calendar::WeeklyClosures, RepeatingClosures, SingleClosures, and Exceptions searches. The internal data structures remain identical (weekly_closed_days array, day_month_closed_days hash, _holidays hash with YYYYMMDD keys) so all consumers (is_holiday, addDuration, next_open_days) continue to work unchanged. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 196982 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196982&action=edit Bug 42310: (follow-up) Remove DBIC schema files for dropped tables Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 196984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196984&action=edit Bug 42310: (follow-up) Add CRUD methods to Koha::Calendar, delegate C4::Calendar Add to Koha::Calendar: add_weekly_closure, add_repeating_closure, add_single_closure, add_exception, delete_weekly_closure, delete_repeating_closure, delete_single_closure, delete_exception, copy_to, _clear_cache C4::Calendar write methods now delegate to Koha::Calendar instead of using raw DBI. No old table references remain in C4::Calendar. Tests in t/db_dependent/Koha/Calendar.t cover all CRUD operations: create, verify via is_holiday, delete, copy_to (including idempotency). Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |kyle@bywatersolutions.com, | |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196978|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=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196980|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=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196982|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=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196984|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=42310 --- Comment #17 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197021&action=edit Bug 42310: Add Koha::Calendar::* Object classes Four Koha::Object + Koha::Objects pairs following the Koha::City(ies) pattern: - Koha::Calendar::WeeklyClosure(s) -> library_weekly_closures - Koha::Calendar::RepeatingClosure(s) -> library_repeating_closures - Koha::Calendar::SingleClosure(s) -> library_single_closures - Koha::Calendar::Exception(s) -> library_closure_exceptions DBIC schema files to be generated via the 'dbic' alias in KTD after the database migration runs. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #19 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197023 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197023&action=edit Bug 42310: (follow-up) Add missing POD Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197025 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197025&action=edit Bug 42310: (follow-up) Adapt Calendar tests to new schema Replace all references to the dropped tables (RepeatableHoliday, SpecialHoliday resultsets and raw DBI inserts) with Koha::Object classes: Koha::Calendar::WeeklyClosure, RepeatingClosure, SingleClosure, and Exception. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #23 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197027 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197027&action=edit Bug 42310: (follow-up) Bake cache invalidation into Koha::Object classes When using the new Koha::Calendar::* classes for writing the CRUD API endpoints I noticed that in my attempt to keep C4::Calendar similar to what it was, I ended up with cache invalidation not always done in the right place. The holidays cache (keyed by {branchcode}_holidays) stores dates and their open/closed status, not titles or descriptions. This means only inserts and deletes need to invalidate it — metadata updates (title/description) do not. Following the pattern established by Koha::AuthorisedValue and Koha::ItemType, I added store() and delete() overrides to Koha::Calendar::SingleClosure and Koha::Calendar::Exception: - store() flushes the cache only on insert (!in_storage), not on updates, since the cache doesn't store titles or descriptions. - delete() always flushes, since removing a closure changes the cached set. This allowed removing the now-redundant _clear_cache calls from Koha::Calendar::add_single_closure, add_exception, delete_single_closure, delete_exception, and from C4::Calendar::ModSingleholiday and ModExceptionholiday. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 David Nind <david@davidnind.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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197021|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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197023|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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197025|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=42310 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197027|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=42310 --- Comment #26 from David Nind <david@davidnind.com> --- Created attachment 197031 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197031&action=edit Bug 42310: Add Koha::Calendar::* Object classes Four Koha::Object + Koha::Objects pairs following the Koha::City(ies) pattern: - Koha::Calendar::WeeklyClosure(s) -> library_weekly_closures - Koha::Calendar::RepeatingClosure(s) -> library_repeating_closures - Koha::Calendar::SingleClosure(s) -> library_single_closures - Koha::Calendar::Exception(s) -> library_closure_exceptions DBIC schema files to be generated via the 'dbic' alias in KTD after the database migration runs. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #28 from David Nind <david@davidnind.com> --- Created attachment 197033 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197033&action=edit Bug 42310: (follow-up) Add missing POD Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #30 from David Nind <david@davidnind.com> --- Created attachment 197035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197035&action=edit Bug 42310: (follow-up) Adapt Calendar tests to new schema Replace all references to the dropped tables (RepeatableHoliday, SpecialHoliday resultsets and raw DBI inserts) with Koha::Object classes: Koha::Calendar::WeeklyClosure, RepeatingClosure, SingleClosure, and Exception. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #32 from David Nind <david@davidnind.com> --- Created attachment 197037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197037&action=edit Bug 42310: (follow-up) Bake cache invalidation into Koha::Object classes When using the new Koha::Calendar::* classes for writing the CRUD API endpoints I noticed that in my attempt to keep C4::Calendar similar to what it was, I ended up with cache invalidation not always done in the right place. The holidays cache (keyed by {branchcode}_holidays) stores dates and their open/closed status, not titles or descriptions. This means only inserts and deletes need to invalidate it — metadata updates (title/description) do not. Following the pattern established by Koha::AuthorisedValue and Koha::ItemType, I added store() and delete() overrides to Koha::Calendar::SingleClosure and Koha::Calendar::Exception: - store() flushes the cache only on insert (!in_storage), not on updates, since the cache doesn't store titles or descriptions. - delete() always flushes, since removing a closure changes the cached set. This allowed removing the now-redundant _clear_cache calls from Koha::Calendar::add_single_closure, add_exception, delete_single_closure, delete_exception, and from C4::Calendar::ModSingleholiday and ModExceptionholiday. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42334 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42334 [Bug 42334] Introduce Koha::Calendar::DateCalculator for date arithmetic -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197030|0 |1 is obsolete| | Attachment #197031|0 |1 is obsolete| | Attachment #197032|0 |1 is obsolete| | Attachment #197033|0 |1 is obsolete| | Attachment #197034|0 |1 is obsolete| | Attachment #197035|0 |1 is obsolete| | Attachment #197036|0 |1 is obsolete| | Attachment #197037|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=42310 --- Comment #35 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197990&action=edit Bug 42310: Add Koha::Calendar::* Object classes Four Koha::Object + Koha::Objects pairs following the Koha::City(ies) pattern: - Koha::Calendar::WeeklyClosure(s) -> library_weekly_closures - Koha::Calendar::RepeatingClosure(s) -> library_repeating_closures - Koha::Calendar::SingleClosure(s) -> library_single_closures - Koha::Calendar::Exception(s) -> library_closure_exceptions DBIC schema files to be generated via the 'dbic' alias in KTD after the database migration runs. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #37 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197992&action=edit Bug 42310: (follow-up) Add missing POD Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #39 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197994 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197994&action=edit Bug 42310: (follow-up) Adapt Calendar tests to new schema Replace all references to the dropped tables (RepeatableHoliday, SpecialHoliday resultsets and raw DBI inserts) with Koha::Object classes: Koha::Calendar::WeeklyClosure, RepeatingClosure, SingleClosure, and Exception. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #41 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197996 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197996&action=edit Bug 42310: (follow-up) Bake cache invalidation into Koha::Object classes When using the new Koha::Calendar::* classes for writing the CRUD API endpoints I noticed that in my attempt to keep C4::Calendar similar to what it was, I ended up with cache invalidation not always done in the right place. The holidays cache (keyed by {branchcode}_holidays) stores dates and their open/closed status, not titles or descriptions. This means only inserts and deletes need to invalidate it — metadata updates (title/description) do not. Following the pattern established by Koha::AuthorisedValue and Koha::ItemType, I added store() and delete() overrides to Koha::Calendar::SingleClosure and Koha::Calendar::Exception: - store() flushes the cache only on insert (!in_storage), not on updates, since the cache doesn't store titles or descriptions. - delete() always flushes, since removing a closure changes the cached set. This allowed removing the now-redundant _clear_cache calls from Koha::Calendar::add_single_closure, add_exception, delete_single_closure, delete_exception, and from C4::Calendar::ModSingleholiday and ModExceptionholiday. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #43 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197998 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197998&action=edit Bug 42310: (QA follow-up) Harden Koha::Calendar->copy_to Wrap the body in schema->txn_do so a failure partway through does not leave the target library partially populated. Flush the target's _holidays cache after the copy, and document the behaviour in POD: weekly and repeating closures are always copied, but single closures and exceptions are filtered to today-or-later dates by design. Test plan: 1) prove t/db_dependent/Koha/Calendar.t 2) Confirm existing copy_to tests still pass. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #45 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198000 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198000&action=edit Bug 42310: (QA follow-up) Add closed_dates_in_range to Koha::Calendar Introduces a public Koha::Calendar->closed_dates_in_range($from_dt, $to_dt) method that returns an arrayref of ISO date strings representing every closed day in the inclusive range, honouring weekly, annual repeating, single and exception (open-override) closures. This replaces callers that were reaching into Koha::Calendar private state (_holidays, weekly_closed_days, day_month_closed_days) to reconstruct the same logic. A follow-up under Bug 42206 converts the REST controller; further callers can be migrated incrementally. Test plan: 1. prove t/db_dependent/Koha/Calendar.t 2. Verify the new 'closed_dates_in_range' subtest passes -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #47 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198002&action=edit Bug 42310: (follow-up) Move Koha::Calendar tree under Koha::Library::Calendar Koha::Calendar is, by construction, a per-library object — it croaks without a branchcode argument and every method is branch-scoped. The top-level namespace hides that. Move the whole tree under Koha::Library::Calendar to match the actual semantics, the existing /libraries/{library_id}/calendar REST routes, and the established Koha::Library::* convention (Group, Hour, FloatLimit, OverDriveInfo). This commit: - renames the Koha::Calendar(::*) tree to Koha::Library::Calendar(::*) - updates package declarations, POD and internal 'use' statements within the moved tree - updates the four DBIC Schema Result overrides (koha_object_class / koha_objects_class) to the new namespace - updates callers inside C4::* modules (Calendar, Circulation, Reserves, Overdues, HoldsQueue), Koha::{Patron,Hold,Checkouts, CurbsidePickup}, circ/returns.pl, the installer and cronjobs - updates the 42310-owned tests (Calendar tests, HoldsQueue.t, Holds/WaitingReserves.t, Holidays.t, Koha/CurbsidePickups.t, Reserves/CancelExpiredReserves.t and the libraries.t CRUD block) Koha::Library->calendar, the REST controllers and the 42311 tests are updated in separate follow-up commits under their respective bugs. Test plan: - prove t/db_dependent/Koha/Library/Calendar.t - prove t/db_dependent/Calendar.t - prove t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t - prove t/db_dependent/Holds/WaitingReserves.t - prove t/db_dependent/Reserves/CancelExpiredReserves.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Bug 42310 depends on bug 42206, which changed state. Bug 42206 Summary: Add REST endpoint GET /libraries/{library_id}/closed_dates https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42206 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #49 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203151 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203151&action=edit Bug 42310: Normalize calendar tables Replace the two overloaded tables (repeatable_holidays, special_holidays) with four purpose-specific tables: - library_weekly_closures: weekly closed days (e.g. every Sunday) - library_repeating_closures: annual repeating closures (e.g. Dec 25) - library_single_closures: one-off closures with a DATE column - library_closure_exceptions: open overrides with a DATE column All use library_id FK and follow current community standards. It points to API usage in a short term so it follows the API naming conventions. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #51 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203153 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203153&action=edit Bug 42310: Refactor Koha::Calendar to use Koha::Calendar::* classes Replace raw DBI queries in _init() and _holidays() with Koha::Calendar::WeeklyClosures, RepeatingClosures, SingleClosures, and Exceptions searches. The internal data structures remain identical (weekly_closed_days array, day_month_closed_days hash, _holidays hash with YYYYMMDD keys) so all consumers (is_holiday, addDuration, next_open_days) continue to work unchanged. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #53 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203155&action=edit Bug 42310: DBIC schema update Regenerated via dbic --force after the library_weekly_closures, library_repeating_closures, library_single_closures, and library_closure_exceptions tables replaced repeatable_holidays and special_holidays. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #55 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203157&action=edit Bug 42310: (follow-up) Adapt Calendar tests to new schema Replace all references to the dropped tables (RepeatableHoliday, SpecialHoliday resultsets and raw DBI inserts) with Koha::Object classes: Koha::Calendar::WeeklyClosure, RepeatingClosure, SingleClosure, and Exception. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #57 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203159 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203159&action=edit Bug 42310: (follow-up) Bake cache invalidation into Koha::Object classes When using the new Koha::Calendar::* classes for writing the CRUD API endpoints I noticed that in my attempt to keep C4::Calendar similar to what it was, I ended up with cache invalidation not always done in the right place. The holidays cache (keyed by {branchcode}_holidays) stores dates and their open/closed status, not titles or descriptions. This means only inserts and deletes need to invalidate it — metadata updates (title/description) do not. Following the pattern established by Koha::AuthorisedValue and Koha::ItemType, I added store() and delete() overrides to Koha::Calendar::SingleClosure and Koha::Calendar::Exception: - store() flushes the cache only on insert (!in_storage), not on updates, since the cache doesn't store titles or descriptions. - delete() always flushes, since removing a closure changes the cached set. This allowed removing the now-redundant _clear_cache calls from Koha::Calendar::add_single_closure, add_exception, delete_single_closure, delete_exception, and from C4::Calendar::ModSingleholiday and ModExceptionholiday. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #59 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203161 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203161&action=edit Bug 42310: (QA follow-up) Harden Koha::Calendar->copy_to Wrap the body in schema->txn_do so a failure partway through does not leave the target library partially populated. Flush the target's _holidays cache after the copy, and document the behaviour in POD: weekly and repeating closures are always copied, but single closures and exceptions are filtered to today-or-later dates by design. Test plan: 1) prove t/db_dependent/Koha/Calendar.t 2) Confirm existing copy_to tests still pass. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #61 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203163 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203163&action=edit Bug 42310: (QA follow-up) Add closed_dates_in_range to Koha::Calendar Introduces a public Koha::Calendar->closed_dates_in_range($from_dt, $to_dt) method that returns an arrayref of ISO date strings representing every closed day in the inclusive range, honouring weekly, annual repeating, single and exception (open-override) closures. This replaces callers that were reaching into Koha::Calendar private state (_holidays, weekly_closed_days, day_month_closed_days) to reconstruct the same logic. A follow-up under Bug 42206 converts the REST controller; further callers can be migrated incrementally. Test plan: 1. prove t/db_dependent/Koha/Calendar.t 2. Verify the new 'closed_dates_in_range' subtest passes -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #63 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203165&action=edit Bug 42310: (follow-up) Update Holidays.t for normalized calendar tables The normalization commit dropped `repeatable_holidays` and `special_holidays` but t/db_dependent/Holidays.t still referenced them directly via raw SQL, so the test blew up on the first `DELETE FROM special_holidays` with "Table doesn't exist". The earlier namespace follow-up touched this file but only renamed Koha::Calendar references, missing the SQL. Replace the legacy table references with the four new normalized tables: repeatable_holidays (weekday set) -> library_weekly_closures repeatable_holidays (weekday NULL) -> library_repeating_closures special_holidays (isexception=0) -> library_single_closures special_holidays (isexception=1) -> library_closure_exceptions - Two fixture-cleanup DELETEs now wipe all four new tables. - The copy_to_branch 'no duplicates' check, previously two raw-SQL queries against the old two tables, becomes four queries against the new tables, keyed on each table's natural unique key (library_id+weekday / library_id+day+month / library_id+date). - Plan for the copy_to_branch subtest bumps from 8 to 10 to cover the two extra duplicate-check assertions. Test plan: 1. prove t/db_dependent/Holidays.t 2. All 15 top-level tests pass, including the 10 subtest assertions in copy_to_branch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #65 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203167 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203167&action=edit Bug 42310: (QA follow-up) Restore weekday validation on weekly closures C4::Calendar::insert_week_day_holiday used to croak "Invalid weekday $weekday" unless $weekday =~ m/^[0-6]$/. That guard was dropped when the insert delegated to Koha::Library::Calendar::WeeklyClosure, and nothing replaced it - an out-of-range or undef weekday now persists silently to library_weekly_closures instead of failing at the API boundary. Add the check to WeeklyClosure->store() rather than the legacy wrapper, so every caller (C4::Calendar, Koha::Library::Calendar->add_weekly_closure, and any future REST/admin caller) gets it, not just the old entry point. Test plan: 1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t t/db_dependent/Holds/WaitingReserves.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #67 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203169 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203169&action=edit Bug 42310: (QA follow-up) Refresh in-process closure state on add/delete_weekly_closure and add/delete_repeating_closure add_weekly_closure/delete_weekly_closure and add_repeating_closure/ delete_repeating_closure returned $self without refreshing $self->{weekly_closed_days}/{day_month_closed_days}, so a calendar object would keep reporting the pre-change state to is_holiday (and everything built on it - days_forward, next_open_days, etc.) until it was re-instantiated. This is distinct from the _holidays Koha::Caches entry, which single/exception closures already flush correctly on store/delete. Call _init to rebuild both structures from the database after each add/delete, so the same object reflects the change immediately. Test plan: 1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t t/db_dependent/Holds/WaitingReserves.t t/db_dependent/Koha/CurbsidePickups.t t/db_dependent/Reserves/CancelExpiredReserves.t t/db_dependent/api/v1/libraries.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 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=42310 --- Comment #69 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203289&action=edit Bug 42310: Normalize calendar tables Replace the two overloaded tables (repeatable_holidays, special_holidays) with four purpose-specific tables: - library_weekly_closures: weekly closed days (e.g. every Sunday) - library_repeating_closures: annual repeating closures (e.g. Dec 25) - library_single_closures: one-off closures with a DATE column - library_closure_exceptions: open overrides with a DATE column All use library_id FK and follow current community standards. It points to API usage in a short term so it follows the API naming conventions. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #71 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203291 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203291&action=edit Bug 42310: Refactor Koha::Calendar to use Koha::Calendar::* classes Replace raw DBI queries in _init() and _holidays() with Koha::Calendar::WeeklyClosures, RepeatingClosures, SingleClosures, and Exceptions searches. The internal data structures remain identical (weekly_closed_days array, day_month_closed_days hash, _holidays hash with YYYYMMDD keys) so all consumers (is_holiday, addDuration, next_open_days) continue to work unchanged. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #73 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203293 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203293&action=edit Bug 42310: DBIC schema update Regenerated via dbic --force after the library_weekly_closures, library_repeating_closures, library_single_closures, and library_closure_exceptions tables replaced repeatable_holidays and special_holidays. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #75 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203295 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203295&action=edit Bug 42310: (follow-up) Adapt Calendar tests to new schema Replace all references to the dropped tables (RepeatableHoliday, SpecialHoliday resultsets and raw DBI inserts) with Koha::Object classes: Koha::Calendar::WeeklyClosure, RepeatingClosure, SingleClosure, and Exception. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #77 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203297 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203297&action=edit Bug 42310: (follow-up) Bake cache invalidation into Koha::Object classes When using the new Koha::Calendar::* classes for writing the CRUD API endpoints I noticed that in my attempt to keep C4::Calendar similar to what it was, I ended up with cache invalidation not always done in the right place. The holidays cache (keyed by {branchcode}_holidays) stores dates and their open/closed status, not titles or descriptions. This means only inserts and deletes need to invalidate it — metadata updates (title/description) do not. Following the pattern established by Koha::AuthorisedValue and Koha::ItemType, I added store() and delete() overrides to Koha::Calendar::SingleClosure and Koha::Calendar::Exception: - store() flushes the cache only on insert (!in_storage), not on updates, since the cache doesn't store titles or descriptions. - delete() always flushes, since removing a closure changes the cached set. This allowed removing the now-redundant _clear_cache calls from Koha::Calendar::add_single_closure, add_exception, delete_single_closure, delete_exception, and from C4::Calendar::ModSingleholiday and ModExceptionholiday. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #79 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203299 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203299&action=edit Bug 42310: (QA follow-up) Harden Koha::Calendar->copy_to Wrap the body in schema->txn_do so a failure partway through does not leave the target library partially populated. Flush the target's _holidays cache after the copy, and document the behaviour in POD: weekly and repeating closures are always copied, but single closures and exceptions are filtered to today-or-later dates by design. Test plan: 1) prove t/db_dependent/Koha/Calendar.t 2) Confirm existing copy_to tests still pass. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #81 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203301 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203301&action=edit Bug 42310: (QA follow-up) Add closed_dates_in_range to Koha::Calendar Introduces a public Koha::Calendar->closed_dates_in_range($from_dt, $to_dt) method that returns an arrayref of ISO date strings representing every closed day in the inclusive range, honouring weekly, annual repeating, single and exception (open-override) closures. This replaces callers that were reaching into Koha::Calendar private state (_holidays, weekly_closed_days, day_month_closed_days) to reconstruct the same logic. A follow-up under Bug 42206 converts the REST controller; further callers can be migrated incrementally. Test plan: 1. prove t/db_dependent/Koha/Calendar.t 2. Verify the new 'closed_dates_in_range' subtest passes Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #83 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203303 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203303&action=edit Bug 42310: (follow-up) Update Holidays.t for normalized calendar tables The normalization commit dropped `repeatable_holidays` and `special_holidays` but t/db_dependent/Holidays.t still referenced them directly via raw SQL, so the test blew up on the first `DELETE FROM special_holidays` with "Table doesn't exist". The earlier namespace follow-up touched this file but only renamed Koha::Calendar references, missing the SQL. Replace the legacy table references with the four new normalized tables: repeatable_holidays (weekday set) -> library_weekly_closures repeatable_holidays (weekday NULL) -> library_repeating_closures special_holidays (isexception=0) -> library_single_closures special_holidays (isexception=1) -> library_closure_exceptions - Two fixture-cleanup DELETEs now wipe all four new tables. - The copy_to_branch 'no duplicates' check, previously two raw-SQL queries against the old two tables, becomes four queries against the new tables, keyed on each table's natural unique key (library_id+weekday / library_id+day+month / library_id+date). - Plan for the copy_to_branch subtest bumps from 8 to 10 to cover the two extra duplicate-check assertions. Test plan: 1. prove t/db_dependent/Holidays.t 2. All 15 top-level tests pass, including the 10 subtest assertions in copy_to_branch. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #85 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203305 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203305&action=edit Bug 42310: (QA follow-up) Restore weekday validation on weekly closures C4::Calendar::insert_week_day_holiday used to croak "Invalid weekday $weekday" unless $weekday =~ m/^[0-6]$/. That guard was dropped when the insert delegated to Koha::Library::Calendar::WeeklyClosure, and nothing replaced it - an out-of-range or undef weekday now persists silently to library_weekly_closures instead of failing at the API boundary. Add the check to WeeklyClosure->store() rather than the legacy wrapper, so every caller (C4::Calendar, Koha::Library::Calendar->add_weekly_closure, and any future REST/admin caller) gets it, not just the old entry point. Test plan: 1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t t/db_dependent/Holds/WaitingReserves.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #87 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203307 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203307&action=edit Bug 42310: (QA follow-up) Refresh in-process closure state on add/delete_weekly_closure and add/delete_repeating_closure add_weekly_closure/delete_weekly_closure and add_repeating_closure/ delete_repeating_closure returned $self without refreshing $self->{weekly_closed_days}/{day_month_closed_days}, so a calendar object would keep reporting the pre-change state to is_holiday (and everything built on it - days_forward, next_open_days, etc.) until it was re-instantiated. This is distinct from the _holidays Koha::Caches entry, which single/exception closures already flush correctly on store/delete. Call _init to rebuild both structures from the database after each add/delete, so the same object reflects the change immediately. Test plan: 1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t t/db_dependent/Holds/WaitingReserves.t t/db_dependent/Koha/CurbsidePickups.t t/db_dependent/Reserves/CancelExpiredReserves.t t/db_dependent/api/v1/libraries.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@openfifth.co.u | |k Status|Passed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203289|0 |1 is obsolete| | Attachment #203290|0 |1 is obsolete| | Attachment #203291|0 |1 is obsolete| | Attachment #203292|0 |1 is obsolete| | Attachment #203293|0 |1 is obsolete| | Attachment #203294|0 |1 is obsolete| | Attachment #203295|0 |1 is obsolete| | Attachment #203296|0 |1 is obsolete| | Attachment #203297|0 |1 is obsolete| | Attachment #203298|0 |1 is obsolete| | Attachment #203299|0 |1 is obsolete| | Attachment #203300|0 |1 is obsolete| | Attachment #203301|0 |1 is obsolete| | Attachment #203302|0 |1 is obsolete| | Attachment #203303|0 |1 is obsolete| | Attachment #203304|0 |1 is obsolete| | Attachment #203305|0 |1 is obsolete| | Attachment #203306|0 |1 is obsolete| | Attachment #203307|0 |1 is obsolete| | Attachment #203308|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=42310 --- Comment #90 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205800 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205800&action=edit Bug 42310: Add Koha::Calendar::* Object classes Four Koha::Object + Koha::Objects pairs following the Koha::City(ies) pattern: - Koha::Calendar::WeeklyClosure(s) -> library_weekly_closures - Koha::Calendar::RepeatingClosure(s) -> library_repeating_closures - Koha::Calendar::SingleClosure(s) -> library_single_closures - Koha::Calendar::Exception(s) -> library_closure_exceptions DBIC schema files to be generated via the 'dbic' alias in KTD after the database migration runs. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #92 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205802&action=edit Bug 42310: (follow-up) Add missing POD Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #94 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205804&action=edit Bug 42310: DBIC schema manual updates Add koha_object_class/koha_objects_class overrides for the four new Result classes backing the normalized calendar tables, below the auto-generated marker. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #96 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205806&action=edit Bug 42310: (follow-up) Add CRUD methods to Koha::Calendar, delegate C4::Calendar Add to Koha::Calendar: add_weekly_closure, add_repeating_closure, add_single_closure, add_exception, delete_weekly_closure, delete_repeating_closure, delete_single_closure, delete_exception, copy_to, _clear_cache C4::Calendar write methods now delegate to Koha::Calendar instead of using raw DBI. No old table references remain in C4::Calendar. Tests in t/db_dependent/Koha/Calendar.t cover all CRUD operations: create, verify via is_holiday, delete, copy_to (including idempotency). Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #98 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205808 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205808&action=edit Bug 42310: (QA follow-up) Fire per-row delete hooks in Koha::Calendar delete_* methods The four delete_*_closure / delete_exception methods in Koha::Calendar were calling ResultSet->delete, which issues a single bulk SQL DELETE and never calls the row-level delete() override. The cache-invalidation hooks on Koha::Calendar::SingleClosure->delete and Koha::Calendar::Exception->delete were therefore skipped when callers went through $library->calendar->delete_single_closure(...) or delete_exception(...), and the <library>_holidays cache kept reporting deleted dates as closed until its TTL expired. The underlying tables enforce UNIQUE constraints on (library_id, weekday), (library_id, day, month) and (library_id, date), so each search can match at most one row. Switch the four methods to ->search({...}, { rows => 1 })->single followed by ->delete on the row, which both makes the single-row expectation explicit and ensures the per-row delete override runs. A regression test is added to t/db_dependent/Koha/Calendar.t asserting that the _holidays cache is cleared after delete_single_closure and delete_exception. Test plan: 1) prove t/db_dependent/Koha/Calendar.t 2) Confirm the new 'delete_*_closure clears _holidays cache' subtest passes. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #100 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205810 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205810&action=edit Bug 42310: (QA follow-up) Report duplicate rows skipped during migration The new calendar tables add UNIQUE(library_id, weekday), UNIQUE(library_id, day, month) and UNIQUE(library_id, date) constraints that did not exist on repeatable_holidays / special_holidays, so the INSERT IGNORE migration can silently drop legacy duplicate rows. This patch compares source vs destination row counts for each of the four migrations and emits a say_warning naming the constraint and pointing the operator at their pre-upgrade backup so titles or descriptions can be recovered if needed. Success lines now include the migrated row count. DROP statements for the legacy tables are guarded with TableExists and reported via say_success for consistency with the rest of the atomicupdate. Test plan: 1) Apply the update on a database containing legacy duplicates and verify the warning is emitted with the correct count. 2) Apply on a database with no duplicates and verify no warning appears. 3) Re-run the update and verify it is idempotent (no warnings, no re-migration). Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #102 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205812&action=edit Bug 42310: (follow-up) Move Koha::Calendar tree under Koha::Library::Calendar Koha::Calendar is, by construction, a per-library object — it croaks without a branchcode argument and every method is branch-scoped. The top-level namespace hides that. Move the whole tree under Koha::Library::Calendar to match the actual semantics, the existing /libraries/{library_id}/calendar REST routes, and the established Koha::Library::* convention (Group, Hour, FloatLimit, OverDriveInfo). This commit: - renames the Koha::Calendar(::*) tree to Koha::Library::Calendar(::*) - updates package declarations, POD and internal 'use' statements within the moved tree - updates the four DBIC Schema Result overrides (koha_object_class / koha_objects_class) to the new namespace - updates callers inside C4::* modules (Calendar, Circulation, Reserves, Overdues, HoldsQueue), Koha::{Patron,Hold,Checkouts, CurbsidePickup}, circ/returns.pl, the installer and cronjobs - updates the 42310-owned tests (Calendar tests, HoldsQueue.t, Holds/WaitingReserves.t, Holidays.t, Koha/CurbsidePickups.t, Reserves/CancelExpiredReserves.t and the libraries.t CRUD block) Koha::Library->calendar, the REST controllers and the 42311 tests are updated in separate follow-up commits under their respective bugs. Test plan: - prove t/db_dependent/Koha/Library/Calendar.t - prove t/db_dependent/Calendar.t - prove t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t - prove t/db_dependent/Holds/WaitingReserves.t - prove t/db_dependent/Reserves/CancelExpiredReserves.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #104 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205814 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205814&action=edit Bug 42310: (QA follow-up) Restore in-memory holiday cache sync in C4::Calendar insert_single_holiday and insert_exception_holiday delegate to Koha::Library::Calendar::SingleClosure/Exception, but stopped updating $self->{single_holidays}/{exception_holidays} on the C4::Calendar object itself, unlike the sibling insert_week_day_holiday/insert_day_month_holiday methods which still do. isHoliday() reads exactly those hashes, so a date just inserted via insert_single_holiday/insert_exception_holiday would read back as "not a holiday" for the rest of that object's lifetime - reachable from tools/newHolidays.pl and tools/exceptionHolidays.pl, which loop over several dates on one $calendar object, checking isHoliday() before each insert to skip duplicates. While adding a regression test, also found that _init() has the same class of bug on the DB-load path: it builds the single/exception lookup keys from `split(/-/, $row->{date})`, which keeps the zero-padding from the DATE string (e.g. "06"), while isHoliday() numifies its lookup key (e.g. "6"). Any single/exception holiday with a single-digit month or day was therefore invisible to isHoliday() even on a freshly reloaded C4::Calendar object. Both call sites now numify year/month/day before building the hash key. Test plan: 1) prove t/db_dependent/Holidays.t t/db_dependent/Calendar.t t/db_dependent/Koha/Library/Calendar.t t/db_dependent/HoldsQueue.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #106 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205816 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205816&action=edit Bug 42310: (QA follow-up) Reject in-place date changes on SingleClosure/Exception store() only flushes the _holidays cache on insert, on the assumption that updates only ever touch title/description. That assumption held for every current caller (ModSingleholiday/ModExceptionholiday only ever change those two fields), but nothing enforced it - a future caller updating date in place would silently desync the cache for up to its TTL, with no error. Make the assumption explicit: reject an update that changes date, rather than silently allowing it. Callers that need to move a closure/exception to a different date must delete and re-add. Test plan: 1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #108 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205818&action=edit Bug 42310: (QA follow-up) Add test coverage for add_* cache invalidation, empty calendars, and leap-year repeating closures Coverage gaps identified during QA review: - add_single_closure/add_exception were only ever tested against a cold cache; nothing proved they invalidate a cache already warmed with the stale answer before the add. - No test exercised a library with no closures at all. - No test exercised a repeating (day/month) closure on 29 February across a leap and a non-leap year. Atomicupdate migration coverage is intentionally left out of scope. Test plan: 1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #110 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205820 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205820&action=edit Bug 42310: (follow-up) Make 16.12.00.032 upgrade self-contained The historical 16.12.00.032 upgrade step (Bug 12063) instantiated Koha::Calendar to push reserves.expirationdate forward by the max pickup delay in open days. Depending on a live application class from a frozen upgrade step is fragile: when this bug renamed and removed Koha::Calendar, the migration had to be edited to chase the new namespace. Frozen updatedatabase.pl steps should be self-contained and prefer direct DB access. Reimplement the holiday-aware date advance inline, reading the legacy repeatable_holidays / special_holidays tables (which still exist at this point in the upgrade path) directly, with no dependency on any Koha::* calendar class. This keeps the step working regardless of later refactors and removes the calendar-class reference from updatedatabase.pl entirely. The weekly / day-month / single-closure logic and exception (open override) handling mirror the historical C4::Calendar semantics. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #112 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi all Issue 1: cleanup_database.pl script still refers to special_holidays (which no longer exists after this patchset): perl misc/cronjobs/cleanup_database.pl --unique-holidays 30 --confirm DBI Exception: DBD::mysql::st execute failed: Table 'koha_kohadev.special_holidays' doesn't exist Issue 2: kohastructure.sql's comment header above library_repeating_closures still refers to repeatable_holidays Issue 3: The 16.12.00.032 upgrade step (installer/data/mysql/updatedatabase.pl) used to respect the useDaysMode system preference when advancing a waiting hold's pickup expiration date past closed days (via Koha::Library::Calendar->days_forward()). The patch "Make 16.12.00.032 upgrade self-contained" replaced that with a hand-rolled loop that always advances 1 day at a time, regardless of useDaysMode, so Dayweek mode's "skip to the same weekday next week" behaviour is silently gone. Steps to reproduce: A) perl -MKoha::Library::Calendar -MDateTime -MC4::Context -e ' C4::Context->set_preference("useDaysMode", "Dayweek"); my $c = Koha::Library::Calendar->new( branchcode => "MPL", days_mode => C4::Context->preference("useDaysMode") ); print $c->get_push_amt( DateTime->now ), "\n"; ' B) Output: 7 - this is Koha's real, current calendar logic: pushing forward 7 days (not 1) when skipping a closed day under Dayweek mode. C) installer/data/mysql/updatedatabase.pl's 16.12.00.032 step only ever does $expirationdate->add( days => 1 ) in its closed-day loop, never checking useDaysMode. Issue 4 (Non-blocker): C4::Overdues::GetSpecialHolidays/GetRepeatableHolidays/GetWdayFromItemnumber still reference the dropped tables via raw SQL, but they're dead code. This is not something introduced by this patchset, but worth cleaning up while we're at it imo. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #205820|0 |1 is obsolete| | Attachment #205821|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=42310 --- Comment #114 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205895&action=edit Bug 42310: (follow-up) Retire t/db_dependent/Holidays.t t/db_dependent/Holidays.t was a legacy file whose coverage is now provided by the two dedicated calendar test files. Its calendar behaviour assertions (is_holiday for weekly/day-month/single holidays) are already exercised by t/db_dependent/Calendar.t and t/db_dependent/Koha/Library/Calendar.t. The remaining unique cases queried the special_holidays / repeatable_holidays tables directly, which this bug drops during the migration to the Koha::Library::Calendar::*Closure schema, so the file could no longer run. This commit folds the unique coverage into t/db_dependent/Koha/Library/Calendar.t, rewritten against the new closure objects/API (no raw legacy-table SQL): - is_holiday DST edge case (America/Santiago days_between) - next_open_days / prev_open_days throwing NoOpenDays when the library is never open, and finding an open day created via an exception - copy_to completeness: repeating-closure copy, exception copy, and exclusion of past-dated single closures and exceptions, plus the no-duplication guarantee for all four closure types and then deletes t/db_dependent/Holidays.t. Test plan: - prove t/db_dependent/Koha/Library/Calendar.t - prove t/db_dependent/Calendar.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #116 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Pedro Amorim (ammopt) from comment #112)

Hi all

Issue 1: cleanup_database.pl script still refers to special_holidays (which no longer exists after this patchset): perl misc/cronjobs/cleanup_database.pl --unique-holidays 30 --confirm DBI Exception: DBD::mysql::st execute failed: Table 'koha_kohadev.special_holidays' doesn't exist

I totally missed that script! I made it use the Koha::Object-based classes now, as the DELETE should be triggering cache invalidation as well!

Issue 2: kohastructure.sql's comment header above library_repeating_closures still refers to repeatable_holidays

Done.

Issue 3: The 16.12.00.032 upgrade step (installer/data/mysql/updatedatabase.pl) used to respect the useDaysMode system preference when advancing a waiting hold's pickup expiration date past closed days (via Koha::Library::Calendar->days_forward()). The patch "Make 16.12.00.032 upgrade self-contained" replaced that with a hand-rolled loop that always advances 1 day at a time, regardless of useDaysMode, so Dayweek mode's "skip to the same weekday next week" behaviour is silently gone.

Steps to reproduce: A) perl -MKoha::Library::Calendar -MDateTime -MC4::Context -e ' C4::Context->set_preference("useDaysMode", "Dayweek"); my $c = Koha::Library::Calendar->new( branchcode => "MPL", days_mode => C4::Context->preference("useDaysMode") ); print $c->get_push_amt( DateTime->now ), "\n"; ' B) Output: 7 - this is Koha's real, current calendar logic: pushing forward 7 days (not 1) when skipping a closed day under Dayweek mode. C) installer/data/mysql/updatedatabase.pl's 16.12.00.032 step only ever does $expirationdate->add( days => 1 ) in its closed-day loop, never checking useDaysMode.

Done!

Issue 4 (Non-blocker): C4::Overdues::GetSpecialHolidays/GetRepeatableHolidays/GetWdayFromItemnumber still reference the dropped tables via raw SQL, but they're dead code. This is not something introduced by this patchset, but worth cleaning up while we're at it imo.

Done! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 --- Comment #117 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.