diff --git a/C4/Letters.pm b/C4/Letters.pm index 9e12e0cd87..4acc931992 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -375,12 +375,16 @@ sub SendAlerts { Message => Encode::encode( "utf8", "" . $letter->{content} ), 'Content-Type' => 'text/plain; charset="utf8"', ); + $mail{'Reply-to'} = C4::Context->preference('ReplytoDefault') if C4::Context->preference('ReplytoDefault'); $mail{'Sender'} = C4::Context->preference('ReturnpathDefault') if C4::Context->preference('ReturnpathDefault'); - sendmail(%mail) or carp $Mail::Sendmail::error; + unless ( sendmail(%mail) ) { + carp $Mail::Sendmail::error; + return { error => $Mail::Sendmail::error }; + } logaction( "ACQUISITION", diff --git a/C4/Serials.pm b/C4/Serials.pm index bcb37e3707..d127066596 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -96,14 +96,14 @@ the array is in name order sub GetSuppliersWithLateIssues { my $dbh = C4::Context->dbh; my $query = qq| - SELECT DISTINCT id, name + SELECT DISTINCT id, name FROM subscription LEFT JOIN serial ON serial.subscriptionid=subscription.subscriptionid LEFT JOIN aqbooksellers ON subscription.aqbooksellerid = aqbooksellers.id WHERE id > 0 AND ( (planneddate < now() AND serial.status=1) - OR serial.STATUS IN (3, 4, 41, 42, 43, 44) + OR serial.STATUS IN (3, 4, 41, 42, 43, 44, 7) ) AND subscription.closed = 0 ORDER BY name|; @@ -310,8 +310,12 @@ sub UpdateClaimdateIssues { my $dbh = C4::Context->dbh; $date = strftime( "%Y-%m-%d", localtime ) unless ($date); my $query = " - UPDATE serial SET claimdate = ?, status = 7 - WHERE serialid in (" . join( ",", map { '?' } @$serialids ) . ")"; + UPDATE serial + SET claimdate = ?, + status = 7, + claims_count = claims_count + 1 + WHERE serialid in (" . join( ",", map { '?' } @$serialids ) . ") + "; my $rq = $dbh->prepare($query); $rq->execute($date, @$serialids); return $rq->rows; @@ -1993,14 +1997,14 @@ sub GetLateOrMissingIssues { "SELECT serialid, aqbooksellerid, name, biblio.title, biblioitems.issn, planneddate, serialseq, - serial.status, serial.subscriptionid, claimdate, + serial.status, serial.subscriptionid, claimdate, claims_count, subscription.branchcode - FROM serial - LEFT JOIN subscription ON serial.subscriptionid=subscription.subscriptionid + FROM serial + LEFT JOIN subscription ON serial.subscriptionid=subscription.subscriptionid LEFT JOIN biblio ON subscription.biblionumber=biblio.biblionumber LEFT JOIN biblioitems ON subscription.biblionumber=biblioitems.biblionumber LEFT JOIN aqbooksellers ON subscription.aqbooksellerid = aqbooksellers.id - WHERE subscription.subscriptionid = serial.subscriptionid + 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 subscription.aqbooksellerid=$supplierid $byserial @@ -2008,16 +2012,16 @@ sub GetLateOrMissingIssues { ); } else { $sth = $dbh->prepare( - "SELECT + "SELECT serialid, aqbooksellerid, name, biblio.title, planneddate, serialseq, - serial.status, serial.subscriptionid, claimdate, + serial.status, serial.subscriptionid, claimdate, claims_count, subscription.branchcode - FROM serial - LEFT JOIN subscription ON serial.subscriptionid=subscription.subscriptionid + FROM serial + LEFT JOIN subscription ON serial.subscriptionid=subscription.subscriptionid LEFT JOIN biblio ON subscription.biblionumber=biblio.biblionumber LEFT JOIN aqbooksellers ON subscription.aqbooksellerid = aqbooksellers.id - WHERE subscription.subscriptionid = serial.subscriptionid + 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)) $byserial ORDER BY $order" @@ -2095,12 +2099,12 @@ called from claims.pl file sub updateClaim { my ($serialid) = @_; my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( - "UPDATE serial SET claimdate = now() - WHERE serialid = ? - " - ); - $sth->execute($serialid); + $dbh->do(q| + UPDATE serial + SET claimdate = NOW(), + claims_count = claims_count + 1 + WHERE serialid = ? + |, {}, $serialid ); return; } diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 38435b9118..cbd1c61889 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1940,6 +1940,7 @@ CREATE TABLE `serial` ( `notes` text, `publisheddate` date default NULL, `claimdate` date default NULL, + claims_count int(11) default 0, `routingnotes` text, PRIMARY KEY (`serialid`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 08fc7b32ad..40afd2d3e3 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8877,6 +8877,20 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.17.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do(q| + ALTER TABLE serial ADD COLUMN claims_count INT(11) DEFAULT 0 after claimdate + |); + $dbh->do(q| + UPDATE serial + SET claims_count = 1 + WHERE claimdate IS NOT NULL + |); + print "Upgrade to $DBversion done (Bug 5342: Add claims_count field in serial table)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt index de9102e22d..49b8448a60 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt @@ -13,11 +13,7 @@ var sTable = $("#claimst").dataTable($.extend(true, {}, dataTablesDefaults, { "sDom": 't', "aoColumnDefs": [ - [% IF ( letter ) %] - { "aTargets": [ 0,1 ], "bSortable": false, "bSearchable": false }, - [% ELSE %] - { "aTargets": [ 0 ], "bSortable": false, "bSearchable": false }, - [% END %] + { "aTargets": [ 0 ], "bSortable": false, "bSearchable": false }, { 'sType': "anti-the", 'aTargets' : [ 'anti-the'] }, { 'sType': "title-string", 'aTargets' : [ 'title-string'] } ], @@ -35,7 +31,7 @@ $("#claimst tr:visible :checkbox").attr('checked', $("#CheckAll").is(':checked')); }); - // Generates a dynamic link for exporting the selection's data as CSV + // Generates a dynamic link for exporting the selections data as CSV $("#ExportSelected").click(function() { // We need to use "input[name=serialid]:checked" instead of "input:checked". Otherwise, the "check all" box will pass the value of "on" as a serialid, which produces a SQL error. var selected = $("input[name=serialid]:checked"); @@ -177,6 +173,19 @@

Claims

+ [% IF error_claim %] + [% IF error_claim == 'no_vendor_email' %] +
This vendor has no email defined for late issues.
+ [% ELSIF error_claim == 'no_loggedin_user_email' %] +
No email is configured for your user.
+ [% ELSE %] +
[% error_claim %]
+ [% END %] + [% END %] + [% IF info_claim %] +
Email has been sent.
+ [% END %] + [% IF letters %][% UNLESS ( missingissues ) %][% IF ( supplierid ) %]
No missing issues found.
[% ELSE %]
Please choose a vendor.
[% END %][% END %][% END %] [% IF ( SHOWCONFIRMATION ) %] @@ -258,40 +267,34 @@
-
- - - - - - - - - - - - - + +
VendorLibraryTitleISSNIssue numberStatusSinceClaim date
+ + + + + + + + + + + + + [% FOREACH missingissue IN missingissues %] - + - - + +
VendorLibraryTitleIssue numberStatusSinceClaims countClaim date
- [% missingissue.name %] - [% missingissue.name %] [% Branches.GetName( missingissue.branchcode ) %] [% missingissue.title |html %] - [% missingissue.issn %] - - [% missingissue.serialseq %] - [% missingissue.serialseq %] [% IF ( missingissue.status1 ) %]Expected[% END %] [% IF ( missingissue.status3 ) %]Late[% END %] @@ -309,6 +312,7 @@ [% END %] [% missingissue.claims_count %] [% IF ( missingissue.claimdate ) %] [% missingissue.claimdate %] @@ -320,25 +324,29 @@ [% END %]
- [% IF csv_profiles %] -
- - - Download selected claims - [% END %] + [% IF csv_profiles %] +
+ + + Download selected claims + [% END %] -[% IF ( letters ) %] -
- -
+ [% IF letters %] +
+ + + + + +
[% END %]
diff --git a/serials/claims.pl b/serials/claims.pl index bfe5a0f376..822a1ff163 100755 --- a/serials/claims.pl +++ b/serials/claims.pl @@ -37,7 +37,6 @@ my $op = $input->param('op'); my $claimletter = $input->param('claimletter'); my $supplierid = $input->param('supplierid'); my $suppliername = $input->param('suppliername'); -my $order = $input->param('order'); # open template first (security & userenv set here) my ($template, $loggedinuser, $cookie) @@ -52,21 +51,12 @@ my ($template, $loggedinuser, $cookie) # supplierlist is returned in name order my $supplierlist = GetSuppliersWithLateIssues(); for my $s (@{$supplierlist} ) { - $s->{count} = scalar GetLateOrMissingIssues($s->{id}, q{}, $order); + $s->{count} = scalar GetLateOrMissingIssues($s->{id}); if ($supplierid && $s->{id} == $supplierid) { $s->{selected} = 1; } } -my $letters = GetLetters({ module => 'claimissues' }); - -my @missingissues; -my @supplierinfo; -if ($supplierid) { - @missingissues = GetLateOrMissingIssues($supplierid,$serialid,$order); - @supplierinfo=GetBookSeller($supplierid); -} - my $branchloop = GetBranchesLoop(); my $preview=0; @@ -75,14 +65,37 @@ if($op && $op eq 'preview'){ } else { my @serialnums=$input->param('serialid'); if (@serialnums) { # i.e. they have been flagged to generate claims - SendAlerts('claimissues',\@serialnums,$input->param("letter_code")); - my $cntupdate=UpdateClaimdateIssues(\@serialnums); - ### $cntupdate SHOULD be equal to scalar(@$serialnums) + my $err; + eval { + $err = SendAlerts('claimissues',\@serialnums,$input->param("letter_code")); + if ( not ref $err or not exists $err->{error} ) { + UpdateClaimdateIssues(\@serialnums); + } + }; + if ( $@ ) { + $template->param(error_claim => $@); + } elsif ( ref $err and exists $err->{error} ) { + if ( $err->{error} eq "no_email" ) { + $template->param( error_claim => 'no_vendor_email' ); + } elsif ( $err->{error} =~ m|Bad or missing From address| ) { + $template->param( error_claim => 'no_loggedin_user_email' ); + } + } else { + $template->param( info_claim => 1 ); + } } } +my $letters = GetLetters({ module => 'claimissues' }); + +my @missingissues; +my @supplierinfo; +if ($supplierid) { + @missingissues = GetLateOrMissingIssues($supplierid); + @supplierinfo=GetBookSeller($supplierid); +} + $template->param( - order =>$order, suploop => $supplierlist, phone => $supplierinfo[0]->{phone}, booksellerfax => $supplierinfo[0]->{booksellerfax}, diff --git a/t/db_dependent/Acquisition/OrderFromSubscription.t b/t/db_dependent/Acquisition/OrderFromSubscription.t index 27532483bf..e03c023780 100644 --- a/t/db_dependent/Acquisition/OrderFromSubscription.t +++ b/t/db_dependent/Acquisition/OrderFromSubscription.t @@ -1,6 +1,6 @@ use Modern::Perl; -use Test::More tests => 13; +use Test::More tests => 12; use Data::Dumper; use_ok('C4::Acquisition'); @@ -13,9 +13,6 @@ my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; -my $supplierlist=eval{GetSuppliersWithLateIssues()}; -ok(length($@)==0,"No SQL problem in GetSuppliersWithLateIssues"); - my $booksellerid = C4::Bookseller::AddBookseller( { name => "my vendor", diff --git a/t/db_dependent/Serials/Claims.t b/t/db_dependent/Serials/Claims.t new file mode 100644 index 0000000000..a90563dd45 --- /dev/null +++ b/t/db_dependent/Serials/Claims.t @@ -0,0 +1,145 @@ +use Modern::Perl; +use Test::More tests => 13; + +use C4::Acquisition; +use C4::Budgets; +use_ok('C4::Serials'); + +use Koha::DateUtils qw( dt_from_string output_pref ); + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +my $branchcode = 'CPL'; +my $bpid = AddBudgetPeriod({ + budget_period_startdate => '2015-01-01', + budget_period_enddate => '2015-12-31', + budget_period_description => "budget desc" +}); + +my $budget_id = AddBudget({ + budget_code => "ABCD", + budget_amount => "123.132", + budget_name => "Périodiques", + budget_notes => "This is a note", + budget_period_id => $bpid +}); + +my $record = MARC::Record->new(); +my ( $biblionumber, $biblioitemnumber ) = C4::Biblio::AddBiblio($record, ''); + +my $sample_supplier1 = { + name => 'Name1', + address1 => 'address1_1', + address2 => 'address1-2', + address3 => 'address1_2', + address4 => 'address1_2', + postal => 'postal1', + phone => 'phone1', + accountnumber => 'accountnumber1', + fax => 'fax1', + url => 'url1', + active => 1, + gstreg => 1, + listincgst => 1, + invoiceincgst => 1, + gstrate => '1.0000', + discount => '1.0000', + notes => 'notes1', + deliverytime => undef +}; +my $sample_supplier2 = { + name => 'Name2', + address1 => 'address1_2', + address2 => 'address2-2', + address3 => 'address3_2', + address4 => 'address4_2', + postal => 'postal2', + phone => 'phone2', + accountnumber => 'accountnumber2', + fax => 'fax2', + url => 'url2', + active => 1, + gstreg => 1, + listincgst => 1, + invoiceincgst => 1, + gstrate => '2.0000', + discount => '2.0000', + notes => 'notes2', + deliverytime => 2 +}; + +my $supplier_id1 = C4::Bookseller::AddBookseller($sample_supplier1); +my $supplier_id2 = C4::Bookseller::AddBookseller($sample_supplier2); + +my $supplierlist = eval { GetSuppliersWithLateIssues() }; +is( length($@), 0, "No SQL problem in GetSuppliersWithLateIssues" ); +is ( scalar(@$supplierlist), 0, 'There is no late issues yet'); + +my $subscriptionid_not_late = NewSubscription( + undef, $branchcode, $supplier_id1, undef, $budget_id, $biblionumber, + '2013-01-01', undef, undef, undef, undef, + undef, undef, undef, undef, undef, undef, + 1, "notes",undef, '9999-01-01', undef, undef, + undef, undef, 0, "intnotes", 0, + undef, undef, 0, undef, '2013-12-31', 0 +); +$supplierlist = GetSuppliersWithLateIssues(); +is ( scalar(@$supplierlist), 0, 'There is still no late issues yet'); + +my $subscriptionid_inlate1 = NewSubscription( + undef, $branchcode, $supplier_id1, undef, $budget_id, $biblionumber, + '2013-01-01', undef, undef, undef, undef, + undef, undef, undef, undef, undef, undef, + 1, "notes",undef, '2013-01-01', undef, undef, + undef, undef, 0, "intnotes", 0, + undef, undef, 0, undef, '2013-12-31', 0 +); + +my $subscriptionid_inlate2 = NewSubscription( + undef, $branchcode, $supplier_id2, undef, $budget_id, $biblionumber, + '2013-01-01', undef, undef, undef, undef, + undef, undef, undef, undef, undef, undef, + 1, "notes",undef, '2013-01-01', undef, undef, + undef, undef, 0, "intnotes", 0, + undef, undef, 0, undef, '2013-12-31', 0 +); + +my $subscriptionid_inlate3 = NewSubscription( + undef, $branchcode, $supplier_id2, undef, $budget_id, $biblionumber, + '2013-01-02', undef, undef, undef, undef, + undef, undef, undef, undef, undef, undef, + 1, "notes",undef, '2013-01-02', undef, undef, + undef, undef, 0, "intnotes", 0, + undef, undef, 0, undef, '2013-12-31', 0 +); + + +$supplierlist = GetSuppliersWithLateIssues(); +is ( scalar(@$supplierlist), 2, '2 suppliers should have issues in late'); + +is( GetLateOrMissingIssues(), undef, 'GetLateOrMissingIssues should return undef without parameter' ); + +my @late_or_missing_issues = GetLateOrMissingIssues( $supplier_id1 ); +is( scalar(@late_or_missing_issues), 1, 'supplier 1 should have 1 issue in late' ); + +@late_or_missing_issues = GetLateOrMissingIssues( $supplier_id2); +is( scalar(@late_or_missing_issues), 2, 'supplier 2 should have 2 issues in late' ); + +is( exists $late_or_missing_issues[0]->{claimdate}, 1, 'GetLateOrMissingIssues returns claimdate' ); +is( exists $late_or_missing_issues[0]->{claims_count}, 1, 'GetLateOrMissingIssues returns claims_count' ); +is( $late_or_missing_issues[0]->{claims_count}, 0, 'The issues should not habe been claimed yet' ); + +my $serialid_to_claim = $late_or_missing_issues[0]->{serialid}; +updateClaim( $serialid_to_claim ); + +@late_or_missing_issues = GetLateOrMissingIssues( $supplier_id2); +is( scalar(@late_or_missing_issues), 2, 'supplier 2 should have 2 issues in late (already claimed issues are returns)' ); + +my ( $serial_claimed ) = grep { ($_->{serialid} == $serialid_to_claim) ? $_ : () } @late_or_missing_issues; +is( $serial_claimed->{claims_count}, 1, 'The serial should have been claimed' ); + +my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); +# FIXME: This test should pass. The GetLateOrMissingIssues should not deal with date format! +#is( $serial_claimed->{claimdate}, $today, 'The serial should have been claimed today' ); diff --git a/t/db_dependent/Serials_2.t b/t/db_dependent/Serials_2.t index 51b59955b0..0a233fc412 100644 --- a/t/db_dependent/Serials_2.t +++ b/t/db_dependent/Serials_2.t @@ -1,7 +1,7 @@ #!/usr/bin/perl use Modern::Perl; -use Test::More tests => 37; +use Test::More tests => 36; use MARC::Record; @@ -20,10 +20,6 @@ my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; - -my $supplierlist=eval{GetSuppliersWithLateIssues()}; -ok(length($@)==0,"No SQL problem in GetSuppliersWithLateIssues"); - my $record = MARC::Record->new(); $record->append_fields( MARC::Field->new( '952', '0', '0', a => 'CPL', b => 'CPL' )