From 981eeff7ef1a014b848a52ee9e6e20de106048e3 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 --- Koha/Biblio.pm | 9 +++++++-- Koha/Biblios.pm | 6 +++++- Koha/Item.pm | 10 +++++++--- t/db_dependent/Koha/Biblio.t | 13 +++++++++++-- t/db_dependent/Koha/Biblios.t | 9 ++++++++- t/db_dependent/Koha/Item.t | 11 +++++++++-- 6 files changed, 47 insertions(+), 11 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index cb898f4bc3..82f4c0926b 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; @@ -255,18 +256,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 5ed04bcca4..7251a8a094 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -36,6 +36,7 @@ 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; @@ -734,20 +735,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 1c4311b109..ef41c7636a 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -18,6 +18,7 @@ use Modern::Perl; 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 d4fbac7f8f..75edcb4275 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -443,8 +443,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; @@ -496,6 +497,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.2