The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys.
It is easy to create overdue triggers that don't go away when a category is deleted. Those rules are hidden from the editor and can cause overdue notices to no longer send!
I've just had this problem with a branch that was deleted and the rules didn't go away, crashing out the overdues script!
Created attachment 105368 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Created attachment 105369 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Created attachment 105380 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Created attachment 105382 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Created attachment 105387 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Created attachment 105388 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
> 0) Create some overdue rules with invalid branchcodes and categorycodes Does it means: - Home › Tools › Overdue notice/status triggers - create triggers - check the overduerules table - delete the branch - check that the overduerules are still there > 6) Editor should behave as it did before Does it means checking that "Home › Tools › Overdue notice/status triggers" still works?
(In reply to Victor Grousset/tuxayo from comment #9) > > 0) Create some overdue rules with invalid branchcodes and categorycodes > > Does it means: > - Home › Tools › Overdue notice/status triggers > - create triggers > - check the overduerules table > - delete the branch > - check that the overduerules are still there Yes, that would work. I was lazy and just inserted them directly from the mysql cli :) > > 6) Editor should behave as it did before > > Does it means checking that "Home › Tools › Overdue notice/status triggers" > still works? Yes!
> 6) Editor should behave as it did before Everything was fine until this step: > Could not compile /kohadevbox/koha/tools/overduerules.pl: > syntax error at /kohadevbox/koha/tools/overduerules.pl line 169, near "};" > BEGIN not safe after errors--compilation aborted at > /kohadevbox/koha/tools/overduerules.pl line 313. > at /usr/share/perl5/CGI/Compile.pm line 132 It's a missing parenthesis: }; ↓ }); Even after fixing there is an issue. "Email" and "print" aren't saved when ticked, they disappear after saving.
BTW, how does one makes "phone" and "SMS" tickable?
Created attachment 105424 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
(In reply to Victor Grousset/tuxayo from comment #12) > BTW, how does one makes "phone" and "SMS" tickable? You can set the SMS::Send driver syspref to 'Email', and enable the talking tech itiva syspref iirc.
Thanks! Should the following problem be fixed? > "Email" and "print" aren't saved when ticked, they disappear after saving. It seems the lasted patch only fixed the compilation error.
(In reply to Victor Grousset/tuxayo from comment #15) > Should the following problem be fixed? > > > "Email" and "print" aren't saved when ticked, they disappear after saving. > > It seems the lasted patch only fixed the compilation error. I wanted to doublecheck and something else went wrong - be on master code - reset_all - apply patch - updatedatabase - go to /cgi-bin/koha/tools/overduerules.pl - fill on the first line, some delay, some letter, tick everything you can - submit - "Changes saved" - the UI however shows nothing of what was inputted - try again on the same line - still nothing - pick a library - try again - this time it works So there is something wrong with the default triggers. (Tested on Debian 10 and Debian 9.) - be on master code + the patch - spin up the instance (koha testing docker) - go to /cgi-bin/koha/tools/overduerules.pl - fill on the first line, some delay, some letter, tick everything you can - submit - all saved except email and phone - happens only for default triggers caveat: maybe the 2nd case is wrong. Maybe one shouldn't build a dev env with the path already applied. I don't know how DB are handled when integrated in master which makes this work in master. And might prevent from working on a submitted patch. Anyway, first case is valid, there is still an issue.
Created attachment 105551 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Kyle, we are loosing the default values. To recreate: On master, add some rule for the "Default" library (branchcode="") Apply the patches, run the udpate DB
Created attachment 106201 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Hi Kyle, working through this and it looks good overall until now. But the tests need to be updated as well I think: $ prove t/db_dependent/Overdues.t t/db_dependent/Overdues.t .. 1/17 {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`overduerules`, CONSTRAINT `fk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement " INSERT INTO overduerules ( overduerules_id, branchcode, categorycode ) VALUES (1, 'CPL', 'PT'), (2, 'CPL', 'YA'), (3, '', 'PT'), (4, '', 'YA') "] at /usr/share/perl5/DBIx/Class/Schema.pm line 1118. DBIx::Class::Schema::throw_exception(Koha::Schema=HASH(0x5626c3608c58), "DBI Exception: DBD::mysql::db do failed: Cannot add or update"...) called at /usr/share/perl5/DBIx/Class/Storage.pm line 113 DBIx::Class::Storage::throw_exception(DBIx::Class::Storage::DBI::mysql=HASH(0x5626c3e11be8), "DBI Exception: DBD::mysql::db do failed: Cannot add or update"...) called at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1501 DBIx::Class::Storage::DBI::__ANON__("DBD::mysql::db do failed: Cannot add or update a child row: a"..., DBI::db=HASH(0x5626c44f6520), undef) called at t/db_dependent/Overdues.t line 33 # Looks like your test exited with 255 just after 3. t/db_dependent/Overdues.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 14/17 subtests Test Summary Report ------------------- t/db_dependent/Overdues.t (Wstat: 65280 Tests: 3 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 17 tests but ran 3.
(In reply to Katrin Fischer from comment #20) > Hi Kyle, > > working through this and it looks good overall until now. But the tests need > to be updated as well I think: > > $ prove t/db_dependent/Overdues.t > t/db_dependent/Overdues.t .. 1/17 {UNKNOWN}: DBI Exception: DBD::mysql::db > do failed: Cannot add or update a child row: a foreign key constraint fails > (`koha_kohadev`.`overduerules`, CONSTRAINT `fk_branchcode` FOREIGN KEY > (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON > UPDATE CASCADE) [for Statement " > INSERT INTO overduerules ( overduerules_id, branchcode, categorycode ) > VALUES > (1, 'CPL', 'PT'), > (2, 'CPL', 'YA'), > (3, '', 'PT'), > (4, '', 'YA') > "] at /usr/share/perl5/DBIx/Class/Schema.pm line 1118. > DBIx::Class::Schema::throw_exception(Koha::Schema=HASH(0x5626c3608c58), > "DBI Exception: DBD::mysql::db do failed: Cannot add or update"...) called > at /usr/share/perl5/DBIx/Class/Storage.pm line 113 > DBIx::Class::Storage::throw_exception(DBIx::Class::Storage::DBI:: > mysql=HASH(0x5626c3e11be8), "DBI Exception: DBD::mysql::db do failed: Cannot > add or update"...) called at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line > 1501 > DBIx::Class::Storage::DBI::__ANON__("DBD::mysql::db do failed: Cannot add > or update a child row: a"..., DBI::db=HASH(0x5626c44f6520), undef) called at > t/db_dependent/Overdues.t line 33 > > # Looks like your test exited with 255 just after 3. > t/db_dependent/Overdues.t .. Dubious, test returned 255 (wstat 65280, 0xff00) > Failed 14/17 subtests > > Test Summary Report > ------------------- > t/db_dependent/Overdues.t (Wstat: 65280 Tests: 3 Failed: 0) > Non-zero exit status: 255 > Parse errors: Bad plan. You planned 17 tests but ran 3. I'm unable to recreate your error. Are you using a fresh database?
Created attachment 134730 [details] [review] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before
Created attachment 134731 [details] [review] Bug 18086: (follow-up) Update for export changes
Just a quick rebase.. tests are still failing for obvious reasons (We've set NOT NULL and NULL instead of '') and the tests try to set the data that way. Needs a little digging to understand intentions.
Just got bitten by this one again...
I reckon this bug could be split into 2. Why don't we just add the foreign keys to the database, and then do the Perl changes in a separate bug? This report has been stuck for 5 years now :/