Attachment 110996 Details for Bug 26579 – Bug 26579: Remove unused C4::Acquisition::DelOrder function

[patch] Bug 26579: Remove unused C4::Acquisition::DelOrder function

Bug-26579-Remove-unused-C4AcquisitionDelOrder-func.patch (text/plain), 9.06 KB, created by Tomás Cohen Arazi (tcohen) on 2020-09-30 14:57:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-09-30 14:57:26 UTC

Size: 9.06 KB

patch

obsolete

>From f2d704f66fc452867a2546adefc597074f581439 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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 <tomascohen@theke.io>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 26579: 110996 | 111040 | 111138 | 111726