Attachment 161244 Details for Bug 35386 – Bug 35386: Add RESTAPIRenewalBranch system preference

[patch] Bug 35386: Add RESTAPIRenewalBranch system preference

Bug-35386-Add-RESTAPIRenewalBranch-system-preferen.patch (text/plain), 10.47 KB, created by Brendan Lawlor on 2024-01-22 14:39:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Brendan Lawlor

Created: 2024-01-22 14:39:21 UTC

Size: 10.47 KB

patch

obsolete

>From 146bcb63f6c9e8ebeed65520fbf151ad5a47f9d0 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Mon, 27 Nov 2023 16:31:41 +0000
>Subject: [PATCH] Bug 35386: Add RESTAPIRenewalBranch system preference
>
>This patch adds a new system prefernce, RESTAPIRenewalBranch, analogous to the existing OpacRenewalBranch system preference.
>
>The preference allows choosing how the renewal branch is recorded in the statistics table.
>In order ot preserve existing behaviour, the default is to use the api user's branch.
>
>To test:
>1 - Checkout some items to a patron
>2 - Add an API user account with circulation permissions and a different homebranch
>3 - POST a renewal to: http://localhost:8080/api/v1/checkouts/{checkout_id}/renewal
>4 - Check statistics table and confirm the api users branch was used
>5 - Apply patches, restart all
>6 - Repeat API renewal, confirm same branch used
>7 - Change the RESTAPIRenewal syspref
>8 - Repeat API renewal and confirm specified branch is used
>9 - Confirm the syspref works for all settings
>
>Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
>---
> Koha/Item.pm                                  | 42 +++++++++++++------
> .../data/mysql/atomicupdate/bug_35386.pl      | 17 ++++++++
> installer/data/mysql/mandatory/sysprefs.sql   |  1 +
> .../admin/preferences/web_services.pref       | 11 +++++
> t/db_dependent/Koha/Item.t                    | 30 ++++++++++++-
> 5 files changed, 87 insertions(+), 14 deletions(-)
> create mode 100755 installer/data/mysql/atomicupdate/bug_35386.pl
>
>diff --git a/Koha/Item.pm b/Koha/Item.pm
>index 22eaceac94..424a9509ac 100644
>--- a/Koha/Item.pm
>+++ b/Koha/Item.pm
>@@ -1195,30 +1195,46 @@ Returns the branchcode to be recorded in statistics renewal of the item
> 
> sub renewal_branchcode {
> 
>-    my ($self, $params ) = @_;
>+    my ( $self, $params ) = @_;
> 
>     my $interface = C4::Context->interface;
>     my $branchcode;
>-    if ( $interface eq 'opac' ){
>-        my $renewal_branchcode = C4::Context->preference('OpacRenewalBranch');
>-        if( !defined $renewal_branchcode || $renewal_branchcode eq 'opacrenew' ){
>+    my $renewal_branchcode;
>+
>+    if ( $interface eq 'opac' ) {
>+        $renewal_branchcode = C4::Context->preference('OpacRenewalBranch');
>+        if ( !defined $renewal_branchcode || $renewal_branchcode eq 'opacrenew' ) {
>             $branchcode = 'OPACRenew';
>         }
>-        elsif ( $renewal_branchcode eq 'itemhomebranch' ) {
>-            $branchcode = $self->homebranch;
>+    } elsif ( $interface eq 'api' ) {
>+        $renewal_branchcode = C4::Context->preference('RESTAPIRenewalBranch');
>+        if ( !defined $renewal_branchcode || $renewal_branchcode eq 'apirenew' ) {
>+            $branchcode = 'APIRenew';
>         }
>-        elsif ( $renewal_branchcode eq 'patronhomebranch' ) {
>+    }
>+
>+    return $branchcode if $branchcode;
>+
>+    if ($renewal_branchcode) {
>+        if ( $renewal_branchcode eq 'itemhomebranch' ) {
>+            $branchcode = $self->homebranch;
>+        } elsif ( $renewal_branchcode eq 'patronhomebranch' ) {
>             $branchcode = $self->checkout->patron->branchcode;
>-        }
>-        elsif ( $renewal_branchcode eq 'checkoutbranch' ) {
>+        } elsif ( $renewal_branchcode eq 'checkoutbranch' ) {
>             $branchcode = $self->checkout->branchcode;
>-        }
>-        else {
>+        } elsif ( $renewal_branchcode eq 'apiuserbranch' ) {
>+            $branchcode =
>+                ( C4::Context->userenv && defined C4::Context->userenv->{branch} )
>+                ? C4::Context->userenv->{branch}
>+                : $params->{branch};
>+        } else {
>             $branchcode = "";
>         }
>     } else {
>-        $branchcode = ( C4::Context->userenv && defined C4::Context->userenv->{branch} )
>-            ? C4::Context->userenv->{branch} : $params->{branch};
>+        $branchcode =
>+            ( C4::Context->userenv && defined C4::Context->userenv->{branch} )
>+            ? C4::Context->userenv->{branch}
>+            : $params->{branch};
>     }
>     return $branchcode;
> }
>diff --git a/installer/data/mysql/atomicupdate/bug_35386.pl b/installer/data/mysql/atomicupdate/bug_35386.pl
>new file mode 100755
>index 0000000000..8a5b4cffbc
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_35386.pl
>@@ -0,0 +1,17 @@
>+use Modern::Perl;
>+
>+return {
>+    bug_number  => "35386",
>+    description => "Add RESTAPIRenewalBranch preference",
>+    up          => sub {
>+        my ($args) = @_;
>+        my ( $dbh, $out ) = @$args{qw(dbh out)};
>+
>+        $dbh->do(q{
>+            INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES
>+            ('RESTAPIRenewalBranch','apiuserbranch','itemhomebranch|patronhomebranch|checkoutbranch|apiuserbranch|none','Choose how the branch for an API renewal is recorded in statistics','Choice')
>+        });
>+
>+        say $out "Added new system preference 'RESTAPIRenewalBranch'";
>+    },
>+};
>diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql
>index 7295744c4c..87563bfb87 100644
>--- a/installer/data/mysql/mandatory/sysprefs.sql
>+++ b/installer/data/mysql/mandatory/sysprefs.sql
>@@ -638,6 +638,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('RESTOAuth2ClientCredentials','0',NULL,'If enabled, the OAuth2 client credentials flow is enabled for the REST API.','YesNo'),
> ('RESTPublicAnonymousRequests','1',NULL,'If enabled, the API will allow anonymous access to public routes that don\'t require authenticated access.','YesNo'),
> ('RESTPublicAPI','1',NULL,'If enabled, the REST API will expose the /public endpoints.','YesNo'),
>+('RESTAPIRenewalBranch','apiuserbranch','itemhomebranch|patronhomebranch|checkoutbranch|apiuserbranch|none','Choose how the branch for an API renewal is recorded in statistics','Choice'),
> ('RestrictedPageLocalIPs','',NULL,'Beginning of IP addresses considered as local (comma separated ex: "127.0.0,127.0.2")','Free'),
> ('RestrictedPageContent','',NULL,'HTML content of the restricted page','TextArea'),
> ('RestrictedPageTitle','',NULL,'Title of the restricted page (breadcrumb and header)','Free'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref
>index 1fb5c99916..e8cff9a429 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref
>@@ -35,6 +35,17 @@ Web services:
>                   1: Enable
>                   0: "Disable"
>             - "the /public namespace of the API."
>+        -
>+            - Use
>+            - pref: RESTAPIRenewalBranch
>+              choices:
>+                  apiuserbranch: "the branch of the api user"
>+                  itemhomebranch: "the item's home library"
>+                  patronhomebranch: "the patron's home library"
>+                  checkoutbranch: "the library the item was checked out from"
>+                  none: "NULL"
>+                  apirenew: "'APIRenew'"
>+            - as branchcode to store in the statistics table.
>     OAI-PMH:
>         -
>             - pref: OAI-PMH
>diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t
>index 7a21db18f5..f9798aa4a5 100755
>--- a/t/db_dependent/Koha/Item.t
>+++ b/t/db_dependent/Koha/Item.t
>@@ -1019,7 +1019,7 @@ subtest 'deletion' => sub {
> };
> 
> subtest 'renewal_branchcode' => sub {
>-    plan tests => 13;
>+    plan tests => 25;
> 
>     $schema->storage->txn_begin;
> 
>@@ -1063,6 +1063,34 @@ subtest 'renewal_branchcode' => sub {
>     is( $item->renewal_branchcode, $item->homebranch, "If interface opac and OpacRenewalBranch set to itemhomebranch, we get homebranch of item");
>     is( $item->renewal_branchcode({branch=>'MANATEE'}), $item->homebranch, "If interface opac and OpacRenewalBranch set to itemhomebranch, we get homebranch of item even if branch passed");
> 
>+    C4::Context->interface( 'api' );
>+
>+    t::lib::Mocks::mock_preference('RESTAPIRenewalBranch', undef);
>+    is( $item->renewal_branchcode, 'APIRenew', "If interface api and RESTAPIRenewalBranch undef, we get APIRenew");
>+    is( $item->renewal_branchcode({branch=>'COW'}), 'APIRenew', "If interface api and RESTAPIRenewalBranch undef, we get APIRenew even if branch passed");
>+
>+    t::lib::Mocks::mock_preference('RESTAPIRenewalBranch', 'none');
>+    is( $item->renewal_branchcode, '', "If interface api and RESTAPIRenewalBranch is none, we get blank string");
>+    is( $item->renewal_branchcode({branch=>'COW'}), '', "If interface api and RESTAPIRenewalBranch is none, we get blank string even if branch passed");
>+
>+    t::lib::Mocks::mock_preference('RESTAPIRenewalBranch', 'checkoutbranch');
>+    is( $item->renewal_branchcode, $checkout->branchcode, "If interface api and RESTAPIRenewalBranch set to checkoutbranch, we get branch of checkout");
>+    is( $item->renewal_branchcode({branch=>'MONKEY'}), $checkout->branchcode, "If interface api and RESTAPIRenewalBranch set to checkoutbranch, we get branch of checkout even if branch passed");
>+
>+    t::lib::Mocks::mock_preference('RESTAPIRenewalBranch','patronhomebranch');
>+    is( $item->renewal_branchcode, $checkout->patron->branchcode, "If interface api and RESTAPIRenewalBranch set to patronbranch, we get branch of patron");
>+    is( $item->renewal_branchcode({branch=>'TURKEY'}), $checkout->patron->branchcode, "If interface api and RESTAPIRenewalBranch set to patronbranch, we get branch of patron even if branch passed");
>+
>+    t::lib::Mocks::mock_preference('RESTAPIRenewalBranch','itemhomebranch');
>+    is( $item->renewal_branchcode, $item->homebranch, "If interface api and RESTAPIRenewalBranch set to itemhomebranch, we get homebranch of item");
>+    is( $item->renewal_branchcode({branch=>'MANATEE'}), $item->homebranch, "If interface api and RESTAPIRenewalBranch set to itemhomebranch, we get homebranch of item even if branch passed");
>+
>+    t::lib::Mocks::mock_userenv({ branchcode => $branch->branchcode });
>+    t::lib::Mocks::mock_preference('RESTAPIRenewalBranch','apiuserbranch');
>+    is( $item->renewal_branchcode, $branch->branchcode, "If interface api and RESTAPIRenewalBranch set to apiuserbranch, we get branch from userenv");
>+    is( $item->renewal_branchcode({branch=>'MANATEE'}), $branch->branchcode, "If interface api and RESTAPIRenewalBranch set to apiuserbranch, we get branch from userenv even if branch passed");
>+    C4::Context->set_userenv(51, 'userid4tests', undef, 'firstname', 'surname', undef, undef, 0, undef, undef, undef ); #mock userenv doesn't let us set null branch
>+
>     $schema->storage->txn_rollback;
> };
> 
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 35386: 159278 | 159282 | 161244 | 161428