From 796d117791501ea32e2a9cf2060be73dbd964580 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 Signed-off-by: Martin Renvoize --- 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 ee483ef3e0..ad1e9a70ed 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.20.1