From 3ed77291082b53abe8cdd9cd8f7d66c7b5b62344 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 24 Jul 2017 23:10:12 -0700 Subject: [PATCH 01/13] CRM-20958 - Declare created/modified timestamps for case+activity --- CRM/Activity/DAO/Activity.php | 46 +++++++++++++++++++++++++++++++- CRM/Case/DAO/Case.php | 46 +++++++++++++++++++++++++++++++- xml/schema/Activity/Activity.xml | 20 ++++++++++++++ xml/schema/Case/Case.xml | 20 ++++++++++++++ 4 files changed, 130 insertions(+), 2 deletions(-) diff --git a/CRM/Activity/DAO/Activity.php b/CRM/Activity/DAO/Activity.php index 5b099160668d..d619c2e99029 100644 --- a/CRM/Activity/DAO/Activity.php +++ b/CRM/Activity/DAO/Activity.php @@ -30,7 +30,7 @@ * * Generated from xml/schema/CRM/Activity/Activity.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:a530f1fb1a27c5a15b5d138732b4c581) + * (GenCodeChecksum:dfa63754ef6ea1a9c7148e735dd6ff8a) */ require_once 'CRM/Core/DAO.php'; require_once 'CRM/Utils/Type.php'; @@ -195,6 +195,18 @@ class CRM_Activity_DAO_Activity extends CRM_Core_DAO { * @var boolean */ public $is_star; + /** + * When was the activity was created. + * + * @var timestamp + */ + public $created_date; + /** + * When was the activity (or closely related entity) was created or modified or deleted. + * + * @var timestamp + */ + public $modified_date; /** * Class constructor. */ @@ -642,6 +654,38 @@ static function &fields() { 'bao' => 'CRM_Activity_BAO_Activity', 'localizable' => 0, ) , + 'activity_created_date' => array( + 'name' => 'created_date', + 'type' => CRM_Utils_Type::T_TIMESTAMP, + 'title' => ts('Created Date') , + 'description' => 'When was the activity was created.', + 'required' => false, + 'export' => true, + 'where' => 'civicrm_activity.created_date', + 'headerPattern' => '', + 'dataPattern' => '', + 'default' => 'NULL', + 'table_name' => 'civicrm_activity', + 'entity' => 'Activity', + 'bao' => 'CRM_Activity_BAO_Activity', + 'localizable' => 0, + ) , + 'activity_modified_date' => array( + 'name' => 'modified_date', + 'type' => CRM_Utils_Type::T_TIMESTAMP, + 'title' => ts('Modified Date') , + 'description' => 'When was the activity (or closely related entity) was created or modified or deleted.', + 'required' => false, + 'export' => true, + 'where' => 'civicrm_activity.modified_date', + 'headerPattern' => '', + 'dataPattern' => '', + 'default' => 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP', + 'table_name' => 'civicrm_activity', + 'entity' => 'Activity', + 'bao' => 'CRM_Activity_BAO_Activity', + 'localizable' => 0, + ) , ); CRM_Core_DAO_AllCoreTables::invoke(__CLASS__, 'fields_callback', Civi::$statics[__CLASS__]['fields']); } diff --git a/CRM/Case/DAO/Case.php b/CRM/Case/DAO/Case.php index dd1bfe3b027b..66bf0d6aff94 100644 --- a/CRM/Case/DAO/Case.php +++ b/CRM/Case/DAO/Case.php @@ -30,7 +30,7 @@ * * Generated from xml/schema/CRM/Case/Case.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:e45e7e2a53a945c4659cf393410a9d7a) + * (GenCodeChecksum:2a046fd795b19790f45c5d9dde06a538) */ require_once 'CRM/Core/DAO.php'; require_once 'CRM/Utils/Type.php'; @@ -97,6 +97,18 @@ class CRM_Case_DAO_Case extends CRM_Core_DAO { * @var boolean */ public $is_deleted; + /** + * When was the case was created. + * + * @var timestamp + */ + public $created_date; + /** + * When was the case (or closely related entity) was created or modified or deleted. + * + * @var timestamp + */ + public $modified_date; /** * Class constructor. */ @@ -275,6 +287,38 @@ static function &fields() { 'bao' => 'CRM_Case_BAO_Case', 'localizable' => 0, ) , + 'case_created_date' => array( + 'name' => 'created_date', + 'type' => CRM_Utils_Type::T_TIMESTAMP, + 'title' => ts('Created Date') , + 'description' => 'When was the case was created.', + 'required' => false, + 'export' => true, + 'where' => 'civicrm_case.created_date', + 'headerPattern' => '', + 'dataPattern' => '', + 'default' => 'NULL', + 'table_name' => 'civicrm_case', + 'entity' => 'Case', + 'bao' => 'CRM_Case_BAO_Case', + 'localizable' => 0, + ) , + 'case_modified_date' => array( + 'name' => 'modified_date', + 'type' => CRM_Utils_Type::T_TIMESTAMP, + 'title' => ts('Modified Date') , + 'description' => 'When was the case (or closely related entity) was created or modified or deleted.', + 'required' => false, + 'export' => true, + 'where' => 'civicrm_case.modified_date', + 'headerPattern' => '', + 'dataPattern' => '', + 'default' => 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP', + 'table_name' => 'civicrm_case', + 'entity' => 'Case', + 'bao' => 'CRM_Case_BAO_Case', + 'localizable' => 0, + ) , ); CRM_Core_DAO_AllCoreTables::invoke(__CLASS__, 'fields_callback', Civi::$statics[__CLASS__]['fields']); } diff --git a/xml/schema/Activity/Activity.xml b/xml/schema/Activity/Activity.xml index 5c5d013bcca7..6b6da122492c 100644 --- a/xml/schema/Activity/Activity.xml +++ b/xml/schema/Activity/Activity.xml @@ -463,4 +463,24 @@ /(activity.)?(star|favorite)/i 4.7 + + created_date + activity_created_date + timestamp + When was the activity was created. + false + true + NULL + 4.7 + + + modified_date + activity_modified_date + timestamp + When was the activity (or closely related entity) was created or modified or deleted. + false + true + CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP + 4.7 + diff --git a/xml/schema/Case/Case.xml b/xml/schema/Case/Case.xml index 37d19180771f..beeaaa987413 100644 --- a/xml/schema/Case/Case.xml +++ b/xml/schema/Case/Case.xml @@ -191,4 +191,24 @@ is_deleted 2.2 + + created_date + case_created_date + timestamp + When was the case was created. + false + true + NULL + 4.7 + + + modified_date + case_modified_date + timestamp + When was the case (or closely related entity) was created or modified or deleted. + false + true + CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP + 4.7 + From f6cecca82b633c59425f51d27f7ae0db689233fc Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 24 Jul 2017 23:13:15 -0700 Subject: [PATCH 02/13] CRM-20958 - On upgrade, declare created/modified timestamps for case+activity --- CRM/Upgrade/Incremental/php/FourSeven.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CRM/Upgrade/Incremental/php/FourSeven.php b/CRM/Upgrade/Incremental/php/FourSeven.php index 08c8a722fbec..c49eaad4c82f 100644 --- a/CRM/Upgrade/Incremental/php/FourSeven.php +++ b/CRM/Upgrade/Incremental/php/FourSeven.php @@ -418,6 +418,17 @@ public function upgrade_4_7_25($rev) { 'civicrm_uf_group', 'cancel_button_text', "varchar(64) COLLATE utf8_unicode_ci DEFAULT NULL COMMENT 'Custom Text to display on the cancel button when used in create or edit mode'", TRUE); $this->addTask('Add Submit button text column to civicrm_uf_group', 'addColumn', 'civicrm_uf_group', 'submit_button_text', "varchar(64) COLLATE utf8_unicode_ci DEFAULT NULL COMMENT 'Custom Text to display on the submit button on profile edit/create screens'", TRUE); + + $this->addTask('CRM-20958 - Add created_date to civicrm_activity', 'addColumn', + 'civicrm_activity', 'created_date', "timestamp NULL DEFAULT NULL COMMENT 'When was the activity was created.'"); + $this->addTask('CRM-20958 - Add modified_date to civicrm_activity', 'addColumn', + 'civicrm_activity', 'modified_date', "timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT 'When was the activity (or closely related entity) was created or modified or deleted.'"); + $this->addTask('CRM-20958 - Add created_date to civicrm_case', 'addColumn', + 'civicrm_case', 'created_date', "timestamp NULL DEFAULT NULL COMMENT 'When was the case was created.'"); + $this->addTask('CRM-20958 - Add modified_date to civicrm_case', 'addColumn', + 'civicrm_case', 'modified_date', "timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT 'When was the case (or closely related entity) was created or modified or deleted.'"); + + return TRUE; } From 6050e9ad11556cf2f67cbe5b0faaee374633c2d8 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 26 Jul 2017 18:08:20 -0700 Subject: [PATCH 03/13] CRM-20958 - Extract helper Civi\Core\SqlTrigger\TimestampTriggers == Before == * SQL triggers to populate `civicrm_contact.created_date` and `civicrm_contact.modified_date` are generate via `CRM_Contact_BAO_Contact::triggerInfo($info, $tableName)` == After == * `CRM_Contact_BAO_Contact::triggerInfo` calls a helper `TimestampTriggers` * The helper `TimestampTriggers` accepts arguments describing the names of the tables/columns which needed for the timestamp triggers. == Comments == To test, I used this command to update and dump the schema: ``` cv api system.flush triggers=1 && mysqldump --triggers ... ``` The schema was identical before and after. (Notably, by alternately hacking the code, I was able to validate the test was capable of revealing discrepencies.) --- CRM/Contact/BAO/Contact.php | 87 +----- Civi/Core/SqlTrigger/TimestampTriggers.php | 337 +++++++++++++++++++++ 2 files changed, 348 insertions(+), 76 deletions(-) create mode 100644 Civi/Core/SqlTrigger/TimestampTriggers.php diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index df8eaf5929a3..9c829b29c172 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -3277,51 +3277,6 @@ public static function getTimestamps($contactId) { } } - /** - * Generate triggers to update the timestamp. - * - * The corresponding civicrm_contact row is updated on insert/update/delete - * to a table that extends civicrm_contact. - * Don't regenerate triggers for all such tables if only asked for one table. - * - * @param array $info - * Reference to the array where generated trigger information is being stored - * @param string|null $reqTableName - * Name of the table for which triggers are being generated, or NULL if all tables - * @param array $relatedTableNames - * Array of all core or all custom table names extending civicrm_contact - * @param string $contactRefColumn - * 'contact_id' if processing core tables, 'entity_id' if processing custom tables - * - * @link https://issues.civicrm.org/jira/browse/CRM-15602 - * @see triggerInfo - */ - public static function generateTimestampTriggers(&$info, $reqTableName, $relatedTableNames, $contactRefColumn) { - // Safety - $contactRefColumn = CRM_Core_DAO::escapeString($contactRefColumn); - // If specific related table requested, just process that one - if (in_array($reqTableName, $relatedTableNames)) { - $relatedTableNames = array($reqTableName); - } - - // If no specific table requested (include all related tables), - // or a specific related table requested (as matched above) - if (empty($reqTableName) || in_array($reqTableName, $relatedTableNames)) { - $info[] = array( - 'table' => $relatedTableNames, - 'when' => 'AFTER', - 'event' => array('INSERT', 'UPDATE'), - 'sql' => "\nUPDATE civicrm_contact SET modified_date = CURRENT_TIMESTAMP WHERE id = NEW.$contactRefColumn;\n", - ); - $info[] = array( - 'table' => $relatedTableNames, - 'when' => 'AFTER', - 'event' => array('DELETE'), - 'sql' => "\nUPDATE civicrm_contact SET modified_date = CURRENT_TIMESTAMP WHERE id = OLD.$contactRefColumn;\n", - ); - } - } - /** * Get a list of triggers for the contact table. * @@ -3343,37 +3298,17 @@ public static function triggerInfo(&$info, $tableName = NULL) { } } - // Update timestamp for civicrm_contact itself - if ($tableName == NULL || $tableName == self::getTableName()) { - $info[] = array( - 'table' => array(self::getTableName()), - 'when' => 'BEFORE', - 'event' => array('INSERT'), - 'sql' => "\nSET NEW.created_date = CURRENT_TIMESTAMP;\n", - ); - } - - // Update timestamp when modifying closely related core tables - $relatedTables = array( - 'civicrm_address', - 'civicrm_email', - 'civicrm_im', - 'civicrm_phone', - 'civicrm_website', - ); - self::generateTimestampTriggers($info, $tableName, $relatedTables, 'contact_id'); - - // Update timestamp when modifying related custom-data tables - $customGroupTables = array(); - $customGroupDAO = CRM_Core_BAO_CustomGroup::getAllCustomGroupsByBaseEntity('Contact'); - $customGroupDAO->is_multiple = 0; - $customGroupDAO->find(); - while ($customGroupDAO->fetch()) { - $customGroupTables[] = $customGroupDAO->table_name; - } - if (!empty($customGroupTables)) { - self::generateTimestampTriggers($info, $tableName, $customGroupTables, 'entity_id'); - } + // Modifications to these records should update the contact timestamps. + \Civi\Core\SqlTrigger\TimestampTriggers::create('civicrm_contact', 'Contact') + ->setRelations(array( + array('table' => 'civicrm_address', 'column' => 'contact_id'), + array('table' => 'civicrm_email', 'column' => 'contact_id'), + array('table' => 'civicrm_im', 'column' => 'contact_id'), + array('table' => 'civicrm_phone', 'column' => 'contact_id'), + array('table' => 'civicrm_website', 'column' => 'contact_id'), + ) + ) + ->alterTriggerInfo($info, $tableName); // Update phone table to populate phone_numeric field if (!$tableName || $tableName == 'civicrm_phone') { diff --git a/Civi/Core/SqlTrigger/TimestampTriggers.php b/Civi/Core/SqlTrigger/TimestampTriggers.php new file mode 100644 index 000000000000..e204332b772e --- /dev/null +++ b/Civi/Core/SqlTrigger/TimestampTriggers.php @@ -0,0 +1,337 @@ + 'civicrm_bar', 'column' => 'foo_id'); + */ + private $relations; + + /** + * @param string $tableName + * SQL table name. + * Ex: 'civicrm_contact', 'civicrm_activity'. + * @param string $customDataEntity + * An entity name (from civicrm_custom_group.extends). + * Ex: 'Contact', 'Activity'. + * @return TimestampTriggers + */ + public static function create($tableName, $customDataEntity) { + return new static($tableName, $customDataEntity); + } + + /** + * TimestampTriggers constructor. + * + * @param string $tableName + * SQL table name. + * Ex: 'civicrm_contact', 'civicrm_activity'. + * @param string $customDataEntity + * An entity name (from civicrm_custom_group.extends). + * Ex: 'Contact', 'Activity'. + * @param string $createdDate + * SQL column name. + * Ex: 'created_date'. + * @param string $modifiedDate + * SQL column name. + * Ex: 'modified_date'. + * @param array $relations + * Ex: $relations[0] == array('table' => 'civicrm_bar', 'column' => 'foo_id'); + */ + public function __construct( + $tableName, + $customDataEntity, + $createdDate = 'created_date', + $modifiedDate = 'modified_date', + $relations = array() + ) { + $this->tableName = $tableName; + $this->customDataEntity = $customDataEntity; + $this->createdDate = $createdDate; + $this->modifiedDate = $modifiedDate; + $this->relations = $relations; + } + + /** + * Add our list of triggers to the global list. + * + * @param \Civi\Core\Event\GenericHookEvent $e + * @see \CRM_Utils_Hook::triggerInfo + */ + public function onTriggerInfo($e) { + $this->alterTriggerInfo($e->info, $e->tableName); + } + + /** + * Add our list of triggers to the global list. + * + * @see \CRM_Utils_Hook::triggerInfo + * @see \CRM_Core_DAO::triggerRebuild + * + * @param array $info + * See hook_civicrm_triggerInfo. + * @param string|NULL $tableFilter + * See hook_civicrm_triggerInfo. + */ + public function alterTriggerInfo(&$info, $tableFilter = NULL) { + //during upgrade, first check for valid version and then create triggers + //i.e the columns created_date and modified_date are introduced in 4.3.alpha1 so dont create triggers for older version + if (\CRM_Core_Config::isUpgradeMode()) { + if (!\CRM_Core_DAO::checkFieldExists($this->getTableName(), + $this->getCreatedDate()) + ) { + return; + } + // $currentVer = CRM_Core_BAO_Domain::version(TRUE); + // if (version_compare($currentVer, '4.3.alpha1') < 0) { + // return; + // } + } + + if ($tableFilter == NULL || $tableFilter == $this->getTableName()) { + $info[] = array( + 'table' => array($this->getTableName()), + 'when' => 'BEFORE', + 'event' => array('INSERT'), + 'sql' => "\nSET NEW.{$this->getCreatedDate()} = CURRENT_TIMESTAMP;\n", + ); + } + + // Update timestamp when modifying closely related tables + $relIdx = \CRM_Utils_Array::index( + array('column', 'table'), + $this->getAllRelations() + ); + foreach ($relIdx as $column => $someRelations) { + $this->generateTimestampTriggers($info, $tableFilter, + array_keys($someRelations), $column); + } + } + + /** + * Generate triggers to update the timestamp. + * + * The corresponding civicrm_FOO row is updated on insert/update/delete + * to a table that extends civicrm_FOO. + * Don't regenerate triggers for all such tables if only asked for one table. + * + * @param array $info + * Reference to the array where generated trigger information is being stored + * @param string|null $tableFilter + * Name of the table for which triggers are being generated, or NULL if all tables + * @param array $relatedTableNames + * Array of all core or all custom table names extending civicrm_FOO + * @param string $contactRefColumn + * 'contact_id' if processing core tables, 'entity_id' if processing custom tables + * + * @link https://issues.civicrm.org/jira/browse/CRM-15602 + * @see triggerInfo + */ + public function generateTimestampTriggers( + &$info, + $tableFilter, + $relatedTableNames, + $contactRefColumn + ) { + // Safety + $contactRefColumn = \CRM_Core_DAO::escapeString($contactRefColumn); + + // If specific related table requested, just process that one. + // (Reply: This feels fishy.) + if (in_array($tableFilter, $relatedTableNames)) { + $relatedTableNames = array($tableFilter); + } + + // If no specific table requested (include all related tables), + // or a specific related table requested (as matched above) + if (empty($tableFilter) || isset($relatedTableNames[$tableFilter])) { + $info[] = array( + 'table' => $relatedTableNames, + 'when' => 'AFTER', + 'event' => array('INSERT', 'UPDATE'), + 'sql' => "\nUPDATE {$this->getTableName()} SET {$this->getModifiedDate()} = CURRENT_TIMESTAMP WHERE id = NEW.$contactRefColumn;\n", + ); + $info[] = array( + 'table' => $relatedTableNames, + 'when' => 'AFTER', + 'event' => array('DELETE'), + 'sql' => "\nUPDATE {$this->getTableName()} SET {$this->getModifiedDate()} = CURRENT_TIMESTAMP WHERE id = OLD.$contactRefColumn;\n", + ); + } + } + + /** + * @return string + */ + public function getTableName() { + return $this->tableName; + } + + /** + * @param string $tableName + * @return TimestampTriggers + */ + public function setTableName($tableName) { + $this->tableName = $tableName; + return $this; + } + + /** + * @return string + */ + public function getCustomDataEntity() { + return $this->customDataEntity; + } + + /** + * @param string $customDataEntity + * @return TimestampTriggers + */ + public function setCustomDataEntity($customDataEntity) { + $this->customDataEntity = $customDataEntity; + return $this; + } + + /** + * @return string + */ + public function getCreatedDate() { + return $this->createdDate; + } + + /** + * @param string $createdDate + * @return TimestampTriggers + */ + public function setCreatedDate($createdDate) { + $this->createdDate = $createdDate; + return $this; + } + + /** + * @return string + */ + public function getModifiedDate() { + return $this->modifiedDate; + } + + /** + * @param string $modifiedDate + * @return TimestampTriggers + */ + public function setModifiedDate($modifiedDate) { + $this->modifiedDate = $modifiedDate; + return $this; + } + + /** + * @return array + * Each item is an array('table' => string, 'column' => string) + */ + public function getRelations() { + return $this->relations; + } + + /** + * @param array $relations + * @return TimestampTriggers + */ + public function setRelations($relations) { + $this->relations = $relations; + return $this; + } + + /** + * Get a list of all tracked relations. + * + * This is basically the curated list (`$this->relations`) plus any custom data. + * + * @return array + * Each item is an array('table' => string, 'column' => string) + */ + public function getAllRelations() { + $relations = $this->getRelations(); + + if ($this->getCustomDataEntity()) { + $customGroupDAO = \CRM_Core_BAO_CustomGroup::getAllCustomGroupsByBaseEntity($this->getCustomDataEntity()); + $customGroupDAO->is_multiple = 0; + $customGroupDAO->find(); + while ($customGroupDAO->fetch()) { + $relations[] = array( + 'table' => $customGroupDAO->table_name, + 'column' => 'entity_id', + ); + } + } + + return $relations; + } + +} From 4421274777264782f0f10c48f622dc838f339654 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 26 Jul 2017 20:40:02 -0700 Subject: [PATCH 04/13] CRM-20958 - Add helper Civi\Core\SqlTrigger\StaticTriggers --- Civi/Core/SqlTrigger/StaticTriggers.php | 127 ++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 Civi/Core/SqlTrigger/StaticTriggers.php diff --git a/Civi/Core/SqlTrigger/StaticTriggers.php b/Civi/Core/SqlTrigger/StaticTriggers.php new file mode 100644 index 000000000000..e4747a8d2b8e --- /dev/null +++ b/Civi/Core/SqlTrigger/StaticTriggers.php @@ -0,0 +1,127 @@ + 'civicrm_case', 'column'=> 'modified_date'); + * + * @see \CRM_Utils_Hook::triggerInfo + */ + private $triggers; + + /** + * StaticTriggers constructor. + * @param $triggers + */ + public function __construct($triggers) { + $this->triggers = $triggers; + } + + + /** + * Add our list of triggers to the global list. + * + * @param \Civi\Core\Event\GenericHookEvent $e + * @see \CRM_Utils_Hook::triggerInfo + */ + public function onTriggerInfo($e) { + $this->alterTriggerInfo($e->info, $e->tableName); + } + + /** + * Add our list of triggers to the global list. + * + * @see \CRM_Utils_Hook::triggerInfo + * @see \CRM_Core_DAO::triggerRebuild + * + * @param array $info + * See hook_civicrm_triggerInfo. + * @param string|NULL $tableFilter + * See hook_civicrm_triggerInfo. + */ + public function alterTriggerInfo(&$info, $tableFilter = NULL) { + foreach ($this->getTriggers() as $trigger) { + if ($tableFilter !== NULL) { + // Because sadism. + if (in_array($tableFilter, (array) $trigger['table'])) { + $trigger['table'] = $tableFilter; + } + } + + if (\CRM_Core_Config::isUpgradeMode() && isset($trigger['upgrade_check'])) { + $uc = $trigger['upgrade_check']; + if (!\CRM_Core_DAO::checkFieldExists($uc['table'], $uc['column']) + ) { + continue; + } + } + unset($trigger['upgrade_check']); + $info[] = $trigger; + } + } + + /** + * @return mixed + */ + public function getTriggers() { + return $this->triggers; + } + + /** + * @param mixed $triggers + * @return StaticTriggers + */ + public function setTriggers($triggers) { + $this->triggers = $triggers; + return $this; + } + + /** + * @param $trigger + * @return StaticTriggers + */ + public function addTrigger($trigger) { + $this->triggers[] = $trigger; + return $this; + } + +} From e0a667ad4d7fc32f479dc71df6278b65bd9db15a Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 26 Jul 2017 18:42:54 -0700 Subject: [PATCH 05/13] CRM-20958 - Create activity and case triggers --- Civi/Core/Container.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index b01595f4cdc8..85f6120460e0 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -208,6 +208,16 @@ public function createContainer() { ->setFactory(array($class, 'singleton')); } + $container->setDefinition('civi.activity.triggers', new Definition( + 'Civi\Core\SqlTrigger\TimestampTriggers', + array('civicrm_activity', 'Activity') + ))->addTag('kernel.event_listener', array('event' => 'hook_civicrm_triggerInfo', 'method' => 'onTriggerInfo')); + + $container->setDefinition('civi.case.triggers', new Definition( + 'Civi\Core\SqlTrigger\TimestampTriggers', + array('civicrm_case', 'Case') + ))->addTag('kernel.event_listener', array('event' => 'hook_civicrm_triggerInfo', 'method' => 'onTriggerInfo')); + $container->setDefinition('civi_token_compat', new Definition( 'Civi\Token\TokenCompatSubscriber', array() From b21ffed7895ea4ba55c09dfdfadd599a78a08eb7 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 26 Jul 2017 20:40:26 -0700 Subject: [PATCH 06/13] CRM-20958 - Create case-activity triggers (to update case when activity changes) --- Civi/Core/Container.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index 85f6120460e0..75d95000178e 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -218,6 +218,29 @@ public function createContainer() { array('civicrm_case', 'Case') ))->addTag('kernel.event_listener', array('event' => 'hook_civicrm_triggerInfo', 'method' => 'onTriggerInfo')); + $container->setDefinition('civi.case.staticTriggers', new Definition( + 'Civi\Core\SqlTrigger\StaticTriggers', + array( + array( + array( + 'upgrade_check' => array('table' => 'civicrm_case', 'column' => 'modified_date'), + 'table' => 'civicrm_case_activity', + 'when' => 'AFTER', + 'event' => array('INSERT'), + 'sql' => "\nUPDATE civicrm_case SET modified_date = CURRENT_TIMESTAMP WHERE id = NEW.case_id;\n", + ), + array( + 'upgrade_check' => array('table' => 'civicrm_case', 'column' => 'modified_date'), + 'table' => 'civicrm_activity', + 'when' => 'BEFORE', + 'event' => array('UPDATE', 'DELETE'), + 'sql' => "\nUPDATE civicrm_case SET modified_date = CURRENT_TIMESTAMP WHERE id IN (SELECT ca.case_id FROM civicrm_case_activity ca WHERE ca.activity_id = OLD.id);\n", + ), + ), + ) + )) + ->addTag('kernel.event_listener', array('event' => 'hook_civicrm_triggerInfo', 'method' => 'onTriggerInfo')); + $container->setDefinition('civi_token_compat', new Definition( 'Civi\Token\TokenCompatSubscriber', array() From 3a03a3f74dd1f71e21a35a8e5aa75cb5e13d9594 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 26 Jul 2017 18:58:43 -0700 Subject: [PATCH 07/13] CRM-20958 - api_v3_ActivityTest::testActivityCreateExample - Ignore creation/modification date. The technique of using hard-coded example record doesn't work with this data. --- tests/phpunit/api/v3/ActivityTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/phpunit/api/v3/ActivityTest.php b/tests/phpunit/api/v3/ActivityTest.php index 98b9b2f8bbc5..b2b0f5523c64 100644 --- a/tests/phpunit/api/v3/ActivityTest.php +++ b/tests/phpunit/api/v3/ActivityTest.php @@ -447,6 +447,11 @@ public function testActivityCreateExample() { require_once 'api/v3/examples/Activity/Create.php'; $result = activity_create_example(); $expectedResult = activity_create_expectedresult(); + // Compare everything *except* timestamps. + unset($result['values'][1]['created_date']); + unset($result['values'][1]['modified_date']); + unset($expectedResult['values'][1]['created_date']); + unset($expectedResult['values'][1]['modified_date']); $this->assertEquals($result, $expectedResult); } From 2c6b0b4b53c5656d161a02f36ee7c40b1ca9150f Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 27 Jul 2017 21:09:26 -0700 Subject: [PATCH 08/13] CRM-20958 - api_v3_CaseTest - Add test for timestamp management --- tests/phpunit/api/v3/CaseTest.php | 48 +++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/phpunit/api/v3/CaseTest.php b/tests/phpunit/api/v3/CaseTest.php index 72ea596cf6cf..a1820df11ed5 100644 --- a/tests/phpunit/api/v3/CaseTest.php +++ b/tests/phpunit/api/v3/CaseTest.php @@ -849,4 +849,52 @@ public function caseActivityRevisionExamples() { return $examples; } + public function testTimestamps() { + $params = $this->_params; + $case_created = $this->callAPISuccess('case', 'create', $params); + + $case_1 = $this->callAPISuccess('Case', 'getsingle', array( + 'id' => $case_created['id'], + )); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $case_1['created_date']); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $case_1['modified_date']); + $this->assertApproxEquals(strtotime($case_1['created_date']), strtotime($case_1['modified_date']), 2); + + $activity_1 = $this->callAPISuccess('activity', 'getsingle', array( + 'case_id' => $case_created['id'], + 'options' => array( + 'limit' => 1, + ), + )); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $activity_1['created_date']); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $activity_1['modified_date']); + $this->assertApproxEquals(strtotime($activity_1['created_date']), strtotime($activity_1['modified_date']), 2); + + usleep(1.5 * 1000000); + $this->callAPISuccess('activity', 'create', array( + 'id' => $activity_1['id'], + 'subject' => 'Make cheese', + )); + + $activity_2 = $this->callAPISuccess('activity', 'getsingle', array( + 'id' => $activity_1['id'], + )); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $activity_2['created_date']); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $activity_2['modified_date']); + $this->assertNotEquals($activity_2['created_date'], $activity_2['modified_date']); + + $this->assertEquals($activity_1['created_date'], $activity_2['created_date']); + $this->assertNotEquals($activity_1['modified_date'], $activity_2['modified_date']); + $this->assertLessThan($activity_2['modified_date'], $activity_1['modified_date'], + sprintf("Original modification time (%s) should predate later modification time (%s)", $activity_1['modified_date'], $activity_2['modified_date'])); + + $case_2 = $this->callAPISuccess('Case', 'getsingle', array( + 'id' => $case_created['id'], + )); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $case_2['created_date']); + $this->assertRegExp(';^\d\d\d\d-\d\d-\d\d \d\d:\d\d;', $case_2['modified_date']); + $this->assertEquals($case_1['created_date'], $case_2['created_date']); + $this->assertNotEquals($case_2['created_date'], $case_2['modified_date']); + } + } From ea6a17a90e231f4347c5bb5044f8dcf3500b7d70 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 27 Jul 2017 21:10:30 -0700 Subject: [PATCH 09/13] CRM-20958 - Case, Activity BAOs - Watch out for saving stale timestamps There appears to be some application logic which follows a process like this: 1. Read the case 2. Tweak the data 3. Save updated case The problem is comes if step #4 resaves a timestamp loaded in step #1, which is fairly likely to happen if you read+save the same record. This was specifically observed on the "Manage Case" screen when editing activities, but the data-flow is pretty common, so make a general fix to the BAO. --- CRM/Activity/BAO/Activity.php | 4 ++++ CRM/Case/BAO/Case.php | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 2f4386695d1b..ea9bd39efd1a 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -285,6 +285,10 @@ public static function deleteActivityContact($activityId, $recordTypeID = NULL) * @return CRM_Activity_BAO_Activity|null|object */ public static function create(&$params) { + // CRM-20958 - These fields are managed by MySQL triggers. Watch out for clients resaving stale timestamps. + unset($params['created_date']); + unset($params['modified_date']); + // check required params if (!self::dataExists($params)) { throw new CRM_Core_Exception('Not enough data to create activity object'); diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php index 13da2d59419d..89c77bd94f6b 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -89,6 +89,10 @@ public static function add(&$params) { * @return CRM_Case_BAO_Case */ public static function &create(&$params) { + // CRM-20958 - These fields are managed by MySQL triggers. Watch out for clients resaving stale timestamps. + unset($params['created_date']); + unset($params['modified_date']); + $transaction = new CRM_Core_Transaction(); if (!empty($params['id'])) { From c7006a4b6964c1b8e8ee34f467f07558a652452c Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 17 Aug 2017 21:45:57 -0700 Subject: [PATCH 10/13] CRM-20958 - Health check - If NULL timestamps are still around, show link to DoctorWhen --- CRM/Utils/Check/Component/Case.php | 46 ++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/CRM/Utils/Check/Component/Case.php b/CRM/Utils/Check/Component/Case.php index 2a7199a40b24..f6f5a9eb1199 100644 --- a/CRM/Utils/Check/Component/Case.php +++ b/CRM/Utils/Check/Component/Case.php @@ -32,6 +32,8 @@ */ class CRM_Utils_Check_Component_Case extends CRM_Utils_Check_Component { + const DOCTOR_WHEN = 'https://github.com/civicrm/org.civicrm.doctorwhen'; + /** * @var CRM_Case_XMLRepository */ @@ -116,4 +118,48 @@ public function checkCaseTypeNameConsistency() { return $messages; } + /** + * Check that the timestamp columns are populated. (CRM-20958) + * + * @return array + * An empty array, or a list of warnings + */ + public function checkNullTimestamps() { + $messages = array(); + + $nullCount = 0; + $nullCount += CRM_Utils_SQL_Select::from('civicrm_activity') + ->where('created_date IS NULL OR modified_date IS NULL') + ->select('COUNT(*)') + ->execute() + ->fetchValue(); + $nullCount += CRM_Utils_SQL_Select::from('civicrm_case') + ->where('created_date IS NULL OR modified_date IS NULL') + ->select('COUNT(*)') + ->execute() + ->fetchValue(); + + if ($nullCount > 0) { + $messages[] = new CRM_Utils_Check_Message( + __FUNCTION__, + '

' . + ts('The tables "civicrm_activity" and "civicrm_case" were updated to support two new fields, "created_date" and "modified_date". For historical data, these fields may appear blank. (%1 records have NULL timestamps.)', array( + 1 => $nullCount, + )) . + '

' . + ts('At time of writing, this is not a problem. However, future extensions and improvements could rely on these fields, so it may be useful to back-fill them.') . + '

' . + ts('For further discussion, please visit %1', array( + 1 => sprintf('%s', self::DOCTOR_WHEN, self::DOCTOR_WHEN), + )) . + '

', + ts('Timestamps for Activities and Cases'), + \Psr\Log\LogLevel::NOTICE, + 'fa-clock-o' + ); + } + + return $messages; + } + } From 32c1003dcb4ad6652e16946e51504e9ee81e96c4 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 31 Jul 2017 19:55:18 -0700 Subject: [PATCH 11/13] CRM-20958 - api_v3_CaseTest::testCaseUpdate - Ignore creation/modification date. Checking the `modified_date` is a bit racy -- depending on sub-second performance/alignment, the original `Case` creation and the subsequent `Case` update may have the same `modified_date` or may have different `modified_date`. --- tests/phpunit/api/v3/CaseTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/api/v3/CaseTest.php b/tests/phpunit/api/v3/CaseTest.php index a1820df11ed5..0d429bd2bbf0 100644 --- a/tests/phpunit/api/v3/CaseTest.php +++ b/tests/phpunit/api/v3/CaseTest.php @@ -193,8 +193,13 @@ public function testCaseUpdate() { $params['subject'] = $case['subject'] = 'Something Else'; $this->callAPISuccess('case', 'create', $params); - // Verify that updated case is exactly equal to the original with new subject. + // Verify that updated case is equal to the original with new subject. $result = $this->callAPISuccessGetSingle('Case', array('case_id' => $id)); + // Modification dates are likely to differ by 0-2 sec. Check manually. + $this->assertTrue($result['modified_date'] >= $case['modified_date']); + unset($result['modified_date']); + unset($case['modified_date']); + // Everything else should be identical. $this->assertAPIArrayComparison($result, $case); } From 4ccc3cddbc1f14321c98feafa7b84d1b0f14997e Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 2 Aug 2017 14:19:00 -0700 Subject: [PATCH 12/13] CRM-20958 - TimestampTriggers - Update docblock --- Civi/Core/SqlTrigger/TimestampTriggers.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Civi/Core/SqlTrigger/TimestampTriggers.php b/Civi/Core/SqlTrigger/TimestampTriggers.php index e204332b772e..60c35a9c224e 100644 --- a/Civi/Core/SqlTrigger/TimestampTriggers.php +++ b/Civi/Core/SqlTrigger/TimestampTriggers.php @@ -141,18 +141,15 @@ public function onTriggerInfo($e) { * See hook_civicrm_triggerInfo. */ public function alterTriggerInfo(&$info, $tableFilter = NULL) { - //during upgrade, first check for valid version and then create triggers - //i.e the columns created_date and modified_date are introduced in 4.3.alpha1 so dont create triggers for older version + // If we haven't upgraded yet, then the created_date/modified_date may not exist. + // In the past, this was a version-based check, but checkFieldExists() + // seems more robust. if (\CRM_Core_Config::isUpgradeMode()) { if (!\CRM_Core_DAO::checkFieldExists($this->getTableName(), $this->getCreatedDate()) ) { return; } - // $currentVer = CRM_Core_BAO_Domain::version(TRUE); - // if (version_compare($currentVer, '4.3.alpha1') < 0) { - // return; - // } } if ($tableFilter == NULL || $tableFilter == $this->getTableName()) { From ed30b24910cecea3d9c7e152d96ff44a5ac3cd92 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 3 Aug 2017 15:15:28 -0700 Subject: [PATCH 13/13] CRM-20958 - api_v3_CaseTest::testCaseUpdate - Style tweak --- tests/phpunit/api/v3/CaseTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/api/v3/CaseTest.php b/tests/phpunit/api/v3/CaseTest.php index 0d429bd2bbf0..9330a6958e94 100644 --- a/tests/phpunit/api/v3/CaseTest.php +++ b/tests/phpunit/api/v3/CaseTest.php @@ -196,7 +196,7 @@ public function testCaseUpdate() { // Verify that updated case is equal to the original with new subject. $result = $this->callAPISuccessGetSingle('Case', array('case_id' => $id)); // Modification dates are likely to differ by 0-2 sec. Check manually. - $this->assertTrue($result['modified_date'] >= $case['modified_date']); + $this->assertGreaterThanOrEqual($result['modified_date'], $case['modified_date']); unset($result['modified_date']); unset($case['modified_date']); // Everything else should be identical.