Bugzilla – Attachment 101239 Details for
Bug 24759
OpacRenewalBranch code should be a Koha::Item method
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24759: Move OpacRenewalBranch code to Koha::Item
Bug-24759-Move-OpacRenewalBranch-code-to-KohaItem.patch (text/plain), 8.45 KB, created by
Andrew Fuerste-Henry
on 2020-03-20 15:11:26 UTC
(
hide
)
Description:
Bug 24759: Move OpacRenewalBranch code to Koha::Item
Filename:
MIME Type:
Creator:
Andrew Fuerste-Henry
Created:
2020-03-20 15:11:26 UTC
Size:
8.45 KB
patch
obsolete
>From 86a2b89a5a7aa783cc77ea44d18fddd49b5a9d32 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Fri, 28 Feb 2020 13:09:29 +0000 >Subject: [PATCH] Bug 24759: Move OpacRenewalBranch code to Koha::Item > >This patchset moves all code to calculate the correct renewal branch into Koha::Item.pm > >When interface is opac we follow the syspref, otherwise we use the current userenv, or pass through >a defined branch > >To test: >1 - Check out an item to a patron >2 - Set allowed renewals in the circ rules to 100 (just so you can keep testing) >3 - Renew the item in staff interface, confirm it is recorded correctly in statistics table (as signed in branch) >4 - Renew via the opac, testing with each setting of OpacRenewalbranch >5 - prove -v t/db_dependent/Koha/Item.t > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >--- > C4/Circulation.pm | 6 +----- > Koha/Item.pm | 39 ++++++++++++++++++++++++++++++++++ > opac/opac-renew.pl | 21 +----------------- > t/db_dependent/Koha/Item.t | 53 +++++++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 93 insertions(+), 26 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 2c75ec68c4..0013e17b17 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -3049,14 +3049,10 @@ sub AddRenewal { > DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); > } > >- unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl >- $branch = ( C4::Context->userenv && defined C4::Context->userenv->{branch} ) ? C4::Context->userenv->{branch} : $branch; >- } >- > # Add the renewal to stats > UpdateStats( > { >- branch => $branch, >+ branch => $item_object->renewalbranch({branch => $branch}), > type => 'renew', > amount => $charge, > itemnumber => $itemnumber, >diff --git a/Koha/Item.pm b/Koha/Item.pm >index d2b1b9d154..11866921ce 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -490,6 +490,45 @@ sub as_marc_field { > return $field; > } > >+=head3 renewalbranch >+ >+Returns the branch to be recorded in statistics renewal of the item >+ >+=cut >+ >+sub renewalbranch { >+ >+ my ($self, $params ) = @_; >+ >+ my $interface = C4::Context->interface; >+ my $branchcode; >+ if ( $interface eq 'opac' ){ >+ my $renewalbranch = C4::Context->preference('OpacRenewalBranch'); >+ if( !defined $renewalbranch ){ >+ $branchcode = 'OPACRenew'; >+ } >+ elsif ( $renewalbranch eq 'itemhomebranch' ) { >+ $branchcode = $self->homebranch; >+ } >+ elsif ( $renewalbranch eq 'patronhomebranch' ) { >+ $branchcode = $self->checkout->patron->branchcode; >+ } >+ elsif ( $renewalbranch eq 'checkoutbranch' ) { >+ $branchcode = $self->checkout->branchcode; >+ } >+ elsif ( $renewalbranch eq 'NULL' ) { >+ $branchcode = ''; >+ } >+ else { >+ $branchcode = 'OPACRenew'; >+ } >+ } else { >+ $branchcode = ( C4::Context->userenv && defined C4::Context->userenv->{branch} ) >+ ? C4::Context->userenv->{branch} : $params->{branch}; >+ } >+ return $branchcode; >+} >+ > =head3 to_api_mapping > > This method returns the mapping for representing a Koha::Item object >diff --git a/opac/opac-renew.pl b/opac/opac-renew.pl >index 85570d54ad..cb9043d9ad 100755 >--- a/opac/opac-renew.pl >+++ b/opac/opac-renew.pl >@@ -63,26 +63,7 @@ else { > my ( $status, $error ) = > CanBookBeRenewed( $borrowernumber, $itemnumber ); > if ( $status == 1 && $opacrenew == 1 ) { >- my $renewalbranch = C4::Context->preference('OpacRenewalBranch'); >- my $branchcode; >- if ( $renewalbranch eq 'itemhomebranch' ) { >- my $item = Koha::Items->find($itemnumber); >- $branchcode = $item->homebranch; >- } >- elsif ( $renewalbranch eq 'patronhomebranch' ) { >- $branchcode = Koha::Patrons->find( $borrowernumber )->branchcode; >- } >- elsif ( $renewalbranch eq 'checkoutbranch' ) { >- my $issue = GetOpenIssue($itemnumber); # FIXME Should not be $item->checkout? >- $branchcode = $issue->{'branchcode'}; >- } >- elsif ( $renewalbranch eq 'NULL' ) { >- $branchcode = ''; >- } >- else { >- $branchcode = 'OPACRenew'; >- } >- AddRenewal( $borrowernumber, $itemnumber, $branchcode, undef, undef ); >+ AddRenewal( $borrowernumber, $itemnumber, undef, undef, undef ); > push( @renewed, $itemnumber ); > } > else { >diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t >index 3a2d2a8c67..32364879b9 100644 >--- a/t/db_dependent/Koha/Item.t >+++ b/t/db_dependent/Koha/Item.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 4; >+use Test::More tests => 5; > > use C4::Biblio; > >@@ -329,3 +329,54 @@ subtest 'pickup_locations' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'renewalbranch' => sub { >+ plan tests => 15; >+ >+ $schema->storage->txn_begin; >+ >+ my $item = $builder->build_sample_item(); >+ my $branch = $builder->build_object({ class => 'Koha::Libraries' }); >+ my $checkout = $builder->build_object({ >+ class => 'Koha::Checkouts', >+ value => { >+ itemnumber => $item->itemnumber, >+ } >+ }); >+ >+ >+ C4::Context->interface( 'intranet' ); >+ t::lib::Mocks::mock_userenv({ branchcode => $branch->branchcode }); >+ >+ is( $item->renewalbranch, $branch->branchcode, "If interface not opac, we get the branch from context"); >+ is( $item->renewalbranch({ branch => "PANDA"}), $branch->branchcode, "If interface not opac, we get the branch from context even if we pass one in"); >+ C4::Context->set_userenv(51, 'userid4tests', undef, 'firstname', 'surname', undef, undef, 0, undef, undef, undef ); #mock userenv doesn't let us set null branch >+ is( $item->renewalbranch({ branch => "PANDA"}), "PANDA", "If interface not opac, we get the branch we pass one in if context not set"); >+ >+ C4::Context->interface( 'opac' ); >+ >+ t::lib::Mocks::mock_preference('OpacRenewalBranch', ''); >+ is( $item->renewalbranch, 'OPACRenew', "If interface opac and OpacRenewalBranch blank, we get the OPACRenew"); >+ is( $item->renewalbranch({branch=>'CHICKEN'}), 'OPACRenew', "If interface opac and OpacRenewalBranch blank, we get the OPACRenew even if branch passes"); >+ >+ t::lib::Mocks::mock_preference('OpacRenewalBranch', undef); >+ is( $item->renewalbranch, 'OPACRenew', "If interface opac and OpacRenewalBranch undef, we get OPACRenew"); >+ is( $item->renewalbranch({branch=>'COW'}), 'OPACRenew', "If interface opac and OpacRenewalBranch undef, we get OPACRenew even if branch passed"); >+ >+ t::lib::Mocks::mock_preference('OpacRenewalBranch', 'NULL'); >+ is( $item->renewalbranch, '', "If interface opac and OpacRenewalBranch is string 'NULL', we get blank string"); >+ is( $item->renewalbranch({branch=>'COW'}), '', "If interface opac and OpacRenewalBranch is string 'NULL', we get blank string even if branch passed"); >+ >+ t::lib::Mocks::mock_preference('OpacRenewalBranch', 'checkoutbranch'); >+ is( $item->renewalbranch, $checkout->branchcode, "If interface opac and OpacRenewalBranch set to checkoutbranch, we get branch of checkout"); >+ is( $item->renewalbranch({branch=>'MONKEY'}), $checkout->branchcode, "If interface opac and OpacRenewalBranch set to checkoutbranch, we get branch of checkout even if branch passed"); >+ >+ t::lib::Mocks::mock_preference('OpacRenewalBranch','patronhomebranch'); >+ is( $item->renewalbranch, $checkout->patron->branchcode, "If interface opac and OpacRenewalBranch set to patronbranch, we get branch of patron"); >+ is( $item->renewalbranch({branch=>'TURKEY'}), $checkout->patron->branchcode, "If interface opac and OpacRenewalBranch set to patronbranch, we get branch of patron even if branch passed"); >+ >+ t::lib::Mocks::mock_preference('OpacRenewalBranch','itemhomebranch'); >+ is( $item->renewalbranch, $item->homebranch, "If interface opac and OpacRenewalBranch set to itemhomebranch, we get homebranch of item"); >+ is( $item->renewalbranch({branch=>'MANATEE'}), $item->homebranch, "If interface opac and OpacRenewalBranch set to itemhomebranch, we get homebranch of item even if branch passed"); >+ >+}; >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24759
:
99763
|
99764
|
99780
|
99953
|
100002
|
100003
|
101238
|
101239
|
101240
|
101241
|
101242
|
101243
|
101244
|
101245
|
101285
|
101286
|
101287
|
101288
|
101289
|
101290
|
101291