From b83ecc1c7d0c5fc2b74655dff5f6a55290252796 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 Rebased-by: Victor Grousset/tuxayo --- C4/Serials.pm | 106 +++++++++++------- .../data/mysql/atomicupdate/bug_18745.perl | 29 +++++ installer/data/mysql/kohastructure.sql | 9 +- serials/subscription-add.pl | 4 +- t/db_dependent/Serials/Claims.t | 28 ++++- 5 files changed, 126 insertions(+), 50 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_18745.perl diff --git a/C4/Serials.pm b/C4/Serials.pm index 6a1274f20a..c2bdfd5021 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -135,17 +135,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 => {} }); } @@ -1325,6 +1325,8 @@ sub ModSubscription { $itemtype, $previousitemtype, $mana_id, $ccode ) = @_; + $aqbooksellerid ||= undef; + my $subscription = Koha::Subscriptions->find($subscriptionid); $subscription->set( { @@ -1407,6 +1409,8 @@ sub NewSubscription { ) = @_; my $dbh = C4::Context->dbh; + $aqbooksellerid ||= undef; + my $subscription = Koha::Subscription->new( { librarian => $auser, @@ -1486,6 +1490,7 @@ sub NewSubscription { serialseq_y => $subscription->{'lastvalue2'}, serialseq_z => $subscription->{'lastvalue3'}, subscriptionid => $subscriptionid, + aqbooksellerid => $subscription->{aqbooksellerid}, biblionumber => $biblionumber, status => EXPECTED, planneddate => $firstacquidate, @@ -1629,6 +1634,7 @@ sub NewIssue { serialseq_y => $subscription->lastvalue2(), serialseq_z => $subscription->lastvalue3(), subscriptionid => $subscriptionid, + aqbooksellerid => $subscription->aqbooksellerid(), biblionumber => $biblionumber, status => $status, planneddate => $planneddate, @@ -1854,7 +1860,7 @@ sub GetLateOrMissingIssues { my $sth; my $byserial = ''; if ($serialid) { - $byserial = "and serialid = " . $serialid; + $byserial = "and s.serialid = " . $serialid; } if ($order) { $order .= ", title"; @@ -1863,39 +1869,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, - subscription.branchcode, serial.publisheddate - 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" - ); + $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, s.publisheddate + 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, - subscription.branchcode, serial.publisheddate - 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 = ?)) + $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, s.publisheddate + 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..dbc90e042b --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_18745.perl @@ -0,0 +1,29 @@ +$DBversion = 'XXX'; +if( CheckVersion( $DBversion ) ) { + $dbh->do(q{ + UPDATE subscription + SET aqbooksellerid = NULL + WHERE aqbooksellerid = 0 + }); + $dbh->do(q{ + ALTER TABLE subscription + MODIFY COLUMN aqbooksellerid int(11) NULL DEFAULT NULL, + ADD CONSTRAINT subscription_ibfk_aqbooksellerid FOREIGN KEY (aqbooksellerid) REFERENCES aqbooksellers (id) ON DELETE RESTRICT ON UPDATE CASCADE + }); + + 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 79b1b6ea4f..423c747efd 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4775,6 +4775,7 @@ CREATE TABLE `serial` ( `serialid` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique key for the issue', `biblionumber` int(11) NOT NULL COMMENT 'foreign key for the biblio.biblionumber that this issue is attached to', `subscriptionid` int(11) NOT NULL COMMENT 'foreign key to the subscription.subscriptionid that this issue is part of', + `aqbooksellerid` int(11) NULL DEFAULT NULL COMMENT 'the subscription''s bookseller at the time of creation (used for claims)', `serialseq` varchar(100) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '' COMMENT 'issue information (volume, number, etc)', `serialseq_x` varchar(100) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'first part of issue information', `serialseq_y` varchar(100) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'second part of issue information', @@ -4791,7 +4792,8 @@ CREATE TABLE `serial` ( KEY `serial_ibfk_1` (`biblionumber`), KEY `serial_ibfk_2` (`subscriptionid`), CONSTRAINT `serial_ibfk_1` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT `serial_ibfk_2` FOREIGN KEY (`subscriptionid`) REFERENCES `subscription` (`subscriptionid`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `serial_ibfk_2` FOREIGN KEY (`subscriptionid`) REFERENCES `subscription` (`subscriptionid`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `serial_ibfk_3` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; @@ -5003,7 +5005,7 @@ CREATE TABLE `subscription` ( `subscriptionid` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique key for this subscription', `librarian` varchar(100) COLLATE utf8mb4_unicode_ci DEFAULT '' COMMENT 'the librarian''s username from borrowers.userid', `startdate` date DEFAULT NULL COMMENT 'start date for this subscription', - `aqbooksellerid` int(11) DEFAULT 0 COMMENT 'foreign key for aqbooksellers.id to link to the vendor', + `aqbooksellerid` int(11) NULL DEFAULT NULL COMMENT 'foreign key for aqbooksellers.id to link to the vendor', `cost` int(11) DEFAULT 0, `aqbudgetid` int(11) DEFAULT 0, `weeklength` int(11) DEFAULT 0 COMMENT 'subscription length in weeks (will not be filled in if monthlength or numberlength is set)', @@ -5049,7 +5051,8 @@ CREATE TABLE `subscription` ( KEY `subscription_ibfk_3` (`biblionumber`), CONSTRAINT `subscription_ibfk_1` FOREIGN KEY (`periodicity`) REFERENCES `subscription_frequencies` (`id`) ON DELETE SET NULL ON UPDATE CASCADE, CONSTRAINT `subscription_ibfk_2` FOREIGN KEY (`numberpattern`) REFERENCES `subscription_numberpatterns` (`id`) ON DELETE SET NULL ON UPDATE CASCADE, - CONSTRAINT `subscription_ibfk_3` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `subscription_ibfk_3` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `subscription_ibfk_4` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`) ON DELETE RESTRICT ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index c08142b897..6fcc7f018d 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -297,7 +297,7 @@ sub redirect_add_subscription { my $auser = $query->param('user'); my $branchcode = $query->param('branchcode'); - my $aqbooksellerid = $query->param('aqbooksellerid'); + my $aqbooksellerid = $query->param('aqbooksellerid') || undef; my $cost = $query->param('cost'); my $aqbudgetid = $query->param('aqbudgetid'); my @irregularity = $query->multi_param('irregularity'); @@ -393,7 +393,7 @@ sub redirect_mod_subscription { my $librarian => scalar $query->param('librarian'), my $branchcode = $query->param('branchcode'); my $cost = $query->param('cost'); - my $aqbooksellerid = $query->param('aqbooksellerid'); + my $aqbooksellerid = $query->param('aqbooksellerid') || undef; my $biblionumber = $query->param('biblionumber'); my $aqbudgetid = $query->param('aqbudgetid'); diff --git a/t/db_dependent/Serials/Claims.t b/t/db_dependent/Serials/Claims.t index afc3a18e66..a17087492e 100755 --- a/t/db_dependent/Serials/Claims.t +++ b/t/db_dependent/Serials/Claims.t @@ -1,11 +1,11 @@ use Modern::Perl; -use Test::More tests => 17; +use Test::More tests => 18; use C4::Acquisition; use C4::Budgets qw( AddBudgetPeriod AddBudget ); use Koha::Database; use Koha::Acquisition::Booksellers; -use_ok('C4::Serials', qw( GetSuppliersWithLateIssues NewSubscription GetLateOrMissingIssues updateClaim )); +use_ok('C4::Serials', qw( GetSuppliersWithLateIssues NewSubscription GetLateOrMissingIssues updateClaim NewIssue )); use Koha::DateUtils qw( dt_from_string output_pref ); @@ -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({ name => 'Bookseller #1' })->store; + my $bookseller2 = Koha::Acquisition::Bookseller->new({ name => 'Bookseller #2' })->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.37.1