From 371db0c3c03e3c2c333fccc75b006f2449cfc457 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 20 Apr 2023 15:04:29 -0300 Subject: [PATCH] Bug 33447: Make *->pickup_locations methods throw an exception on missing parameter This patch fixes the FIXME for making the methods throw an exception. Tests are added, and POD is adapted as well. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/patrons* \ t/db_dependent/api/v1/holds.t \ t/db_dependent/Reserves* \ t/db_dependent/Hold* \ t/db_dependent/Koha/Hold* \ t/db_dependent/Items* => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 981eeff7ef1a014b848a52ee9e6e20de106048e3) --- Koha/Biblio.pm | 9 +++++++-- Koha/Biblios.pm | 6 +++++- Koha/Item.pm | 12 +++++++++--- t/db_dependent/Koha/Biblio.t | 15 ++++++++++++--- t/db_dependent/Koha/Biblios.t | 9 ++++++++- t/db_dependent/Koha/Item.t | 11 +++++++++-- 6 files changed, 50 insertions(+), 12 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 5c82b271a3..9819e68e42 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -38,6 +38,7 @@ use Koha::Biblioitems; use Koha::Cache::Memory::Lite; use Koha::Checkouts; use Koha::CirculationRules; +use Koha::Exceptions; use Koha::Item::Transfer::Limits; use Koha::Items; use Koha::Libraries; @@ -240,18 +241,22 @@ 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 and if item can be transferred to each pickup location. -Patron is a required parameter. +Throws a I exception if the B parameter I +is not passed. =cut sub pickup_locations { my ( $self, $params ) = @_; + Koha::Exceptions::MissingParameter->throw( parameter => 'patron' ) + unless exists $params->{patron}; + my $patron = $params->{patron}; my $memory_cache = Koha::Cache::Memory::Lite->get_instance(); diff --git a/Koha/Biblios.pm b/Koha/Biblios.pm index af8de37cb1..2e105214d0 100644 --- a/Koha/Biblios.pm +++ b/Koha/Biblios.pm @@ -42,13 +42,17 @@ Koha::Biblios - Koha Biblio object set class For a given resultset, it returns all the pickup locations. -Patron is a required parameter. +Throws a I exception if the B parameter I +is not passed. =cut sub pickup_locations { my ( $self, $params ) = @_; + Koha::Exceptions::MissingParameter->throw( parameter => 'patron' ) + unless exists $params->{patron}; + my $patron = $params->{patron}; my @pickup_locations; diff --git a/Koha/Item.pm b/Koha/Item.pm index 55c02e31e4..f2e00a9916 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -36,6 +36,9 @@ use Koha::Biblio::ItemGroups; use Koha::Checkouts; use Koha::CirculationRules; use Koha::CoverImages; +use Koha::Exceptions; +use Koha::Exceptions::Checkin; +use Koha::Exceptions::Item::Bundle; use Koha::Exceptions::Item::Transfer; use Koha::Item::Attributes; use Koha::Exceptions::Item::Bundle; @@ -733,20 +736,23 @@ sub can_be_transferred { =head3 pickup_locations -$pickup_locations = $item->pickup_locations( {patron => $patron } ) + my $pickup_locations = $item->pickup_locations({ patron => $patron }) 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. +Throws a I exception if the B parameter I +is not passed. =cut sub pickup_locations { my ($self, $params) = @_; + Koha::Exceptions::MissingParameter->throw( parameter => 'patron' ) + unless exists $params->{patron}; + my $patron = $params->{patron}; - # FIXME We should throw an exception if not passed my $circ_control_branch = C4::Reserves::GetReservesControlBranch( $self->unblessed(), $patron->unblessed ); diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index 085384e78f..bed7047dae 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -17,7 +17,8 @@ use Modern::Perl; -use Test::More tests => 22; # +1 +use Test::More tests => 23; # +1 +use Test::Exception; use Test::Warn; use C4::Biblio qw( AddBiblio ModBiblio ModBiblioMarc ); @@ -214,8 +215,9 @@ subtest 'is_serial() tests' => sub { $schema->storage->txn_rollback; }; -subtest 'pickup_locations' => sub { - plan tests => 9; +subtest 'pickup_locations() tests' => sub { + + plan tests => 11; $schema->storage->txn_begin; @@ -357,6 +359,13 @@ subtest 'pickup_locations' => sub { my $biblio1 = $builder->build_sample_biblio({ title => '1' }); my $biblio2 = $builder->build_sample_biblio({ title => '2' }); + throws_ok + { $biblio1->pickup_locations } + 'Koha::Exceptions::MissingParameter', + 'Exception thrown on missing parameter'; + + is( $@->parameter, 'patron', 'Exception param correctly set' ); + my $item1_1 = $builder->build_sample_item({ biblionumber => $biblio1->biblionumber, homebranch => $library1->branchcode, diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index bf9d36d216..69a18edf23 100755 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -234,7 +234,7 @@ $schema->storage->txn_rollback; subtest 'pickup_locations() tests' => sub { - plan tests => 1; + plan tests => 3; $schema->storage->txn_begin; @@ -286,6 +286,13 @@ subtest 'pickup_locations() tests' => sub { } ); + throws_ok + { $biblios->pickup_locations } + 'Koha::Exceptions::MissingParameter', + 'Exception thrown on missing parameter'; + + is( $@->parameter, 'patron', 'Exception param correctly set' ); + my $library_ids = [ Koha::Libraries->search( { diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index be8fbf61cf..3eb3c0592a 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -404,8 +404,9 @@ subtest "as_marc_field() tests" => sub { Koha::Caches->get_instance->clear_from_cache( "MarcStructure-1-" ); }; -subtest 'pickup_locations' => sub { - plan tests => 66; +subtest 'pickup_locations() tests' => sub { + + plan tests => 68; $schema->storage->txn_begin; @@ -457,6 +458,12 @@ subtest 'pickup_locations' => sub { } ); + throws_ok + { $item1->pickup_locations } + 'Koha::Exceptions::MissingParameter', + 'Exception thrown on missing parameter'; + + is( $@->parameter, 'patron', 'Exception param correctly set' ); my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library1->branchcode, firstname => '1' } } ); my $patron4 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library4->branchcode, firstname => '4' } } ); -- 2.39.5