From 528ce76f644617993ed5eb7d6b72e162191c7d4b Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 9 Aug 2017 16:30:03 +0200 Subject: [PATCH] [SIGNED-OFF] [SIGNED-OFF] Bug 19004: Adjust AddReturn for retrieving item type In the regular situation, you can get itemtype via biblio and then biblioitem as well as via biblioitem (at least when item-level_itypes is set to biblio). But since Koha unfortunately defined two relations in item, one for biblioitemnumber (the good one) and one for biblionumber (redundant), TestBuilder (correctly) builds one biblioitem and two biblios. If you item-level_itypes to biblio record, this will result in failing tests when calling AddReturn (in this case Koha/Patrons.t). It will crash on: Can't call method "itemtype" on an undefined value at C4/Circulation.pm line 1826. Cause: AddReturn goes via the biblionumber to biblio and than to biblioitems, and it does not find a biblioitem. (Not a fault from TestBuilder but a database design problem.) This patch makes a small change in AddReturn to retrieve the itemtype via biblioitem. It actually is a shorter road than items->biblio->biblioitems. Note: I do not test the Biblioitems->find call, since we already checked the GetItem call before and we have a foreign key constraint. I did not call $item->effective_itemtype since we still use GetItem; this could be done later. Adjusted Circulation/Returns.t too: If we add an item with TestBuilder and we called AddBiblio before, we should link biblioitemnumber as well. Test plan: [1] Do not apply this patch yet. [2] Set item-level_itypes to biblio record. [3] Run t/db_dependent/Koha/Patrons.t. (It should fail.) [4] Apply this patch. [5] Run t/db_dependent/Koha/Patrons.t again. [6] Run t/db_dependent/Circulation/Returns.t [7] Git grep on AddReturn and run a few other tests calling it. Note: Bugs 19070/19071 address three tests that call AddReturn too. [8] In the interface, check in a book. Signed-off-by: Marcel de Rooy Note: Bugs 19070 and 19071 are already pushed. The command in comment #4 has all the tests successful. Signed-off-by: Mark Tompsett --- C4/Circulation.pm | 8 ++++---- t/db_dependent/Circulation/Returns.t | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index fba723f..b0e26b9 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -41,6 +41,7 @@ use Algorithm::CheckDigits; use Data::Dumper; use Koha::Account; use Koha::AuthorisedValues; +use Koha::Biblioitems; use Koha::DateUtils; use Koha::Calendar; use Koha::Checkouts; @@ -1820,10 +1821,9 @@ sub AddReturn { } my $itemnumber = $item->{ itemnumber }; - - my $item_level_itypes = C4::Context->preference("item-level_itypes"); - my $biblio = $item_level_itypes ? undef : Koha::Biblios->find( $item->{ biblionumber } ); # don't get bib data unless we need it - my $itemtype = $item_level_itypes ? $item->{itype} : $biblio->biblioitem->itemtype; + my $itemtype = C4::Context->preference("item-level_itypes") + ? $item->{itype} + : Koha::Biblioitems->find( $item->{biblioitemnumber} )->itemtype; my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ); if ( $issue ) { diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index 4135d66..a90f623 100644 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -202,7 +202,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { t::lib::Mocks::mock_preference( "IssueLog", 1 ); t::lib::Mocks::mock_preference( "ReturnLog", 1 ); - # Set item-level item types + # Set biblio level item types t::lib::Mocks::mock_preference( "item-level_itypes", 0 ); # Create an itemtype for biblio-level item type @@ -234,6 +234,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { source => 'Item', value => { biblionumber => $biblionumber, + biblioitemnumber => $biblioitemnumber, homebranch => $branch, holdingbranch => $branch, itype => $ilevel_itemtype @@ -243,6 +244,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { source => 'Item', value => { biblionumber => $biblionumber, + biblioitemnumber => $biblioitemnumber, homebranch => $branch, holdingbranch => $branch, itype => undef -- 2.1.4