From 12d2470b6dcde8c154810925f9f1f3e6dcab62e1 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 | 79 +++++++++++++++++++++++++---------------------- serials/serials-edit.pl | 3 +- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 9d3fe91..36b6a9e 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 @@ -333,10 +333,10 @@ sub UpdateClaimdateIssues { my $dbh = C4::Context->dbh; $date = strftime( "%Y-%m-%d", localtime ) unless ($date); my $query = " - UPDATE serial SET claimdate = ?, status = 7 + UPDATE serial SET claimdate = ?, status = ? WHERE serialid in (" . join( ",", map { '?' } @$serialids ) . ")"; my $rq = $dbh->prepare($query); - $rq->execute($date, @$serialids); + $rq->execute($date, @$serialids, CLAIMED); return $rq->rows; } @@ -843,23 +843,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"; @@ -899,11 +903,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); @@ -1186,12 +1191,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'} . "; "; } } @@ -1210,7 +1215,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'} . "; "; @@ -1253,7 +1258,7 @@ sub ModSerialStatus { # change status & update subscriptionhistory my $val; - if ( $status == 6 ) { + if ( $status == DELETED ) { DelIssue( { 'serialid' => $serialid, 'subscriptionid' => $subscriptionid, 'serialseq' => $serialseq } ); } else { @@ -1265,22 +1270,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}); @@ -1301,7 +1305,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} ); } @@ -1339,7 +1343,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{ @@ -1387,7 +1391,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; } @@ -1557,7 +1561,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"); @@ -1672,13 +1676,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| @@ -2041,6 +2045,7 @@ sub GetLateOrMissingIssues { } else { $order = "title"; } + my $missing_statuses_string = join ',', (MISSING_STATUSES); if ($supplierid) { $sth = $dbh->prepare( "SELECT @@ -2054,7 +2059,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" @@ -2071,12 +2076,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 ) { @@ -2784,7 +2789,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 = ? @@ -2792,13 +2797,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 } ); - $sth->execute( $subscriptionid ); + $sth->execute( STOPPED, $subscriptionid ); } =head2 ReopenSubscription @@ -2808,7 +2813,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 = ? @@ -2816,13 +2821,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 WHERE subscriptionid = ? - AND status = 8 + AND status = ? } ); - $sth->execute( $subscriptionid ); + $sth->execute( $subscriptionid, STOPPED ); } =head2 subscriptionCurrentlyOnOrder 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