From 35c8f5d651e980e325ce86c3545c2efe52d960cd Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 11 Nov 2020 13:26:58 +0000 Subject: [PATCH] Bug 26963: (QA follow-up) 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 Signed-off-by: Tomas Cohen Arazi --- 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 db87a0aaf7e..10d4eb2c481 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 401254404d7..b983cad9348 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.29.2