There is a FIXME to add a foreign key between vendors and subscriptions
Created attachment 171971 [details] [review] Bug 38011: Add a foreign key between vendors and subscriptions Test plan: prove t/db_dependent/Koha/Subscription.t
Created attachment 171972 [details] [review] Bug 38011: Add a foreign key between vendors and subscriptions Test plan: prove t/db_dependent/Koha/Subscription.t
The tests failed for me (using KTD). If I don't run the databaseupdate, the tests pass: prove t/db_dependent/Koha/Subscription.t t/db_dependent/Koha/Subscription.t .. ok All tests successful. Files=1, Tests=10, 2 wallclock secs ( 0.01 usr 0.00 sys + 1.32 cusr 0.39 csys = 1.72 CPU) Result: PASS If I do a databaseupdate and restart everything, the tests fail: prove t/db_dependent/Koha/Subscription.t t/db_dependent/Koha/Subscription.t .. 1/10 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`subscription`, CONSTRAINT `subscription_ibfk_4` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`) ON DELETE SET NULL ON UPDATE CASCADE) at /kohadevbox/koha/Koha/Object.pm line 172 # No tests run! # Failed test 'No tests run for subtest "Koha::Subscription->biblio"' # at t/db_dependent/Koha/Subscription.t line 49. Broken FK constraint# Looks like your test exited with 255 just after 2. t/db_dependent/Koha/Subscription.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 9/10 subtests Test Summary Report ------------------- t/db_dependent/Koha/Subscription.t (Wstat: 65280 (exited 255) Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 255 Parse errors: Bad plan. You planned 10 tests but ran 2. Files=1, Tests=2, 2 wallclock secs ( 0.02 usr 0.01 sys + 0.92 cusr 0.17 csys = 1.12 CPU) Result: FAIL Am I supposed to run the databaseupdate?
Created attachment 173859 [details] [review] Bug 38011: Add a foreign key between vendors and subscriptions Test plan: prove t/db_dependent/Koha/Subscription.t
Missed something from the DB update file that I had in a forgotten commit in my local branch - added that back in and it should work now!
Created attachment 173860 [details] [review] Bug 38011: Add a foreign key between vendors and subscriptions Test plan: prove t/db_dependent/Koha/Subscription.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 174180 [details] [review] Bug 38011: Add a foreign key between vendors and subscriptions Test plan: prove t/db_dependent/Koha/Subscription.t prove t/db_dependent/Koha/Acquisition/Booksellers.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 174181 [details] [review] Bug 38011: Schema update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 174182 [details] [review] Bug 38011: (QA follow-up) Additional unit tests Add an additional unit test for the default null on the new constraint. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 174183 [details] [review] Bug 38011: (QA follow-up) Improve database update This patch ensure's we fix errant values in subscriptions prior to adding the foreign key constraint. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 174185 [details] [review] Bug 38011: Add a foreign key between vendors and subscriptions Test plan: prove t/db_dependent/Koha/Subscription.t prove t/db_dependent/Koha/Acquisition/Booksellers.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Amended-by: Jonathan Druart Rename $rs Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 174186 [details] [review] Bug 38011: Schema update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 174187 [details] [review] Bug 38011: (QA follow-up) Additional unit tests Add an additional unit test for the default null on the new constraint. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 174188 [details] [review] Bug 38011: (QA follow-up) Improve database update This patch ensure's we fix errant values in subscriptions prior to adding the foreign key constraint. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The nice output is not so nice when you have no subscriptions to update: Updated 0E0 subscriptions with NULL aqbooksellerid where aqbooksellerid was not found in aqbooksellers
Pushed for 24.11! Well done everyone, thank you!
(In reply to Katrin Fischer from comment #15) > The nice output is not so nice when you have no subscriptions to update: > > Updated 0E0 subscriptions with NULL aqbooksellerid where aqbooksellerid was > not found in aqbooksellers 0E0 is the 'non-error' zero for DBI.. it needs catching explicitly.. I've done a few updates where that's the case.
Created attachment 174397 [details] [review] Bug 38011: (QA follow-up) Improve database update output Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Follow-up picked for main - thanks!
Created attachment 174492 [details] [review] Bug 38011: Fix ReNewSubscription.t t/db_dependent/Serials/ReNewSubscription.t .. Value not allowed for auto_incr id in Aqbookseller at /kohadevbox/koha/t/lib/TestBuilder.pm line 402. Null value for subscriptionid in Subscriptionhistory not allowed at /kohadevbox/koha/t/lib/TestBuilder.pm line 415. Can't call method "enddate" on an undefined value at t/db_dependent/Serials/ReNewSubscription.t line 102. It's failing when no vendor with id=1 in DB.
Picked follow-up for main.