Bugzilla – Attachment 149968 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.54 KB, created by
Nick Clemens
on 2023-04-20 15:46:23 UTC
(
hide
)
Description:
Bug 33447: (follow-up) Fix tests and make assumption explicit
Filename:
MIME Type:
Creator:
Nick Clemens
Created:
2023-04-20 15:46:23 UTC
Size:
4.54 KB
patch
obsolete
>From 492c72ec5a378e260013e9b470936971c2feb7d2 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 | 5 +++-- > Koha/Biblios.pm | 4 +++- > Koha/Item.pm | 11 ++++++----- > t/db_dependent/Koha/Biblios.t | 4 +++- > 4 files changed, 15 insertions(+), 9 deletions(-) > >diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm >index a94665ddc8..3801828612 100644 >--- a/Koha/Biblio.pm >+++ b/Koha/Biblio.pm >@@ -258,8 +258,9 @@ sub can_be_transferred { > 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 cf1cbdb83a..af8de37cb1 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 0d29655938..5ed04bcca4 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 b93efebb21..bf9d36d216 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.30.2
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