From a851aaeebf60e2da4ea9381151521e5f03a994c8 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 30 Sep 2020 15:39:05 -0300 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- C4/Acquisition.pm | 24 +------------- Koha/EDI.pm | 4 +-- acqui/basket.pl | 31 ++++++++++--------- t/db_dependent/Acquisition.t | 2 +- .../Acquisition/GetBasketsInfosByBookseller.t | 4 +-- .../Acquisition/close_reopen_basket.t | 6 ++-- t/db_dependent/Letters.t | 2 +- 7 files changed, 27 insertions(+), 46 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index e29cbe8733..2ac15473b4 100644 --- a/C4/Acquisition.pm +++ b/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); diff --git a/Koha/EDI.pm b/Koha/EDI.pm index 8fc8d0d828..445998d51e 100644 --- a/Koha/EDI.pm +++ b/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; } } diff --git a/acqui/basket.pl b/acqui/basket.pl index ae918b93d3..f77d86b88c 100755 --- a/acqui/basket.pl +++ b/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') ) { diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index d6d830bd50..5669923b65 100755 --- a/t/db_dependent/Acquisition.t +++ b/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" ); diff --git a/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t b/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t index 0cd4f599f4..fce3cf6033 100755 --- a/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t +++ b/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]; diff --git a/t/db_dependent/Acquisition/close_reopen_basket.t b/t/db_dependent/Acquisition/close_reopen_basket.t index 09225e8588..7faaac1611 100755 --- a/t/db_dependent/Acquisition/close_reopen_basket.t +++ b/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} ); diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 9a629bdfbf..766e21d7f4 100755 --- a/t/db_dependent/Letters.t +++ b/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' ) } -- 2.39.5