Bug 17160 - Foreign key constraint for message_transports is too loose
Summary: Foreign key constraint for message_transports is too loose
Status: RESOLVED DUPLICATE of bug 17762
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 5334
  Show dependency tree / graph
 
Reported: 2016-08-22 14:23 UTC by Barton Chittenden
Modified: 2019-12-16 10:13 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Barton Chittenden 2016-08-22 14:23:03 UTC 
I think this is the probable cause for Bug 13171.

http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=installer/data/mysql/kohastructure.sql;h=a05ef0684ae98f3fa1808fbc1533e43f684ebe9f;hb=HEAD#l2544

The message transports definition

--
-- Table structure for table `message_transports`
--

DROP TABLE IF EXISTS `message_transports`;
CREATE TABLE `message_transports` (
  `message_attribute_id` int(11) NOT NULL,
  `message_transport_type` varchar(20) NOT NULL,
  `is_digest` tinyint(1) NOT NULL default '0',
  `letter_module` varchar(20) NOT NULL default '',
  `letter_code` varchar(20) NOT NULL default '',
  `branchcode` varchar(10) NOT NULL default '',
  PRIMARY KEY  (`message_attribute_id`,`message_transport_type`,`is_digest`),
  KEY `message_transport_type` (`message_transport_type`),
  KEY `letter_module` (`letter_module`,`letter_code`),
  CONSTRAINT `message_transports_ibfk_1` FOREIGN KEY (`message_attribute_id`) REFERENCES `message_attributes` (`message_attribute_id`) ON DELETE CASCADE ON UPDATE CASCADE,
  CONSTRAINT `message_transports_ibfk_2` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDATE CASCADE,
  CONSTRAINT `message_transports_ibfk_3` FOREIGN KEY (`letter_module`, `letter_code`, `branchcode`) REFERENCES `letter` (`module`, `code`, `branchcode`) ON DELETE CASCADE ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

Has

`message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDATE CASCADE,
  CONSTRAINT `message_transports_ibfk_3` FOREIGN KEY (`letter_module`, `letter_code`, `branchcode`) REFERENCES `letter` (`module`, `code`, `branchcode`) ON DELETE CASCADE ON UPDATE CASCADE

The foreign key contstraints `module`, `code`, `branchcode` do *NOT* uniquely identify a row in the letter table, because message_transport_type may vary. Therefore, a deletion *any* message transport type will delete *all* rows matching `module`, `code`, `branchcode`.
Comment 1 Jonathan Druart 2016-08-23 16:20:33 UTC 
Barton, I understand the problem but it does not look so easy to fix it.
In the message_transports we have

MariaDB [koha]> select message_transport_type, letter_module, letter_code from message_transports;                            
+------------------------+---------------+-------------+
| message_transport_type | letter_module | letter_code |
+------------------------+---------------+-------------+
| email                  | circulation   | CHECKIN     |
| sms                    | circulation   | CHECKIN     |
| email                  | circulation   | CHECKOUT    |
| sms                    | circulation   | CHECKOUT    |
| email                  | circulation   | DUE         |
| sms                    | circulation   | DUE         |
| email                  | circulation   | DUEDGST     |
| sms                    | circulation   | DUEDGST     |
| email                  | circulation   | PREDUE      |
| sms                    | circulation   | PREDUE      |
| email                  | circulation   | PREDUEDGST  |
| sms                    | circulation   | PREDUEDGST  |
| email                  | reserves      | HOLD        |
| sms                    | reserves      | HOLD        |
+------------------------+---------------+-------------+
14 rows in set (0.10 sec)

But the letter table does necessarily contain a sms template for CHECKIN or CHECKOUT (or whatever).
If it does not exist, we won't be able to create the FK correctly.
Comment 2 Barton Chittenden 2016-08-23 19:32:23 UTC 
(In reply to Jonathan Druart from comment #1)
> Barton, I understand the problem but it does not look so easy to fix it.

<snip>

> But the letter table does necessarily contain a sms template for CHECKIN or
> CHECKOUT (or whatever).
> If it does not exist, we won't be able to create the FK correctly.

I see your point... can we remove

    ON DELETE CASCADE ON UPDATE CASCADE

from

    CONSTRAINT `message_transports_ibfk_3` FOREIGN KEY (`letter_module`, `letter_code`, `branchcode`) REFERENCES `letter` (`module`, `code`, `branchcode`) ON DELETE CASCADE ON UPDATE CASCADE


So that at least we're not losing lines in 'message_transports' if we happen to delete a row from the letter table?
Comment 3 Katrin Fischer 2016-08-23 21:08:35 UTC 
Wondering - how would you delete a single message transport line without SQL? In the GUI the letter with all its message types appears as one thing that can only be deleted completely.
Comment 4 Barton Chittenden 2016-11-02 19:35:13 UTC 
(In reply to Katrin Fischer from comment #3)
> Wondering - how would you delete a single message transport line without
> SQL? In the GUI the letter with all its message types appears as one thing
> that can only be deleted completely.

Cait,

I'm pretty sure that updatedatabase did it on Bug 13171.
Comment 5 Jonathan Druart 2019-12-16 10:13:53 UTC 
message_transports_ibfk_3 has been removed on bug 17762.

*** This bug has been marked as a duplicate of bug 17762 ***