[Bug 42311] New: REST API CRUD endpoints for library calendar
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Bug ID: 42311 Summary: REST API CRUD endpoints for library calendar Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: tomascohen@gmail.com Add endpoints under /libraries/{library_id}/calendar for managing closure definitions: weekly closures, repeating annual closures, single closures, and exceptions. Includes a copy operation to duplicate one library's calendar to another. Depends on the Koha::Calendar::* classes from bug 42310. Permission: tools: edit_calendar for writes, catalogue: 1 for reads. -- 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=42311 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |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. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Medium patch Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197028 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197028&action=edit Bug 42311: Unit tests 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=42311 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197029 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197029&action=edit Bug 42311: REST API CRUD endpoints for library calendar With the Koha::Calendar::* classes and the normalized schema from bug 42310 in place, this patch adds the REST API layer for managing library calendar closures. Each closure type gets its own controller and spec file, following the established Koha::REST::V1::Cities pattern: GET /libraries/{id}/calendar Returns all closure definitions grouped by type. POST /libraries/{id}/calendar/weekly_closures GET /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} PUT /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} DELETE /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} POST /libraries/{id}/calendar/repeating_closures GET /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} PUT /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} DELETE /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} POST /libraries/{id}/calendar/single_closures GET /libraries/{id}/calendar/single_closures/{single_closure_id} PUT /libraries/{id}/calendar/single_closures/{single_closure_id} DELETE /libraries/{id}/calendar/single_closures/{single_closure_id} POST /libraries/{id}/calendar/closure_exceptions GET /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} PUT /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} DELETE /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} POST /libraries/{id}/calendar/copy Copies all closure rules from another library. Permissions: catalogue:1 for reads, tools:edit_calendar for writes. To test: 1. Apply the patches from bug 42310, then this one 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/api/v1/libraries_calendar.t => SUCCESS: Tests pass! 3. Sign off :-D 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=42311 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, | |nick@bywatersolutions.com --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I'm happy with this CRUD implementation. My only doubt is the 'copy' action, which is not RESTful at all. I replicated what's being done in the preservation module, though. Happy to remove it from this report if it makes things easier on QA. It will only be required when time comes and we rewrite the admin page in Vue.js. If it is accepted as-is, I'd be happy too. Let's discuss! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- I had a go at testing, but I am getting test failures. If I apply 42310 first, then apply 42311 and say no to 42310 as a dependency, I get failures. If I just apply 42311 and say yes to all the dependencies, I get failures as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- @davidnind, it would be great if you shared your failures info. I just did the following and all tests succeeded: 1) Launch a clean 'main' KTD because 'misc4dev' fails to inject holidays on the new schema ```shell git checkout origin/main -b qa_42311 ktd --proxy --name qa42311 up -d ktd --name qa42311 --wait-ready 200 ``` 2) Apply the patches (replied 'y' to all questions): ```shell ~/git/koha qa_42311:main *27 ?2 01:57:01 p. m. ❯ git bz apply 42311 📋 Bug 42311 depends on bug 42314 (Signed Off) Follow? [(y)es, (n)o] y 📋 Bug 42314 depends on bug 42310 (Signed Off) Follow? [(y)es, (n)o] y 📋 Bug 42310 depends on bug 42206 (Signed Off) Follow? [(y)es, (n)o] y 📋 Bug 42206 - Add REST endpoint GET /libraries/{library_id}/closed_dates • 196969 - Bug 42206: Add GET /libraries/{id}/holidays endpoint • 196970 - Bug 42206: (follow-up) Rename to /closed_dates, improve efficiency Apply? [(y)es, (n)o, (i)nteractive] y Preparing 2 patch(es): ✓ Prepared 2 patch(es) Applying 2 patch(es): ✓ [1/2] Applied Bug 42206: Add GET /libraries/{id}/holidays endpoint ✓ [2/2] Applied Bug 42206: (follow-up) Rename to /closed_dates, improve eff… ✓ Successfully applied 2 patch(es) from bug 42206 ⠧ Fetching bug 42310 attachments...^R 📋 Bug 42310 - Move calendar CRUD to Koha::Calendar::* with normalized schema • 197030 - Bug 42310: Normalize calendar tables • 197031 - Bug 42310: Add Koha::Calendar::* Object classes • 197032 - Bug 42310: Refactor Koha::Calendar to use Koha::Calendar::* classes • 197033 - Bug 42310: (follow-up) Add missing POD • 197034 - Bug 42310: (follow-up) Remove DBIC schema files for dropped tables • 197035 - Bug 42310: (follow-up) Adapt Calendar tests to new schema • 197036 - Bug 42310: (follow-up) Add CRUD methods to Koha::Calendar, delegate C4::Calendar • 197037 - Bug 42310: (follow-up) Bake cache invalidation into Koha::Object classes Apply? [(y)es, (n)o, (i)nteractive] y Preparing 8 patch(es): ✓ Prepared 8 patch(es) Applying 8 patch(es): ✓ [1/8] Applied Bug 42310: Normalize calendar tables ✓ [2/8] Applied Bug 42310: Add Koha::Calendar::* Object classes ✓ [3/8] Applied Bug 42310: Refactor Koha::Calendar to use Koha::Calendar::*… ✓ [4/8] Applied Bug 42310: (follow-up) Add missing POD ✓ [5/8] Applied Bug 42310: (follow-up) Remove DBIC schema files for dropped… ✓ [6/8] Applied Bug 42310: (follow-up) Adapt Calendar tests to new schema ✓ [7/8] Applied Bug 42310: (follow-up) Add CRUD methods to Koha::Calendar, … ✓ [8/8] Applied Bug 42310: (follow-up) Bake cache invalidation into Koha::O… ✓ Successfully applied 8 patch(es) from bug 42310 📋 Bug 42314 - Add `Koha::Library->calendar` • 196976 - Bug 42314: Add Koha::Library->calendar accessor • 196977 - Bug 42314: (follow-up) Use Koha::Library->calendar accessor Apply? [(y)es, (n)o, (i)nteractive] y Preparing 2 patch(es): ✓ Prepared 2 patch(es) Applying 2 patch(es): ✓ [1/2] Applied Bug 42314: Add Koha::Library->calendar accessor ✓ [2/2] Applied Bug 42314: (follow-up) Use Koha::Library->calendar accessor ✓ Successfully applied 2 patch(es) from bug 42314 📋 Bug 42311 - REST API CRUD endpoints for library calendar • 197028 - Bug 42311: Unit tests • 197029 - Bug 42311: REST API CRUD endpoints for library calendar Apply? [(y)es, (n)o, (i)nteractive] y Preparing 2 patch(es): ✓ Prepared 2 patch(es) Applying 2 patch(es): ✓ [1/2] Applied Bug 42311: Unit tests ✓ [2/2] Applied Bug 42311: REST API CRUD endpoints for library calendar ✓ Successfully applied 2 patch(es) from bug 42311 ~/git/koha qa_42311:main ⇡14 *27 ?2 40s 01:57:46 p. m. ❯ git checkout qa_42311 Switched to branch 'qa_42311' Your branch is ahead of 'origin/main' by 14 commits. (use "git push" to publish your local commits) ~/git/koha qa_42311:main ⇡14 *27 ?2 02:00:30 p. m. ``` 3) Then update within KTD: ```shell ktd --name qa42311 --shell updatedatabase yarn api:bundle ``` 4) Run the `qa` script which will run the tests: ```shell $ qa Tag v25.05.00-1 found in /kohadevbox/koha — checking out branch main in /kohadevbox/qa-test-tools Already on 'main' Your branch is up to date with 'origin/main'. testing 2 commit(s) (applied to 0f8d960 'e5 Bug 42314: (follow-up) Use Koha::L') Processing files [RUNNING] Koha/REST/V1/Libraries/Calendar.pm [RUNNING] Koha/REST/V1/Libraries/Calendar/ClosureExceptions.pm [RUNNING] Koha/REST/V1/Libraries/Calendar/RepeatingClosures.pm [RUNNING] Koha/REST/V1/Libraries/Calendar/SingleClosures.pm [RUNNING] Koha/REST/V1/Libraries/Calendar/WeeklyClosures.pm [RUNNING] api/v1/swagger/paths/libraries_calendar.yaml [PASS] api/v1/swagger/paths/libraries_calendar.yaml [RUNNING] api/v1/swagger/paths/libraries_calendar_closure_exceptions.yaml [PASS] api/v1/swagger/paths/libraries_calendar_closure_exceptions.yaml [RUNNING] api/v1/swagger/paths/libraries_calendar_repeating_closures.yaml [PASS] api/v1/swagger/paths/libraries_calendar_repeating_closures.yaml [RUNNING] api/v1/swagger/paths/libraries_calendar_single_closures.yaml [PASS] api/v1/swagger/paths/libraries_calendar_single_closures.yaml [RUNNING] api/v1/swagger/paths/libraries_calendar_weekly_closures.yaml [PASS] api/v1/swagger/paths/libraries_calendar_weekly_closures.yaml [RUNNING] api/v1/swagger/swagger.yaml [PASS] api/v1/swagger/swagger.yaml [PASS] Koha/REST/V1/Libraries/Calendar.pm [PASS] Koha/REST/V1/Libraries/Calendar/WeeklyClosures.pm [PASS] Koha/REST/V1/Libraries/Calendar/RepeatingClosures.pm [PASS] Koha/REST/V1/Libraries/Calendar/ClosureExceptions.pm [PASS] Koha/REST/V1/Libraries/Calendar/SingleClosures.pm [RUNNING] t/db_dependent/api/v1/libraries_calendar.t [PASS] t/db_dependent/api/v1/libraries_calendar.t Result: [PASS] Koha/REST/V1/Libraries/Calendar.pm [PASS] Koha/REST/V1/Libraries/Calendar/ClosureExceptions.pm [PASS] Koha/REST/V1/Libraries/Calendar/RepeatingClosures.pm [PASS] Koha/REST/V1/Libraries/Calendar/SingleClosures.pm [PASS] Koha/REST/V1/Libraries/Calendar/WeeklyClosures.pm [PASS] api/v1/swagger/paths/libraries_calendar.yaml [PASS] api/v1/swagger/paths/libraries_calendar_closure_exceptions.yaml [PASS] api/v1/swagger/paths/libraries_calendar_repeating_closures.yaml [PASS] api/v1/swagger/paths/libraries_calendar_single_closures.yaml [PASS] api/v1/swagger/paths/libraries_calendar_weekly_closures.yaml [PASS] api/v1/swagger/swagger.yaml [PASS] t/db_dependent/api/v1/libraries_calendar.t Processing additional checks PASS! * Was this bug sponsored by anyone? Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/api/v1/libraries_calendar.t PASS ``` Tests pass when run manually, and also when running `qa c 14` (i.e. the whole patch series): ```shell ... [PASS] t/db_dependent/Koha/Library.t [PASS] t/db_dependent/Reserves/CancelExpiredReserves.t [PASS] t/db_dependent/api/v1/libraries.t [PASS] t/db_dependent/api/v1/libraries_calendar.t Processing additional checks PASS! Running tests (10) * Proving /kohadevbox/koha/t/db_dependent/Calendar.t PASS * Proving /kohadevbox/koha/t/db_dependent/Circulation.t PASS * Proving /kohadevbox/koha/t/db_dependent/Circulation/maxsuspensiondays.t PASS * Proving /kohadevbox/koha/t/db_dependent/Holds/WaitingReserves.t PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/Calendar.t PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/CurbsidePickups.t PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/Library.t PASS * Proving /kohadevbox/koha/t/db_dependent/Reserves/CancelExpiredReserves.t PASS * Proving /kohadevbox/koha/t/db_dependent/api/v1/libraries.t PASS * Proving /kohadevbox/koha/t/db_dependent/api/v1/libraries_calendar.t PASS ``` -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #6 from David Nind <david@davidnind.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #5)
@davidnind, it would be great if you shared your failures info. I just did the following and all tests succeeded: ...
Apologies Tomás, I normally do, but there was quite a lot of output when I ran it. For whatever reason, it is all now working as expected with tests passing! Looks like I didn't clean up properly after testing other bugs... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 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=42311 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197028|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=42311 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197029|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=42311 --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 197129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197129&action=edit Bug 42311: Unit tests 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=42311 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 197130 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197130&action=edit Bug 42311: REST API CRUD endpoints for library calendar With the Koha::Calendar::* classes and the normalized schema from bug 42310 in place, this patch adds the REST API layer for managing library calendar closures. Each closure type gets its own controller and spec file, following the established Koha::REST::V1::Cities pattern: GET /libraries/{id}/calendar Returns all closure definitions grouped by type. POST /libraries/{id}/calendar/weekly_closures GET /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} PUT /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} DELETE /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} POST /libraries/{id}/calendar/repeating_closures GET /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} PUT /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} DELETE /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} POST /libraries/{id}/calendar/single_closures GET /libraries/{id}/calendar/single_closures/{single_closure_id} PUT /libraries/{id}/calendar/single_closures/{single_closure_id} DELETE /libraries/{id}/calendar/single_closures/{single_closure_id} POST /libraries/{id}/calendar/closure_exceptions GET /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} PUT /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} DELETE /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} POST /libraries/{id}/calendar/copy Copies all closure rules from another library. Permissions: catalogue:1 for reads, tools:edit_calendar for writes. To test: 1. Apply the patches from bug 42310, then this one 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/api/v1/libraries_calendar.t => SUCCESS: Tests pass! 3. Sign off :-D 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=42311 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=42311 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197129|0 |1 is obsolete| | Attachment #197130|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=42311 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198009&action=edit Bug 42311: Unit tests 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=42311 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198010&action=edit Bug 42311: REST API CRUD endpoints for library calendar With the Koha::Calendar::* classes and the normalized schema from bug 42310 in place, this patch adds the REST API layer for managing library calendar closures. Each closure type gets its own controller and spec file, following the established Koha::REST::V1::Cities pattern: GET /libraries/{id}/calendar Returns all closure definitions grouped by type. POST /libraries/{id}/calendar/weekly_closures GET /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} PUT /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} DELETE /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} POST /libraries/{id}/calendar/repeating_closures GET /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} PUT /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} DELETE /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} POST /libraries/{id}/calendar/single_closures GET /libraries/{id}/calendar/single_closures/{single_closure_id} PUT /libraries/{id}/calendar/single_closures/{single_closure_id} DELETE /libraries/{id}/calendar/single_closures/{single_closure_id} POST /libraries/{id}/calendar/closure_exceptions GET /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} PUT /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} DELETE /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} POST /libraries/{id}/calendar/copy Copies all closure rules from another library. Permissions: catalogue:1 for reads, tools:edit_calendar for writes. To test: 1. Apply the patches from bug 42310, then this one 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/api/v1/libraries_calendar.t => SUCCESS: Tests pass! 3. Sign off :-D 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=42311 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198011&action=edit Bug 42311: (QA follow-up) Standardise CRUD controllers on new_from_api Two of the four calendar CRUD controllers previously used the $library->calendar->add_* accessor path on POST, while the other two (WeeklyClosures, RepeatingClosures) used Koha::Calendar::*->new_from_api. The accessor path skips new_from_api and therefore skips the API-name to column-name translation: the two styles happen to behave identically today because the spec's field names match column names, but any future divergence would silently break only the two accessor-based endpoints. Switch SingleClosures and ClosureExceptions add() to the new_from_api pattern so all four controllers follow the same Koha REST convention. While here, add the missing "use Scalar::Util qw( blessed );" to every controller in the directory: each was calling blessed() in its catch block without importing it. Test plan: 1. prove t/db_dependent/api/v1/libraries_calendar.t 2. POST /api/v1/libraries/{id}/calendar/single_closures still creates a closure; duplicate POST returns 409 3. POST /api/v1/libraries/{id}/calendar/closure_exceptions same -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198012 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198012&action=edit Bug 42311: (QA follow-up) Add response schemas to calendar CRUD endpoints The POST 201 and GET/PUT 200 responses for the four calendar CRUD endpoints (weekly_closures, repeating_closures, single_closures, closure_exceptions) had a bare `description:` with no schema, so the response body shape was undocumented in the OpenAPI spec. Declare an inline response schema on each success response matching the actual shape returned by `$c->objects->to_api($closure)`: the internally assigned identifier, library_id, the rule-specific fields (weekday / day+month / date), title, and description. Test plan: 1. prove t/db_dependent/api/v1/libraries_calendar.t 2. GET /api/v1/.swagger should validate; no breaking change to any existing client since the spec was underspecified, not wrong -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 198013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198013&action=edit Bug 42311: (follow-up) Use Koha::Library::Calendar namespace Point the calendar REST controllers and their tests at the new Koha::Library::Calendar(::*) namespace: - Koha::REST::V1::Libraries::Calendar - Koha::REST::V1::Libraries::Calendar::ClosureExceptions - Koha::REST::V1::Libraries::Calendar::RepeatingClosures - Koha::REST::V1::Libraries::Calendar::SingleClosures - Koha::REST::V1::Libraries::Calendar::WeeklyClosures - t/db_dependent/api/v1/libraries_calendar.t Controller class names, OpenAPI routes, and DB table names are unchanged; only internal 'use' statements and fully-qualified class references are updated. Test plan: - prove t/db_dependent/api/v1/libraries_calendar.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198009|0 |1 is obsolete| | Attachment #198010|0 |1 is obsolete| | Attachment #198011|0 |1 is obsolete| | Attachment #198012|0 |1 is obsolete| | Attachment #198013|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=42311 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203178 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203178&action=edit Bug 42311: Unit tests 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=42311 --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203179 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203179&action=edit Bug 42311: REST API CRUD endpoints for library calendar With the Koha::Calendar::* classes and the normalized schema from bug 42310 in place, this patch adds the REST API layer for managing library calendar closures. Each closure type gets its own controller and spec file, following the established Koha::REST::V1::Cities pattern: GET /libraries/{id}/calendar Returns all closure definitions grouped by type. POST /libraries/{id}/calendar/weekly_closures GET /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} PUT /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} DELETE /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} POST /libraries/{id}/calendar/repeating_closures GET /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} PUT /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} DELETE /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} POST /libraries/{id}/calendar/single_closures GET /libraries/{id}/calendar/single_closures/{single_closure_id} PUT /libraries/{id}/calendar/single_closures/{single_closure_id} DELETE /libraries/{id}/calendar/single_closures/{single_closure_id} POST /libraries/{id}/calendar/closure_exceptions GET /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} PUT /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} DELETE /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} POST /libraries/{id}/calendar/copy Copies all closure rules from another library. Permissions: catalogue:1 for reads, tools:edit_calendar for writes. To test: 1. Apply the patches from bug 42310, then this one 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/api/v1/libraries_calendar.t => SUCCESS: Tests pass! 3. Sign off :-D 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=42311 --- Comment #16 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203180 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203180&action=edit Bug 42311: (QA follow-up) Standardise CRUD controllers on new_from_api Two of the four calendar CRUD controllers previously used the $library->calendar->add_* accessor path on POST, while the other two (WeeklyClosures, RepeatingClosures) used Koha::Calendar::*->new_from_api. The accessor path skips new_from_api and therefore skips the API-name to column-name translation: the two styles happen to behave identically today because the spec's field names match column names, but any future divergence would silently break only the two accessor-based endpoints. Switch SingleClosures and ClosureExceptions add() to the new_from_api pattern so all four controllers follow the same Koha REST convention. While here, add the missing "use Scalar::Util qw( blessed );" to every controller in the directory: each was calling blessed() in its catch block without importing it. Test plan: 1. prove t/db_dependent/api/v1/libraries_calendar.t 2. POST /api/v1/libraries/{id}/calendar/single_closures still creates a closure; duplicate POST returns 409 3. POST /api/v1/libraries/{id}/calendar/closure_exceptions same -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #17 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203181 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203181&action=edit Bug 42311: (QA follow-up) Add response schemas to calendar CRUD endpoints The POST 201 and GET/PUT 200 responses for the four calendar CRUD endpoints (weekly_closures, repeating_closures, single_closures, closure_exceptions) had a bare `description:` with no schema, so the response body shape was undocumented in the OpenAPI spec. Declare an inline response schema on each success response matching the actual shape returned by `$c->objects->to_api($closure)`: the internally assigned identifier, library_id, the rule-specific fields (weekday / day+month / date), title, and description. Test plan: 1. prove t/db_dependent/api/v1/libraries_calendar.t 2. GET /api/v1/.swagger should validate; no breaking change to any existing client since the spec was underspecified, not wrong -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 --- Comment #18 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203182 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203182&action=edit Bug 42311: (follow-up) Use Koha::Library::Calendar namespace Point the calendar REST controllers and their tests at the new Koha::Library::Calendar(::*) namespace: - Koha::REST::V1::Libraries::Calendar - Koha::REST::V1::Libraries::Calendar::ClosureExceptions - Koha::REST::V1::Libraries::Calendar::RepeatingClosures - Koha::REST::V1::Libraries::Calendar::SingleClosures - Koha::REST::V1::Libraries::Calendar::WeeklyClosures - t/db_dependent/api/v1/libraries_calendar.t Controller class names, OpenAPI routes, and DB table names are unchanged; only internal 'use' statements and fully-qualified class references are updated. Test plan: - prove t/db_dependent/api/v1/libraries_calendar.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203178|0 |1 is obsolete| | Attachment #203179|0 |1 is obsolete| | Attachment #203180|0 |1 is obsolete| | Attachment #203181|0 |1 is obsolete| | Attachment #203182|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=42311 --- Comment #19 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203317 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203317&action=edit Bug 42311: Unit tests 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=42311 --- Comment #20 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203318 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203318&action=edit Bug 42311: REST API CRUD endpoints for library calendar With the Koha::Calendar::* classes and the normalized schema from bug 42310 in place, this patch adds the REST API layer for managing library calendar closures. Each closure type gets its own controller and spec file, following the established Koha::REST::V1::Cities pattern: GET /libraries/{id}/calendar Returns all closure definitions grouped by type. POST /libraries/{id}/calendar/weekly_closures GET /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} PUT /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} DELETE /libraries/{id}/calendar/weekly_closures/{weekly_closure_id} POST /libraries/{id}/calendar/repeating_closures GET /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} PUT /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} DELETE /libraries/{id}/calendar/repeating_closures/{repeating_closure_id} POST /libraries/{id}/calendar/single_closures GET /libraries/{id}/calendar/single_closures/{single_closure_id} PUT /libraries/{id}/calendar/single_closures/{single_closure_id} DELETE /libraries/{id}/calendar/single_closures/{single_closure_id} POST /libraries/{id}/calendar/closure_exceptions GET /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} PUT /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} DELETE /libraries/{id}/calendar/closure_exceptions/{closure_exception_id} POST /libraries/{id}/calendar/copy Copies all closure rules from another library. Permissions: catalogue:1 for reads, tools:edit_calendar for writes. To test: 1. Apply the patches from bug 42310, then this one 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/api/v1/libraries_calendar.t => SUCCESS: Tests pass! 3. Sign off :-D 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=42311 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203319 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203319&action=edit Bug 42311: (QA follow-up) Standardise CRUD controllers on new_from_api Two of the four calendar CRUD controllers previously used the $library->calendar->add_* accessor path on POST, while the other two (WeeklyClosures, RepeatingClosures) used Koha::Calendar::*->new_from_api. The accessor path skips new_from_api and therefore skips the API-name to column-name translation: the two styles happen to behave identically today because the spec's field names match column names, but any future divergence would silently break only the two accessor-based endpoints. Switch SingleClosures and ClosureExceptions add() to the new_from_api pattern so all four controllers follow the same Koha REST convention. While here, add the missing "use Scalar::Util qw( blessed );" to every controller in the directory: each was calling blessed() in its catch block without importing it. Test plan: 1. prove t/db_dependent/api/v1/libraries_calendar.t 2. POST /api/v1/libraries/{id}/calendar/single_closures still creates a closure; duplicate POST returns 409 3. POST /api/v1/libraries/{id}/calendar/closure_exceptions same 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=42311 --- Comment #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203320 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203320&action=edit Bug 42311: (QA follow-up) Add response schemas to calendar CRUD endpoints The POST 201 and GET/PUT 200 responses for the four calendar CRUD endpoints (weekly_closures, repeating_closures, single_closures, closure_exceptions) had a bare `description:` with no schema, so the response body shape was undocumented in the OpenAPI spec. Declare an inline response schema on each success response matching the actual shape returned by `$c->objects->to_api($closure)`: the internally assigned identifier, library_id, the rule-specific fields (weekday / day+month / date), title, and description. Test plan: 1. prove t/db_dependent/api/v1/libraries_calendar.t 2. GET /api/v1/.swagger should validate; no breaking change to any existing client since the spec was underspecified, not wrong 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=42311 --- Comment #23 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203321 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203321&action=edit Bug 42311: (follow-up) Use Koha::Library::Calendar namespace Point the calendar REST controllers and their tests at the new Koha::Library::Calendar(::*) namespace: - Koha::REST::V1::Libraries::Calendar - Koha::REST::V1::Libraries::Calendar::ClosureExceptions - Koha::REST::V1::Libraries::Calendar::RepeatingClosures - Koha::REST::V1::Libraries::Calendar::SingleClosures - Koha::REST::V1::Libraries::Calendar::WeeklyClosures - t/db_dependent/api/v1/libraries_calendar.t Controller class names, OpenAPI routes, and DB table names are unchanged; only internal 'use' statements and fully-qualified class references are updated. Test plan: - prove t/db_dependent/api/v1/libraries_calendar.t Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Thanks, Martin, for the outstanding QA work on this tree and the great follow-ups. Teamwork for the win! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35937 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42311 Bug 42311 depends on bug 42314, which changed state. Bug 42314 Summary: Add `Koha::Library->calendar` https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42314 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |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=42311 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@openfifth.co.u | |k Status|Passed QA |Failed QA --- Comment #25 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi all, a few blockers here worth fixing before push: Setup: Inside KTD, rebuild the API bundle and restart plack: $ ktd --shell k$ yarn api:bundle k$ koha-plack --restart kohadev Blocker 1: GET /libraries/{library_id}/calendar silently truncates each list at RESTdefaultPageSize list() calls $c->objects->search for each closure type, so each list is paginated to 20 rows with no way to request more (the spec doesn't declare _per_page). The 42310 migration copies all historic special_holidays, so most upgraded libraries will be over 20 single closures from day one. 1) Add 25 single closures to CPL: k$ for d in $(seq -w 1 25); do curl -s -o /dev/null -u koha:koha -H 'Content-Type: application/json' -d "{\"date\":\"2099-01-$d\",\"description\":\"\"}" http://localhost:8081/api/v1/libraries/CPL/calendar/single_closures; done 2) Count what the calendar endpoint returns: k$ curl -s -u koha:koha http://localhost:8081/api/v1/libraries/CPL/calendar | perl -MJSON -0ne 'print scalar @{decode_json($_)->{single_closures}}, "\n"' 3) Confirm it says 20, not 25 4) Look at the pagination header: k$ curl -s -D - -o /dev/null -u koha:koha http://localhost:8081/api/v1/libraries/CPL/calendar | grep -i x-total-count 5) Confirm it's garbage: 'X-Total-Count: 0, 0, 25, 0' Blocker 2: POST with only the required fields explodes 'description' is NOT NULL with no default in all 4 tables, but optional in all 4 POST specs. 1) Add a weekly closure with just 'weekday' (the only required field in the spec): k$ curl -s -u koha:koha -H 'Content-Type: application/json' -d '{"weekday":3}' http://localhost:8081/api/v1/libraries/CPL/calendar/weekly_closures 2) Confirm it explodes with a nasty 500 error, and the log shows 'Koha::Exceptions::Object::NotNull': k$ tail -n 5 /var/log/koha/kohadev/api-error.log 3) Same thing happens for repeating_closures ('day'+'month'), single_closures and closure_exceptions ('date') Blocker 3: repeating closures accept impossible dates The spec has no bounds on 'day'/'month' and RepeatingClosure has no store validation, so invalid rows get stored and would need an atomicupdate to clean up later. 1) Add a repeating closure on day 99 of month 13: k$ curl -s -u koha:koha -H 'Content-Type: application/json' -d '{"day":99,"month":13,"description":""}' http://localhost:8081/api/v1/libraries/CPL/calendar/repeating_closures 2) Confirm it's stored and returned (201) instead of rejected with a 400 Blocker 4: copy() test starts failing on 2027-01-02 The test hardcodes a '2027-01-01' single closure, and copy_to only copies single closures dated today or later. 1) Run the tests normally, confirm they pass: k$ prove t/db_dependent/api/v1/libraries_calendar.t 2) Run them again with the clock set to 2027-01-02: k$ perl -MTest::MockTime=set_absolute_time -e 'BEGIN { set_absolute_time("2027-01-02T12:00:00Z") } do "./t/db_dependent/api/v1/libraries_calendar.t"; die $@ if $@' 2>&1 | grep -E '^(not )?ok|Failed test' 3) Confirm 'not ok 7 - copy() tests' / 'Failed test 'Single copied'' -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org