Summary: | Overdue rules do not save (delay field should only accept numbers) | ||
---|---|---|---|
Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
Component: | Tools | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | jonathan.druart, veron |
Version: | unspecified | Keywords: | Academy |
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13624 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 19259: Enforce type of the delay value when editing overdue rules - template side only
Bug 19259: Enforce type of the delay value when editing overdue rules - template side only Bug 19259: Enforce type of the delay value when editing overdue rules - template side only |
Description
Aleisha Amohia
2017-09-06 00:00:16 UTC
Confirmed. In my tests it saves columns Delay, Letter and Restrict. It does NOT save columns Email and Print (Phone and SMS not tested). While saving I get the following in plack-error.log: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`overduerules_transport_types`, CONSTRAINT `overduerules_fk` FOREIGN KEY (`overduerules_id`) REFERENCES `old_overduerules` (`overduerules_id`) ON DELETE CASCADE ON UPDATE CASCAD) [for Statement " INSERT INTO overduerules_transport_types( overduerules_id, letternumber, message_transport_type ) VALUES ( (SELECT overduerules_id FROM overduerules WHERE branchcode = ? AND categorycode = ?), ?, ? ) " with ParamValues: 0='', 1='K', 2=1, 3="email"] at /home/marc/koha/tools/overduerules.pl line 198. It works for me on master with a fresh install. old_overduerules sounds related to 3.23.00.007 What returns "show create table overduerules_transport_types"? (In reply to Jonathan Druart from comment #2) > It works for me on master with a fresh install. > > old_overduerules sounds related to 3.23.00.007 > > What returns "show create table overduerules_transport_types"? mysql> show create table overduerules_transport_types; +------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | Table | Create Table | +------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | overduerules_transport_types | CREATE TABLE `overduerules_transport_types` ( `id` int(11) NOT NULL AUTO_INCREMENT, `letternumber` int(1) NOT NULL DEFAULT '1', `message_transport_type` varchar(20) COLLATE utf8_unicode_ci NOT NULL DEFAULT 'email', `overduerules_id` int(11) NOT NULL, PRIMARY KEY (`id`), KEY `overduerules_fk` (`overduerules_id`), KEY `mtt_fk` (`message_transport_type`), CONSTRAINT `mtt_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `overduerules_fk` FOREIGN KEY (`overduerules_id`) REFERENCES `old_overduerules` (`overduerules_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci | +------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ 1 row in set (0.00 sec) Reading: https://dev.mysql.com/doc/refman/5.5/en/rename-table.html ------- RENAME TABLE changes internally generated foreign key constraint names and user-defined foreign key constraint names that contain the string “tbl_name_ibfk_” to reflect the new table name. InnoDB interprets foreign key constraint names that contain the string “tbl_name_ibfk_” as internally generated names. Foreign key constraint names that point to the renamed table are automatically updated unless there is a conflict, in which case, the statement fails with an error. A conflict occurs if the renamed constraint name already exists. In such cases, you must drop and re-create the foreign keys in order for them to function properly. ------- Line 11590 in updatedatabase.pl renames table overduerules to old_overduerules, then it creates a new table overduerules. The renaming action changed / renamed REFERENCES in constraints of overduerules_transport_types (maybe in other tables as well?). That would explain that the error can not be reproduced on a fresh install (i.e. an install later than 3.23.00.007). Sorry I should clarify: When I first filed this bug I thought the whole feature was broken, but I was wrong. The overdue rule does not save when you put letters in the delay field (rather than numbers) and there is no error message. Otherwise I am actually able to save overdue rules. Marc, % git checkout 0aa6ba1599bb0de923619a365f529119dc5c67a6 # DBrev 3.23.00.006 % reset_all > SHOW CREATE TABLE overduerules_transport_types; | overduerules_transport_types | CREATE TABLE `overduerules_transport_types` ( `id` int(11) NOT NULL AUTO_INCREMENT, `branchcode` varchar(10) COLLATE utf8_unicode_ci NOT NULL DEFAULT '', `categorycode` varchar(10) COLLATE utf8_unicode_ci NOT NULL DEFAULT '', `letternumber` int(1) NOT NULL DEFAULT '1', `message_transport_type` varchar(20) COLLATE utf8_unicode_ci NOT NULL DEFAULT 'email', PRIMARY KEY (`id`), KEY `overduerules_fk` (`branchcode`,`categorycode`), KEY `mtt_fk` (`message_transport_type`), CONSTRAINT `mtt_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `overduerules_fk` FOREIGN KEY (`branchcode`, `categorycode`) REFERENCES `overduerules` (`branchcode`, `categorycode`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci | % git checkout master % restart_all % sudo koha-shell kohadev % perl installer/data/mysql/updatedatabase.pl > SHOW CREATE TABLE overduerules_transport_types; | overduerules_transport_types | CREATE TABLE `overduerules_transport_types` ( `id` int(11) NOT NULL AUTO_INCREMENT, `letternumber` int(1) NOT NULL DEFAULT '1', `message_transport_type` varchar(20) COLLATE utf8_unicode_ci NOT NULL DEFAULT 'email', `overduerules_id` int(11) NOT NULL, PRIMARY KEY (`id`), KEY `mtt_fk` (`message_transport_type`), KEY `overduerules_transport_types_fk` (`overduerules_id`), CONSTRAINT `mtt_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `overduerules_transport_types_fk` FOREIGN KEY (`overduerules_id`) REFERENCES `overduerules` (`overduerules_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci | Without data it seems that the update works fine. Maybe you got some errors during the upgrade? Is it a DB in production? Seems ok with data too (In reply to Jonathan Druart from comment #6) (...) > > Without data it seems that the update works fine. > > Maybe you got some errors during the upgrade? Is it a DB in production? Thanks for testing. It's not a DB for production. It's on a VM I installed a couple of years ago; best possible that while upgrading something went wrong. Question: Is there any script that can be used to check database structure integrity? (E.g. Typo3 CMS has such script in it's install section) ...and let's not forget the problem reported by Aleisha (see comment 5) - The problem described in comment #1 was related to a local database issue. I fixed the database structure, and I can no longer reproduce the problem (not saving Email and Print flags) - It is possible to save changes with the exception of comment #5: It does not save when you put letters in the delay field (rather than numbers) - Changing importance to "normal" - Changing - Changing .... the title for better description (In reply to Marc Véron from comment #4) > Reading: > https://dev.mysql.com/doc/refman/5.5/en/rename-table.html > ------- > RENAME TABLE changes internally generated foreign key constraint names and > user-defined foreign key constraint names that contain the string > “tbl_name_ibfk_” to reflect the new table name. InnoDB interprets foreign > key constraint names that contain the string “tbl_name_ibfk_” as internally > generated names. > > Foreign key constraint names that point to the renamed table are > automatically updated unless there is a conflict, in which case, the > statement fails with an error. A conflict occurs if the renamed constraint > name already exists. In such cases, you must drop and re-create the foreign > keys in order for them to function properly. > ------- > > Line 11590 in updatedatabase.pl renames table overduerules to > old_overduerules, then it creates a new table overduerules. > > The renaming action changed / renamed REFERENCES in constraints of > overduerules_transport_types (maybe in other tables as well?). > > That would explain that the error can not be reproduced on a fresh install > (i.e. an install later than 3.23.00.007). For the records: The database issue could be fixed with following hack: mysql> show create table overduerules_transport_types; (Verify that you see REFERENCES `old_overduerules`) mysql> RENAME TABLE overduerules TO old_overduerules; mysql> RENAME TABLE old_overduerules TO overduerules; mysql> show create table overduerules_transport_types; (Verify that REFERENCES has changed to REFERENCES `overduerules`) Created attachment 67400 [details] [review] Bug 19259: Enforce type of the delay value when editing overdue rules - template side only Overdue rules do not save when a delay that is not an integer > 0, without any warnings. This patch forces the user to select a number > 0. Test plan: Without this patch applied, enter non-integer value or negative or 0 => The rule will not be inserted without any warnings. With this patch applied you will get an message from the browser and the form will not save Created attachment 67482 [details] [review] Bug 19259: Enforce type of the delay value when editing overdue rules - template side only Overdue rules do not save when a delay that is not an integer > 0, without any warnings. This patch forces the user to select a number > 0. Test plan: Without this patch applied, enter non-integer value or negative or 0 => The rule will not be inserted without any warnings. With this patch applied you will get an message from the browser and the form will not save Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Created attachment 67921 [details] [review] Bug 19259: Enforce type of the delay value when editing overdue rules - template side only Overdue rules do not save when a delay that is not an integer > 0, without any warnings. This patch forces the user to select a number > 0. Test plan: Without this patch applied, enter non-integer value or negative or 0 => The rule will not be inserted without any warnings. With this patch applied you will get an message from the browser and the form will not save Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 17.11, thanks to everybody involved! |