From b2d628ee76624be8d6a7369ddc7f55531c13c597 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 cb898f4bc3e..82f4c0926b3 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 af8de37cb1b..2e105214d0b 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 5ed04bcca44..7251a8a094f 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 1c4311b1094..ef41c7636ad 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 bf9d36d216f..69a18edf230 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 d4fbac7f8f3..75edcb42754 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.34.1