From 0e769e6d2f75c62f1eba82c9bbbc0f0b5355f117 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 20 Jun 2024 17:20:53 +0100 Subject: [PATCH] Bug 36641: (follow-up) Requested changes This patch does a few things: 1) Moves /circulation-rules to /circulation_rules 2) Adds a the missing permission to /circulation_rules/kinds 3) Updates the circ-rule-kind definition to allow for the new fields added since the route was first defined 4) Adds a 'render_invalid_parameter_value' helper to ::Responses 5) Uses the above helper to render a 400 when an invalid branchcode, itemtype or patron category code is used for filtering 6) Converts from required path based parameters to optional query based parameters. 7) Refined the definition for the 'rules' response so we now only allow for 'string' or 'integer' values for the keys in the response. NOTE: The multi for the 'rules' query parameter fails to work at the moment. --- Koha/REST/Plugin/Responses.pm | 23 ++++++ Koha/REST/V1/CirculationRules.pm | 27 +++++-- .../swagger/definitions/circ-rule-kind.yaml | 2 +- ...tion-rules.yaml => circulation_rules.yaml} | 79 +++++++++++-------- api/v1/swagger/swagger.yaml | 8 +- 5 files changed, 93 insertions(+), 46 deletions(-) rename api/v1/swagger/paths/{circulation-rules.yaml => circulation_rules.yaml} (81%) diff --git a/Koha/REST/Plugin/Responses.pm b/Koha/REST/Plugin/Responses.pm index ed3f7673629..402f22e5b1e 100644 --- a/Koha/REST/Plugin/Responses.pm +++ b/Koha/REST/Plugin/Responses.pm @@ -77,6 +77,29 @@ Provides a generic method rendering the standard response for resource not found ); } ); + +=head3 render_invalid_parameter_value + + $c->render_invalid_parameter_value + +Provides a generic method rendering the standard response for invalid parameter value passed. + +=cut + + $app->helper( + 'render_invalid_parameter_value' => sub { + my ( $c, $path ) = @_; + + $c->render( + status => 400, + openapi => { + error => "Invalid parameter value", + error_code => 'invalid_parameter_value', + path => $path, + }, + ); + } + ); } 1; diff --git a/Koha/REST/V1/CirculationRules.pm b/Koha/REST/V1/CirculationRules.pm index f692181c0ac..3cfb576db60 100644 --- a/Koha/REST/V1/CirculationRules.pm +++ b/Koha/REST/V1/CirculationRules.pm @@ -21,7 +21,6 @@ use Mojo::Base 'Mojolicious::Controller'; use Koha::CirculationRules; - =head1 API =head2 Methods @@ -36,12 +35,11 @@ sub get_kinds { my $c = shift->openapi->valid_input or return; return $c->render( - status => 200, + status => 200, openapi => Koha::CirculationRules->rule_kinds, ); } - =head3 list_effective_rules List all effective rules for the requested patron/item/branch combination @@ -52,16 +50,31 @@ sub list_effective_rules { my $c = shift->openapi->valid_input or return; my $item_type = $c->param('itemtype'); - my $library = $c->param('library'); + my $branchcode = $c->param('library'); my $patron_category = $c->param('category'); - my $rules = $c->param('rules') // [ keys %{Koha::CirculationRules->rule_kinds} ]; + my $rules = $c->param('rules') // [ keys %{ Koha::CirculationRules->rule_kinds } ]; + + if ($item_type) { + my $type = Koha::ItemTypes->find($item_type); + return $c->render_invalid_parameter_value('/query/itemtype') unless $type; + } + + if ($branchcode) { + my $library = Koha::Libraries->find($branchcode); + return $c->render_invalid_parameter_value('/query/library') unless $library; + } + + if ($patron_category) { + my $category = Koha::Patron::Categories->find($patron_category); + return $c->render_invalid_parameter_value('/query/category') unless $category; + } my $effective_rules = Koha::CirculationRules->get_effective_rules( { categorycode => $patron_category, itemtype => $item_type, - branchcode => $library, - rules => $rules + branchcode => $branchcode, + rules => $rules } ); diff --git a/api/v1/swagger/definitions/circ-rule-kind.yaml b/api/v1/swagger/definitions/circ-rule-kind.yaml index 3a3a766cc83..f4e82088644 100644 --- a/api/v1/swagger/definitions/circ-rule-kind.yaml +++ b/api/v1/swagger/definitions/circ-rule-kind.yaml @@ -10,6 +10,6 @@ properties: - branchcode - categorycode - itemtype -additionalProperties: false +additionalProperties: true required: - scope diff --git a/api/v1/swagger/paths/circulation-rules.yaml b/api/v1/swagger/paths/circulation_rules.yaml similarity index 81% rename from api/v1/swagger/paths/circulation-rules.yaml rename to api/v1/swagger/paths/circulation_rules.yaml index 742227932d6..f0ae2d12b73 100644 --- a/api/v1/swagger/paths/circulation-rules.yaml +++ b/api/v1/swagger/paths/circulation_rules.yaml @@ -1,20 +1,48 @@ --- -/circulation-rules/kinds: +/circulation_rules: get: - x-mojo-to: CirculationRules#get_kinds - operationId: getCirculationRuleKinds + x-mojo-to: CirculationRules#list_effective_rules + operationId: listCirculationRules tags: - circulation_rules - summary: Get circulation rules kinds + summary: Get circulation rules for this item/library/patron combination produces: - application/json + parameters: + - name: itemtype + in: query + description: The itemtype + required: false + type: string + - name: library + in: query + description: The library code + required: false + type: string + - name: category + in: query + description: The patron category + required: false + type: string + - name: rules + in: query + description: A comma-separated list of rule kinds + required: false + type: array + items: + type: string + collectionFormat: multi responses: "200": - description: A map of rule kind information + description: A list of rules for this itemtype, library and patron category combination schema: type: object additionalProperties: - $ref: "../swagger.yaml#/definitions/circ-rule-kind" + type: [ 'string', 'integer' ] + "400": + description: Bad request + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: @@ -30,43 +58,25 @@ description: Under maintenance schema: $ref: "../swagger.yaml#/definitions/error" -/circulation-rules/{itemtype}/{library}/{category}: + x-koha-authorization: + permissions: + - circulate: circulate_remaining_permissions +/circulation_rules/kinds: get: - x-mojo-to: CirculationRules#list_effective_rules - operationId: listCirculationRules + x-mojo-to: CirculationRules#get_kinds + operationId: getCirculationRuleKinds tags: - circulation_rules - summary: Get circulation rules for this item/library/patron combination + summary: Get circulation rules kinds produces: - application/json - parameters: - - name: itemtype - in: path - description: The itemtype - required: true - type: string - - name: library - in: path - description: The library code - required: true - type: string - - name: category - in: path - description: The patron category - required: true - type: string - - name: rules - in: query - description: A comma-separated list of rule kinds - type: array - items: - type: string - collectionFormat: csv responses: "200": - description: A list of rules for this itemtype, library and patron category combination + description: A map of rule kind information schema: type: object + additionalProperties: + $ref: "../swagger.yaml#/definitions/circ-rule-kind" "403": description: Access forbidden schema: @@ -85,3 +95,4 @@ x-koha-authorization: permissions: - circulate: circulate_remaining_permissions + diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 812287bfb63..e7ff2c60f22 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -279,10 +279,10 @@ paths: $ref: "./paths/checkouts.yaml#/~1checkouts~1{checkout_id}~1renewal" "/checkouts/availability": $ref: "./paths/checkouts.yaml#/~1checkouts~1availability" - /circulation-rules/kinds: - $ref: ./paths/circulation-rules.yaml#/~1circulation-rules~1kinds - /circulation-rules/{itemtype}/{library}/{category}: - $ref: ./paths/circulation-rules.yaml#/~1circulation-rules~1{itemtype}~1{library}~1{category} + /circulation_rules: + $ref: ./paths/circulation_rules.yaml#/~1circulation_rules + /circulation_rules/kinds: + $ref: ./paths/circulation_rules.yaml#/~1circulation_rules~1kinds /cities: $ref: ./paths/cities.yaml#/~1cities "/cities/{city_id}": -- 2.45.2