| 
        
          Description
        
        
          Matt Blenkinsop
        
        
        
        
          2024-09-26 13:24:25 UTC
        
       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! Pushed for 24.11! Well done everyone, thank you! 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. Bug fix, and updates to tests. No changes required to the manual. |