From a815cdaa97849259c68fd6688a139fff4ae98d70 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 11 Nov 2020 13:26:58 +0000 Subject: [PATCH] Bug 26963: [GOLF] Fix up tests and cover case of undefined ccode While this technically shouldn't happen, if a library creates itemtype limits, then flips the pref, those rules are still in the db until a ccode rule is saved. To be extra safe, when checking for rules of one type, we should make sure the other type is undef - i.e. if looking for ccode rules, we should confirm the itype is undef for those rules Additionally, we shouldn't set the barcode now that we are not deleting all items, so we use copynumber for our item identification field as it doesn't need to be unique in the DB Signed-off-by: Nick Clemens --- Koha/Item.pm | 10 ++++++++-- t/db_dependent/Koha/Item.t | 24 ++++++++++++++++++------ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index db87a0aaf7..10d4eb2c48 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -599,11 +599,17 @@ sub pickup_locations { ) unless C4::Context->preference('UseBranchTransferLimits'); my $limittype = C4::Context->preference('BranchTransferLimitsType'); - my $limiter = $limittype eq 'itemtype' ? $self->effective_itemtype : $self->ccode; + my ($ccode, $itype) = (undef, undef); + if( $limittype eq 'ccode' ){ + $ccode = $self->ccode; + } else { + $itype = $self->itype; + } my $limits = Koha::Item::Transfer::Limits->search( { fromBranch => $self->holdingbranch, - $limittype => $limiter + ccode => $ccode, + itemtype => $itype, }, { columns => ['toBranch'] } ); diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 401254404d..b983cad934 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -178,7 +178,7 @@ subtest 'pickup_locations' => sub { { homebranch => $library1->branchcode, holdingbranch => $library2->branchcode, - barcode => '1', + copynumber => 1, ccode => 'Gollum' } )->store; @@ -187,7 +187,7 @@ subtest 'pickup_locations' => sub { { homebranch => $library3->branchcode, holdingbranch => $library4->branchcode, - barcode => '3', + copynumber => 3, itype => $item1->itype, } )->store; @@ -208,7 +208,7 @@ subtest 'pickup_locations' => sub { my $patron4 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library4->branchcode, firstname => '4' } } ); my $all_count = Koha::Libraries->search({ pickup_location => 1})->count(); - plan tests => ($all_count +1) * 7 + 31 + 60; + plan tests => ($all_count +1) * 7 + 31 + 61; my $results = { "1-1-1-any" => $all_count, @@ -295,13 +295,13 @@ subtest 'pickup_locations' => sub { } ok( scalar(@pl) == $results->{ - $item->barcode . '-' + $item->copynumber . '-' . $patron->firstname . '-' . $ha . '-' . $hfp }, 'item' - . $item->barcode + . $item->copynumber . ', patron' . $patron->firstname . ', holdallowed: ' @@ -310,7 +310,7 @@ subtest 'pickup_locations' => sub { . $hfp . ' should return ' . $results->{ - $item->barcode . '-' + $item->copynumber . '-' . $patron->firstname . '-' . $ha . '-' . $hfp @@ -334,6 +334,15 @@ subtest 'pickup_locations' => sub { } # Now test that branchtransferlimits will further filter the pickup locations + + my $item_no_ccode = $builder->build_sample_item( + { + homebranch => $library1->branchcode, + holdingbranch => $library2->branchcode, + itype => $item1->itype, + } + )->store; + t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1); t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); Koha::CirculationRules->set_rules( @@ -381,6 +390,9 @@ subtest 'pickup_locations' => sub { $pickup_locations = $item1->pickup_locations( { patron => $patron1 } )->as_list; is( scalar @$pickup_locations, $all_count, "With no transfer limits of type ccode we get back the libraries that are pickup locations"); + $pickup_locations = $item_no_ccode->pickup_locations( { patron => $patron1 } )->as_list; + is( scalar @$pickup_locations, $all_count, "With no transfer limits of type ccode and an item with no ccode we get back the libraries that are pickup locations"); + $builder->build_object( { class => 'Koha::Item::Transfer::Limits', -- 2.11.0