From f64c8debde8a74f6be78106a810fed56889a094e Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 2 Aug 2024 14:19:18 +0100 Subject: [PATCH] Bug 37256: Defined 'context' of circulation rules This is a follow-up to the previous bug exposing circulation rules over the API. We formally define a definition for a circulation rules set including a new 'context' object the defines the context, library_id, patron_category_id and item_type_id combination of the rules returned. This is just a patch I missed in the last submission that I'd meant to add and it's important for the PUT part of this patchset. --- Koha/REST/V1/CirculationRules.pm | 14 ++++++++++++- .../definitions/circulation_rules.yaml | 19 ++++++++++++++++++ api/v1/swagger/paths/circulation_rules.yaml | 2 +- api/v1/swagger/swagger.yaml | 2 ++ t/db_dependent/api/v1/circulation_rules.t | 20 +++++++++++-------- 5 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 api/v1/swagger/definitions/circulation_rules.yaml diff --git a/Koha/REST/V1/CirculationRules.pm b/Koha/REST/V1/CirculationRules.pm index 41cb1cdf06a..92c38d7f84a 100644 --- a/Koha/REST/V1/CirculationRules.pm +++ b/Koha/REST/V1/CirculationRules.pm @@ -50,7 +50,7 @@ sub list_rules { my $c = shift->openapi->valid_input or return; return try { - my $effective = $c->param('effective') // 1; + my $effective = $c->param('effective') // 1; my $kinds = defined( $c->param('rules') ) ? [ split /\s*,\s*/, $c->param('rules') ] @@ -157,6 +157,18 @@ sub list_rules { } + # Map context into rules + @{$rules} = map { + my %new_rule = %$_; + my %context = ( + "library_id" => delete $new_rule{"branchcode"} // "*", + "patron_category_id" => delete $new_rule{"categorycode"} // "*", + "item_type_id" => delete $new_rule{"itemtype"} // "*", + ); + $new_rule{"context"} = \%context; + \%new_rule; + } @{$rules}; + return $c->render( status => 200, openapi => $rules diff --git a/api/v1/swagger/definitions/circulation_rules.yaml b/api/v1/swagger/definitions/circulation_rules.yaml new file mode 100644 index 00000000000..62f89fe4849 --- /dev/null +++ b/api/v1/swagger/definitions/circulation_rules.yaml @@ -0,0 +1,19 @@ +--- +type: object +properties: + context: + type: object + properties: + patron_category_id: + type: string + description: Patron category id + library_id: + type: string + description: Library branch id + item_type_id: + type: string + description: Item type id + description: Context of the ruleset +additionalProperties: true +required: + - context diff --git a/api/v1/swagger/paths/circulation_rules.yaml b/api/v1/swagger/paths/circulation_rules.yaml index 1c689e2ee09..53eadc17afb 100644 --- a/api/v1/swagger/paths/circulation_rules.yaml +++ b/api/v1/swagger/paths/circulation_rules.yaml @@ -40,7 +40,7 @@ schema: type: array items: - type: object + $ref: "../swagger.yaml#/definitions/circulation_rules" "400": description: Bad request schema: diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index dd72361380e..22418fd6fae 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -12,6 +12,8 @@ definitions: $ref: ./definitions/authorised_value.yaml authorised_value_category: $ref: ./definitions/authorised_value_category.yaml + circulation_rules: + $ref: ./definitions/circulation_rules.yaml identity_provider: $ref: ./definitions/identity_provider.yaml identity_provider_domain: diff --git a/t/db_dependent/api/v1/circulation_rules.t b/t/db_dependent/api/v1/circulation_rules.t index 8be6e47fa4d..f69ca251cee 100755 --- a/t/db_dependent/api/v1/circulation_rules.t +++ b/t/db_dependent/api/v1/circulation_rules.t @@ -143,8 +143,12 @@ subtest 'list_rules() tests' => sub { ->json_is( '/0/finedays' => 5, "Default finedays rule returned when no library is added to request query" ); # Limit to only rules we're interested in - $t->get_ok("//$userid:$password@/api/v1/circulation_rules?rules=fine,finedays")->status_is(200) - ->json_is( '/0' => { fine => 2, finedays => 5 }, "Only the two rules we asked for are returned" ); + $t->get_ok("//$userid:$password@/api/v1/circulation_rules?rules=fine,finedays")->status_is(200)->json_is( + '/0' => { + context => { item_type_id => '*', patron_category_id => '*', library_id => '*' }, fine => 2, finedays => 5 + }, + "Only the two rules we asked for are returned" + ); # Warn on unsupported query parameter $t->get_ok("//$userid:$password@/api/v1/circulation_rules?rules_blah=blah")->status_is(400) @@ -236,9 +240,9 @@ subtest 'list_rules() tests' => sub { # First rule set should march default, default, default if ( $index == 0 ) { - ok( $pointer->get('/branchcode') eq "*" - && $pointer->get('/itemtype') eq '*' - && $pointer->get('/categorycode') eq '*', "Default rules returned first" ); + ok( $pointer->get('/context/library_id') eq "*" + && $pointer->get('/context/item_type_id') eq '*' + && $pointer->get('/context/patron_category_id') eq '*', "Default rules returned first" ); } # Iterate over the list of expected keys for each hash @@ -265,9 +269,9 @@ subtest 'list_rules() tests' => sub { # First (and only) rule set should match branchcode, default, default. if ( $index == 0 ) { - ok( $pointer->get('/branchcode') eq $branchcode - && $pointer->get('/itemtype') eq '*' - && $pointer->get('/categorycode') eq '*', "Branchcode rule set returned when filtered" ); + ok( $pointer->get('/context/library_id') eq $branchcode + && $pointer->get('/context/item_type_id') eq '*' + && $pointer->get('/context/patron_category_id') eq '*', "Branchcode rule set returned when filtered" ); } # Iterate over the list of expected keys for each hash -- 2.45.2