From f2d704f66fc452867a2546adefc597074f581439 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 30 Sep 2020 11:47:03 -0300 Subject: [PATCH] Bug 26579: Remove unused C4::Acquisition::DelOrder function This patch removes an unused function, its tests, and adjusts other test files that relied on it by replacing it with $order->cancel calls. To test: 1. Run: $ kshell k$ prove t/db_dependent/Acquisition.t \ t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t \ t/db_dependent/Acquisition/close_reopen_basket.t => SUCCESS: Tests pass! 2. Apply this patch 3. Repeat (1) => SUCCESS: Tests pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- C4/Acquisition.pm | 63 +------------------ t/db_dependent/Acquisition.t | 41 +----------- .../Acquisition/GetBasketsInfosByBookseller.t | 4 +- .../Acquisition/close_reopen_basket.t | 4 +- 4 files changed, 8 insertions(+), 104 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 3aff14d667..1a6de25e27 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -66,7 +66,7 @@ BEGIN { &ModBasketgroup &NewBasketgroup &DelBasketgroup &GetBasketgroup &CloseBasketgroup &GetBasketgroups &ReOpenBasketgroup - &DelOrder &ModOrder &GetOrder &GetOrders &GetOrdersByBiblionumber + &ModOrder &GetOrder &GetOrders &GetOrdersByBiblionumber &GetOrderFromItemnumber &SearchOrders &GetHistory &GetRecentAcqui &ModReceiveOrder &CancelReceipt @@ -1825,67 +1825,6 @@ sub SearchOrders { #------------------------------------------------------------# -=head3 DelOrder - - &DelOrder($biblionumber, $ordernumber); - -Cancel the order with the given order and biblio numbers. It does not -delete any entries in the aqorders table, it merely marks them as -cancelled. - -=cut - -sub DelOrder { - my ( $bibnum, $ordernumber, $delete_biblio, $reason ) = @_; - my $error; - my $dbh = C4::Context->dbh; - my $query = " - UPDATE aqorders - SET datecancellationprinted=now(), orderstatus='cancelled' - "; - if($reason) { - $query .= ", cancellationreason = ? "; - } - $query .= " - WHERE biblionumber=? AND ordernumber=? - "; - my $sth = $dbh->prepare($query); - if($reason) { - $sth->execute($reason, $bibnum, $ordernumber); - } else { - $sth->execute( $bibnum, $ordernumber ); - } - $sth->finish; - - my $order = Koha::Acquisition::Orders->find($ordernumber); - my $items = $order->items; - while ( my $item = $items->next ) { # Should be moved to Koha::Acquisition::Order->delete - my $delcheck = $item->safe_delete; - - if($delcheck ne '1') { - $error->{'delitem'} = 1; - } - } - - if($delete_biblio) { - # We get the number of remaining items - my $biblio = Koha::Biblios->find( $bibnum ); - my $itemcount = $biblio->items->count; - - # If there are no items left, - if ( $itemcount == 0 ) { - # We delete the record - my $delcheck = DelBiblio($bibnum); - - if($delcheck) { - $error->{'delbiblio'} = 1; - } - } - } - - return $error; -} - =head3 TransferOrder my $newordernumber = TransferOrder($ordernumber, $basketno); diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index 6028fea2b3..f3e009c866 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 => 79; +use Test::More tests => 65; use t::lib::Mocks; use Koha::Database; use Koha::DateUtils qw(dt_from_string output_pref); @@ -292,7 +292,7 @@ for ( 0 .. 5 ) { $order_content[$_]->{str}->{ordernumber} = $ordernumbers[$_]; } -DelOrder( $order_content[3]->{str}->{biblionumber}, $ordernumbers[3] ); +Koha::Acquisition::Orders->find($ordernumbers[3])->cancel; my $invoiceid = AddInvoice( invoicenumber => 'invoice', @@ -527,40 +527,6 @@ is( $nonexistent_order, undef, 'GetOrder returns undef if no ordernumber is give $nonexistent_order = GetOrder( 424242424242 ); is( $nonexistent_order, undef, 'GetOrder returns undef if a nonexistent ordernumber is given' ); -# Tests for DelOrder -$order1 = GetOrder($ordernumbers[0]); -my $error = DelOrder($order1->{biblionumber}, $order1->{ordernumber}); -ok((not defined $error), "DelOrder does not fail"); -$order1 = GetOrder($order1->{ordernumber}); -ok((defined $order1->{datecancellationprinted}), "order is cancelled"); -ok((not defined $order1->{cancellationreason}), "order has no cancellation reason"); -ok((defined Koha::Biblios->find( $order1->{biblionumber} )), "biblio still exists"); - -$order2 = GetOrder($ordernumbers[1]); -$error = DelOrder($order2->{biblionumber}, $order2->{ordernumber}, 1); -ok((not defined $error), "DelOrder does not fail"); -$order2 = GetOrder($order2->{ordernumber}); -ok((defined $order2->{datecancellationprinted}), "order is cancelled"); -ok((not defined $order2->{cancellationreason}), "order has no cancellation reason"); -ok((not defined Koha::Biblios->find( $order2->{biblionumber} )), "biblio does not exist anymore"); - -my $order4 = GetOrder($ordernumbers[3]); -$error = DelOrder($order4->{biblionumber}, $order4->{ordernumber}, 1, "foobar"); -ok((not defined $error), "DelOrder does not fail"); -$order4 = GetOrder($order4->{ordernumber}); -ok((defined $order4->{datecancellationprinted}), "order is cancelled"); -ok(($order4->{cancellationreason} eq "foobar"), "order has cancellation reason \"foobar\""); -ok((not defined Koha::Biblios->find( $order4->{biblionumber} )), "biblio does not exist anymore"); - -my $order5 = GetOrder($ordernumbers[4]); -Koha::Item->new({ barcode => '0102030405', biblionumber => $order5->{biblionumber} })->store; -$error = DelOrder($order5->{biblionumber}, $order5->{ordernumber}, 1); -$order5 = GetOrder($order5->{ordernumber}); -ok((defined $order5->{datecancellationprinted}), "order is cancelled"); -ok((defined Koha::Biblios->find( $order5->{biblionumber} )), "biblio still exists"); - -# End of tests for DelOrder - subtest 'ModOrder' => sub { plan tests => 1; ModOrder( { ordernumber => $order1->{ordernumber}, unitprice => 42 } ); @@ -640,8 +606,7 @@ sub run_flavoured_tests { $orders = GetHistory( isbn => '0136019706' ); is( scalar(@$orders), 1, "GetHistory searches correctly by ISBN" ); - my $order = GetOrder($ordernumber); - DelOrder($order->{biblionumber}, $order->{ordernumber}, 1); + Koha::Acquisition::Orders->find($ordernumber)->cancel; } # Do "flavoured" tests diff --git a/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t b/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t index ec8fceb378..0cd4f599f4 100755 --- a/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t +++ b/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t @@ -83,7 +83,7 @@ $baskets = C4::Acquisition::GetBasketsInfosByBookseller( $supplierid ); $basket = $baskets->[0]; is( $basket->{uncertainprices}, 0, "Uncertain prcies returns number of uncertain items"); -C4::Acquisition::DelOrder( $biblionumber2, $ordernumber2 ); +Koha::Acquisition::Orders->find($ordernumber2)->cancel; $baskets = C4::Acquisition::GetBasketsInfosByBookseller( $supplierid ); is( scalar(@$baskets), 1, 'Order2 deleted, still 1 basket' ); $basket = $baskets->[0]; @@ -93,7 +93,7 @@ is( $basket->{total_items_cancelled}, 4, 'Order2 deleted, 4 items cancelled' ); is( $basket->{expected_items}, 2, 'Order2 deleted, now 2 items are expected' ); is( $basket->{total_biblios_cancelled}, 1, 'Order2 deleted, 1 biblios cancelled' ); -C4::Acquisition::DelOrder( $biblionumber1, $ordernumber1 ); +Koha::Acquisition::Orders->find($ordernumber1)->cancel; $baskets = C4::Acquisition::GetBasketsInfosByBookseller( $supplierid ); is( scalar(@$baskets), 1, 'Both orders deleted, still 1 basket' ); $basket = $baskets->[0]; diff --git a/t/db_dependent/Acquisition/close_reopen_basket.t b/t/db_dependent/Acquisition/close_reopen_basket.t index 1cb20d591a..09225e8588 100755 --- a/t/db_dependent/Acquisition/close_reopen_basket.t +++ b/t/db_dependent/Acquisition/close_reopen_basket.t @@ -81,10 +81,10 @@ C4::Acquisition::ReopenBasket( $basketno ); is ( scalar( map { $_->{orderstatus} eq 'ordered' ? 1 : () } @orders ), 0, "No order is ordered, the basket is reopened" ); is ( scalar( map { $_->{orderstatus} eq 'new' ? 1 : () } @orders ), 2, "2 orders are new, the basket is reopened" ); -C4::Acquisition::DelOrder( $biblionumber1, $ordernumber1 ); +Koha::Acquisition::Orders->find($ordernumber1)->cancel; my ( $order ) = C4::Acquisition::GetOrders( $basketno, {cancelled => 1} ); is( $order->{ordernumber}, $ordernumber1, 'The order returned by GetOrders should have been the right one' ); -is( $order->{orderstatus}, 'cancelled', 'DelOrder should have set status to cancelled' ); +is( $order->{orderstatus}, 'cancelled', 'cancelling the order should have set status to cancelled' ); C4::Acquisition::CloseBasket( $basketno ); ( $order ) = C4::Acquisition::GetOrders( $basketno, {cancelled => 1} ); -- 2.28.0