From 2c54f5ac80c05adf22dbe380b527d7a385189d11 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 13 Nov 2014 10:07:15 +0100 Subject: [PATCH] [PASSED QA] 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. Signed-off-by: Paola Rossi Signed-off-by: Kyle M Hall --- 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..8b54af3 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 ) ) && ( $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 == EXPECTED && $status != EXPECTED ) { 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 planneddateprepare( "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; } } -- 1.7.2.5