From 986765f3a0054923aafe9ac32d1f8b2d668a42b9 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Fri, 18 Dec 2015 14:59:02 +0000
Subject: [PATCH] Bug 14598 [QA Followup] - Correct the behavior of GetItem
Content-Type: text/plain; charset=utf-8

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.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 C4/Items.pm            |   45 +++++++++-----------------
 t/db_dependent/Items.t |   84 ++++++++++++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 49 deletions(-)

diff --git a/C4/Items.pm b/C4/Items.pm
index baafdc2..0fc7f10 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -156,38 +156,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;
-    }
-
-    return unless ( $data );
-
-    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;
-	}
+        $item = Koha::Items->find( { barcode => $barcode } );
+    }
+
+    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();
+    }
+
     return $data;
 }    # sub GetItem
 
diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t
index 3e886e2..e3c00c8 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
     t::lib::Mocks::mock_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
     t::lib::Mocks::mock_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 $library = Koha::Libraries->find( $library1->{branchcode} );
     $library->opac_info("homebranch OPAC info");
@@ -274,6 +299,9 @@ subtest 'SearchItems test' => sub {
     my $library2 = $builder->build({
         source => 'Branch',
     });
+    my $itemtype = $builder->build({
+        source => 'Itemtype',
+    });
 
     t::lib::Mocks::mock_preference('marcflavour', 'MARC21');
     my $cpl_items_before = SearchItemsByField( 'homebranch', $library1->{branchcode});
@@ -286,10 +314,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);
@@ -443,11 +473,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
     t::lib::Mocks::mock_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 );
@@ -476,6 +516,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 = [
@@ -497,7 +540,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