From d52da8e6969bb8d8c14f865d53f8e0293b077af0 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 18 Dec 2015 14:59:02 +0000 Subject: [PATCH] Bug 14598 [QA Followup] - Correct the behavior of GetItem Currently GetItem sets itemtype to the biblio itemtype if no item level itemtype exists. Instead, it should only do this if item_level-itypes is not set. --- C4/Items.pm | 38 ++++++++-------------- t/db_dependent/Items.t | 84 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 77 insertions(+), 45 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index a7db6ef..9914376 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -37,6 +37,7 @@ use Data::Dumper; # used as part of logging item record changes, not just for use Koha::DateUtils qw/dt_from_string/; use Koha::Database; +use Koha::Items; use vars qw($VERSION @ISA @EXPORT); @@ -149,38 +150,25 @@ names to values. If C<$serial> is true, include serial publication data. sub GetItem { my ($itemnumber,$barcode, $serial) = @_; my $dbh = C4::Context->dbh; - my $data; + my $item; if ($itemnumber) { - my $sth = $dbh->prepare(" - SELECT * FROM items - WHERE itemnumber = ?"); - $sth->execute($itemnumber); - $data = $sth->fetchrow_hashref; + $item = Koha::Items->find( $itemnumber ); } else { - my $sth = $dbh->prepare(" - SELECT * FROM items - WHERE barcode = ?" - ); - $sth->execute($barcode); - $data = $sth->fetchrow_hashref; + $item = Koha::Item->find( { barcode => $barcode } ); } - return unless ( $data ); + return unless ( $item ); + + my $data = $item->unblessed(); + $data->{itype} = $item->effective_itemtype(); # set the correct itype - if ( $serial) { - my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=?"); - $ssth->execute($data->{'itemnumber'}) ; - ($data->{'serialseq'} , $data->{'publisheddate'}) = $ssth->fetchrow_array(); + if ($serial) { + my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=?"); + $ssth->execute( $data->{'itemnumber'} ); + ( $data->{'serialseq'}, $data->{'publisheddate'} ) = $ssth->fetchrow_array(); } - #if we don't have an items.itype, use biblioitems.itemtype. - # FIXME this should respect the itypes systempreference - # if (C4::Context->preference('item-level_itypes')) { - if( ! $data->{'itype'} ) { - my $sth = $dbh->prepare("SELECT itemtype FROM biblioitems WHERE biblionumber = ?"); - $sth->execute($data->{'biblionumber'}); - ($data->{'itype'}) = $sth->fetchrow_array; - } + return $data; } # sub GetItem diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 35755c6..70cdf47 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -40,7 +40,7 @@ my $location = 'My Location'; subtest 'General Add, Get and Del tests' => sub { - plan tests => 14; + plan tests => 16; $schema->storage->txn_begin; @@ -48,13 +48,16 @@ subtest 'General Add, Get and Del tests' => sub { my $library = $builder->build({ source => 'Branch', }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); # Create a biblio instance for testing C4::Context->set_preference('marcflavour', 'MARC21'); my ($bibnum, $bibitemnum) = get_biblio(); # Add an item. - my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location } , $bibnum); + my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, itype => $itemtype->{itemtype} } , $bibnum); cmp_ok($item_bibnum, '==', $bibnum, "New item is linked to correct biblionumber."); cmp_ok($item_bibitemnum, '==', $bibitemnum, "New item is linked to correct biblioitemnumber."); @@ -75,7 +78,7 @@ subtest 'General Add, Get and Del tests' => sub { my $getdeleted = GetItem($itemnumber); is($getdeleted->{'itemnumber'}, undef, "Item deleted as expected."); - ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, permanent_location => 'my permanent location' } , $bibnum); + ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, permanent_location => 'my permanent location', itype => $itemtype->{itemtype} } , $bibnum); $getitem = GetItem($itemnumber); is( $getitem->{location}, $location, "The location should not have been modified" ); is( $getitem->{permanent_location}, 'my permanent location', "The permanent_location should not have modified" ); @@ -90,6 +93,13 @@ subtest 'General Add, Get and Del tests' => sub { is( $getitem->{location}, 'CART', "The location should have been set to CART" ); is( $getitem->{permanent_location}, $location, "The permanent_location should not have been set to CART" ); + C4::Context->set_preference('item-level_itypes', '1'); + $getitem = GetItem($itemnumber); + is( $getitem->{itype}, $itemtype->{itemtype}, "Itemtype set correctly when using item_level-itypes" ); + C4::Context->set_preference('item-level_itypes', '0'); + $getitem = GetItem($itemnumber); + is( $getitem->{itype}, undef, "Itemtype set correctly when not using item_level-itypes" ); + $schema->storage->txn_rollback; }; @@ -109,23 +119,32 @@ subtest 'GetHiddenItemnumbers tests' => sub { my $library2 = $builder->build({ source => 'Branch', }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); # Create a new biblio C4::Context->set_preference('marcflavour', 'MARC21'); my ($biblionumber, $biblioitemnumber) = get_biblio(); # Add two items - my ($item1_bibnum, $item1_bibitemnum, $item1_itemnumber) = AddItem( - { homebranch => $library1->{branchcode}, - holdingbranch => $library1->{branchcode}, - withdrawn => 1 }, - $biblionumber + my ( $item1_bibnum, $item1_bibitemnum, $item1_itemnumber ) = AddItem( + { + homebranch => $library1->{branchcode}, + holdingbranch => $library1->{branchcode}, + withdrawn => 1, + itype => $itemtype->{itemtype}, + }, + $biblionumber ); - my ($item2_bibnum, $item2_bibitemnum, $item2_itemnumber) = AddItem( - { homebranch => $library2->{branchcode}, - holdingbranch => $library2->{branchcode}, - withdrawn => 0 }, - $biblionumber + my ( $item2_bibnum, $item2_bibitemnum, $item2_itemnumber ) = AddItem( + { + homebranch => $library2->{branchcode}, + holdingbranch => $library2->{branchcode}, + withdrawn => 0, + itype => $itemtype->{itemtype}, + }, + $biblionumber ); my $opachiddenitems; @@ -192,15 +211,21 @@ subtest 'GetItemsInfo tests' => sub { my $library2 = $builder->build({ source => 'Branch', }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); # Add a biblio my ($biblionumber, $biblioitemnumber) = get_biblio(); # Add an item - my ($item_bibnum, $item_bibitemnum, $itemnumber) - = AddItem({ - homebranch => $library1->{branchcode}, - holdingbranch => $library2->{branchcode}, - }, $biblionumber ); + my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem( + { + homebranch => $library1->{branchcode}, + holdingbranch => $library2->{branchcode}, + itype => $itemtype->{itemtype}, + }, + $biblionumber + ); my $branch = GetBranchDetail( $library1->{branchcode} ); $branch->{ opac_info } = "homebranch OPAC info"; @@ -279,6 +304,9 @@ subtest 'SearchItems test' => sub { my $library2 = $builder->build({ source => 'Branch', }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); C4::Context->set_preference('marcflavour', 'MARC21'); my $cpl_items_before = SearchItemsByField( 'homebranch', $library1->{branchcode}); @@ -291,10 +319,12 @@ subtest 'SearchItems test' => sub { my (undef, undef, $item1_itemnumber) = AddItem({ homebranch => $library1->{branchcode}, holdingbranch => $library1->{branchcode}, + itype => $itemtype->{itemtype}, }, $biblionumber); my (undef, undef, $item2_itemnumber) = AddItem({ homebranch => $library2->{branchcode}, holdingbranch => $library2->{branchcode}, + itype => $itemtype->{itemtype}, }, $biblionumber); my ($items, $total_results); @@ -442,11 +472,21 @@ subtest 'Koha::Item(s) tests' => sub { my $library2 = $builder->build({ source => 'Branch', }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); # Create a biblio and item for testing C4::Context->set_preference('marcflavour', 'MARC21'); my ($bibnum, $bibitemnum) = get_biblio(); - my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library1->{branchcode}, holdingbranch => $library2->{branchcode} } , $bibnum); + my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem( + { + homebranch => $library1->{branchcode}, + holdingbranch => $library2->{branchcode}, + itype => $itemtype->{itemtype}, + }, + $bibnum + ); # Get item. my $item = Koha::Items->find( $itemnumber ); @@ -475,6 +515,9 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { my $library2 = $builder->build({ source => 'Branch', }); + my $itemtype = $builder->build({ + source => 'Itemtype', + }); my ( $biblionumber, $biblioitemnumber ) = get_biblio(); my $item_infos = [ @@ -496,7 +539,8 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { my ( undef, undef, $itemnumber ) = AddItem( { homebranch => $item_info->{homebranch}, - holdingbranch => $item_info->{holdingbanch} + holdingbranch => $item_info->{holdingbanch}, + itype => $itemtype->{itemtype}, }, $biblionumber ); -- 1.7.10.4