Bugzilla – Attachment 33991 Details for
Bug 11814
Serial statuses should be stored in constants
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11814 - (follow-up) Use constants to describe statuses
0002-Bug-11814-follow-up-Use-constants-to-describe-status.patch (text/plain), 12.59 KB, created by
Paola Rossi
on 2014-11-27 11:20:22 UTC
(
hide
)
Description:
Bug 11814 - (follow-up) Use constants to describe statuses
Filename:
MIME Type:
Creator:
Paola Rossi
Created:
2014-11-27 11:20:22 UTC
Size:
12.59 KB
patch
obsolete
>From 5f36e6a8c4d723d2211891464e52ba879df933b4 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 2/3] 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 <paola.rossi@cineca.it> >--- > 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 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; > } > } >-- >1.7.10.4 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
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