Bug 26584: Remove unused C4::Acquisition::CloseBasket function

This patch makes code use the new Koha::Acquisition::Basket->close
method and makes CloseBasket obsolete.

It then removes it, and adapts the few places in which it was used.

1. Apply this patch
2. Run:
   $ kshell
  k$ git diff origin/master --name-only | grep -e '\.t$' | xargs prove
=> SUCCESS: Tests pass!
3. Try playing with baskets, closing them
=> SUCCESS: All works as expected!
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/Acquisition.pm

@@ -53,7 +53,7 @@ BEGIN {
     require Exporter;
     @ISA    = qw(Exporter);
     @EXPORT = qw(
-        &GetBasket &NewBasket &CloseBasket &ReopenBasket &ModBasket
+        &GetBasket &NewBasket &ReopenBasket &ModBasket
         &GetBasketAsCSV &GetBasketGroupAsCSV
         &GetBasketsByBookseller &GetBasketsByBasketgroup
         &GetBasketsInfosByBookseller
@@ -206,28 +206,6 @@ sub NewBasket {
     return $basket;
 }
 
-#------------------------------------------------------------#
-
-=head3 CloseBasket
-
-  &CloseBasket($basketno);
-
-close a basket (becomes unmodifiable, except for receives)
-
-=cut
-
-sub CloseBasket {
-    my ($basketno) = @_;
-    my $dbh        = C4::Context->dbh;
-    $dbh->do('UPDATE aqbasket SET closedate=now() WHERE basketno=?', {}, $basketno );
-
-    $dbh->do(
-q{UPDATE aqorders SET orderstatus = 'ordered' WHERE basketno = ? AND orderstatus NOT IN ( 'complete', 'cancelled')},
-        {}, $basketno
-    );
-    return;
-}
-
 =head3 ReopenBasket
 
   &ReopenBasket($basketno);

Koha/EDI.pm

@@ -28,7 +28,7 @@ use DateTime;
 use C4::Context;
 use Koha::Database;
 use Koha::DateUtils;
-use C4::Acquisition qw( NewBasket CloseBasket ModOrder);
+use C4::Acquisition qw( NewBasket ModOrder);
 use C4::Suggestions qw( ModSuggestion );
 use C4::Biblio qw( AddBiblio TransformKohaToMarc GetMarcBiblio GetFrameworkCode GetMarcFromKohaField );
 use Koha::Edifact::Order;
@@ -592,7 +592,7 @@ sub process_quote {
                     basketno => $b,
                 }
             );
-            CloseBasket($b);
+            Koha::Acquisition::Baskets->find($b)->close;
         }
     }
 

acqui/basket.pl

@@ -191,9 +191,12 @@ if ( $op eq 'delete_confirm' ) {
 } elsif ($op eq 'close') {
     my $confirm = $query->param('confirm') || $confirm_pref eq '2';
     if ($confirm) {
-        my $basketno = $query->param('basketno');
-        my $booksellerid = $query->param('booksellerid');
-        $basketno =~ /^\d+$/ and CloseBasket($basketno);
+
+        # close the basket
+        # FIXME: we should fetch the object at the beginning of this script
+        #        and get rid of the hash that is passed around
+        Koha::Acquisition::Baskets->find($basketno)->close;
+
         # if requested, create basket group, close it and attach the basket
         if ($query->param('createbasketgroup')) {
             my $branchcode;
@@ -524,17 +527,17 @@ sub get_order_infos {
 sub edi_close_and_order {
     my $confirm = $query->param('confirm') || $confirm_pref eq '2';
     if ($confirm) {
-            my $edi_params = {
-                basketno => $basketno,
-                ean    => $ean,
-            };
-            if ( $basket->{branch} ) {
-                $edi_params->{branchcode} = $basket->{branch};
-            }
-            if ( create_edi_order($edi_params) ) {
-                #$template->param( edifile => 1 );
-            }
-        CloseBasket($basketno);
+        my $edi_params = {
+            basketno => $basketno,
+            ean    => $ean,
+        };
+        if ( $basket->{branch} ) {
+            $edi_params->{branchcode} = $basket->{branch};
+        }
+        if ( create_edi_order($edi_params) ) {
+            #$template->param( edifile => 1 );
+        }
+        Koha::Acquisition::Baskets->find($basketno)->close;
 
         # if requested, create basket group, close it and attach the basket
         if ( $query->param('createbasketgroup') ) {

t/db_dependent/Acquisition.t

@@ -394,7 +394,7 @@ ok( GetBudgetByOrderNumber( $ordernumbers[0] )->{'budget_id'} eq $budgetid,
 my $lateorders = Koha::Acquisition::Orders->filter_by_lates({ delay => 0 });
 is( $lateorders->search({ 'me.basketno' => $basketno })->count,
     0, "GetLateOrders does not get orders from opened baskets" );
-C4::Acquisition::CloseBasket($basketno);
+Koha::Acquisition::Baskets->find($basketno)->close;
 $lateorders = Koha::Acquisition::Orders->filter_by_lates({ delay => 0 });
 isnt( $lateorders->search({ 'me.basketno' => $basketno })->count,
     0, "GetLateOrders gets orders from closed baskets" );

t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t

@@ -103,7 +103,7 @@ is( $basket->{total_items_cancelled}, 6, 'Both orders deleted, 6 items cancelled
 is( $basket->{expected_items}, 0, 'Both orders delete, now 0 items are expected' );
 is( $basket->{total_biblios_cancelled}, 2, 'Both orders deleted, 2 biblios cancelled' );
 
-C4::Acquisition::CloseBasket( $basketno );
+Koha::Acquisition::Baskets->find( $basketno )->close;
 $baskets = C4::Acquisition::GetBasketsInfosByBookseller( $supplierid );
 is( scalar(@$baskets), 0, 'Basket is closed, 0 basket opened' );
 $baskets = C4::Acquisition::GetBasketsInfosByBookseller( $supplierid, 1 );
@@ -120,7 +120,7 @@ my $order3 = Koha::Acquisition::Order->new(
 )->store;
 my $ordernumber3 = $order3->ordernumber;
 
-C4::Acquisition::CloseBasket( $basketno );
+Koha::Acquisition::Baskets->find( $basketno )->close;
 $baskets = C4::Acquisition::GetBasketsInfosByBookseller( $supplierid );
 is( scalar(@$baskets), 1, 'Basket is closed and has items to receive' );
 $basket = $baskets->[0];

t/db_dependent/Acquisition/close_reopen_basket.t

@@ -72,7 +72,7 @@ my @orders = C4::Acquisition::GetOrders( $basketno );
 is( scalar(@orders), 2, "2 orders are created" );
 is ( scalar( map { $_->{orderstatus} eq 'new' ? 1 : () } @orders ), 2, "2 orders are new before closing the basket" );
 
-C4::Acquisition::CloseBasket( $basketno );
+Koha::Acquisition::Baskets->find( $basketno )->close;
 @orders = C4::Acquisition::GetOrders( $basketno );
 is ( scalar( map { $_->{orderstatus} eq 'ordered' ? 1 : () } @orders ), 2, "2 orders are ordered, the basket is closed" );
 
@@ -86,10 +86,10 @@ 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', 'cancelling the order should have set status to cancelled' );
 
-C4::Acquisition::CloseBasket( $basketno );
+Koha::Acquisition::Baskets->find( $basketno )->close;
 ( $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', 'CloseBasket should not reset the status to ordered for cancelled orders' );
+is( $order->{orderstatus}, 'cancelled', '$basket->close should not reset the status to ordered for cancelled orders' );
 
 C4::Acquisition::ReopenBasket( $basketno );
 ( $order ) = C4::Acquisition::GetOrders( $basketno, {cancelled => 1} );

t/db_dependent/Letters.t

@@ -436,7 +436,7 @@ my $order = Koha::Acquisition::Order->new(
 )->store;
 my $ordernumber = $order->ordernumber;
 
-C4::Acquisition::CloseBasket( $basketno );
+Koha::Acquisition::Baskets->find( $basketno )->close;
 my $err;
 warning_like {
     $err = SendAlerts( 'claimacquisition', [ $ordernumber ], 'TESTACQCLAIM' ) }