From 927b153d4ed1de31e94b830f613a67650edf28dc Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Tue, 11 Dec 2012 11:16:54 +0100 Subject: [PATCH] [PASSED QA] Bug 7688: Fix generating next serial when there is no 'Expected' issue It can happen when the Expected issue is claimed. In this case the status of the last serial is 'Claimed' This patch change the API of GetNextSeq and GetSeq Test plan: - Create a subscription which starts a long time ago so that serials automatically appear in late issues - Receive the first serial - Go to claims page and claim the 2nd serial. - Go back to the subscription page and click on 'Serial collection' - You should have 2 serials, one 'Arrived' and one 'Claimed'. - Click on Generate Next. This should fail with a software error message ("can't call method output ...") - Apply this patch and click again on Generate Next. A new issue must be created with status 'Expected'. Signed-off-by: Bernardo Gonzalez Kriegel Signed-off-by: Paul Poulain --- C4/Serials.pm | 134 ++++++++++++++++++-------------------- serials/serials-collection.pl | 62 +++++++++--------- serials/showpredictionpattern.pl | 20 +++--- 3 files changed, 106 insertions(+), 110 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 5676883..b37aa55 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -889,26 +889,32 @@ sub GetDistributedTo { =head2 GetNextSeq -GetNextSeq($val) -$val is a hashref containing all the attributes of the table 'subscription' + my ( + $nextseq, $newlastvalue1, $newlastvalue2, $newlastvalue3, + $newinnerloop1, $newinnerloop2, $newinnerloop3 + ) = GetNextSeq( $subscription, $pattern, $planneddate ); + +$subscription is a hashref containing all the attributes of the table +'subscription'. +$pattern is a hashref containing all the attributes of the table +'subscription_numberpatterns'. +$planneddate is a C4::Dates object. This function get the next issue for the subscription given on input arg -return: -a list containing all the input params updated. =cut sub GetNextSeq { - my ($val, $planneddate) = @_; + my ($subscription, $pattern, $planneddate) = @_; my ( $calculated, $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, $newinnerloop2, $newinnerloop3 ); my $count = 1; - if($val->{'skip_serialseq'}) { - my @irreg = split /;/, $val->{'irregularity'}; + if ($subscription->{'skip_serialseq'}) { + my @irreg = split /;/, $subscription->{'irregularity'}; if(@irreg > 0) { my $irregularities = {}; $irregularities->{$_} = 1 foreach(@irreg); - my $issueno = GetFictiveIssueNumber($val, $planneddate) + 1; + my $issueno = GetFictiveIssueNumber($subscription, $planneddate) + 1; while($irregularities->{$issueno}) { $count++; $issueno++; @@ -916,62 +922,62 @@ sub GetNextSeq { } } - my $pattern = $val->{numberpattern}; - $calculated = $val->{numberingmethod}; - my $locale = $val->{locale}; - $newlastvalue1 = $val->{lastvalue1} || 0; - $newlastvalue2 = $val->{lastvalue2} || 0; - $newlastvalue3 = $val->{lastvalue3} || 0; - $newinnerloop1 = $val->{innerloop1} || 0; - $newinnerloop2 = $val->{innerloop2} || 0; - $newinnerloop3 = $val->{innerloop3} || 0; + my $numberingmethod = $pattern->{numberingmethod}; + $calculated = $numberingmethod; + my $locale = $subscription->{locale}; + $newlastvalue1 = $subscription->{lastvalue1} || 0; + $newlastvalue2 = $subscription->{lastvalue2} || 0; + $newlastvalue3 = $subscription->{lastvalue3} || 0; + $newinnerloop1 = $subscription->{innerloop1} || 0; + $newinnerloop2 = $subscription->{innerloop2} || 0; + $newinnerloop3 = $subscription->{innerloop3} || 0; my %calc; foreach(qw/X Y Z/) { - $calc{$_} = 1 if ($val->{'numberingmethod'} =~ /\{$_\}/); + $calc{$_} = 1 if ($numberingmethod =~ /\{$_\}/); } for(my $i = 0; $i < $count; $i++) { if($calc{'X'}) { # check if we have to increase the new value. $newinnerloop1 += 1; - if ($newinnerloop1 >= $val->{every1}) { + if ($newinnerloop1 >= $pattern->{every1}) { $newinnerloop1 = 0; - $newlastvalue1 += $val->{add1}; + $newlastvalue1 += $pattern->{add1}; } # reset counter if needed. - $newlastvalue1 = $val->{setto1} if ($newlastvalue1 > $val->{whenmorethan1}); + $newlastvalue1 = $pattern->{setto1} if ($newlastvalue1 > $pattern->{whenmorethan1}); } if($calc{'Y'}) { # check if we have to increase the new value. $newinnerloop2 += 1; - if ($newinnerloop2 >= $val->{every2}) { + if ($newinnerloop2 >= $pattern->{every2}) { $newinnerloop2 = 0; - $newlastvalue2 += $val->{add2}; + $newlastvalue2 += $pattern->{add2}; } # reset counter if needed. - $newlastvalue2 = $val->{setto2} if ($newlastvalue2 > $val->{whenmorethan2}); + $newlastvalue2 = $pattern->{setto2} if ($newlastvalue2 > $pattern->{whenmorethan2}); } if($calc{'Z'}) { # check if we have to increase the new value. $newinnerloop3 += 1; - if ($newinnerloop3 >= $val->{every3}) { + if ($newinnerloop3 >= $pattern->{every3}) { $newinnerloop3 = 0; - $newlastvalue3 += $val->{add3}; + $newlastvalue3 += $pattern->{add3}; } # reset counter if needed. - $newlastvalue3 = $val->{setto3} if ($newlastvalue3 > $val->{whenmorethan3}); + $newlastvalue3 = $pattern->{setto3} if ($newlastvalue3 > $pattern->{whenmorethan3}); } } if($calc{'X'}) { - my $newlastvalue1string = _numeration( $newlastvalue1, $val->{numbering1}, $locale ); + my $newlastvalue1string = _numeration( $newlastvalue1, $pattern->{numbering1}, $locale ); $calculated =~ s/\{X\}/$newlastvalue1string/g; } if($calc{'Y'}) { - my $newlastvalue2string = _numeration( $newlastvalue2, $val->{numbering2}, $locale ); + my $newlastvalue2string = _numeration( $newlastvalue2, $pattern->{numbering2}, $locale ); $calculated =~ s/\{Y\}/$newlastvalue2string/g; } if($calc{'Z'}) { - my $newlastvalue3string = _numeration( $newlastvalue3, $val->{numbering3}, $locale ); + my $newlastvalue3string = _numeration( $newlastvalue3, $pattern->{numbering3}, $locale ); $calculated =~ s/\{Z\}/$newlastvalue3string/g; } @@ -982,31 +988,31 @@ sub GetNextSeq { =head2 GetSeq -$calculated = GetSeq($val) -$val is a hashref containing all the attributes of the table 'subscription' +$calculated = GetSeq($subscription, $pattern) +$subscription is a hashref containing all the attributes of the table 'subscription' +$pattern is a hashref containing all the attributes of the table 'subscription_numberpatterns' this function transforms {X},{Y},{Z} to 150,0,0 for example. return: -the sequence in integer format +the sequence in string format =cut sub GetSeq { - my ($val) = @_; - my $locale = $val->{locale}; + my ($subscription, $pattern) = @_; + my $locale = $subscription->{locale}; - my $pattern = $val->{numberpattern}; - my $calculated = $val->{numberingmethod}; + my $calculated = $pattern->{numberingmethod}; - my $newlastvalue1 = $val->{'lastvalue1'} || 0; - $newlastvalue1 = _numeration($newlastvalue1, $val->{numbering1}, $locale) if ($val->{numbering1}); # reset counter if needed. + my $newlastvalue1 = $subscription->{'lastvalue1'} || 0; + $newlastvalue1 = _numeration($newlastvalue1, $pattern->{numbering1}, $locale) if ($pattern->{numbering1}); # reset counter if needed. $calculated =~ s/\{X\}/$newlastvalue1/g; - my $newlastvalue2 = $val->{'lastvalue2'} || 0; - $newlastvalue2 = _numeration($newlastvalue2, $val->{numbering2}, $locale) if ($val->{numbering2}); # reset counter if needed. + my $newlastvalue2 = $subscription->{'lastvalue2'} || 0; + $newlastvalue2 = _numeration($newlastvalue2, $pattern->{numbering2}, $locale) if ($pattern->{numbering2}); # reset counter if needed. $calculated =~ s/\{Y\}/$newlastvalue2/g; - my $newlastvalue3 = $val->{'lastvalue3'} || 0; - $newlastvalue3 = _numeration($newlastvalue3, $val->{numbering3}, $locale) if ($val->{numbering3}); # reset counter if needed. + my $newlastvalue3 = $subscription->{'lastvalue3'} || 0; + $newlastvalue3 = _numeration($newlastvalue3, $pattern->{numbering3}, $locale) if ($pattern->{numbering3}); # reset counter if needed. $calculated =~ s/\{Z\}/$newlastvalue3/g; return $calculated; } @@ -1205,34 +1211,29 @@ sub ModSerialStatus { # create new waited entry if needed (ie : was a "waited" and has changed) if ( $oldstatus == 1 && $status != 1 ) { - my $query = qq{ - SELECT subscription.*, subscription_numberpatterns.*, - subscription_frequencies.* - FROM subscription - LEFT JOIN subscription_numberpatterns ON subscription.numberpattern = subscription_numberpatterns.id - LEFT JOIN subscription_frequencies ON subscription.periodicity = subscription_frequencies.id - WHERE subscriptionid = ? - }; - $sth = $dbh->prepare($query); - $sth->execute($subscriptionid); - my $val = $sth->fetchrow_hashref; + my $subscription = GetSubscription($subscriptionid); + my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern}); # next issue number - my ( $newserialseq, $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, $newinnerloop2, $newinnerloop3 ) = GetNextSeq($val, $publisheddate); + my ( + $newserialseq, $newlastvalue1, $newlastvalue2, $newlastvalue3, + $newinnerloop1, $newinnerloop2, $newinnerloop3 + ) + = GetNextSeq( $subscription, $pattern, $publisheddate ); # next date (calculated from actual date & frequency parameters) - my $nextpublisheddate = GetNextDate($val, $publisheddate, 1); + my $nextpublisheddate = GetNextDate($subscription, $publisheddate, 1); my $nextpubdate = $nextpublisheddate; - NewIssue( $newserialseq, $subscriptionid, $val->{'biblionumber'}, 1, $nextpubdate, $nextpubdate ); + NewIssue( $newserialseq, $subscriptionid, $subscription->{'biblionumber'}, 1, $nextpubdate, $nextpubdate ); $query = "UPDATE subscription SET lastvalue1=?, lastvalue2=?, lastvalue3=?, innerloop1=?, innerloop2=?, innerloop3=? WHERE subscriptionid = ?"; $sth = $dbh->prepare($query); $sth->execute( $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, $newinnerloop2, $newinnerloop3, $subscriptionid ); # check if an alert must be sent... (= a letter is defined & status became "arrived" - if ( $val->{letter} && $status == 2 && $oldstatus != 2 ) { + if ( $subscription->{letter} && $status == 2 && $oldstatus != 2 ) { require C4::Letters; - C4::Letters::SendAlerts( 'issue', $val->{subscriptionid}, $val->{letter} ); + C4::Letters::SendAlerts( 'issue', $subscription->{subscriptionid}, $subscription->{letter} ); } } @@ -1275,7 +1276,7 @@ sub GetNextExpected { SELECT * FROM serial WHERE subscriptionid = ? - ORDER BY planneddate DESC + ORDER BY publisheddate DESC LIMIT 1 }; $sth = $dbh->prepare($query); @@ -1475,18 +1476,11 @@ sub NewSubscription { $sth->execute( $biblionumber, $subscriptionid, $startdate, $notes, $internalnotes ); # reread subscription to get a hash (for calculation of the 1st issue number) - $query = qq( - SELECT * - FROM subscription - LEFT JOIN subscription_numberpatterns ON subscription.numberpattern = subscription_numberpatterns.id - WHERE subscriptionid = ? - ); - $sth = $dbh->prepare($query); - $sth->execute($subscriptionid); - my $val = $sth->fetchrow_hashref; + my $subscription = GetSubscription($subscriptionid); + my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern}); # calculate issue number - my $serialseq = GetSeq($val); + my $serialseq = GetSeq($subscription, $pattern); $query = qq| INSERT INTO serial (serialseq,subscriptionid,biblionumber,status, planneddate, publisheddate) diff --git a/serials/serials-collection.pl b/serials/serials-collection.pl index cdeb6eb..28b4d6a 100755 --- a/serials/serials-collection.pl +++ b/serials/serials-collection.pl @@ -61,38 +61,40 @@ if($op eq 'gennext' && @subscriptionid){ my $status = defined( $nbissues ) ? 2 : 3; $nbissues ||= 1; for ( my $i = 0; $i < $nbissues; $i++ ){ - $sth->execute($subscriptionid); - # modify actual expected issue, to generate the next - if ( my $issue = $sth->fetchrow_hashref ) { - ModSerialStatus( $issue->{serialid}, $issue->{serialseq}, - $issue->{planneddate}, $issue->{publisheddate}, - $status, "" ); - }else{ + $sth->execute($subscriptionid); + # modify actual expected issue, to generate the next + if ( my $issue = $sth->fetchrow_hashref ) { + ModSerialStatus( $issue->{serialid}, $issue->{serialseq}, + $issue->{planneddate}, $issue->{publisheddate}, + $status, "" ); + } else { + require C4::Serials::Numberpattern; my $subscription = GetSubscription($subscriptionid); + my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern}); my $expected = GetNextExpected($subscriptionid); - my ( - $newserialseq, $newlastvalue1, $newlastvalue2, $newlastvalue3, - $newinnerloop1, $newinnerloop2, $newinnerloop3 - ) = GetNextSeq($subscription); - - ## We generate the next publication date - my $nextpublisheddate = GetNextDate( $expected->{planneddate}->output('iso'), $subscription ); - ## Creating the new issue - NewIssue( $newserialseq, $subscriptionid, $subscription->{'biblionumber'}, - 1, $nextpublisheddate, $nextpublisheddate ); - - ## Updating the subscription seq status - my $squery = "UPDATE subscription SET lastvalue1=?, lastvalue2=?, lastvalue3=?, innerloop1=?, innerloop2=?, innerloop3=? - WHERE subscriptionid = ?"; - my $seqsth = $dbh->prepare($squery); - $seqsth->execute( - $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, - $newinnerloop2, $newinnerloop3, $subscriptionid - ); - - } - last if $nbissues == 1; - last if HasSubscriptionExpired($subscriptionid) > 0; + my ( + $newserialseq, $newlastvalue1, $newlastvalue2, $newlastvalue3, + $newinnerloop1, $newinnerloop2, $newinnerloop3 + ) = GetNextSeq($subscription, $pattern, $expected->{publisheddate}); + + ## We generate the next publication date + my $nextpublisheddate = GetNextDate($subscription, $expected->{publisheddate}, 1); + ## Creating the new issue + NewIssue( $newserialseq, $subscriptionid, $subscription->{'biblionumber'}, + 1, $nextpublisheddate, $nextpublisheddate ); + + ## Updating the subscription seq status + my $squery = "UPDATE subscription SET lastvalue1=?, lastvalue2=?, lastvalue3=?, innerloop1=?, innerloop2=?, innerloop3=? + WHERE subscriptionid = ?"; + my $seqsth = $dbh->prepare($squery); + $seqsth->execute( + $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, + $newinnerloop2, $newinnerloop3, $subscriptionid + ); + + } + last if $nbissues == 1; + last if HasSubscriptionExpired($subscriptionid) > 0; } print $query->redirect('/cgi-bin/koha/serials/serials-collection.pl?subscriptionid='.$subscriptionid); } diff --git a/serials/showpredictionpattern.pl b/serials/showpredictionpattern.pl index e56695c..1d9b867 100755 --- a/serials/showpredictionpattern.pl +++ b/serials/showpredictionpattern.pl @@ -55,15 +55,11 @@ my $sublength = $input->param('sublength'); my $custompattern = $input->param('custompattern'); -my %val = ( - locale => $input->param('locale') // '', +my %pattern = ( numberingmethod => $input->param('numberingmethod') // '', numbering1 => $input->param('numbering1') // '', numbering2 => $input->param('numbering2') // '', numbering3 => $input->param('numbering3') // '', - lastvalue1 => $input->param('lastvalue1') // '', - lastvalue2 => $input->param('lastvalue2') // '', - lastvalue3 => $input->param('lastvalue3') // '', add1 => $input->param('add1') // '', add2 => $input->param('add2') // '', add3 => $input->param('add3') // '', @@ -76,9 +72,6 @@ my %val = ( every1 => $input->param('every1') // '', every2 => $input->param('every2') // '', every3 => $input->param('every3') // '', - innerloop1 => $input->param('innerloop1') // '', - innerloop2 => $input->param('innerloop2') // '', - innerloop3 => $input->param('innerloop3') // '', ); if(!defined $firstacquidate || $firstacquidate eq ''){ @@ -100,6 +93,13 @@ if($nextacquidate) { my $date = $nextacquidate; my %subscription = ( + locale => $input->param('locale') // '', + lastvalue1 => $input->param('lastvalue1') // '', + lastvalue2 => $input->param('lastvalue2') // '', + lastvalue3 => $input->param('lastvalue3') // '', + innerloop1 => $input->param('innerloop1') // '', + innerloop2 => $input->param('innerloop2') // '', + innerloop3 => $input->param('innerloop3') // '', irregularity => '', periodicity => $frequencyid, countissuesperunit => 1, @@ -114,7 +114,7 @@ if(defined $subscriptionid) { } my @predictions_loop; -my ($calculated) = GetSeq(\%val); +my ($calculated) = GetSeq(\%subscription, \%pattern); push @predictions_loop, { number => $calculated, publicationdate => $date, @@ -159,7 +159,7 @@ while( $i < 1000 ) { last; } - ($calculated, $val{'lastvalue1'}, $val{'lastvalue2'}, $val{'lastvalue3'}, $val{'innerloop1'}, $val{'innerloop2'}, $val{'innerloop3'}) = GetNextSeq(\%val); + ($calculated, $subscription{'lastvalue1'}, $subscription{'lastvalue2'}, $subscription{'lastvalue3'}, $subscription{'innerloop1'}, $subscription{'innerloop2'}, $subscription{'innerloop3'}) = GetNextSeq(\%subscription, \%pattern); $issuenumber++; $line{'number'} = $calculated; $line{'issuenumber'} = $issuenumber; -- 1.7.9.5