Bug 7688: (follow-up) rewrite t/db_dependant/Serials.t

A lot of tests were just a call to a funtion without any arguments,
whereas the function expect at least one argument.
These tests were kept, but all return values are now undef when a
mandatory argument is missing, so return values are consistent.

The part where subscription periodicity is changed could not work
because of ',' appended to each key in ModSubscription call. So it's
rewritten, taking into account the new API for subscription frequencies.
This script should leave your database intact because it revert any
modification made.

Also fix some warnings in C4::Serials and in C4::Items.
And fix a typo in koha-tmpl/.../subscription-numberpatterns.tt

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Work as described. No errors
Tested, again, without trouble.
With a suscription, tests successful.
No koha-qa errors

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This commit is contained in:
Julian Maurice 2013-04-08 12:04:45 +00:00 committed by Galen Charlton
parent e1593c898f
commit d93ec41012
4 changed files with 199 additions and 99 deletions

View File

@ -2700,7 +2700,7 @@ sub PrepareItemrecordDisplay {
#---- branch
if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) {
if ( ( C4::Context->preference("IndependentBranches") )
&& ( C4::Context->userenv->{flags} % 2 != 1 ) ) {
&& ( C4::Context->userenv && C4::Context->userenv->{flags} % 2 != 1 ) ) {
my $sth = $dbh->prepare( "SELECT branchcode,branchname FROM branches WHERE branchcode = ? ORDER BY branchname" );
$sth->execute( C4::Context->userenv->{branch} );
push @authorised_values, ""
@ -2720,7 +2720,7 @@ sub PrepareItemrecordDisplay {
}
}
$defaultvalue = C4::Context->userenv->{branch};
$defaultvalue = C4::Context->userenv ? C4::Context->userenv->{branch} : undef;
if ( $defaultvalues and $defaultvalues->{branchcode} ) {
$defaultvalue = $defaultvalues->{branchcode};
}

View File

@ -121,6 +121,9 @@ name,title,planneddate,serialseq,serial.subscriptionid from tables : subscriptio
sub GetLateIssues {
my ($supplierid) = @_;
return unless ($supplierid);
my $dbh = C4::Context->dbh;
my $sth;
if ($supplierid) {
@ -153,10 +156,8 @@ sub GetLateIssues {
}
my @issuelist;
my $last_title;
my $odd = 0;
while ( my $line = $sth->fetchrow_hashref ) {
$odd++ unless $line->{title} eq $last_title;
$line->{title} = "" if $line->{title} eq $last_title;
$line->{title} = "" if $last_title and $line->{title} eq $last_title;
$last_title = $line->{title} if ( $line->{title} );
$line->{planneddate} = format_date( $line->{planneddate} );
push @issuelist, $line;
@ -287,6 +288,9 @@ returns the number of rows affected
sub AddItem2Serial {
my ( $serialid, $itemnumber ) = @_;
return unless ($serialid and $itemnumber);
my $dbh = C4::Context->dbh;
my $rq = $dbh->prepare("INSERT INTO `serialitems` SET serialid=? , itemnumber=?");
$rq->execute( $serialid, $itemnumber );
@ -304,6 +308,9 @@ Update Claimdate for issues in @$serialids list with date $date
sub UpdateClaimdateIssues {
my ( $serialids, $date ) = @_;
return unless ($serialids);
my $dbh = C4::Context->dbh;
$date = strftime( "%Y-%m-%d", localtime ) unless ($date);
my $query = "
@ -369,6 +376,9 @@ sub GetSubscription {
sub GetFullSubscription {
my ($subscriptionid) = @_;
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $query = qq|
SELECT serial.serialid,
@ -415,6 +425,9 @@ sub GetFullSubscription {
sub PrepareSerialsData {
my ($lines) = @_;
return unless ($lines);
my %tmpresults;
my $year;
my @res;
@ -471,6 +484,9 @@ startdate, histstartdate,opacnote,missinglist,recievedlist,periodicity,status &
sub GetSubscriptionsFromBiblionumber {
my ($biblionumber) = @_;
return unless ($biblionumber);
my $dbh = C4::Context->dbh;
my $query = qq(
SELECT subscription.*,
@ -749,6 +765,9 @@ FIXME: We should return \@serials.
sub GetSerials {
my ( $subscriptionid, $count ) = @_;
return unless $subscriptionid;
my $dbh = C4::Context->dbh;
# status = 2 is "arrived"
@ -815,6 +834,9 @@ this number is used to see if a subscription can be deleted (=it must have only
sub GetSerials2 {
my ( $subscription, $status ) = @_;
return unless ($subscription and $status);
my $dbh = C4::Context->dbh;
my $query = qq|
SELECT serialid,serialseq, status, planneddate, publisheddate,notes, routingnotes
@ -854,6 +876,9 @@ a ref to an array which contains all of the latest serials stored into a hash.
sub GetLatestSerials {
my ( $subscriptionid, $limit ) = @_;
return unless ($subscriptionid and $limit);
my $dbh = C4::Context->dbh;
# status = 2 is "arrived"
@ -886,7 +911,10 @@ This function returns the field distributedto for the subscription matching subs
sub GetDistributedTo {
my $dbh = C4::Context->dbh;
my $distributedto;
my $subscriptionid = @_;
my ($subscriptionid) = @_;
return unless ($subscriptionid);
my $query = "SELECT distributedto FROM subscription WHERE subscriptionid=?";
my $sth = $dbh->prepare($query);
$sth->execute($subscriptionid);
@ -911,7 +939,10 @@ This function get the next issue for the subscription given on input arg
sub GetNextSeq {
my ($subscription, $pattern, $planneddate) = @_;
my ( $calculated, $newlastvalue1, $newlastvalue2, $newlastvalue3,
return unless ($subscription and $pattern);
my ( $newlastvalue1, $newlastvalue2, $newlastvalue3,
$newinnerloop1, $newinnerloop2, $newinnerloop3 );
my $count = 1;
@ -929,63 +960,66 @@ sub GetNextSeq {
}
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 ($numberingmethod =~ /\{$_\}/);
}
my $calculated = "";
if ($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 ($numberingmethod =~ /\{$_\}/);
}
for(my $i = 0; $i < $count; $i++) {
if($calc{'X'}) {
# check if we have to increase the new value.
$newinnerloop1 += 1;
if ($newinnerloop1 >= $pattern->{every1}) {
$newinnerloop1 = 0;
$newlastvalue1 += $pattern->{add1};
for(my $i = 0; $i < $count; $i++) {
if($calc{'X'}) {
# check if we have to increase the new value.
$newinnerloop1 += 1;
if ($newinnerloop1 >= $pattern->{every1}) {
$newinnerloop1 = 0;
$newlastvalue1 += $pattern->{add1};
}
# reset counter if needed.
$newlastvalue1 = $pattern->{setto1} if ($newlastvalue1 > $pattern->{whenmorethan1});
}
# reset counter if needed.
$newlastvalue1 = $pattern->{setto1} if ($newlastvalue1 > $pattern->{whenmorethan1});
if($calc{'Y'}) {
# check if we have to increase the new value.
$newinnerloop2 += 1;
if ($newinnerloop2 >= $pattern->{every2}) {
$newinnerloop2 = 0;
$newlastvalue2 += $pattern->{add2};
}
# reset counter if needed.
$newlastvalue2 = $pattern->{setto2} if ($newlastvalue2 > $pattern->{whenmorethan2});
}
if($calc{'Z'}) {
# check if we have to increase the new value.
$newinnerloop3 += 1;
if ($newinnerloop3 >= $pattern->{every3}) {
$newinnerloop3 = 0;
$newlastvalue3 += $pattern->{add3};
}
# reset counter if needed.
$newlastvalue3 = $pattern->{setto3} if ($newlastvalue3 > $pattern->{whenmorethan3});
}
}
if($calc{'X'}) {
my $newlastvalue1string = _numeration( $newlastvalue1, $pattern->{numbering1}, $locale );
$calculated =~ s/\{X\}/$newlastvalue1string/g;
}
if($calc{'Y'}) {
# check if we have to increase the new value.
$newinnerloop2 += 1;
if ($newinnerloop2 >= $pattern->{every2}) {
$newinnerloop2 = 0;
$newlastvalue2 += $pattern->{add2};
}
# reset counter if needed.
$newlastvalue2 = $pattern->{setto2} if ($newlastvalue2 > $pattern->{whenmorethan2});
my $newlastvalue2string = _numeration( $newlastvalue2, $pattern->{numbering2}, $locale );
$calculated =~ s/\{Y\}/$newlastvalue2string/g;
}
if($calc{'Z'}) {
# check if we have to increase the new value.
$newinnerloop3 += 1;
if ($newinnerloop3 >= $pattern->{every3}) {
$newinnerloop3 = 0;
$newlastvalue3 += $pattern->{add3};
}
# reset counter if needed.
$newlastvalue3 = $pattern->{setto3} if ($newlastvalue3 > $pattern->{whenmorethan3});
my $newlastvalue3string = _numeration( $newlastvalue3, $pattern->{numbering3}, $locale );
$calculated =~ s/\{Z\}/$newlastvalue3string/g;
}
}
if($calc{'X'}) {
my $newlastvalue1string = _numeration( $newlastvalue1, $pattern->{numbering1}, $locale );
$calculated =~ s/\{X\}/$newlastvalue1string/g;
}
if($calc{'Y'}) {
my $newlastvalue2string = _numeration( $newlastvalue2, $pattern->{numbering2}, $locale );
$calculated =~ s/\{Y\}/$newlastvalue2string/g;
}
if($calc{'Z'}) {
my $newlastvalue3string = _numeration( $newlastvalue3, $pattern->{numbering3}, $locale );
$calculated =~ s/\{Z\}/$newlastvalue3string/g;
}
return ($calculated,
$newlastvalue1, $newlastvalue2, $newlastvalue3,
@ -1005,6 +1039,9 @@ the sequence in string format
sub GetSeq {
my ($subscription, $pattern) = @_;
return unless ($subscription and $pattern);
my $locale = $subscription->{locale};
my $calculated = $pattern->{numberingmethod};
@ -1036,6 +1073,9 @@ the enddate or undef
sub GetExpirationDate {
my ( $subscriptionid, $startdate ) = @_;
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $subscription = GetSubscription($subscriptionid);
my $enddate;
@ -1085,6 +1125,9 @@ the number of subscriptions
sub CountSubscriptionFromBiblionumber {
my ($biblionumber) = @_;
return unless ($biblionumber);
my $dbh = C4::Context->dbh;
my $query = "SELECT count(*) FROM subscription WHERE biblionumber=?";
my $sth = $dbh->prepare($query);
@ -1104,6 +1147,9 @@ returns the number of rows affected
sub ModSubscriptionHistory {
my ( $subscriptionid, $histstartdate, $enddate, $receivedlist, $missinglist, $opacnote, $librariannote ) = @_;
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $query = "UPDATE subscriptionhistory
SET histstartdate=?,histenddate=?,recievedlist=?,missinglist=?,opacnote=?,librariannote=?
@ -1173,6 +1219,7 @@ Note : if we change from "waited" to something else,then we will have to create
sub ModSerialStatus {
my ( $serialid, $serialseq, $planneddate, $publisheddate, $status, $notes ) = @_;
return unless ($serialid);
#It is a usual serial
# 1st, get previous status :
@ -1588,6 +1635,8 @@ sub NewIssue {
my ( $serialseq, $subscriptionid, $biblionumber, $status, $planneddate, $publisheddate, $notes ) = @_;
### FIXME biblionumber CAN be provided by subscriptionid. So Do we STILL NEED IT ?
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $query = qq|
INSERT INTO serial
@ -1640,6 +1689,9 @@ return :
sub ItemizeSerials {
my ( $serialid, $info ) = @_;
return unless ($serialid);
my $now = POSIX::strftime( "%Y-%m-%d", localtime );
my $dbh = C4::Context->dbh;
@ -1774,6 +1826,9 @@ sub HasSubscriptionStrictlyExpired {
# Getting end of subscription date
my ($subscriptionid) = @_;
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $subscription = GetSubscription($subscriptionid);
my $expirationdate = $subscription->{enddate} || GetExpirationDate($subscriptionid);
@ -1815,6 +1870,9 @@ return :
sub HasSubscriptionExpired {
my ($subscriptionid) = @_;
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $subscription = GetSubscription($subscriptionid);
my $frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});
@ -1952,6 +2010,9 @@ name,title,planneddate,serialseq,serial.subscriptionid from tables : subscriptio
sub GetLateOrMissingIssues {
my ( $supplierid, $serialid, $order ) = @_;
return unless ($supplierid);
my $dbh = C4::Context->dbh;
my $sth;
my $byserial = '';
@ -2026,6 +2087,9 @@ called when a missing issue is found from the serials-recieve.pl file
sub removeMissingIssue {
my ( $sequence, $subscriptionid ) = @_;
return unless ($sequence and $subscriptionid);
my $dbh = C4::Context->dbh;
my $sth = $dbh->prepare("SELECT * FROM subscriptionhistory WHERE subscriptionid = ?");
$sth->execute($subscriptionid);
@ -2111,6 +2175,9 @@ used to show either an 'add' or 'edit' link
sub check_routing {
my ($subscriptionid) = @_;
return unless ($subscriptionid);
my $dbh = C4::Context->dbh;
my $sth = $dbh->prepare(
"SELECT count(routingid) routingids FROM subscription LEFT JOIN subscriptionroutinglist
@ -2136,6 +2203,9 @@ of either 1 or highest current rank + 1
sub addroutingmember {
my ( $borrowernumber, $subscriptionid ) = @_;
return unless ($borrowernumber and $subscriptionid);
my $rank;
my $dbh = C4::Context->dbh;
my $sth = $dbh->prepare( "SELECT max(ranking) rank FROM subscriptionroutinglist WHERE subscriptionid = ?" );

View File

@ -182,7 +182,7 @@ function show_blocking_subs() {
[% END %]
</select>
[% END %]
<td>Fromatting</td>
<td>Formatting</td>
<td>[% PROCESS numbering_select name="numbering1" value=numbering1 %]</td>
<td>[% PROCESS numbering_select name="numbering2" value=numbering2 %]</td>
<td>[% PROCESS numbering_select name="numbering3" value=numbering3 %]</td>

View File

@ -3,13 +3,14 @@
# This Koha test module is a stub!
# Add more tests here!!!
use strict;
use warnings;
use Modern::Perl;
use YAML;
use CGI;
use C4::Serials;
use C4::Serials::Frequency;
use C4::Debug;
use Test::More tests => 34;
use Test::More tests => 35;
BEGIN {
use_ok('C4::Serials');
@ -17,83 +18,112 @@ BEGIN {
my $subscriptionid = 1;
my $subscriptioninformation = GetSubscription( $subscriptionid );
$debug && warn Dump($subscriptioninformation);
my @subscriptions = GetSubscriptions( $$subscriptioninformation{bibliotitle} );
isa_ok( \@subscriptions, 'ARRAY' );
$debug && warn scalar(@subscriptions);
@subscriptions = GetSubscriptions( undef, $$subscriptioninformation{issn} );
isa_ok( \@subscriptions, 'ARRAY' );
$debug && warn scalar(@subscriptions);
@subscriptions = GetSubscriptions( undef, undef, $$subscriptioninformation{ean} );
isa_ok( \@subscriptions, 'ARRAY' );
$debug && warn scalar(@subscriptions);
@subscriptions = GetSubscriptions( undef, undef, undef, $$subscriptioninformation{bibnum} );
isa_ok( \@subscriptions, 'ARRAY' );
$debug && warn scalar(@subscriptions);
if ($subscriptioninformation->{periodicity} % 16==0){
$subscriptioninformation->{periodicity}=7;
ModSubscription(@$subscriptioninformation{qw(librarian, branchcode, aqbooksellerid, cost, aqbudgetid, startdate, periodicity, firstacquidate,
dow, irregularity, numberpattern, numberlength, weeklength, monthlength, add1, every1,
whenmorethan1, setto1, lastvalue1, innerloop1, add2, every2, whenmorethan2, setto2,
lastvalue2, innerloop2, add3, every3, whenmorethan3, setto3, lastvalue3, innerloop3,
numberingmethod, status, biblionumber, callnumber, notes, letter, hemisphere, manualhistory,
internalnotes, serialsadditems, staffdisplaycount, opacdisplaycount, graceperiod, location, enddate, subscriptionid
)});
my $frequency = GetSubscriptionFrequency($subscriptioninformation->{periodicity});
my $old_frequency;
if (not $frequency->{unit}) {
$old_frequency = $frequency->{id};
$frequency->{unit} = "month";
$frequency->{unitsperissue} = 1;
$frequency->{issuesperunit} = 1;
$frequency->{description} = "Frequency created by t/db_dependant/Serials.t";
$subscriptioninformation->{periodicity} = AddSubscriptionFrequency($frequency);
ModSubscription( @$subscriptioninformation{qw(
librarian branchcode aqbooksellerid cost aqbudgetid startdate
periodicity firstacquidate irregularity numberpattern locale
numberlength weeklength monthlength lastvalue1 innerloop1 lastvalue2
innerloop2 lastvalue3 innerloop3 status biblionumber callnumber notes
letter manualhistory internalnotes serialsadditems staffdisplaycount
opacdisplaycount graceperiod location enddate subscriptionid
skip_serialseq
)} );
}
my $expirationdate = GetExpirationDate(1) ;
my $expirationdate = GetExpirationDate($subscriptionid) ;
ok( $expirationdate, "not NULL" );
$debug && warn "$expirationdate";
is(C4::Serials::GetLateIssues(),"0", 'test getting late issues');
is(C4::Serials::GetLateIssues(), undef, 'test getting late issues');
ok(C4::Serials::GetSubscriptionHistoryFromSubscriptionId(), 'test getting history from sub-scription');
ok(C4::Serials::GetSubscriptionHistoryFromSubscriptionId($subscriptionid), 'test getting history from sub-scription');
ok(C4::Serials::GetSerialStatusFromSerialId(), 'test getting Serial Status From Serial Id');
my ($serials_count, @serials) = GetSerials($subscriptionid);
ok($serials_count > 0, 'Subscription has at least one serial');
my $serial = $serials[0];
ok(C4::Serials::GetSerialInformation(), 'test getting Serial Information');
ok(C4::Serials::GetSerialStatusFromSerialId($serial->{serialid}), 'test getting Serial Status From Serial Id');
ok(C4::Serials::AddItem2Serial(), 'test adding item to serial');
isa_ok(C4::Serials::GetSerialInformation($serial->{serialid}), 'HASH', 'test getting Serial Information');
ok(C4::Serials::UpdateClaimdateIssues(), 'test updating claim date');
# Delete created frequency
if ($old_frequency) {
my $freq_to_delete = $subscriptioninformation->{periodicity};
$subscriptioninformation->{periodicity} = $old_frequency;
ok(C4::Serials::GetFullSubscription(), 'test getting full subscription');
ModSubscription( @$subscriptioninformation{qw(
librarian branchcode aqbooksellerid cost aqbudgetid startdate
periodicity firstacquidate irregularity numberpattern locale
numberlength weeklength monthlength lastvalue1 innerloop1 lastvalue2
innerloop2 lastvalue3 innerloop3 status biblionumber callnumber notes
letter manualhistory internalnotes serialsadditems staffdisplaycount
opacdisplaycount graceperiod location enddate subscriptionid
skip_serialseq
)} );
ok(C4::Serials::PrepareSerialsData(), 'test preparing serial data');
DelSubscriptionFrequency($freq_to_delete);
}
ok(C4::Serials::GetSubscriptionsFromBiblionumber(), 'test getting subscriptions form biblio number');
is(C4::Serials::GetSerials(),"0", 'test getting serials when you enter nothing');
is(C4::Serials::GetSerials2(),"0", 'test getting serials when you enter nothing');
# Test calling subs without parameters
is(C4::Serials::AddItem2Serial(), undef, 'test adding item to serial');
is(C4::Serials::UpdateClaimdateIssues(), undef, 'test updating claim date');
is(C4::Serials::GetFullSubscription(), undef, 'test getting full subscription');
is(C4::Serials::PrepareSerialsData(), undef, 'test preparing serial data');
is(C4::Serials::GetSubscriptionsFromBiblionumber(), undef, 'test getting subscriptions form biblio number');
ok(C4::Serials::GetLatestSerials(), 'test getting lastest serials');
is(C4::Serials::GetSerials(), undef, 'test getting serials when you enter nothing');
is(C4::Serials::GetSerials2(), undef, 'test getting serials when you enter nothing');
is(C4::Serials::GetDistributedTo(),"0", 'test getting distributed when nothing is entered');
is(C4::Serials::GetLatestSerials(), undef, 'test getting lastest serials');
is(C4::Serials::GetNextSeq(),"0", 'test getting next seq when you enter nothing');
is(C4::Serials::GetDistributedTo(), undef, 'test getting distributed when nothing is entered');
is(C4::Serials::GetSeq(),undef, 'test getting seq when you enter nothing');
is(C4::Serials::GetNextSeq(), undef, 'test getting next seq when you enter nothing');
is(C4::Serials::CountSubscriptionFromBiblionumber(),"0", 'test counting subscription when nothing is entered');
is(C4::Serials::GetSeq(), undef, 'test getting seq when you enter nothing');
is(C4::Serials::ModSubscriptionHistory(),"0", 'test modding subscription history');
is(C4::Serials::CountSubscriptionFromBiblionumber(), undef, 'test counting subscription when nothing is entered');
is(C4::Serials::ModSubscriptionHistory(), undef, 'test modding subscription history');
is(C4::Serials::ModSerialStatus(),undef, 'test modding serials');
is(C4::Serials::NewIssue(),"0", 'test getting 0 when nothing is entered');
is(C4::Serials::NewIssue(), undef, 'test getting 0 when nothing is entered');
is(C4::Serials::ItemizeSerials(),undef, 'test getting nothing when nothing is entered');
ok(C4::Serials::HasSubscriptionStrictlyExpired(), 'test if the subscriptions has expired');
is(C4::Serials::HasSubscriptionExpired(),"0", 'test if the subscriptions has expired');
is(C4::Serials::HasSubscriptionStrictlyExpired(), undef, 'test if the subscriptions has expired');
is(C4::Serials::HasSubscriptionExpired(), undef, 'test if the subscriptions has expired');
is(C4::Serials::GetLateOrMissingIssues(),"0", 'test getting last or missing issues');
is(C4::Serials::GetLateOrMissingIssues(), undef, 'test getting last or missing issues');
is(C4::Serials::removeMissingIssue(),undef, 'test removing a missing issue');
is(C4::Serials::removeMissingIssue(), undef, 'test removing a missing issue');
is(C4::Serials::updateClaim(),undef, 'test updating claim');
is(C4::Serials::getsupplierbyserialid(),undef, 'test getting supplier idea');
is(C4::Serials::check_routing(),"0", 'test checking route');
is(C4::Serials::check_routing(), undef, 'test checking route');
is(C4::Serials::addroutingmember(),undef, 'test adding route member');