From d99954978795087e948f920b4dd1b7debd34fe84 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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