From 146bcb63f6c9e8ebeed65520fbf151ad5a47f9d0 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 Signed-off-by: Brendan Lawlor --- 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