Bug 13244: Merge GetOrders and GetCancelledOrders

These two subroutines did the same job (same select, same join, etc.)

Test plan:
Go on the basket list page and verify you see the pending and the
cancelled baskets.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Two small things are adjusted in separate follow-ups.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

C4/Acquisition.pm

@@ -62,7 +62,7 @@ BEGIN {
         &GetLateOrders &GetOrderFromItemnumber
         &SearchOrders &GetHistory &GetRecentAcqui
         &ModReceiveOrder &CancelReceipt
-        &GetCancelledOrders &TransferOrder
+        &TransferOrder
         &GetLastOrderNotReceivedFromSubscriptionid &GetLastOrderReceivedFromSubscriptionid
         &ModItemOrder
 
@@ -1036,43 +1036,75 @@ sub GetBasketgroups {
 
 =head3 GetOrders
 
-  @orders = &GetOrders($basketnumber, $orderby);
+  @orders = &GetOrders( $basketno, { orderby => 'biblio.title', cancelled => 0|1 } );
 
 Looks up the pending (non-cancelled) orders with the given basket
-number. If C<$booksellerID> is non-empty, only orders from that seller
-are returned.
+number.
 
-return :
-C<&basket> returns a two-element array. C<@orders> is an array of
-references-to-hash, whose keys are the fields from the aqorders,
-biblio, and biblioitems tables in the Koha database.
+If cancelled is set, only cancelled orders will be returned.
 
 =cut
 
 sub GetOrders {
-    my ( $basketno, $orderby ) = @_;
+    my ( $basketno, $params ) = @_;
+
     return () unless $basketno;
+
+    my $orderby = $params->{orderby};
+    my $cancelled = $params->{cancelled} || 0;
+
     my $dbh   = C4::Context->dbh;
-    my $query  ="
+    my $query = q|
         SELECT biblio.*,biblioitems.*,
                 aqorders.*,
                 aqbudgets.*,
+        |;
+    $query .= $cancelled
+      ? q|
+                aqorders_transfers.ordernumber_from AS transferred_to,
+                aqorders_transfers.timestamp AS transferred_to_timestamp
+    |
+      : q|
                 aqorders_transfers.ordernumber_from AS transferred_from,
                 aqorders_transfers.timestamp AS transferred_from_timestamp
+    |;
+    $query .= q|
         FROM    aqorders
             LEFT JOIN aqbudgets        ON aqbudgets.budget_id = aqorders.budget_id
             LEFT JOIN biblio           ON biblio.biblionumber = aqorders.biblionumber
             LEFT JOIN biblioitems      ON biblioitems.biblionumber =biblio.biblionumber
+    |;
+    $query .= $cancelled
+      ? q|
+            LEFT JOIN aqorders_transfers ON aqorders_transfers.ordernumber_from = aqorders.ordernumber
+    |
+      : q|
             LEFT JOIN aqorders_transfers ON aqorders_transfers.ordernumber_to = aqorders.ordernumber
+
+    |;
+    $query .= q|
         WHERE   basketno=?
+    |;
+
+    if ($cancelled) {
+        $orderby ||= q|biblioitems.publishercode, biblio.title|;
+        $query .= q|
+            AND (datecancellationprinted IS NOT NULL
+               AND datecancellationprinted <> '0000-00-00')
+        |;
+    }
+    else {
+        $orderby ||=
+          q|aqorders.datecancellationprinted desc, aqorders.timestamp desc|;
+        $query .= q|
             AND (datecancellationprinted IS NULL OR datecancellationprinted='0000-00-00')
-    ";
+        |;
+    }
 
-    $orderby = "biblioitems.publishercode,biblio.title" unless $orderby;
     $query .= " ORDER BY $orderby";
-    my $result_set =
+    my $orders =
       $dbh->selectall_arrayref( $query, { Slice => {} }, $basketno );
-    return @{$result_set};
+    return @{$orders};
 
 }
 
@@ -1306,51 +1338,6 @@ sub ModItemOrder {
     return $sth->execute($ordernumber, $itemnumber);
 }
 
-#------------------------------------------------------------#
-
-=head3 GetCancelledOrders
-
-  my @orders = GetCancelledOrders($basketno, $orderby);
-
-Returns cancelled orders for a basket
-
-=cut
-
-sub GetCancelledOrders {
-    my ( $basketno, $orderby ) = @_;
-
-    return () unless $basketno;
-
-    my $dbh   = C4::Context->dbh;
-    my $query = "
-        SELECT
-            biblio.*,
-            biblioitems.*,
-            aqorders.*,
-            aqbudgets.*,
-            aqorders_transfers.ordernumber_to AS transferred_to,
-            aqorders_transfers.timestamp AS transferred_to_timestamp
-        FROM aqorders
-          LEFT JOIN aqbudgets   ON aqbudgets.budget_id = aqorders.budget_id
-          LEFT JOIN biblio      ON biblio.biblionumber = aqorders.biblionumber
-          LEFT JOIN biblioitems ON biblioitems.biblionumber = biblio.biblionumber
-          LEFT JOIN aqorders_transfers ON aqorders_transfers.ordernumber_from = aqorders.ordernumber
-        WHERE basketno = ?
-          AND (datecancellationprinted IS NOT NULL
-               AND datecancellationprinted <> '0000-00-00')
-    ";
-
-    $orderby = "aqorders.datecancellationprinted desc, aqorders.timestamp desc"
-        unless $orderby;
-    $query .= " ORDER BY $orderby";
-    my $sth = $dbh->prepare($query);
-    $sth->execute($basketno);
-    my $results = $sth->fetchall_arrayref( {} );
-
-    return @$results;
-}
-
-
 #------------------------------------------------------------#
 
 =head3 ModReceiveOrder

acqui/basket.pl

@@ -366,7 +366,7 @@ if ( $op eq 'delete_confirm' ) {
     push @book_foot_loop, map {$_} values %foot;
 
     # Get cancelled orders
-    my @cancelledorders = GetCancelledOrders($basketno);
+    my @cancelledorders = GetOrders($basketno, { cancelled => 1 });
     my @cancelledorders_loop;
     for my $order (@cancelledorders) {
         $order = C4::Acquisition::populate_order_with_prices({ order => $order, booksellerid => $booksellerid, ordering => 1 });

t/db_dependent/Acquisition.t

@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use POSIX qw(strftime);
 
-use Test::More tests => 88;
+use Test::More tests => 87;
 
 BEGIN {
     use_ok('C4::Acquisition');
@@ -525,14 +525,12 @@ ok(
 );
 
 #
-# Test GetCancelledOrders
+# Test GetOrders { cancelled => 1 }
 #
 
 @expectedfields =
   ( @base_expectedfields, ( 'transferred_to_timestamp', 'transferred_to' ) );
-is( GetCancelledOrders(), undef,
-    "GetCancelledOrders with no params returns undef" );
-@get_orders = GetCancelledOrders($basketno);
+@get_orders = GetOrders($basketno, { cancelled => 1 });
 (
     $test_missing_fields,   $test_extra_fields,
     $test_different_fields, $test_nbr_fields
@@ -541,21 +539,21 @@ is( GetCancelledOrders(), undef,
 is(
     $$test_nbr_fields[0],
     scalar @expectedfields,
-    "GetCancelledOrders gets orders with the right number of fields"
+    "GetOrders { cancelled => 1 } gets orders with the right number of fields"
 );
 is( join( " ", @$test_missing_fields ),
-    '', "GetCancelledOrders gets orders with no missing fields" );
+    '', "GetOrders { cancelled => 1 } gets orders with no missing fields" );
 is( join( " ", @$test_extra_fields ),
-    '', "GetCancelledOrders gets orders with no unexpected fields" );
+    '', "GetOrders { cancelled => 1 } gets orders with no unexpected fields" );
 is( join( " ", @$test_different_fields ),
     '',
-    "GetCancelledOrders gets orders with the right content in every fields" );
+    "GetOrders { cancelled => 1 } gets orders with the right content in every fields" );
 ok(
     (
         ( scalar @get_orders == 1 )
           and grep ( $_->{ordernumber} eq $ordernumbers[3], @get_orders )
     ),
-    "GetCancelledOrders only gets cancelled orders"
+    "GetOrders { cancelled => 1 } only gets cancelled orders"
 );
 
 #