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>

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index c113c16..3fca9eb 100644 (file)
--- 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
index 8f177b0..3b2bee9 100644 (file)
--- 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
index 284b082..4940821 100755 (executable)
--- a/acqui/basket.pl
+++ b/acqui/basket.pl
@@ -130,10 +130,11 @@ if ( $op eq 'delete_confirm' ) {
         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
index 22a036f..4e11f2b 100755 (executable)
--- a/acqui/parcel.pl
+++ b/acqui/parcel.pl
@@ -240,7 +240,7 @@ unless( defined $invoice->{closedate} ) {
         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
index 4679b9e..bb9a29c 100755 (executable)
--- a/catalogue/ISBDdetail.pl
+++ b/catalogue/ISBDdetail.pl
@@ -47,6 +47,8 @@ use C4::Members; # to use GetMember
 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 @@ if ( not defined $record ) {
        exit;
 }
 
+my $biblio = Koha::Biblios->find( $biblionumber );
 my $framework = GetFrameworkCode( $biblionumber );
 my $record_processor = Koha::RecordProcessor->new({
     filters => 'ViewPolicy',
@@ -114,7 +117,7 @@ if($query->cookie("holdfor")){
 }
 
 # 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
index 09f9778..8b32935 100755 (executable)
--- a/catalogue/MARCdetail.pl
+++ b/catalogue/MARCdetail.pl
@@ -59,6 +59,8 @@ use C4::Acquisition;
 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 @@ if ( not defined $record ) {
     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 @@ if($query->cookie("holdfor")){
 }
 
 #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
index fe64ecc..c5c4cd0 100755 (executable)
--- a/catalogue/imageviewer.pl
+++ b/catalogue/imageviewer.pl
@@ -29,6 +29,8 @@ use C4::Images;
 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 ( $template, $borrowernumber, $cookie ) = get_template_and_user(
 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
index c4b9019..6b554f4 100755 (executable)
--- a/catalogue/labeledMARCdetail.pl
+++ b/catalogue/labeledMARCdetail.pl
@@ -31,6 +31,7 @@ use C4::Members; # to use GetMember
 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 @@ if ( not defined $record ) {
     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 @@ if($query->cookie("holdfor")){
 }
 
 #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
index b51dc9c..1973cf2 100755 (executable)
--- 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
index 2920da9..7cc3fe9 100755 (executable)
--- a/tools/batchMod.pl
+++ b/tools/batchMod.pl
@@ -34,6 +34,8 @@ use C4::Debug;
 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 ($op eq "action") {
 
                        # 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 );