Bugzilla – Attachment 149981 Details for
Bug 33447
Add caching to Biblio->pickup_locations
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33447: (follow-up) Fix tests and make assumption explicit
Bug-33447-follow-up-Fix-tests-and-make-assumption-.patch (text/plain), 4.65 KB, created by
Tomás Cohen Arazi (tcohen)
on 2023-04-20 18:08:40 UTC
(
hide
)
Description:
Bug 33447: (follow-up) Fix tests and make assumption explicit
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2023-04-20 18:08:40 UTC
Size:
4.65 KB
patch
obsolete
>From 0293c773c5a45d72d1250e37bd06600fa550d475 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Thu, 20 Apr 2023 15:44:53 +0000 >Subject: [PATCH] Bug 33447: (follow-up) Fix tests and make assumption explicit > >The patches made an assumption that patron would always be passed. It is >within Koha, but not in the Biblios tests. > >There is no scenario where we can determine pickup locations that are not in >reference to a patron (who is picking it up?) so we should always have >this parameter >--- > Koha/Biblio.pm | 7 ++++--- > Koha/Biblios.pm | 4 +++- > Koha/Item.pm | 11 ++++++----- > t/db_dependent/Koha/Biblios.t | 4 +++- > 4 files changed, 16 insertions(+), 10 deletions(-) > >diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm >index a94665ddc81..cb898f4bc3e 100644 >--- a/Koha/Biblio.pm >+++ b/Koha/Biblio.pm >@@ -255,11 +255,12 @@ sub can_be_transferred { > > =head3 pickup_locations > >- my $pickup_locations = $biblio->pickup_locations( {patron => $patron } ); >+ my $pickup_locations = $biblio->pickup_locations( { patron => $patron } ); > > Returns a Koha::Libraries set of possible pickup locations for this biblio's items, >-according to patron's home library (if patron is defined and holds are allowed >-only from hold groups) and if item can be transferred to each pickup location. >+according to patron's home library and if item can be transferred to each pickup location. >+ >+Patron is a required parameter. > > =cut > >diff --git a/Koha/Biblios.pm b/Koha/Biblios.pm >index cf1cbdb83ad..af8de37cb1b 100644 >--- a/Koha/Biblios.pm >+++ b/Koha/Biblios.pm >@@ -40,7 +40,9 @@ Koha::Biblios - Koha Biblio object set class > my $biblios = Koha::Biblios->search(...); > my $pickup_locations = $biblios->pickup_locations({ patron => $patron }); > >-For a given resultset, it returns all the pickup locations >+For a given resultset, it returns all the pickup locations. >+ >+Patron is a required parameter. > > =cut > >diff --git a/Koha/Item.pm b/Koha/Item.pm >index 0d29655938d..5ed04bcca44 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -736,25 +736,26 @@ sub can_be_transferred { > > $pickup_locations = $item->pickup_locations( {patron => $patron } ) > >-Returns possible pickup locations for this item, according to patron's home library (if patron is defined and holds are allowed only from hold groups) >+Returns possible pickup locations for this item, according to patron's home library > and if item can be transferred to each pickup location. > >+Patron parameter is required. >+ > =cut > > sub pickup_locations { > my ($self, $params) = @_; > > my $patron = $params->{patron}; >+ # FIXME We should throw an exception if not passed > > my $circ_control_branch = > C4::Reserves::GetReservesControlBranch( $self->unblessed(), $patron->unblessed ); > my $branchitemrule = > C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype ); > >- if(defined $patron) { >- return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_local_hold_group' && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} ); >- return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_home_library' && $self->home_branch->branchcode ne $patron->branchcode; >- } >+ return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_local_hold_group' && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} ); >+ return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_home_library' && $self->home_branch->branchcode ne $patron->branchcode; > > my $pickup_libraries = Koha::Libraries->search(); > if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup') { >diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t >index b93efebb212..bf9d36d216f 100755 >--- a/t/db_dependent/Koha/Biblios.t >+++ b/t/db_dependent/Koha/Biblios.t >@@ -278,6 +278,8 @@ subtest 'pickup_locations() tests' => sub { > my $item_2_1 = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber }); > my $item_2_2 = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber }); > >+ my $patron = $builder->build_object({ class => 'Koha::Patrons' }); >+ > my $biblios = Koha::Biblios->search( > { > biblionumber => [ $biblio_1->biblionumber, $biblio_2->biblionumber ] >@@ -298,7 +300,7 @@ subtest 'pickup_locations() tests' => sub { > ]; > > my $pickup_locations_ids = [ >- $biblios->pickup_locations->_resultset->get_column('branchcode')->all >+ $biblios->pickup_locations({ patron => $patron })->_resultset->get_column('branchcode')->all > ]; > > is_deeply( >-- >2.34.1
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 33447
:
149295
|
149296
|
149389
|
149390
|
149790
|
149791
|
149792
|
149968
| 149981 |
149982