From 862089f6d666c18d777cc4b25d18c2f546af9bbc Mon Sep 17 00:00:00 2001 From: Nick Clemens 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 --- 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 22eaceac948..424a9509ac2 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 00000000000..8a5b4cffbcd --- /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 b58114d802e..3d4ad404824 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -637,6 +637,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 1fb5c999166..bfecdb2341f 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 7a21db18f55..f9798aa4a55 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