gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 42206: (follow-up) Rename to /closed_dates, improve efficiency 

Rename /libraries/{id}/holidays to /libraries/{id}/closed_dates
for consistency with existing sub-resource naming (desks,
cash_registers).

Efficiency: instead of calling $calendar->is_holiday() per day
(which re-checks all three data sources each time), access the
pre-loaded data structures directly: weekly_closed_days array,
day_month_closed_days hash, and _holidays hash. The logic properly
handles exceptions (dates marked open despite a weekly/annual
closure).

Additional fixes:
- Add 365-day max range guard
- Add missing 401/403 responses in swagger spec
- Add request_id_header parameter
- Test exception handling (open on normally-closed day)
- Remove fragile 'empty defaults' assertion

Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Tomás Cohen Arazi 2026-04-08 12:10:23 -03:00 committed by Lucas Gass
parent 2a3376b243
commit 42b2758a76
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
4 changed files with 114 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
Koha/REST/V1/Libraries.pm
View File

@ -212,14 +212,13 @@ sub list_cash_registers {
};
}
=head3 list_holidays
=head3 list_closed_dates
Controller function that returns closed days for a library within a date range.
Used by booking calendar to disable selection of closed days.
Controller function that returns closed dates for a library within a date range.
=cut
sub list_holidays {
sub list_closed_dates {
my $c = shift->openapi->valid_input or return;
my $library_id = $c->param('library_id');
@ -238,24 +237,48 @@ sub list_holidays {
if ( $to_dt->compare($from_dt) < 0 ) {
return $c->render(
status => 400,
openapi => { error => "'to' date must be after 'from' date" }
openapi => { error => "'to' date must be after 'from' date", error_code => 'invalid_date_range' }
);
}
my $calendar = Koha::Calendar->new( branchcode => $library_id );
my @holidays;
if ( $to_dt->delta_days($from_dt)->in_units('days') > 365 ) {
return $c->render(
status => 400,
openapi => { error => "Date range cannot exceed 365 days", error_code => 'date_range_too_large' }
);
}
my $calendar = Koha::Calendar->new( branchcode => $library_id );
my $holidays = $calendar->_holidays;
my $weekly = $calendar->{weekly_closed_days};
my $day_month = $calendar->{day_month_closed_days};
my @closed;
my $current = $from_dt->clone;
while ( $current <= $to_dt ) {
if ( $calendar->is_holiday($current) ) {
push @holidays, $current->ymd;
my $dominated;
my $ymd = $current->ymd('');
# Check special holidays hash first (includes exceptions)
if ( defined $holidays->{$ymd} ) {
$dominated = $holidays->{$ymd}; # 1 = closed, 0 = exception (open)
}
unless ( defined $dominated && $dominated == 0 ) {
if ( $dominated
|| $weekly->[ $current->day_of_week % 7 ]
|| $day_month->{ $current->month }->{ $current->day } )
{
push @closed, $current->ymd;
}
}
$current->add( days => 1 );
}
return $c->render(
status => 200,
openapi => \@holidays
openapi => \@closed
);
} catch {
$c->unhandled_exception($_);

36
api/v1/swagger/paths/libraries.yaml
View File

@ -425,42 +425,58 @@
x-koha-authorization:
permissions:
catalogue: 1
"/libraries/{library_id}/holidays":
"/libraries/{library_id}/closed_dates":
get:
x-mojo-to: Libraries#list_holidays
operationId: listLibraryHolidays
x-mojo-to: Libraries#list_closed_dates
operationId: listLibraryClosedDates
tags:
- libraries
summary: List holidays for a library
summary: List closed dates for a library
description: |
Returns a list of closed days (holidays) for a library within a date range.
Used by booking calendar to disable selection of closed days.
Returns a list of dates on which the library is closed within the
given range. Accounts for weekly closures, repeating annual closures,
single closures, and exceptions.
The date range cannot exceed 365 days.
parameters:
- $ref: "../swagger.yaml#/parameters/library_id_pp"
- name: from
in: query
description: Start date for the range (ISO 8601 format, e.g., 2024-01-01). Defaults to today.
description: Start date (ISO 8601). Defaults to today.
required: false
type: string
format: date
- name: to
in: query
description: End date for the range (ISO 8601 format, e.g., 2024-03-31). Defaults to 3 months from 'from'.
description: End date (ISO 8601). Defaults to 3 months from 'from'. Max range 365 days.
required: false
type: string
format: date
- $ref: "../swagger.yaml#/parameters/request_id_header"
produces:
- application/json
responses:
"200":
description: A list of holiday dates in YYYY-MM-DD format
description: A list of closed dates in YYYY-MM-DD format
schema:
type: array
items:
type: string
format: date
"400":
description: Bad request
description: |
Bad request. Possible `error_code` attribute values:
* `invalid_date_range` - 'to' date is before 'from' date
* `date_range_too_large` - Date range exceeds 365 days
schema:
$ref: "../swagger.yaml#/definitions/error"
"401":
description: Authentication required
schema:
$ref: "../swagger.yaml#/definitions/error"
"403":
description: Access forbidden
schema:
$ref: "../swagger.yaml#/definitions/error"
"404":

4
api/v1/swagger/swagger.yaml
View File

@ -499,8 +499,8 @@ paths:
$ref: "./paths/libraries.yaml#/~1libraries~1{library_id}~1cash_registers"
"/libraries/{library_id}/desks":
$ref: "./paths/libraries.yaml#/~1libraries~1{library_id}~1desks"
"/libraries/{library_id}/holidays":
$ref: "./paths/libraries.yaml#/~1libraries~1{library_id}~1holidays"
"/libraries/{library_id}/closed_dates":
$ref: "./paths/libraries.yaml#/~1libraries~1{library_id}~1closed_dates"
"/oauth/login/{provider_code}/{interface}":
$ref: ./paths/oauth.yaml#/~1oauth~1login~1{provider_code}~1{interface}
/oauth/token:

97
t/db_dependent/api/v1/libraries.t
View File

@ -440,9 +440,9 @@ subtest 'list_cash_registers() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'list_holidays() tests' => sub {
subtest 'list_closed_dates() tests' => sub {
plan tests => 16;
plan tests => 20;
$schema->storage->txn_begin;
@ -460,79 +460,88 @@ subtest 'list_holidays() tests' => sub {
my $branchcode = $library->branchcode;
my $cache = Koha::Caches->get_instance();
# Clear existing holidays for this branch
# Clear all closure data so we fully control what exists
$schema->resultset('SpecialHoliday')->search( { branchcode => $branchcode } )->delete;
$schema->resultset('RepeatableHoliday')->search( { branchcode => $branchcode } )->delete;
$cache->clear_from_cache( $branchcode . '_holidays' );
# Add a specific holiday
# Weekly closure: Saturday (6)
# 2026-06-13 and 2026-06-20 are Saturdays
$builder->build(
{
source => 'SpecialHoliday',
source => 'RepeatableHoliday',
value => {
branchcode => $branchcode,
day => 15,
month => 6,
year => 2026,
title => 'Test holiday',
description => '',
isexception => 0,
branchcode => $branchcode, weekday => 6, day => undef, month => undef,
title => 'Saturdays', description => '',
},
}
);
# Add a second holiday
# Single closure: 2026-06-15 (a Monday)
$builder->build(
{
source => 'SpecialHoliday',
value => {
branchcode => $branchcode,
day => 20,
month => 6,
year => 2026,
title => 'Another holiday',
description => '',
isexception => 0,
branchcode => $branchcode, day => 15, month => 6, year => 2026,
title => 'Test', description => '', isexception => 0,
},
}
);
# Exception: 2026-06-20 (a Saturday — normally closed, but open this day)
$builder->build(
{
source => 'SpecialHoliday',
value => {
branchcode => $branchcode, day => 20, month => 6, year => 2026,
title => 'Exception', description => '', isexception => 1,
},
}
);
$cache->clear_from_cache( $branchcode . '_holidays' );
# Returns holidays within range
$t->get_ok("//$userid:$password@/api/v1/libraries/$branchcode/holidays?from=2026-06-01&to=2026-06-30")
# Range covering 2026-06-13 (Sat) through 2026-06-21 (Sun)
$t->get_ok("//$userid:$password\@/api/v1/libraries/$branchcode/closed_dates?from=2026-06-13&to=2026-06-21")
->status_is(200);
my $holidays = $t->tx->res->json;
ok( ( grep { $_ eq '2026-06-15' } @$holidays ), 'First holiday found in response' );
ok( ( grep { $_ eq '2026-06-20' } @$holidays ), 'Second holiday found in response' );
my $dates = $t->tx->res->json;
ok( ( grep { $_ eq '2026-06-13' } @$dates ), 'Weekly closure (Saturday 13th) found' );
ok( ( grep { $_ eq '2026-06-15' } @$dates ), 'Single closure (Monday 15th) found' );
ok( !( grep { $_ eq '2026-06-20' } @$dates ), 'Exception (Saturday 20th) excluded — open despite weekly closure' );
ok( !( grep { $_ eq '2026-06-16' } @$dates ), 'Regular weekday (Tuesday 16th) not closed' );
# Returns empty list when no holidays in range
$t->get_ok("//$userid:$password@/api/v1/libraries/$branchcode/holidays?from=2026-07-01&to=2026-07-05")
->status_is(200);
my $no_holidays = $t->tx->res->json;
my @filtered = grep { $_ eq '2026-06-15' || $_ eq '2026-06-20' } @$no_holidays;
is( scalar @filtered, 0, 'No test holidays outside their range' );
# Range with no closures: Mon-Fri week with no specials
# 2026-06-22 Mon through 2026-06-26 Fri — no Saturday, no specials
$t->get_ok("//$userid:$password\@/api/v1/libraries/$branchcode/closed_dates?from=2026-06-22&to=2026-06-26")
->status_is(200)
->json_is( '' => [], 'No closed dates in a weekday-only range' );
# 400 when to < from
$t->get_ok("//$userid:$password@/api/v1/libraries/$branchcode/holidays?from=2026-06-30&to=2026-06-01")
$t->get_ok("//$userid:$password\@/api/v1/libraries/$branchcode/closed_dates?from=2026-06-30&to=2026-06-01")
->status_is(400)
->json_is( '/error' => "'to' date must be after 'from' date" );
->json_is( '/error_code' => 'invalid_date_range' );
# 400 when range > 365 days
$t->get_ok("//$userid:$password\@/api/v1/libraries/$branchcode/closed_dates?from=2026-01-01&to=2028-01-01")
->status_is(400)
->json_is( '/error_code' => 'date_range_too_large' );
# 404 for non-existent library
my $non_existent_code = $library->branchcode;
my $gone = $library->branchcode;
$library->delete;
$cache->clear_from_cache( $non_existent_code . '_holidays' );
$cache->clear_from_cache( $gone . '_holidays' );
$t->get_ok("//$userid:$password\@/api/v1/libraries/$gone/closed_dates?from=2026-06-01&to=2026-06-30")
->status_is(404);
$t->get_ok("//$userid:$password@/api/v1/libraries/$non_existent_code/holidays?from=2026-06-01&to=2026-06-30")
->status_is(404)
->json_is( '/error' => 'Library not found' );
# Defaults work (no from/to params)
my $library2 = $builder->build_object( { class => 'Koha::Libraries' } );
$t->get_ok( "//$userid:$password@/api/v1/libraries/" . $library2->branchcode . "/holidays" )
# Defaults work (no from/to) — fresh library with no closures at all
my $lib2 = $builder->build_object( { class => 'Koha::Libraries' } );
my $branchcode2 = $lib2->branchcode;
$schema->resultset('RepeatableHoliday')->search( { branchcode => $branchcode2 } )->delete;
$schema->resultset('SpecialHoliday')->search( { branchcode => $branchcode2 } )->delete;
$cache->clear_from_cache( $branchcode2 . '_holidays' );
$t->get_ok("//$userid:$password\@/api/v1/libraries/$branchcode2/closed_dates")
->status_is(200)
->json_is( '' => [], 'Empty holidays for fresh library with defaults' );
->json_is( '' => [], 'Empty for library with no closures defined' );
$schema->storage->txn_rollback;
};