From 8d7919a9db47789b3219f4cc7f7544041ca859d3 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 | 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