From 6bbc383dc4c9070079986235fa145cafc3c72eeb Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 17 Apr 2018 12:03:00 +1200 Subject: [PATCH] Fix trigger generation for modified_date on custom data In 4.7.25 https://github.com/civicrm/civicrm-core/pull/10754/commits introduced some modifications to the generation of triggers to update the modified date field. It basically derived the entity being extended by a table and then if that entity had a modified_date field then a trigger would be created to update that field. However, a bug in the CRM_Core_BAO_CustomGroup::getAllCustomGroupsByBaseEntity function meant that incorrect additional tables are also being updated for custom fields. For entities extending contact the contact table AND the mailing table are updated. e.g ``` CREATE TRIGGER {mycustomtable}... UPDATE civicrm_contact SET modified_date = CURRENT_TIMESTAMP WHERE id = NEW.entity_id; UPDATE civicrm_mailing SET modified_date = CURRENT_TIMESTAMP WHERE id = NEW.entity_id; ``` For entities that extend tables that should not attract a trigger ONLY the mailing table is updated. The bug in CRM_Core_BAO_CustomGroup::getAllCustomGroupsByBaseEntity is that it adds the WHERE clause 'AND extends IN ($entityType)' ONLY if $entityType is in a whitelist. Invalid entities result in no filtering. As a fix using a whitelist is no longer valid - we support any entity that might be configured now so simply filtering on the entity makes sense. Other paths to this function seem unlikely to pass in invalid entities & hence trigger this bug. --- CRM/Core/BAO/CustomGroup.php | 12 +++++------- tests/phpunit/CRM/Logging/SchemaTest.php | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/CRM/Core/BAO/CustomGroup.php b/CRM/Core/BAO/CustomGroup.php index 1ec8b47144..6653d6b4f7 100644 --- a/CRM/Core/BAO/CustomGroup.php +++ b/CRM/Core/BAO/CustomGroup.php @@ -1259,6 +1259,10 @@ ORDER BY civicrm_custom_group.weight, */ private static function _addWhereAdd(&$customGroupDAO, $entityType, $entityID = NULL, $allSubtypes = FALSE) { $addSubtypeClause = FALSE; + // This function isn't really accessible with user data but since the string + // is not passed as a param to the query CRM_Core_DAO::escapeString seems like a harmless + // precaution. + $entityType = CRM_Core_DAO::escapeString($entityType); switch ($entityType) { case 'Contact': @@ -1281,13 +1285,7 @@ ORDER BY civicrm_custom_group.weight, } break; - case 'Case': - case 'Location': - case 'Address': - case 'Activity': - case 'Contribution': - case 'Membership': - case 'Participant': + default: $customGroupDAO->whereAdd("extends IN ('$entityType')"); break; } diff --git a/tests/phpunit/CRM/Logging/SchemaTest.php b/tests/phpunit/CRM/Logging/SchemaTest.php index 9c8b7a1a7a..2b238077e1 100644 --- a/tests/phpunit/CRM/Logging/SchemaTest.php +++ b/tests/phpunit/CRM/Logging/SchemaTest.php @@ -43,6 +43,26 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { } } + /** + * Test correct creation of modified date triggers. + * + * Specifically we are testing that the contact table modified date and + * ONLY the contact table modified date is updated when the custom field is updated. + * + * (At point of writing this the modification was leaking to the mailing table). + */ + public function testTriggers() { + $customGroup = $this->entityCustomGroupWithSingleFieldCreate('Contact', 'ContactTest....'); + Civi::service('sql_triggers')->rebuild(); + $log_table = CRM_Core_DAO::executeQuery("SHOW TRIGGERS WHERE `Trigger` LIKE 'civicrm_value_contact_{$customGroup['custom_group_id']}_after_insert%'"); + + while ($log_table->fetch()) { + $this->assertContains('UPDATE civicrm_contact SET modified_date = CURRENT_TIMESTAMP WHERE id = NEW.entity_id;', $log_table->Statement, "Contact modification update should be in the trigger :\n" . $log_table->Statement); + $this->assertNotContains('civicrm_mailing', $log_table->Statement, 'Contact field should not update mailing table'); + $this->assertEquals(1, substr_count($log_table->Statement, 'SET modified_date'), 'Modified date should only be updated on one table (here it is contact)'); + } + } + public function testAutoIncrementNonIdColumn() { CRM_Core_DAO::executeQuery("CREATE TABLE `civicrm_test_table` ( test_id int(10) unsigned NOT NULL AUTO_INCREMENT, -- 2.25.1