[Bug 42310] New: Move calendar CRUD to Koha::Calendar::* with normalized schema
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42310 Bug ID: 42310 Summary: Move calendar CRUD to Koha::Calendar::* with normalized schema Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The calendar system currently relies on C4::Calendar for all write operations using raw DBI, and two overloaded tables where column values determine the type of closure (repeatable_holidays.weekday IS NOT NULL = weekly, IS NULL = annual; special_holidays.isexception = closure vs exception). Single dates are stored as separate day/month/year smallints instead of a DATE column. The goal is to replace this with proper Koha::Object classes under the Koha::Calendar::* namespace backed by normalized, purpose-specific tables, and refactor Koha::Calendar to consume them. This unblocks REST API CRUD endpoints and a Vue-based rewrite of the holidays admin page in the future. -- 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=42310 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=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 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Large patch -- 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 #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196960&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 #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 #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196962&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 #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 #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 196964 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196964&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 #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 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42314 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42314 [Bug 42314] Add `Koha::Library->calendar` -- 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 #196959|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 #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 #196961|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 #196963|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 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196965|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 #9 from David Nind <david@davidnind.com> --- Created attachment 196979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196979&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 #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 #11 from David Nind <david@davidnind.com> --- Created attachment 196981 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196981&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 #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 #13 from David Nind <david@davidnind.com> --- Created attachment 196983 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196983&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 #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 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #15 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Applied the patch 2. Uppdated the database: updatedatabase 3. Rebuild things: yarn build 4. DBIC: dbic 5. Restart everything: restart_all 6. Tests pass: prove t/db_dependent/api/v1/libraries.t t/db_dependent/Calendar.t t/db_dependent/Circulation.t t/db_dependent/Circulation/maxsuspensiondays.t t/db_dependent/Holds/WaitingReserves.t t/db_dependent/Koha/Calendar.t t/db_dependent/Koha/CurbsidePickups.t t/db_dependent/Reserves/CancelExpiredReserves.t Note: No UI testing done. Not sure if this is sufficient for testing. If it isn't, please change back to needs signoff. -- 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 ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- 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 #196979|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 #196981|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 #196983|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 #16 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197020&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 #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 #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197022 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197022&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 #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 #20 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197024 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197024&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 #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 #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197026 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197026&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 --- 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 --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- @davidnind please retest. I changed my mind on the cache handling after playing with this in real life (API). -- 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 #197020|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 #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 #197022|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 #197024|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 #197026|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 #25 from David Nind <david@davidnind.com> --- Created attachment 197030 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197030&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 #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 #27 from David Nind <david@davidnind.com> --- Created attachment 197032 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197032&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 #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 #29 from David Nind <david@davidnind.com> --- Created attachment 197034 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197034&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 #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 #31 from David Nind <david@davidnind.com> --- Created attachment 197036 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197036&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 --- 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 --- Comment #33 from David Nind <david@davidnind.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #24)
@davidnind please retest. I changed my mind on the cache handling after playing with this in real life (API).
No problem! (Although I can't comment on the technical details 8-).) Same testing as in comment #15 -- 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 ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |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 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 #34 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197989 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197989&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 #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 #36 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197991&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 #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 #38 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197993&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> 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 #40 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197995 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197995&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> -- 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 #42 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197997 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197997&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. -- 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 #44 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197999 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197999&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). -- 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 #46 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198001&action=edit Bug 42310: (QA follow-up) Run perltidy Normalize whitespace in Koha::Calendar->copy_to and the bug_42310 atomicupdate so koha-qa reports PASS on tidiness. Test plan: 1. perl misc/devel/tidy.pl --check Koha/Calendar.pm \ installer/data/mysql/atomicupdate/bug_42310.pl 2. koha-qa.pl on this branch no longer reports tidiness failures on these two files -- 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 --- Comment #48 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198003 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198003&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 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 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197989|0 |1 is obsolete| | Attachment #197990|0 |1 is obsolete| | Attachment #197991|0 |1 is obsolete| | Attachment #197992|0 |1 is obsolete| | Attachment #197993|0 |1 is obsolete| | Attachment #197994|0 |1 is obsolete| | Attachment #197995|0 |1 is obsolete| | Attachment #197996|0 |1 is obsolete| | Attachment #197997|0 |1 is obsolete| | Attachment #197998|0 |1 is obsolete| | Attachment #197999|0 |1 is obsolete| | Attachment #198000|0 |1 is obsolete| | Attachment #198001|0 |1 is obsolete| | Attachment #198002|0 |1 is obsolete| | Attachment #198003|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 #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 #50 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203152 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203152&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 #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 #52 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203154 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203154&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 #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 #54 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203156&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. -- 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 #56 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203158 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203158&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> -- 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 #58 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203160 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203160&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. -- 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 #60 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203162 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203162&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). -- 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 #62 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203164&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 --- 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 #64 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203166 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203166&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 -- 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 #66 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203168&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 -- 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 --- Comment #68 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203170 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203170&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 -- 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 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203151|0 |1 is obsolete| | Attachment #203152|0 |1 is obsolete| | Attachment #203153|0 |1 is obsolete| | Attachment #203154|0 |1 is obsolete| | Attachment #203155|0 |1 is obsolete| | Attachment #203156|0 |1 is obsolete| | Attachment #203157|0 |1 is obsolete| | Attachment #203158|0 |1 is obsolete| | Attachment #203159|0 |1 is obsolete| | Attachment #203160|0 |1 is obsolete| | Attachment #203161|0 |1 is obsolete| | Attachment #203162|0 |1 is obsolete| | Attachment #203163|0 |1 is obsolete| | Attachment #203164|0 |1 is obsolete| | Attachment #203165|0 |1 is obsolete| | Attachment #203166|0 |1 is obsolete| | Attachment #203167|0 |1 is obsolete| | Attachment #203168|0 |1 is obsolete| | Attachment #203169|0 |1 is obsolete| | Attachment #203170|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 #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 #70 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203290 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203290&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 #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 #72 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203292 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203292&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 #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 #74 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203294 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203294&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 #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 #76 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203296 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203296&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 #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 #78 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203298 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203298&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 #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 #80 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203300&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 #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 #82 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203302 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203302&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 #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 #84 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203304 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203304&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 #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 #86 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203306 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203306&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 #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 --- Comment #88 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203308 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203308&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.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org