From e6b3d147a8e10d27fd001d70afecba9b216554f3 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 8 Jul 2024 16:54:00 +0100 Subject: [PATCH] Bug 36641: (follow-up) Update to allow returning non-effective rules This patch updates the existing /circulation_rules endpoint introduced in this patchset to allow return of all rule sets rather than only the effective set. We continue to default to the effective set for the parameters passed which will mean by default you will get an arrayref containing one hashref entry with each rule kind as a key in that hashref. However, if you add 'effective=false' as a query parameter, you will now have an arrayref of all rule sets that match your passed criteria including rules that may have fallen back to defaults in the effective case. WIP: Need to add support for * in branch, itemtype, patron category query parameters to allow explicit return of default rules vs no filter passed. (i.e. for the effective=false case where all rules for should be returned when no filters are passed vs * passed to filter to just default rules vs specifics passed for branch/item/patron filtering. --- Koha/REST/V1/CirculationRules.pm | 47 +++++++++++++++------ api/v1/swagger/paths/circulation_rules.yaml | 13 ++++-- t/db_dependent/api/v1/circulation_rules.t | 13 +++--- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/Koha/REST/V1/CirculationRules.pm b/Koha/REST/V1/CirculationRules.pm index 30e045f0370..00b69893e1e 100644 --- a/Koha/REST/V1/CirculationRules.pm +++ b/Koha/REST/V1/CirculationRules.pm @@ -40,20 +40,21 @@ sub get_kinds { ); } -=head3 list_effective_rules +=head3 list_rules -List all effective rules for the requested patron/item/branch combination +Get effective rules for the requested patron/item/branch combination =cut -sub list_effective_rules { +sub list_rules { my $c = shift->openapi->valid_input or return; return try { + my $effective = $c->param('effective') // 1; my $item_type = $c->param('item_type_id'); my $branchcode = $c->param('library_id'); my $patron_category = $c->param('patron_category_id'); - my $rules = $c->param('rules') // [ keys %{ Koha::CirculationRules->rule_kinds } ]; + my $kinds = $c->param('rules') // [ keys %{ Koha::CirculationRules->rule_kinds } ]; if ($item_type) { my $type = Koha::ItemTypes->find($item_type); @@ -94,18 +95,40 @@ sub list_effective_rules { ) unless $category; } - my $effective_rules = Koha::CirculationRules->get_effective_rules( - { - categorycode => $patron_category, - itemtype => $item_type, - branchcode => $branchcode, - rules => $rules + my $rules; + if ($effective) { + + my $effective_rules = Koha::CirculationRules->get_effective_rules( + { + categorycode => $patron_category, + itemtype => $item_type, + branchcode => $branchcode, + rules => $kinds + } + ) // {}; + push @{$rules}, $effective_rules; + } else { + my $select = [ 'branchcode', 'categorycode', 'itemtype' ]; + my $as = [ 'branchcode', 'categorycode', 'itemtype' ]; + for my $kind ( @{$kinds} ) { + push @{$select}, { max => \[ "CASE WHEN rule_name = ? THEN rule_value END", $kind ], -as => $kind }; + push @{$as}, $kind; } - ); + + $rules = Koha::CirculationRules->search( + {}, + { + select => $select, + as => $as, + group_by => [ 'branchcode', 'categorycode', 'itemtype' ] + } + )->unblessed; + + } return $c->render( status => 200, - openapi => $effective_rules ? $effective_rules : {} + openapi => $rules ); } catch { $c->unhandled_exception($_); diff --git a/api/v1/swagger/paths/circulation_rules.yaml b/api/v1/swagger/paths/circulation_rules.yaml index 3f9647943da..1c146682201 100644 --- a/api/v1/swagger/paths/circulation_rules.yaml +++ b/api/v1/swagger/paths/circulation_rules.yaml @@ -1,7 +1,7 @@ --- /circulation_rules: get: - x-mojo-to: CirculationRules#list_effective_rules + x-mojo-to: CirculationRules#list_rules operationId: listCirculationRules tags: - circulation_rules @@ -9,6 +9,11 @@ produces: - application/json parameters: + - name: effective + in: query + description: Boolean indicating whether to return effective rules or all rules. Defaults to true. + required: false + type: boolean - name: item_type_id in: query description: The item type identifier @@ -36,9 +41,9 @@ "200": description: A list of rules for this item type, library and patron category combination schema: - type: object - additionalProperties: - type: [ 'string', 'integer' ] + type: array + items: + type: object "400": description: Bad request schema: diff --git a/t/db_dependent/api/v1/circulation_rules.t b/t/db_dependent/api/v1/circulation_rules.t index a24cd689494..f1ec45a56ec 100755 --- a/t/db_dependent/api/v1/circulation_rules.t +++ b/t/db_dependent/api/v1/circulation_rules.t @@ -32,8 +32,7 @@ my $builder = t::lib::TestBuilder->new; my $t = Test::Mojo->new('Koha::REST::V1'); t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); -subtest 'list_effective_rules() tests' => sub { - +subtest 'list_rules() tests' => sub { plan tests => 32; $schema->storage->txn_begin; @@ -65,7 +64,7 @@ subtest 'list_effective_rules() tests' => sub { ## Authorized user tests # No circulation_rules, so empty hash should be returned - $t->get_ok("//$userid:$password@/api/v1/circulation_rules")->status_is(200)->json_is( {} ); + $t->get_ok("//$userid:$password@/api/v1/circulation_rules")->status_is(200)->json_is( '/0' => {} ); # One rule created, should get returned ok( @@ -82,7 +81,7 @@ subtest 'list_effective_rules() tests' => sub { ); $t->get_ok("//$userid:$password@/api/v1/circulation_rules")->status_is(200) - ->json_is( '' => { 'fine' => 2 }, "Our single rule is returned" ); + ->json_is( '/0' => { 'fine' => 2 }, "Our single rule is returned" ); # Two circulation_rules created, they should both be returned ok( @@ -99,7 +98,7 @@ subtest 'list_effective_rules() tests' => sub { ); $t->get_ok("//$userid:$password@/api/v1/circulation_rules")->status_is(200)->json_is( - '' => { + '/0' => { fine => 2, finedays => 5, }, @@ -121,7 +120,7 @@ subtest 'list_effective_rules() tests' => sub { ); $t->get_ok("//$userid:$password@/api/v1/circulation_rules?library_id=$branchcode")->status_is(200)->json_is( - '' => { + '/0' => { fine => 4, finedays => 5, }, @@ -129,7 +128,7 @@ subtest 'list_effective_rules() tests' => sub { ); $t->get_ok("//$userid:$password@/api/v1/circulation_rules")->status_is(200)->json_is( - '' => { + '/0' => { fine => 2, finedays => 5, }, -- 2.45.2