@@ -, +, @@ a) Does not apply the patch. b) Remove all your claimissues notices and be sure you have some serial issues in late. c) remove email address for the vendor you will use. d) remove email address for the logged in user. e) Export claims using the csv export => The selected issues will be marked as claimed. f) logout/login (to update the email address). <> <> The following issues are in late: <>, <> (<>) --- C4/Letters.pm | 6 +- C4/Serials.pm | 90 +++++-------- installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 13 ++ .../prog/en/modules/serials/claims.tt | 102 ++++++++------- serials/claims.pl | 43 +++--- t/db_dependent/Acquisition/OrderFromSubscription.t | 3 - t/db_dependent/Serials/Claims.t | 145 +++++++++++++++++++++ t/db_dependent/Serials_2.t | 6 +- 9 files changed, 281 insertions(+), 128 deletions(-) create mode 100644 t/db_dependent/Serials/Claims.t --- a/C4/Letters.pm +++ a/C4/Letters.pm @@ -368,7 +368,11 @@ sub SendAlerts { Message => Encode::encode( "utf8", "" . $letter->{content} ), 'Content-Type' => 'text/plain; charset="utf8"', ); - sendmail(%mail) or carp $Mail::Sendmail::error; + + unless ( sendmail(%mail) ) { + carp $Mail::Sendmail::error; + return { error => $Mail::Sendmail::error }; + } logaction( "ACQUISITION", --- a/C4/Serials.pm +++ a/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; @@ -1973,57 +1977,29 @@ name,title,planneddate,serialseq,serial.subscriptionid from tables : subscriptio =cut sub GetLateOrMissingIssues { - my ( $supplierid, $serialid, $order ) = @_; + my ( $supplierid ) = @_; - return unless ( $supplierid or $serialid ); + return unless $supplierid; my $dbh = C4::Context->dbh; my $sth; - my $byserial = ''; - if ($serialid) { - $byserial = "and serialid = " . $serialid; - } - if ($order) { - $order .= ", title"; - } else { - $order = "title"; - } - if ($supplierid) { - $sth = $dbh->prepare( - "SELECT - serialid, aqbooksellerid, name, - biblio.title, biblioitems.issn, planneddate, serialseq, - serial.status, serial.subscriptionid, claimdate, - subscription.branchcode - 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 - 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 - ORDER BY $order" - ); - } else { - $sth = $dbh->prepare( - "SELECT - serialid, aqbooksellerid, name, - biblio.title, planneddate, serialseq, - serial.status, serial.subscriptionid, claimdate, - subscription.branchcode - 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 - 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" - ); - } - $sth->execute; + $sth = $dbh->prepare( + "SELECT + serialid, aqbooksellerid, name, + biblio.title, biblioitems.issn, planneddate, serialseq, + serial.status, serial.subscriptionid, claimdate, claims_count, + subscription.branchcode + 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 + 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=? + ORDER BY title" + ); + $sth->execute( $supplierid ); my @issuelist; while ( my $line = $sth->fetchrow_hashref ) { @@ -2095,12 +2071,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; } --- a/installer/data/mysql/kohastructure.sql +++ a/installer/data/mysql/kohastructure.sql @@ -1935,6 +1935,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; --- a/installer/data/mysql/updatedatabase.pl +++ a/installer/data/mysql/updatedatabase.pl @@ -8669,6 +8669,19 @@ 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 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt +++ a/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.
+ [% ELSIF error_claim == 'no_loggedin_user_email' %] +
No email is configured for this account.
+ [% 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 %]
--- a/serials/claims.pl +++ a/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}, --- a/t/db_dependent/Acquisition/OrderFromSubscription.t +++ a/t/db_dependent/Acquisition/OrderFromSubscription.t @@ -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", --- a/t/db_dependent/Serials/Claims.t +++ a/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' ); --- a/t/db_dependent/Serials_2.t +++ a/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' ) --