From b1aee9ec486d51e172c83e1897e95e8edaef9b00 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Wed, 7 Jun 2017 16:59:17 +0200 Subject: [PATCH] Bug 18745: Serial claims: save supplier info on serial-level This allows to change the subscription's supplier while keeping the info of supplier for previous serials, which allows to claim late/missing serials to the right supplier Test plan: 1. Create 2 suppliers, S1 and S2 2. Create a new subscription with S1 3. Generate some late/missing serials 4. Change supplier to S2 5. Generate some more late/missing serials 6. Go to /cgi-bin/koha/serials/claims.pl and see you have late/missing issues for both suppliers 7. Check that you have the expected issues for each supplier 8. prove t/db_dependent/Serials/Claims.t --- C4/Serials.pm | 98 +++++++++++-------- .../data/mysql/atomicupdate/bug_18745.perl | 18 ++++ installer/data/mysql/kohastructure.sql | 2 + t/db_dependent/Serials/Claims.t | 26 ++++- 4 files changed, 102 insertions(+), 42 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_18745.perl diff --git a/C4/Serials.pm b/C4/Serials.pm index e929e219ef..be6ed77b18 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -121,17 +121,17 @@ sub GetSuppliersWithLateIssues { my $dbh = C4::Context->dbh; my $statuses = join(',', ( LATE, MISSING_STATUSES, CLAIMED ) ); my $query = qq| - 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 ( $statuses ) - ) - AND subscription.closed = 0 - ORDER BY name|; + SELECT DISTINCT aqbooksellers.id, aqbooksellers.name + FROM aqbooksellers + LEFT JOIN serial ON serial.aqbooksellerid = aqbooksellers.id + LEFT JOIN subscription ON serial.subscriptionid = subscription.subscriptionid + WHERE ( + (serial.planneddate < NOW() AND serial.status = 1) + OR serial.status IN ( $statuses ) + ) + AND subscription.closed = 0 + ORDER BY name + |; return $dbh->selectall_arrayref($query, { Slice => {} }); } @@ -1440,6 +1440,7 @@ sub NewSubscription { serialseq_y => $subscription->{'lastvalue2'}, serialseq_z => $subscription->{'lastvalue3'}, subscriptionid => $subscriptionid, + aqbooksellerid => $subscription->{aqbooksellerid}, biblionumber => $biblionumber, status => EXPECTED, planneddate => $firstacquidate, @@ -1550,6 +1551,7 @@ sub NewIssue { serialseq_y => $subscription->lastvalue2(), serialseq_z => $subscription->lastvalue3(), subscriptionid => $subscriptionid, + aqbooksellerid => $subscription->aqbooksellerid(), biblionumber => $biblionumber, status => $status, planneddate => $planneddate, @@ -1776,7 +1778,7 @@ sub GetLateOrMissingIssues { my $sth; my $byserial = ''; if ($serialid) { - $byserial = "and serialid = " . $serialid; + $byserial = "and s.serialid = " . $serialid; } if ($order) { $order .= ", title"; @@ -1785,39 +1787,53 @@ sub GetLateOrMissingIssues { } my $missing_statuses_string = join ',', (MISSING_STATUSES); if ($supplierid) { - $sth = $dbh->prepare( - "SELECT - serialid, aqbooksellerid, name, - biblio.title, biblioitems.issn, planneddate, serialseq, - serial.status, serial.subscriptionid, claimdate, claims_count, + $sth = $dbh->prepare(qq{ + SELECT + s.serialid, s.aqbooksellerid, aqbooksellers.name, + biblio.title, biblioitems.issn, s.planneddate, s.serialseq, + s.status, s.subscriptionid, s.claimdate, s.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 ($missing_statuses_string) OR ((planneddate < now() AND serial.STATUS = ?) OR serial.STATUS = ? OR serial.STATUS = ?)) - AND subscription.aqbooksellerid=$supplierid - $byserial - ORDER BY $order" - ); + FROM serial s + LEFT JOIN subscription ON s.subscriptionid = subscription.subscriptionid + LEFT JOIN biblio ON subscription.biblionumber = biblio.biblionumber + LEFT JOIN biblioitems ON subscription.biblionumber = biblioitems.biblionumber + LEFT JOIN aqbooksellers ON s.aqbooksellerid = aqbooksellers.id + WHERE + ( + s.status IN ($missing_statuses_string) + OR ( + (s.planneddate < NOW() AND s.status = ?) + OR s.status = ? + OR s.status = ? + ) + ) + AND s.aqbooksellerid = $supplierid + $byserial + ORDER BY $order + }); } else { - $sth = $dbh->prepare( - "SELECT - serialid, aqbooksellerid, name, - biblio.title, planneddate, serialseq, - serial.status, serial.subscriptionid, claimdate, claims_count, + $sth = $dbh->prepare(qq{ + SELECT + s.serialid, s.aqbooksellerid, aqbooksellers.name, + biblio.title, s.planneddate, s.serialseq, + s.status, s.subscriptionid, s.claimdate, s.claims_count, 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 ($missing_statuses_string) OR ((planneddate < now() AND serial.STATUS = ?) OR serial.STATUS = ? OR serial.STATUS = ?)) + FROM serial s + LEFT JOIN subscription ON s.subscriptionid = subscription.subscriptionid + LEFT JOIN biblio ON subscription.biblionumber = biblio.biblionumber + LEFT JOIN aqbooksellers ON s.aqbooksellerid = aqbooksellers.id + WHERE + ( + s.status IN ($missing_statuses_string) + OR ( + (s.planneddate < now() AND s.status = ?) + OR s.status = ? + OR s.status = ? + ) + ) $byserial - ORDER BY $order" - ); + ORDER BY $order + }); } $sth->execute( EXPECTED, LATE, CLAIMED ); my @issuelist; diff --git a/installer/data/mysql/atomicupdate/bug_18745.perl b/installer/data/mysql/atomicupdate/bug_18745.perl new file mode 100644 index 0000000000..c301d03eb7 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_18745.perl @@ -0,0 +1,18 @@ +$DBversion = 'XXX'; +if( CheckVersion( $DBversion ) ) { + if (!column_exists('serial', 'aqbooksellerid')) { + $dbh->do(q{ + ALTER TABLE `serial` + ADD COLUMN `aqbooksellerid` int(11) NULL DEFAULT NULL AFTER `subscriptionid`, + ADD CONSTRAINT `serial_aqbooksellerid_fk` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`) + }); + $dbh->do(q{ + UPDATE serial s + JOIN subscription sub ON s.subscriptionid = sub.subscriptionid + SET s.aqbooksellerid = sub.aqbooksellerid + }); + } + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 18745 - Serial claims: save supplier info on serial-level)\n"; +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 89e71c94a5..65b0ff3e42 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1556,6 +1556,7 @@ CREATE TABLE `serial` ( -- issues related to subscriptions `serialid` int(11) NOT NULL auto_increment, -- unique key for the issue `biblionumber` varchar(100) NOT NULL default '', -- foreign key for the biblio.biblionumber that this issue is attached to `subscriptionid` varchar(100) NOT NULL default '', -- foreign key to the subscription.subscriptionid that this issue is part of + `aqbooksellerid` int(11) NULL DEFAULT NULL, -- the subscription's bookseller at the time of creation (used for claims) `serialseq` varchar(100) NOT NULL default '', -- issue information (volume, number, etc) `serialseq_x` varchar( 100 ) NULL DEFAULT NULL, -- first part of issue information `serialseq_y` varchar( 100 ) NULL DEFAULT NULL, -- second part of issue information @@ -1568,6 +1569,7 @@ CREATE TABLE `serial` ( -- issues related to subscriptions `claimdate` date default NULL, -- date claimed claims_count int(11) default 0, -- number of claims made related to this issue `routingnotes` MEDIUMTEXT, -- notes from the routing list + CONSTRAINT `serial_aqbooksellerid_fk` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`), PRIMARY KEY (`serialid`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/t/db_dependent/Serials/Claims.t b/t/db_dependent/Serials/Claims.t index 6458497219..d95d1761ce 100644 --- a/t/db_dependent/Serials/Claims.t +++ b/t/db_dependent/Serials/Claims.t @@ -1,5 +1,5 @@ use Modern::Perl; -use Test::More tests => 17; +use Test::More tests => 18; use C4::Acquisition; use C4::Budgets; @@ -162,3 +162,27 @@ 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' ); + +subtest "Claims when subscription's supplier has changed" => sub { + plan tests => 2; + + my $bookseller1 = Koha::Acquisition::Bookseller->new()->store; + my $bookseller2 = Koha::Acquisition::Bookseller->new()->store; + my $subscriptionid = NewSubscription( + undef, $branchcode, $bookseller1->id, 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 + ); + NewIssue('2', $subscriptionid, $biblionumber, 3, '2013-01-02', '2013-01-02'); # 3 is late + my $subscription = Koha::Subscriptions->find($subscriptionid); + $subscription->aqbooksellerid($bookseller2->id)->store(); + NewIssue('3', $subscriptionid, $biblionumber, 3, '2013-01-03', '2013-01-03'); # 3 is late + + my @lateissues = GetLateOrMissingIssues($bookseller1->id); + is(scalar @lateissues, 2, '2 late issues for bookseller1'); + @lateissues = GetLateOrMissingIssues($bookseller2->id); + is(scalar @lateissues, 1, '1 late issue for bookseller2'); +}; -- 2.20.1