From 8d7919a9db47789b3219f4cc7f7544041ca859d3 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Fri, 21 Feb 2014 15:27:45 +0100
Subject: [PATCH] Bug 11814 - (follow-up) Use constants to describe statuses

This patch deals with all hard-coded status codes in the C4::Serials
module.

Test plan:
Test a complete workflow in the serial module (create, order, receive,
generate next) trying to use all statuses.
---
 C4/Serials.pm           |   57 ++++++++++++++++++++++++++---------------------
 serials/serials-edit.pl |    3 ++-
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/C4/Serials.pm b/C4/Serials.pm
index ec21e8a..520a487 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -55,7 +55,6 @@ use constant MISSING_STATUSES => (
     MISSING_LOST
 );
 
-
 BEGIN {
     $VERSION = 3.07.00.049;    # set version for version checking
     require Exporter;
@@ -118,6 +117,7 @@ the array is in name order
 sub GetSuppliersWithLateIssues {
     my $dbh   = C4::Context->dbh;
     my $statuses = join(',', ( LATE, MISSING_STATUSES ) );
+
     my $query = qq|
         SELECT DISTINCT id, name
     FROM            subscription
@@ -846,23 +846,27 @@ sub GetSerials {
 
 =head2 GetSerials2
 
-@serials = GetSerials2($subscriptionid,$status);
+@serials = GetSerials2($subscriptionid,$statuses);
 this function returns every serial waited for a given subscription
 as well as the number of issues registered in the database (all types)
 this number is used to see if a subscription can be deleted (=it must have only 1 issue)
 
+$statuses is an arrayref of statuses.
+
 =cut
 
 sub GetSerials2 {
-    my ( $subscription, $status ) = @_;
+    my ( $subscription, $statuses ) = @_;
+
+    return unless ($subscription and @$statuses);
 
-    return unless ($subscription and $status);
+    my $statuses_string = join ',', @$statuses;
 
     my $dbh   = C4::Context->dbh;
     my $query = qq|
                  SELECT   serialid,serialseq, status, planneddate, publisheddate,notes, routingnotes
                  FROM     serial 
-                 WHERE    subscriptionid=$subscription AND status IN ($status)
+                 WHERE    subscriptionid=$subscription AND status IN ($statuses_string)
                  ORDER BY publisheddate,serialid DESC
                     |;
     $debug and warn "GetSerials2 query: $query";
@@ -902,11 +906,12 @@ sub GetLatestSerials {
 
     my $dbh = C4::Context->dbh;
 
+    my $statuses = join( ',', ( ARRIVED, MISSING_STATUSES ) );
     # status = 2 is "arrived"
     my $strsth = "SELECT   serialid,serialseq, status, planneddate, publisheddate, notes
                         FROM     serial
                         WHERE    subscriptionid = ?
-                        AND      status IN (2, 4, 41, 42, 43, 44)
+                        AND      status IN ($statuses)
                         ORDER BY publisheddate DESC LIMIT 0,$limit
                 ";
     my $sth = $dbh->prepare($strsth);
@@ -1189,12 +1194,12 @@ sub _update_missinglist {
     my $subscriptionid = shift;
 
     my $dbh = C4::Context->dbh;
-    my @missingserials = GetSerials2($subscriptionid, "4,41,42,43,44,5");
+    my @missingserials = GetSerials2($subscriptionid, [ MISSING_STATUSES, NOT_ISSUED ]);
     my $missinglist;
     foreach my $missingserial (@missingserials) {
-        if ( grep { $_ == $missingserial->{status} } qw( 4 41 42 43 44 ) ) {
+        if ( grep { $_ == $missingserial->{status} } MISSING_STATUSES ) {
             $missinglist .= $missingserial->{'serialseq'} . "; ";
-        } elsif($missingserial->{'status'} == 5) {
+        } elsif($missingserial->{'status'} == NOT_ISSUED) {
             $missinglist .= "not issued " . $missingserial->{'serialseq'} . "; ";
         }
     }
@@ -1213,7 +1218,7 @@ sub _update_receivedlist {
     my $subscriptionid = shift;
 
     my $dbh = C4::Context->dbh;
-    my @receivedserials = GetSerials2($subscriptionid, "2");
+    my @receivedserials = GetSerials2($subscriptionid, [ARRIVED]);
     my $receivedlist;
     foreach (@receivedserials) {
         $receivedlist .= $_->{'serialseq'} . "; ";
@@ -1256,7 +1261,7 @@ sub ModSerialStatus {
 
     # change status & update subscriptionhistory
     my $val;
-    if ( $status == 6 ) {
+    if ( $status == DELETED ) {
         DelIssue( { 'serialid' => $serialid, 'subscriptionid' => $subscriptionid, 'serialseq' => $serialseq } );
     } else {
 
@@ -1272,22 +1277,21 @@ sub ModSerialStatus {
         $sth->execute($subscriptionid);
         my $val = $sth->fetchrow_hashref;
         unless ( $val->{manualhistory} ) {
-            if ( $status == 2 || ($oldstatus == 2 && $status != 2) ) {
+            if ( $status == ARRIVED || ($oldstatus == ARRIVED && $status != ARRIVED) ) {
                   _update_receivedlist($subscriptionid);
             }
-            my @missing_statuses = qw( 4 41 42 43 44 );
-            if ( (  grep { $_ == $status } ( @missing_statuses, 5 ) )
+            if ( (  grep { $_ == $status } ( MISSING_STATUSES, NOT_ISSUED ) )
               || (
-                  ( grep { $_ == $oldstatus } @missing_statuses )
-                  && ! ( grep { $_ == $status } @missing_statuses ) )
-              || ($oldstatus == 5 && $status != 5)) {
+                  ( grep { $_ == $oldstatus } ( MISSING_STATUSES ) )
+                  && ! ( grep { $_ == $status } ( MISSING_STATUSES ) ) )
+              || ($oldstatus == NOT_ISSUED && $status != NOT_ISSUED)) {
                 _update_missinglist($subscriptionid);
             }
         }
     }
 
     # create new waited entry if needed (ie : was a "waited" and has changed)
-    if ( $oldstatus == 1 && $status != 1 ) {
+    if ( $oldstatus == EXPECTED && $status != EXPECTED ) {
         my $subscription = GetSubscription($subscriptionid);
         my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern});
 
@@ -1308,7 +1312,7 @@ sub ModSerialStatus {
         $sth->execute( $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, $newinnerloop2, $newinnerloop3, $subscriptionid );
 
         # check if an alert must be sent... (= a letter is defined & status became "arrived"
-        if ( $subscription->{letter} && $status == 2 && $oldstatus != 2 ) {
+        if ( $subscription->{letter} && $status == ARRIVED && $oldstatus != ARRIVED ) {
             require C4::Letters;
             C4::Letters::SendAlerts( 'issue', $subscription->{subscriptionid}, $subscription->{letter} );
         }
@@ -1346,7 +1350,7 @@ sub GetNextExpected {
     my $sth = $dbh->prepare($query);
 
     # Each subscription has only one 'expected' issue, with serial.status==1.
-    $sth->execute( $subscriptionid, 1 );
+    $sth->execute( $subscriptionid, EXPECTED );
     my $nextissue = $sth->fetchrow_hashref;
     if ( !$nextissue ) {
         $query = qq{
@@ -1394,7 +1398,7 @@ sub ModNextExpected {
     my $sth = $dbh->prepare('UPDATE serial SET planneddate=?,publisheddate=? WHERE subscriptionid=? AND status=?');
 
     # Each subscription has only one 'expected' issue, with serial.status==1.
-    $sth->execute( $date, $date, $subscriptionid, 1 );
+    $sth->execute( $date, $date, $subscriptionid, EXPECTED );
     return 0;
 
 }
@@ -1564,7 +1568,7 @@ sub NewSubscription {
         VALUES (?,?,?,?,?,?)
     |;
     $sth = $dbh->prepare($query);
-    $sth->execute( "$serialseq", $subscriptionid, $biblionumber, 1, $firstacquidate, $firstacquidate );
+    $sth->execute( "$serialseq", $subscriptionid, $biblionumber, EXPECTED, $firstacquidate, $firstacquidate );
 
     logaction( "SERIAL", "ADD", $subscriptionid, "" ) if C4::Context->preference("SubscriptionLog");
 
@@ -1679,13 +1683,13 @@ sub NewIssue {
     $sth->execute($subscriptionid);
     my ( $missinglist, $recievedlist ) = $sth->fetchrow;
 
-    if ( $status == 2 ) {
+    if ( $status == ARRIVED ) {
       ### TODO Add a feature that improves recognition and description.
       ### As such count (serialseq) i.e. : N18,2(N19),N20
       ### Would use substr and index But be careful to previous presence of ()
         $recievedlist .= "; $serialseq" unless (index($recievedlist,$serialseq)>0);
     }
-    if ( $status == 4 ) {
+    if ( grep {/^$status$/} ( MISSING_STATUSES ) ) {
         $missinglist .= "; $serialseq" unless (index($missinglist,$serialseq)>0);
     }
     $query = qq|
@@ -2048,6 +2052,7 @@ sub GetLateOrMissingIssues {
     } else {
         $order = "title";
     }
+    my $missing_statuses_string = join ',', (MISSING_STATUSES);
     if ($supplierid) {
         $sth = $dbh->prepare(
             "SELECT
@@ -2060,7 +2065,7 @@ sub GetLateOrMissingIssues {
                 LEFT JOIN biblio        ON subscription.biblionumber=biblio.biblionumber
                 LEFT JOIN aqbooksellers ON subscription.aqbooksellerid = aqbooksellers.id
                 WHERE subscription.subscriptionid = serial.subscriptionid 
-                AND (serial.STATUS IN (4, 41, 42, 43, 44) OR ((planneddate < now() AND serial.STATUS =1) OR serial.STATUS = 3 OR serial.STATUS = 7))
+                AND (serial.STATUS IN ($missing_statuses_string) OR ((planneddate < now() AND serial.STATUS =1) OR serial.STATUS = 3 OR serial.STATUS = 7))
                 AND subscription.aqbooksellerid=$supplierid
                 $byserial
                 ORDER BY $order"
@@ -2077,7 +2082,7 @@ sub GetLateOrMissingIssues {
                 LEFT JOIN biblio ON subscription.biblionumber=biblio.biblionumber
                 LEFT JOIN aqbooksellers ON subscription.aqbooksellerid = aqbooksellers.id
                 WHERE subscription.subscriptionid = serial.subscriptionid 
-                        AND (serial.STATUS IN (4, 41, 42, 43, 44) OR ((planneddate < now() AND serial.STATUS =1) OR serial.STATUS = 3 OR serial.STATUS = 7))
+                        AND (serial.STATUS IN ($missing_statuses_string) OR ((planneddate < now() AND serial.STATUS =1) OR serial.STATUS = 3 OR serial.STATUS = 7))
                 $byserial
                 ORDER BY $order"
         );
diff --git a/serials/serials-edit.pl b/serials/serials-edit.pl
index 8fb2e60..2247e26 100755
--- a/serials/serials-edit.pl
+++ b/serials/serials-edit.pl
@@ -94,9 +94,10 @@ my @errseq;
 # If user comes from subscription details
 unless (@serialids) {
     my $serstatus = $query->param('serstatus');
+    my @statuses = split ',', $serstatus;
     if ($serstatus) {
         foreach my $subscriptionid (@subscriptionids) {
-            my @tmpser = GetSerials2( $subscriptionid, $serstatus );
+            my @tmpser = GetSerials2( $subscriptionid, \@statuses );
             push @serialids, map { $_->{serialid} } @tmpser;
         }
     }
-- 
1.7.10.4