Bug 18256: Koha::Items - Remove GetItemsCount

C4::Items::GetItemsCount can be replaced with Koha::Biblio->items->count Test plan: Create a bibliographic record with items attached Try to delete the record from a basket (acquisition module), the detail page and the batch item deletion tool. => You should not be able to delete it. Remove the items and then try again to delete the record => Now you must be able to delete it. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
mrenvoize · Mar 22, 2017 · 3115ac6 · 3115ac6
1 parent 0d239e4
commit 3115ac6
Show file tree

Hide file tree

Showing 10 changed files with 60 additions and 36 deletions.
diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
@@ -30,6 +30,7 @@ use C4::Templates qw(gettemplate);
 use Koha::DateUtils qw( dt_from_string output_pref );
 use Koha::Acquisition::Order;
 use Koha::Acquisition::Booksellers;
+use Koha::Biblios;
 use Koha::Number::Price;
 use Koha::Libraries;
 
@@ -1851,7 +1852,8 @@ sub DelOrder {
 
     if($delete_biblio) {
         # We get the number of remaining items
-        my $itemcount = C4::Items::GetItemsCount($bibnum);
+        my $biblio = Koha::Biblios->find( $bibnum );
+        my $itemcount = $biblio->items->count;
 
         # If there are no items left,
         if ( $itemcount == 0 ) {

diff --git a/C4/Items.pm b/C4/Items.pm
@@ -71,7 +71,6 @@ BEGIN {
         GetItemLocation
         GetLostItems
         GetItemsForInventory
-        GetItemsCount
         GetItemInfosOf
         GetItemsByBiblioitemnumber
         GetItemsInfo
@@ -1169,26 +1168,6 @@ sub GetItemsForInventory {
     return (\@results, $iTotalRecords);
 }
 
-=head2 GetItemsCount
-
-  $count = &GetItemsCount( $biblionumber);
-
-This function return count of item with $biblionumber
-
-=cut
-
-sub GetItemsCount {
-    my ( $biblionumber ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $query = "SELECT count(*)
-          FROM  items 
-          WHERE biblionumber=?";
-    my $sth = $dbh->prepare($query);
-    $sth->execute($biblionumber);
-    my $count = $sth->fetchrow;  
-    return ($count);
-}
-
 =head2 GetItemInfosOf
 
   GetItemInfosOf(@itemnumbers);

diff --git a/acqui/basket.pl b/acqui/basket.pl
@@ -130,10 +130,11 @@ =head1 CGI PARAMETERS
         my @cannotdelbiblios ;
         foreach my $myorder (@orders){
             my $biblionumber = $myorder->{'biblionumber'};
+            my $biblio = Koha::Biblios->find( $biblionumber );
             my $countbiblio = CountBiblioInOrders($biblionumber);
             my $ordernumber = $myorder->{'ordernumber'};
             my $subscriptions = scalar GetSubscriptionsId ($biblionumber);
-            my $itemcount = GetItemsCount($biblionumber);
+            my $itemcount = $biblio->items->count;
             my $error;
             if ($countbiblio == 0 && $itemcount == 0 && $subscriptions == 0) {
                 $error = DelBiblio($myorder->{biblionumber}) }
@@ -466,7 +467,7 @@ sub get_order_infos {
     my $countbiblio = CountBiblioInOrders($biblionumber);
     my $ordernumber = $order->{'ordernumber'};
     my @subscriptions = GetSubscriptionsId ($biblionumber);
-    my $itemcount = GetItemsCount($biblionumber);
+    my $itemcount   = $biblio->items->count;
     my $holds_count = $biblio->holds->count;
     my @items = GetItemnumbersFromOrder( $ordernumber );
     my $itemholds;

diff --git a/acqui/parcel.pl b/acqui/parcel.pl
@@ -240,7 +240,7 @@ =head1 CGI PARAMETERS
         my $countbiblio = CountBiblioInOrders($biblionumber);
         my $ordernumber = $line{'ordernumber'};
         my @subscriptions = GetSubscriptionsId ($biblionumber);
-        my $itemcount = GetItemsCount($biblionumber);
+        my $itemcount   = $biblio->items->count;
         my $holds_count = $biblio->holds->count;
         my @items = GetItemnumbersFromOrder( $ordernumber );
         my $itemholds;

diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl
@@ -47,6 +47,8 @@ =head1 FUNCTIONS
 use C4::Serials;    # CountSubscriptionFromBiblionumber
 use C4::Search;		# enabled_staff_search_views
 use C4::Acquisition qw(GetOrdersByBiblionumber);
+
+use Koha::Biblios;
 use Koha::RecordProcessor;
 
 
@@ -87,6 +89,7 @@ =head1 FUNCTIONS
        exit;
 }
 
+my $biblio = Koha::Biblios->find( $biblionumber );
 my $framework = GetFrameworkCode( $biblionumber );
 my $record_processor = Koha::RecordProcessor->new({
     filters => 'ViewPolicy',
@@ -114,7 +117,7 @@ =head1 FUNCTIONS
 }
 
 # count of item linked with biblio
-my $itemcount = GetItemsCount($biblionumber);
+my $itemcount = $biblio->items->count;
 $template->param( count => $itemcount);
 my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber);
 

diff --git a/catalogue/MARCdetail.pl b/catalogue/MARCdetail.pl
@@ -59,6 +59,8 @@ =head1 FUNCTIONS
 use C4::Members; # to use GetMember
 use C4::Serials;    #uses getsubscriptionsfrombiblionumber GetSubscriptionsFromBiblionumber
 use C4::Search;		# enabled_staff_search_views
+
+use Koha::Biblios;
 use Koha::BiblioFrameworks;
 
 use List::MoreUtils qw( uniq );
@@ -98,6 +100,7 @@ =head1 FUNCTIONS
     exit;
 }
 
+my $biblio_object = Koha::Biblios->find( $biblionumber ); # FIXME Should replace $biblio
 my $tagslib = &GetMarcStructure(1,$frameworkcode);
 my $biblio = GetBiblioData($biblionumber);
 
@@ -112,7 +115,7 @@ =head1 FUNCTIONS
 }
 
 #count of item linked
-my $itemcount = GetItemsCount($biblionumber);
+my $itemcount = $biblio_object->items->count;
 $template->param( count => $itemcount,
 					bibliotitle => $biblio->{title}, );
 

diff --git a/catalogue/imageviewer.pl b/catalogue/imageviewer.pl
@@ -29,6 +29,8 @@
 use C4::Search;
 use C4::Acquisition qw(GetOrdersByBiblionumber);
 
+use Koha::Biblios;
+
 my $query = new CGI;
 my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
     {
@@ -43,7 +45,8 @@
 my $biblionumber = $query->param('biblionumber') || $query->param('bib');
 my $imagenumber = $query->param('imagenumber');
 my $biblio = GetBiblio($biblionumber);
-my $itemcount = GetItemsCount($biblionumber);
+my $biblio_object = Koha::Biblios->find( $biblionumber ); # This should replace $biblio
+my $itemcount = $biblio_object->items->count;;
 
 my @items = GetItemsInfo($biblionumber);
 

diff --git a/catalogue/labeledMARCdetail.pl b/catalogue/labeledMARCdetail.pl
@@ -31,6 +31,7 @@
 use C4::Search;		# enabled_staff_search_views
 use C4::Acquisition qw(GetOrdersByBiblionumber);
 
+use Koha::Biblios;
 use Koha::BiblioFrameworks;
 
 my $query        = new CGI;
@@ -65,6 +66,7 @@
     exit;
 }
 
+my $biblio_object = Koha::Biblios->find( $biblionumber ); # FIXME Should replace $biblio
 my $tagslib = GetMarcStructure(1,$frameworkcode);
 my $biblio = GetBiblioData($biblionumber);
 
@@ -79,7 +81,7 @@
 }
 
 #count of item linked
-my $itemcount = GetItemsCount($biblionumber);
+my $itemcount = $biblio_object->items->count;
 $template->param( count => $itemcount,
 					bibliotitle => $biblio->{title}, );
 

diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t
@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use POSIX qw(strftime);
 
-use Test::More tests => 63;
+use Test::More tests => 65;
 use Koha::Database;
 
 BEGIN {
@@ -166,6 +166,7 @@ my ( $biblionumber1, $biblioitemnumber1 ) = AddBiblio( MARC::Record->new, '' );
 my ( $biblionumber2, $biblioitemnumber2 ) = AddBiblio( MARC::Record->new, '' );
 my ( $biblionumber3, $biblioitemnumber3 ) = AddBiblio( MARC::Record->new, '' );
 my ( $biblionumber4, $biblioitemnumber4 ) = AddBiblio( MARC::Record->new, '' );
+my ( $biblionumber5, $biblioitemnumber5 ) = AddBiblio( MARC::Record->new, '' );
 
 # Prepare 5 orders, and make distinction beween fields to be tested with eq and with ==
 # Ex : a price of 50.1 will be stored internally as 5.100000
@@ -257,11 +258,31 @@ my @order_content = (
             uncertainprice => 0,
             tax_rate        => 0
         }
+    },
+    {
+        str => {
+            basketno     => $basketno,
+            biblionumber => $biblionumber5,
+            budget_id    => $budget->{budget_id},
+            order_internalnote => "internal note",
+            order_vendornote   => "vendor note"
+        },
+        num => {
+            quantity       => 1,
+            ecost          => 10,
+            rrp            => 10,
+            listprice      => 10,
+            ecost          => 10,
+            rrp            => 10,
+            discount       => 0,
+            uncertainprice => 0,
+            tax_rate        => 0
+        }
     }
 );
 
-# Create 4 orders in database
-for ( 0 .. 4 ) {
+# Create 5 orders in database
+for ( 0 .. 5 ) {
     my %ocontent;
     @ocontent{ keys %{ $order_content[$_]->{num} } } =
       values %{ $order_content[$_]->{num} };
@@ -298,7 +319,7 @@ my $search_orders = SearchOrders({
 isa_ok( $search_orders, 'ARRAY' );
 ok(
     (
-        ( scalar @$search_orders == 4 )
+        ( scalar @$search_orders == 5 )
           and !grep ( $_->{ordernumber} eq $ordernumbers[3], @$search_orders )
     ),
     "SearchOrders only gets non-cancelled orders"
@@ -311,7 +332,7 @@ $search_orders = SearchOrders({
 });
 ok(
     (
-        ( scalar @$search_orders == 3 ) and !grep ( (
+        ( scalar @$search_orders == 4 ) and !grep ( (
                      ( $_->{ordernumber} eq $ordernumbers[3] )
                   or ( $_->{ordernumber} eq $ordernumbers[4] )
             ),
@@ -368,7 +389,7 @@ $search_orders = SearchOrders({
     pending      => 1,
     ordered      => 1,
 });
-is( scalar (@$search_orders), 3, "SearchOrders with pending and ordered params gets only pending ordered orders. After closing the basket, orders are marked as 'ordered' (bug 11170)" );
+is( scalar (@$search_orders), 4, "SearchOrders with pending and ordered params gets only pending ordered orders. After closing the basket, orders are marked as 'ordered' (bug 11170)" );
 
 #
 # Test AddClaim
@@ -498,6 +519,14 @@ $order4 = GetOrder($order4->{ordernumber});
 ok((defined $order4->{datecancellationprinted}), "order is cancelled");
 ok(($order4->{cancellationreason} eq "foobar"), "order has cancellation reason \"foobar\"");
 ok((not defined GetBiblio($order4->{biblionumber})), "biblio does not exist anymore");
+
+my $order5 = GetOrder($ordernumbers[4]);
+C4::Items::AddItem( { barcode => '0102030405' }, $order5->{biblionumber} );
+$error = DelOrder($order5->{biblionumber}, $order5->{ordernumber}, 1);
+$order5 = GetOrder($order5->{ordernumber});
+ok((defined $order5->{datecancellationprinted}), "order is cancelled");
+ok(GetBiblio($order5->{biblionumber}), "biblio still exists");
+
 # End of tests for DelOrder
 
 subtest 'ModOrder' => sub {

diff --git a/tools/batchMod.pl b/tools/batchMod.pl
@@ -34,6 +34,8 @@
 use C4::Members;
 use MARC::File::XML;
 use List::MoreUtils qw/uniq/;
+
+use Koha::Biblios;
 use Koha::DateUtils;
 
 my $input = new CGI;
@@ -181,7 +183,7 @@
 
 			# If there are no items left, delete the biblio
 			if ( $del_records ) {
-                            my $itemscount = GetItemsCount($itemdata->{'biblionumber'});
+                            my $itemscount = Koha::Biblios->find( $itemdata->{'biblionumber'} )->items->count;
                             if ( $itemscount == 0 ) {
 			        my $error = DelBiblio($itemdata->{'biblionumber'});
 			        $deleted_records++ unless ( $error );