Attachment 49803 Details for Bug 14598 – Bug 14598 [QA Followup] - Correct the behavior of GetItem

[patch] Bug 14598 [QA Followup] - Correct the behavior of GetItem

Bug-14598-QA-Followup---Correct-the-behavior-of-Ge.patch (text/plain), 10.66 KB, created by Tomás Cohen Arazi (tcohen) on 2016-04-01 15:21:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2016-04-01 15:21:34 UTC

Size: 10.66 KB

patch

obsolete

>From d8e280bdcdd928466d217cb8c198a761d43a94c9 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
>
>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>
>---
> 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 a0dfb29..0368717 100644
>--- a/C4/Items.pm
>+++ b/C4/Items.pm
>@@ -38,6 +38,7 @@ use Koha::DateUtils qw/dt_from_string/;
> use Koha::Database;
> 
> use Koha::Database;
>+use Koha::Items;
> 
> use vars qw(@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::Items->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 5df6eb5..e1a52a2 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");
>@@ -279,6 +304,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});
>@@ -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
>     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 );
>@@ -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
>         );
>-- 
>2.7.4

Actions: View | Diff | Splinter Review

Attachments on bug 14598: 41198 | 41199 | 41200 | 41201 | 41202 | 41203 | 41204 | 41205 | 41383 | 41384 | 41385 | 41627 | 41628 | 41629 | 41637 | 42171 | 42177 | 44665 | 44666 | 44667 | 44668 | 45030 | 45047 | 45838 | 45843 | 45844 | 46546 | 46877 | 47108 | 47109 | 47110 | 47111 | 47112 | 47113 | 47114 | 47115 | 47116 | 49797 | 49798 | 49799 | 49800 | 49801 | 49802 | 49803 | 49804 | 49805 | 49807 | 49963 | 49964 | 49965 | 49966 | 49967 | 49968 | 49969 | 49970 | 49971 | 49972 | 49978 | 49979 | 49980 | 49981 | 49982 | 49983 | 49984 | 49985 | 49986 | 49987 | 49988 | 50018 | 56941 | 56942 | 56943 | 56944 | 56945 | 56946 | 56947 | 56948 | 56949 | 56950 | 56951 | 56952 | 56953 | 62539 | 62555 | 62682