Attachment 61052 Details for Bug 18256 – Bug 18256: Koha::Items - Remove GetItemsCount

[patch] Bug 18256: Koha::Items - Remove GetItemsCount

Bug-18256-KohaItems---Remove-GetItemsCount.patch (text/plain), 12.69 KB, created by Jonathan Druart on 2017-03-13 19:29:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-03-13 19:29:34 UTC

Size: 12.69 KB

patch

obsolete

>From d99954978795087e948f920b4dd1b7debd34fe84 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Mon, 13 Mar 2017 01:11:54 -0300
>Subject: [PATCH] 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.
>---
> C4/Acquisition.pm              |  4 +++-
> C4/Items.pm                    | 21 ---------------------
> acqui/basket.pl                |  5 +++--
> acqui/parcel.pl                |  2 +-
> catalogue/ISBDdetail.pl        |  5 ++++-
> catalogue/MARCdetail.pl        |  5 ++++-
> catalogue/imageviewer.pl       |  5 ++++-
> catalogue/labeledMARCdetail.pl |  4 +++-
> t/db_dependent/Acquisition.t   | 41 +++++++++++++++++++++++++++++++++++------
> tools/batchMod.pl              |  4 +++-
> 10 files changed, 60 insertions(+), 36 deletions(-)
>
>diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
>index d5b6b2d..2e87b6d 100644
>--- 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::Orders;
> use Koha::Acquisition::Booksellers;
>+use Koha::Biblios;
> use Koha::Number::Price;
> use Koha::Libraries;
> 
>@@ -1873,7 +1874,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
>--- 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 9f92d7d..3d25946 100755
>--- 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
>--- 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
>--- 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
>--- 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
>--- 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
>--- 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 9049289..72f1ef2 100755
>--- 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
>@@ -256,11 +257,31 @@ my @order_content = (
>             uncertainprice => 0,
>             tax_rate_on_ordering        => 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} };
>@@ -297,7 +318,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"
>@@ -310,7 +331,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] )
>             ),
>@@ -367,7 +388,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
>@@ -497,6 +518,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
>--- 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 );
>-- 
>2.9.3

Actions: View | Diff | Splinter Review

Attachments on bug 18256: 61048 | 61049 | 61052 | 61119 | 61120 | 61259