Attachment 33513 Details for Bug 11814 – Bug 11814 - (follow-up) Use constants to describe statuses

[patch] Bug 11814 - (follow-up) Use constants to describe statuses

Bug-11814---follow-up-Use-constants-to-describe-st.patch (text/plain), 12.24 KB, created by Jonathan Druart on 2014-11-13 09:07:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2014-11-13 09:07:54 UTC

Size: 12.24 KB

patch

obsolete

>From fea8b7eb1e24daf49bc07daa51a2f316cbbb6ef3 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>Date: Thu, 13 Nov 2014 10:07:15 +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           | 79 +++++++++++++++++++++++++------------------------
> serials/serials-edit.pl |  3 +-
> 2 files changed, 43 insertions(+), 39 deletions(-)
>
>diff --git a/C4/Serials.pm b/C4/Serials.pm
>index 9c21897..d70745c 100644
>--- a/C4/Serials.pm
>+++ b/C4/Serials.pm
>@@ -56,7 +56,6 @@ use constant MISSING_STATUSES => (
>     MISSING_LOST
> );
> 
>-
> BEGIN {
>     $VERSION = 3.07.00.049;    # set version for version checking
>     require Exporter;
>@@ -336,12 +335,12 @@ sub UpdateClaimdateIssues {
>     my $query = "
>         UPDATE serial
>         SET claimdate = ?,
>-            status = 7,
>+            status = ?,
>             claims_count = claims_count + 1
>         WHERE  serialid in (" . join( ",", map { '?' } @$serialids ) . ")
>     ";
>     my $rq = $dbh->prepare($query);
>-    $rq->execute($date, @$serialids);
>+    $rq->execute($date, @$serialids, CLAIMED);
>     return $rq->rows;
> }
> 
>@@ -848,23 +847,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 $status);
>+    return unless ($subscription and @$statuses);
>+
>+    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";
>@@ -904,11 +907,11 @@ sub GetLatestSerials {
> 
>     my $dbh = C4::Context->dbh;
> 
>-    # status = 2 is "arrived"
>+    my $statuses = join( ',', ( ARRIVED, MISSING_STATUSES ) );
>     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);
>@@ -1214,7 +1217,7 @@ sub ModSerialStatus {
> 
>     # change status & update subscriptionhistory
>     my $val;
>-    if ( $status == 6 ) {
>+    if ( $status == DELETED ) {
>         DelIssue( { 'serialid' => $serialid, 'subscriptionid' => $subscriptionid, 'serialseq' => $serialseq } );
>     } else {
> 
>@@ -1231,21 +1234,20 @@ sub ModSerialStatus {
>             $sth->execute($subscriptionid);
>             my ( $missinglist, $recievedlist ) = $sth->fetchrow;
> 
>-            if ( $status == 2 || ($oldstatus == 2 && $status != 2) ) {
>+            if ( $status == ARRIVED || ($oldstatus == ARRIVED && $status != ARRIVED) ) {
>                 $recievedlist .= "; $serialseq"
>                     if ($recievedlist !~ /(^|;)\s*$serialseq(?=;|$)/);
>             }
> 
>             # in case serial has been previously marked as missing
>-            if (grep /$status/, (1,2,3,7)) {
>+            if (grep /$status/, (EXPECTED, ARRIVED, LATE, CLAIMED)) {
>                 $missinglist=~ s/(^|;)\s*$serialseq(?=;|$)//g;
>             }
> 
>-            my @missing_statuses = qw( 4 41 42 43 44 );
>             $missinglist .= "; $serialseq"
>-                if ( ( grep { $_ == $status } @missing_statuses ) && ( $missinglist !~/(^|;)\s*$serialseq(?=;|$)/ ) );
>+                if ( ( grep { $_ == $status } ( MISSING_STATUSES, NOT_ISSUED ) && ( $missinglist !~/(^|;)\s*$serialseq(?=;|$)/ ) );
>             $missinglist .= "; not issued $serialseq"
>-                if ( $status == 5 && $missinglist !~ /(^|;)\s*$serialseq(?=;|$)/ );
>+                if ( $status == NOT_ISSUED && $missinglist !~ /(^|;)\s*$serialseq(?=;|$)/ );
> 
>             $query = "UPDATE subscriptionhistory SET recievedlist=?, missinglist=? WHERE  subscriptionid=?";
>             $sth   = $dbh->prepare($query);
>@@ -1256,7 +1258,7 @@ sub ModSerialStatus {
>     }
> 
>     # create new waited entry if needed (ie : was a "waited" and has changed)
>-    if ( $oldstatus == 1 && $status != 1 ) {
>+    if ( $oldstatus == ARRIVED && $status != ARRIVED ) {
>         my $subscription = GetSubscription($subscriptionid);
>         my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern});
> 
>@@ -1277,7 +1279,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} );
>         }
>@@ -1314,8 +1316,8 @@ sub GetNextExpected {
>     };
>     my $sth = $dbh->prepare($query);
> 
>-    # Each subscription has only one 'expected' issue, with serial.status==1.
>-    $sth->execute( $subscriptionid, 1 );
>+    # Each subscription has only one 'expected' issue.
>+    $sth->execute( $subscriptionid, EXPECTED );
>     my $nextissue = $sth->fetchrow_hashref;
>     if ( !$nextissue ) {
>         $query = qq{
>@@ -1362,8 +1364,8 @@ sub ModNextExpected {
>     #FIXME: Would expect to only set planneddate, but we set both on new issue creation, so updating it here
>     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 );
>+    # Each subscription has only one 'expected' issue.
>+    $sth->execute( $date, $date, $subscriptionid, EXPECTED );
>     return 0;
> 
> }
>@@ -1533,7 +1535,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");
> 
>@@ -1648,13 +1650,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|
>@@ -1993,7 +1995,7 @@ sub DelIssue {
> 
> @issuelist = GetLateMissingIssues($supplierid,$serialid)
> 
>-this function selects missing issues on database - where serial.status = 4 or serial.status=3 or planneddate<now
>+this function selects missing issues on database - where serial.status = MISSING* or serial.status = LATE or planneddate<now
> 
> return :
> the issuelist as an array of hash refs. Each element of this array contains 
>@@ -2017,6 +2019,7 @@ sub GetLateOrMissingIssues {
>     } else {
>         $order = "title";
>     }
>+    my $missing_statuses_string = join ',', (MISSING_STATUSES);
>     if ($supplierid) {
>         $sth = $dbh->prepare(
>             "SELECT
>@@ -2030,7 +2033,7 @@ sub GetLateOrMissingIssues {
>                 LEFT JOIN biblioitems   ON subscription.biblionumber=biblioitems.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 = ?) OR serial.STATUS = ? OR serial.STATUS = ?))
>                 AND subscription.aqbooksellerid=$supplierid
>                 $byserial
>                 ORDER BY $order"
>@@ -2047,12 +2050,12 @@ 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 = ?) OR serial.STATUS = ? OR serial.STATUS = ?))
>                 $byserial
>                 ORDER BY $order"
>         );
>     }
>-    $sth->execute;
>+    $sth->execute( EXPECTED, LATE, CLAIMED );
>     my @issuelist;
>     while ( my $line = $sth->fetchrow_hashref ) {
> 
>@@ -2753,7 +2756,7 @@ sub CloseSubscription {
>     my ( $subscriptionid ) = @_;
>     return unless $subscriptionid;
>     my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare( qq{
>+    my $sth = $dbh->prepare( q{
>         UPDATE subscription
>         SET closed = 1
>         WHERE subscriptionid = ?
>@@ -2761,13 +2764,13 @@ sub CloseSubscription {
>     $sth->execute( $subscriptionid );
> 
>     # Set status = missing when status = stopped
>-    $sth = $dbh->prepare( qq{
>+    $sth = $dbh->prepare( q{
>         UPDATE serial
>-        SET status = 8
>+        SET status = ?
>         WHERE subscriptionid = ?
>-        AND status = 1
>+        AND status = ?
>     } );
>-    $sth->execute( $subscriptionid );
>+    $sth->execute( STOPPED, $subscriptionid, EXPECTED );
> }
> 
> =head2 ReopenSubscription
>@@ -2777,7 +2780,7 @@ sub ReopenSubscription {
>     my ( $subscriptionid ) = @_;
>     return unless $subscriptionid;
>     my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare( qq{
>+    my $sth = $dbh->prepare( q{
>         UPDATE subscription
>         SET closed = 0
>         WHERE subscriptionid = ?
>@@ -2785,13 +2788,13 @@ sub ReopenSubscription {
>     $sth->execute( $subscriptionid );
> 
>     # Set status = expected when status = stopped
>-    $sth = $dbh->prepare( qq{
>+    $sth = $dbh->prepare( q{
>         UPDATE serial
>-        SET status = 1
>+        SET status = ?
>         WHERE subscriptionid = ?
>-        AND status = 8
>+        AND status = ?
>     } );
>-    $sth->execute( $subscriptionid );
>+    $sth->execute( EXPECTED, $subscriptionid, STOPPED );
> }
> 
> =head2 subscriptionCurrentlyOnOrder
>diff --git a/serials/serials-edit.pl b/serials/serials-edit.pl
>index 2eeed10..d11eaa5 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;
>         }
>     }
>-- 
>2.1.0

Actions: View | Diff | Splinter Review

Attachments on bug 11814: 25541 | 25542 | 27605 | 33512 | 33513 | 33514 | 33552 | 33890 | 33990 | 33991 | 33992 | 34550 | 34563 | 34564 | 34565 | 34566 | 34567 | 34568 | 34569 | 34570