From 1afd7fe27094c963a279dde949cc81bea75e169e Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Thu, 11 Jul 2013 10:43:05 +0200
Subject: [PATCH] Bug 5336: (follow-up) use understandable codes rather than
 magic numbers for orderstatus

This patch uses understandable codes instead of magical numbers for the
aqorders.orderstatus field.

+ execute sql queries in unit tests into a transaction.

Signed-off-by: Pierre Angot <tredok.pierre@gmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
---
 C4/Acquisition.pm                             | 20 +++++------
 installer/data/mysql/kohastructure.sql        |  2 +-
 installer/data/mysql/updatedatabase.pl        | 10 +++---
 .../prog/en/modules/acqui/histsearch.tt       | 21 ++++++------
 .../Acquisition/OrderFromSubscription.t       |  2 +-
 .../Acquisition/close_reopen_basket.t         | 33 +++++++++----------
 6 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index aa0f9ebc83..ebccac12c0 100644
--- a/C4/Acquisition.pm
+++ b/C4/Acquisition.pm
@@ -236,7 +236,7 @@ sub CloseBasket {
     foreach my $order (@orders) {
         $query = qq{
             UPDATE aqorders
-            SET orderstatus = 1
+            SET orderstatus = 'ordered'
             WHERE ordernumber = ?;
         };
         $sth = $dbh->prepare($query);
@@ -267,7 +267,7 @@ sub ReopenBasket {
     foreach my $order (@orders) {
         $query = qq{
             UPDATE aqorders
-            SET orderstatus = 0
+            SET orderstatus = 'new'
             WHERE ordernumber = ?;
         };
         $sth = $dbh->prepare($query);
@@ -1305,7 +1305,7 @@ sub ModReceiveOrder {
         $sth=$dbh->prepare("
             UPDATE aqorders
             SET quantity = ?,
-                orderstatus = 2
+                orderstatus = 'partial'
             WHERE ordernumber = ?
         ");
 
@@ -1322,7 +1322,7 @@ sub ModReceiveOrder {
         $order->{'unitprice'} = $cost;
         $order->{'rrp'} = $rrp;
         $order->{ecost} = $ecost;
-        $order->{'orderstatus'} = 3;    # totally received
+        $order->{'orderstatus'} = 'complete';
         my $basketno;
         ( $basketno, $new_ordernumber ) = NewOrder($order);
 
@@ -1334,7 +1334,7 @@ sub ModReceiveOrder {
     } else {
         $sth=$dbh->prepare("update aqorders
                             set quantityreceived=?,datereceived=?,invoiceid=?,
-                                unitprice=?,rrp=?,ecost=?,budget_id=?,orderstatus=3
+                                unitprice=?,rrp=?,ecost=?,budget_id=?,orderstatus='complete'
                             where biblionumber=? and ordernumber=?");
         $sth->execute($quantrec,$datereceived,$invoiceid,$cost,$rrp,$ecost,$budget_id,$biblionumber,$ordernumber);
         $sth->finish;
@@ -1385,7 +1385,7 @@ sub CancelReceipt {
             SET quantityreceived = ?,
                 datereceived = ?,
                 invoiceid = ?,
-                orderstatus = 1
+                orderstatus = 'ordered'
             WHERE ordernumber = ?
         };
         $sth = $dbh->prepare($query);
@@ -1413,7 +1413,7 @@ sub CancelReceipt {
         $query = qq{
             UPDATE aqorders
             SET quantity = ?,
-                orderstatus = 1
+                orderstatus = 'ordered'
             WHERE ordernumber = ?
         };
         $sth = $dbh->prepare($query);
@@ -1593,7 +1593,7 @@ sub DelOrder {
     my $dbh = C4::Context->dbh;
     my $query = "
         UPDATE aqorders
-        SET    datecancellationprinted=now(), orderstatus=4
+        SET    datecancellationprinted=now(), orderstatus='cancelled'
         WHERE  biblionumber=? AND ordernumber=?
     ";
     my $sth = $dbh->prepare($query);
@@ -1944,7 +1944,7 @@ sub GetLateOrders {
         $from .= ' AND borrowers.branchcode LIKE ? ';
         push @query_params, C4::Context->userenv->{branch};
     }
-    $from .= " AND orderstatus <> 4 ";
+    $from .= " AND orderstatus <> 'cancelled' ";
     my $query = "$select $from $having\nORDER BY latesince, basketno, borrowers.branchcode, supplier";
     $debug and print STDERR "GetLateOrders query: $query\nGetLateOrders args: " . join(" ",@query_params);
     my $sth = $dbh->prepare($query);
@@ -2067,7 +2067,7 @@ sub GetHistory {
 
     $query .= " WHERE 1 ";
 
-    $query .= " AND (datecancellationprinted is NULL or datecancellationprinted='0000-00-00') " if $orderstatus ne '4';
+    $query .= " AND (datecancellationprinted is NULL or datecancellationprinted='0000-00-00') " if $orderstatus ne 'cancelled';
 
     my @query_params  = ();
 
diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index 6cc907b4c3..b1dcf34066 100644
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -2937,7 +2937,7 @@ CREATE TABLE `aqorders` ( -- information related to the basket line items
   `claimed_date` date default NULL, -- last date a claim was generated
   `subscriptionid` int(11) default NULL, -- links this order line to a subscription (subscription.subscriptionid)
   parent_ordernumber int(11) default NULL, -- ordernumber of parent order line, or same as ordernumber if no parent
-  `orderstatus` tinyint(2) default 0, -- the current status for this line item
+  `orderstatus` varchar(16) default 'new', -- the current status for this line item. Can be 'new', 'ordered', 'partial', 'complete' or 'cancelled'
   PRIMARY KEY  (`ordernumber`),
   KEY `basketno` (`basketno`),
   KEY `biblionumber` (`biblionumber`),
diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index aa2e411428..7d8832e6c5 100755
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -7309,11 +7309,11 @@ if ( CheckVersion($DBversion) ) {
 $DBversion = "3.13.00.XXX";
 if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     my $return_count;
-    $dbh->do("ALTER TABLE aqorders ADD COLUMN orderstatus tinyint(2) DEFAULT 0 AFTER parent_ordernumber");
-    $dbh->do("UPDATE aqorders SET orderstatus=1 WHERE basketno IN (SELECT basketno FROM aqbasket WHERE closedate IS NOT NULL)");
-    $dbh->do("UPDATE aqorders SET orderstatus=2 WHERE quantity > quantityreceived AND quantityreceived > 0");
-    $dbh->do("UPDATE aqorders SET orderstatus=3 WHERE quantity=quantityreceived");
-    $dbh->do("UPDATE aqorders SET orderstatus=4 WHERE datecancellationprinted IS NOT NULL");
+    $dbh->do("ALTER TABLE aqorders ADD COLUMN orderstatus varchar(16) DEFAULT 'new' AFTER parent_ordernumber");
+    $dbh->do("UPDATE aqorders SET orderstatus='ordered' WHERE basketno IN (SELECT basketno FROM aqbasket WHERE closedate IS NOT NULL)");
+    $dbh->do("UPDATE aqorders SET orderstatus='partial' WHERE quantity > quantityreceived AND quantityreceived > 0");
+    $dbh->do("UPDATE aqorders SET orderstatus='complete' WHERE quantity=quantityreceived");
+    $dbh->do("UPDATE aqorders SET orderstatus='cancelled' WHERE datecancellationprinted IS NOT NULL");
     print "Upgrade to $DBversion done (Bug 5336: Add the new column aqorders.orderstatus)\n";
     SetVersion($DBversion);
 }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
index e82c1415cd..fe6623bd3b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
@@ -52,14 +52,13 @@
             <label for="orderstatus">Order status: </label>
             <select name="orderstatus" id="orderstatus">
                 <option value=""></option>
-                <option value="0">New</option>
-                <option value="1">Ordered</option>
-                <option value="2">Partial</option>
-                <option value="3">Complete</option>
-                <option value="4">Deleted</option>
+                <option value="new">New orders</option>
+                <option value="ordered">Pending orders</option>
+                <option value="partial">Partially received orders</option>
+                <option value="complete">Received orders</option>
+                <option value="cancelled">Cancelled orders</option>
             </select>
         </li>
-
         <li>
             <label for="fund">Fund: </label>
             <select name="budget" id="fund">
@@ -143,11 +142,11 @@
                     </td>
                     <td>
                         [% SWITCH order.orderstatus %]
-                            [% CASE '0' %]New
-                            [% CASE '1' %]Ordered
-                            [% CASE '2' %]Partial
-                            [% CASE '3' %]Complete
-                            [% CASE '4' %]Deleted
+                            [% CASE 'new' %]New orders
+                            [% CASE 'ordered' %]Pending orders
+                            [% CASE 'partial' %]Partially received orders
+                            [% CASE 'complete' %]Received orders
+                            [% CASE 'cancelled' %]Cancelled orders
                         [% END %]
                     </td>
                     <td>[% order.quantityreceived %]</td>
diff --git a/t/db_dependent/Acquisition/OrderFromSubscription.t b/t/db_dependent/Acquisition/OrderFromSubscription.t
index f767c56518..847d497006 100644
--- a/t/db_dependent/Acquisition/OrderFromSubscription.t
+++ b/t/db_dependent/Acquisition/OrderFromSubscription.t
@@ -70,7 +70,7 @@ my $ordernumber;
     rrp => $cost,
     ecost => $cost,
     gstrate => 0.0500,
-    orderstatus => 0,
+    orderstatus => 'new',
     subscriptionid => $subscription->{subscriptionid},
     budget_id => $budget_id,
 });
diff --git a/t/db_dependent/Acquisition/close_reopen_basket.t b/t/db_dependent/Acquisition/close_reopen_basket.t
index 7ef63411ca..3e7a280103 100644
--- a/t/db_dependent/Acquisition/close_reopen_basket.t
+++ b/t/db_dependent/Acquisition/close_reopen_basket.t
@@ -2,11 +2,21 @@
 
 use Modern::Perl;
 
-use Test::More;
+use Test::More tests => 6;
 use C4::Acquisition;
 use C4::Biblio qw( AddBiblio DelBiblio );
 use C4::Bookseller;
 use C4::Budgets;
+use C4::Context;
+
+# Start transaction
+my $dbh = C4::Context->dbh;
+$dbh->{AutoCommit} = 0;
+$dbh->{RaiseError} = 1;
+
+$dbh->do(q{
+    DELETE FROM aqorders;
+});
 
 my $booksellerid = C4::Bookseller::AddBookseller(
     {
@@ -56,26 +66,15 @@ my $nb_biblio = C4::Acquisition::GetBiblioCountByBasketno( $basketno );
 is ( $nb_biblio, 2, "There are 2 biblio for this basket" );
 my @orders = C4::Acquisition::GetOrders( $basketno );
 is( scalar(@orders), 2, "2 orders are created" );
-is ( scalar( map { $_->{orderstatus} == 0 ? 1 : () } @orders ), 2, "2 order are new before closing the basket" );
+is ( scalar( map { $_->{orderstatus} eq 'new' ? 1 : () } @orders ), 2, "2 orders are new before closing the basket" );
 
 C4::Acquisition::CloseBasket( $basketno );
 @orders = C4::Acquisition::GetOrders( $basketno );
-is ( scalar( map { $_->{orderstatus} == 1 ? 1 : () } @orders ), 2, "2 orders are ordered, the basket is closed" );
+is ( scalar( map { $_->{orderstatus} eq 'ordered' ? 1 : () } @orders ), 2, "2 orders are ordered, the basket is closed" );
 
 C4::Acquisition::ReopenBasket( $basketno );
 @orders = C4::Acquisition::GetOrders( $basketno );
-is ( scalar( map { $_->{orderstatus} == 1 ? 1 : () } @orders ), 0, "No order are ordered, the basket is reopen" );
-is ( scalar( map { $_->{orderstatus} == 0 ? 1 : () } @orders ), 2, "2 order are new, the basket is reopen" );
-
-
-END {
-    C4::Acquisition::DelOrder( 1, $ordernumber1 );
-    C4::Acquisition::DelOrder( 2, $ordernumber2 );
-    C4::Budgets::DelBudget( $budgetid );
-    C4::Acquisition::DelBasket( $basketno );
-    C4::Bookseller::DelBookseller( $booksellerid );
-    C4::Biblio::DelBiblio($biblionumber1);
-    C4::Biblio::DelBiblio($biblionumber2);
-};
+is ( scalar( map { $_->{orderstatus} eq 'ordered' ? 1 : () } @orders ), 0, "No order are ordered, the basket is reopen" );
+is ( scalar( map { $_->{orderstatus} eq 'new' ? 1 : () } @orders ), 2, "2 orders are new, the basket is reopen" );
 
-done_testing;
+$dbh->rollback;
-- 
2.39.5