Attachment 169020 Details for Bug 36641 – Bug 36641: (follow-up) Update to allow returning non-effective rules

[patch] Bug 36641: (follow-up) Update to allow returning non-effective rules

Bug-36641-follow-up-Update-to-allow-returning-non-.patch (text/plain), 7.52 KB, created by Tomás Cohen Arazi (tcohen) on 2024-07-15 19:00:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2024-07-15 19:00:59 UTC

Size: 7.52 KB

patch

obsolete

>From bc5de45e0ba8c1a8ee662f4b18c74b3b77e2c3e4 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>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.
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 36641: 165086 | 167937 | 167938 | 167946 | 167947 | 168018 | 168019 | 168020 | 168023 | 168024 | 168025 | 168365 | 168366 | 168367 | 168368 | 168369 | 168370 | 168378 | 168379 | 168395 | 168396 | 168397 | 168398 | 168399 | 168400 | 168401 | 168841 | 168874 | 168875 | 168876 | 168877 | 168878 | 168879 | 168880 | 168881 | 168882 | 168883 | 168884 | 168905 | 168906 | 168907 | 168908 | 168909 | 168910 | 168911 | 168912 | 168913 | 168914 | 168915 | 168916 | 168926 | 168927 | 168928 | 168929 | 168930 | 168931 | 168932 | 168933 | 168934 | 168935 | 168936 | 168937 | 168938 | 168940 | 169013 | 169014 | 169015 | 169016 | 169017 | 169018 | 169019 | 169020 | 169021 | 169022 | 169023 | 169024 | 169025 | 169026