From 99008b080be228edef23cd0319d449ae4d63159c Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 6 Apr 2016 10:26:48 +1200 Subject: [PATCH] CRM-18193 Make log_date optional for revert / retrieving changes where uniqueID is in play --- CRM/Logging/Differ.php | 61 ++++++++++++-- api/v3/Logging.php | 2 +- api/v3/examples/Logging/Get.php | 115 +++++++++++++-------------- api/v3/examples/Logging/Revert.php | 2 +- tests/phpunit/api/v3/LoggingTest.php | 62 ++++++++++++++- 5 files changed, 174 insertions(+), 68 deletions(-) diff --git a/CRM/Logging/Differ.php b/CRM/Logging/Differ.php index ffac073531..e09eb40532 100644 --- a/CRM/Logging/Differ.php +++ b/CRM/Logging/Differ.php @@ -80,7 +80,6 @@ class CRM_Logging_Differ { $params = array( 1 => array($this->log_conn_id, 'String'), - 2 => array($this->log_date, 'String'), ); $logging = new CRM_Logging_Schema(); @@ -142,14 +141,21 @@ LEFT JOIN civicrm_activity_contact source ON source.activity_id = lt.id AND sour } } + $logDateClause = ''; + if ($this->log_date) { + $params[2] = array($this->log_date, 'String'); + $logDateClause = " + AND lt.log_date BETWEEN DATE_SUB(%2, INTERVAL {$this->interval}) AND DATE_ADD(%2, INTERVAL {$this->interval}) + "; + } + // find ids in this table that were affected in the given connection (based on connection id and a ±10 s time period around the date) $sql = " SELECT DISTINCT lt.id FROM `{$this->db}`.`log_$table` lt {$join} -WHERE lt.log_conn_id = %1 AND - lt.log_date BETWEEN DATE_SUB(%2, INTERVAL {$this->interval}) AND DATE_ADD(%2, INTERVAL {$this->interval}) - {$contactIdClause}"; - +WHERE lt.log_conn_id = %1 + $logDateClause + {$contactIdClause}"; $dao = CRM_Core_DAO::executeQuery($sql, $params); while ($dao->fetch()) { $diffs = array_merge($diffs, $this->diffsInTableForId($table, $dao->id)); @@ -169,15 +175,23 @@ WHERE lt.log_conn_id = %1 AND $params = array( 1 => array($this->log_conn_id, 'String'), - 2 => array($this->log_date, 'String'), 3 => array($id, 'Integer'), ); // look for all the changes in the given connection that happened less than {$this->interval} s later than log_date to the given id to catch multi-query changes - $changedSQL = "SELECT * FROM `{$this->db}`.`log_$table` WHERE log_conn_id = %1 AND log_date >= %2 AND log_date < DATE_ADD(%2, INTERVAL {$this->interval}) AND id = %3 ORDER BY log_date DESC LIMIT 1"; + $logDateClause = ""; + if ($this->log_date && $this->interval) { + $logDateClause = " AND log_date >= %2 AND log_date < DATE_ADD(%2, INTERVAL {$this->interval})"; + $params[2] = array($this->log_date, 'String'); + } + + $changedSQL = "SELECT * FROM `{$this->db}`.`log_$table` WHERE log_conn_id = %1 $logDateClause AND id = %3 ORDER BY log_date DESC LIMIT 1"; $changedDAO = CRM_Core_DAO::executeQuery($changedSQL, $params); while ($changedDAO->fetch()) { + if (empty($this->log_date) && !$this->checkLogCanBeUsedWithNoLogDate($changedDAO->log_date)) { + throw new CRM_Core_Exception('The connection date must be passed in to disambiguate this logging entry per CRM-18193'); + } $changed = $changedDAO->toArray(); // return early if nothing found @@ -201,6 +215,7 @@ WHERE lt.log_conn_id = %1 AND break; case 'Update': + $params[2] = array($changedDAO->log_date, 'String'); // look for the previous state (different log_conn_id) of the given id $originalSQL = "SELECT * FROM `{$this->db}`.`log_$table` WHERE log_conn_id != %1 AND log_date < %2 AND id = %3 ORDER BY log_date DESC LIMIT 1"; $original = $this->sqlToArray($originalSQL, $params); @@ -420,4 +435,36 @@ ORDER BY log_date return $diffs; } + /** + * Check that the log record relates to a unique log id. + * + * If the record was recorded using the old non-unique style then the + * log_date + * MUST be set to get the (fairly accurate) list of changes. In this case the + * nasty 10 second interval rule is applied. + * + * See CRM-18193 for a discussion of unique log id. + * + * @param string $change_date + * + * @return bool + * @throws \CiviCRM_API3_Exception + */ + protected function checkLogCanBeUsedWithNoLogDate($change_date) { + if (civicrm_api3('Setting', 'getvalue', array('name' => 'logging_all_tables_uniquid', 'group' => 'CiviCRM Preferences'))) { + return TRUE; + }; + $uniqueDate = civicrm_api3('Setting', 'getvalue', array( + 'name' => 'logging_uniqueid_date', + 'group' => 'CiviCRM Preferences', + )); + if (strtotime($uniqueDate) <= strtotime($change_date)) { + return TRUE; + } + else { + return FALSE; + } + + } + } diff --git a/api/v3/Logging.php b/api/v3/Logging.php index f3418a639d..32bab67c39 100644 --- a/api/v3/Logging.php +++ b/api/v3/Logging.php @@ -43,7 +43,7 @@ */ function civicrm_api3_logging_revert($params) { $schema = new CRM_Logging_Schema(); - $reverter = new CRM_Logging_Reverter($params['log_conn_id'], $params['log_date']); + $reverter = new CRM_Logging_Reverter($params['log_conn_id'], CRM_Utils_Array::value('log_date', $params)); $reverter->calculateDiffsFromLogConnAndDate($schema->getLogTablesForContact()); $reverter->revert(); return civicrm_api3_create_success(1); diff --git a/api/v3/examples/Logging/Get.php b/api/v3/examples/Logging/Get.php index 3e181ae209..6e904dee4d 100644 --- a/api/v3/examples/Logging/Get.php +++ b/api/v3/examples/Logging/Get.php @@ -7,8 +7,7 @@ */ function logging_get_example() { $params = array( - 'log_conn_id' => 'wooty woot', - 'log_date' => '2016-04-06 00:08:23', + 'log_conn_id' => 'wooty wop wop', ); try{ @@ -44,173 +43,173 @@ function logging_get_expectedresult() { 'values' => array( '0' => array( 'action' => 'Update', - 'id' => '3', + 'id' => '9', 'field' => 'sort_name', 'from' => 'Anderson, Anthony', 'to' => 'Dwarf, Dopey', 'table' => 'civicrm_contact', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '1' => array( 'action' => 'Update', - 'id' => '3', + 'id' => '9', 'field' => 'display_name', 'from' => 'Mr. Anthony Anderson II', 'to' => 'Mr. Dopey Dwarf II', 'table' => 'civicrm_contact', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '2' => array( 'action' => 'Update', - 'id' => '3', + 'id' => '9', 'field' => 'first_name', 'from' => 'Anthony', 'to' => 'Dopey', 'table' => 'civicrm_contact', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '3' => array( 'action' => 'Update', - 'id' => '3', + 'id' => '9', 'field' => 'last_name', 'from' => 'Anderson', 'to' => 'Dwarf', 'table' => 'civicrm_contact', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '4' => array( 'action' => 'Update', - 'id' => '3', + 'id' => '9', 'field' => 'modified_date', - 'from' => '2016-04-06 00:08:06', - 'to' => '2016-04-06 00:08:23', + 'from' => '2016-04-06 02:53:27', + 'to' => '2016-04-06 02:53:44', 'table' => 'civicrm_contact', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '5' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'id', 'from' => '', - 'to' => '4', + 'to' => '2', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '6' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'contact_id', 'from' => '', - 'to' => '3', + 'to' => '9', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '7' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'location_type_id', 'from' => '', 'to' => '', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '8' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'email', 'from' => '', 'to' => 'dopey@mail.com', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '9' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'is_primary', 'from' => '', 'to' => 0, 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '10' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'is_billing', 'from' => '', 'to' => 0, 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '11' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'on_hold', 'from' => '', 'to' => 0, 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '12' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'is_bulkmail', 'from' => '', 'to' => 0, 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '13' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'hold_date', 'from' => '', 'to' => '', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '14' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'reset_date', 'from' => '', 'to' => '', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '15' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'signature_text', 'from' => '', 'to' => '', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), '16' => array( 'action' => 'Insert', - 'id' => '4', + 'id' => '2', 'field' => 'signature_html', 'from' => '', 'to' => '', 'table' => 'civicrm_email', - 'log_date' => '2016-04-06 00:08:23', - 'log_conn_id' => 'wooty woot', + 'log_date' => '2016-04-06 02:53:44', + 'log_conn_id' => 'wooty wop wop', ), ), ); @@ -220,7 +219,7 @@ function logging_get_expectedresult() { /* * This example has been generated from the API test suite. -* The test that created it is called "testGet" +* The test that created it is called "testGetNoDate" * and can be found at: * https://github.com/civicrm/civicrm-core/blob/master/tests/phpunit/api/v3/LoggingTest.php * diff --git a/api/v3/examples/Logging/Revert.php b/api/v3/examples/Logging/Revert.php index fcdf924195..88a5514e23 100644 --- a/api/v3/examples/Logging/Revert.php +++ b/api/v3/examples/Logging/Revert.php @@ -8,7 +8,7 @@ function logging_revert_example() { $params = array( 'log_conn_id' => 'woot', - 'log_date' => '2016-04-05 23:52:59', + 'log_date' => '2016-04-06 02:52:07', ); try{ diff --git a/tests/phpunit/api/v3/LoggingTest.php b/tests/phpunit/api/v3/LoggingTest.php index 8056dc8d88..e7ea73c091 100644 --- a/tests/phpunit/api/v3/LoggingTest.php +++ b/tests/phpunit/api/v3/LoggingTest.php @@ -246,6 +246,46 @@ class api_v3_LoggingTest extends CiviUnitTestCase { $this->callAPISuccessGetCount('Email', array('id' => $email['id']), 0); } + /** + * Test changes can be reverted. + */ + public function testRevertNoDate() { + $contactId = $this->individualCreate(); + $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); + CRM_Core_DAO::executeQuery("SET @uniqueID = 'Wot woot'"); + $this->callAPISuccess('Contact', 'create', array( + 'id' => $contactId, + 'first_name' => 'Dopey', + 'api.email.create' => array('email' => 'dopey@mail.com')) + ); + $email = $this->callAPISuccessGetSingle('email', array('email' => 'dopey@mail.com')); + $this->callAPISuccess('Logging', 'revert', array('log_conn_id' => 'Wot woot')); + $this->assertEquals('Anthony', $this->callAPISuccessGetValue('contact', array('id' => $contactId, 'return' => 'first_name'))); + $this->callAPISuccessGetCount('Email', array('id' => $email['id']), 0); + } + + /** + * Test changes can be reverted. + */ + public function testRevertNoDateNotUnique() { + $contactId = $this->individualCreate(); + $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); + CRM_Core_DAO::executeQuery("SET @uniqueID = 'Wopity woot'"); + $this->callAPISuccess('Contact', 'create', array( + 'id' => $contactId, + 'first_name' => 'Dopey', + 'api.email.create' => array('email' => 'dopey@mail.com')) + ); + $this->callAPISuccess('Setting', 'create', array('logging_all_tables_uniquid' => FALSE)); + $this->callAPISuccess('Setting', 'create', array('logging_uniqueid_date' => date('Y-m-d H:i:s'))); + $this->callAPIFailure( + 'Logging', + 'revert', + array('log_conn_id' => 'Wopity woot'), + 'Failure in api call for Logging revert: The connection date must be passed in to disambiguate this logging entry per CRM-18193' + ); + } + /** * Test changes can be retrieved. */ @@ -261,7 +301,27 @@ class api_v3_LoggingTest extends CiviUnitTestCase { 'api.email.create' => array('email' => 'dopey@mail.com')) ); $this->callAPISuccessGetSingle('email', array('email' => 'dopey@mail.com')); - $diffs = $this->callAPIAndDocument('Logging', 'get', array('log_conn_id' => 'wooty woot', 'log_date' => $timeStamp), __FUNCTION__, __FILE__); + $diffs = $this->callAPISuccess('Logging', 'get', array('log_conn_id' => 'wooty woot', 'log_date' => $timeStamp), __FUNCTION__, __FILE__); + $this->assertLoggingIncludes($diffs['values'], array('to' => 'Dwarf, Dopey')); + $this->assertLoggingIncludes($diffs['values'], array('to' => 'Mr. Dopey Dwarf II', 'table' => 'civicrm_contact', 'action' => 'Update', 'field' => 'display_name')); + $this->assertLoggingIncludes($diffs['values'], array('to' => 'dopey@mail.com', 'table' => 'civicrm_email', 'action' => 'Insert', 'field' => 'email')); + } + + /** + * Test changes can be retrieved without log_date being required. + */ + public function testGetNoDate() { + $contactId = $this->individualCreate(); + $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); + CRM_Core_DAO::executeQuery("SET @uniqueID = 'wooty wop wop'"); + $this->callAPISuccess('Contact', 'create', array( + 'id' => $contactId, + 'first_name' => 'Dopey', + 'last_name' => 'Dwarf', + 'api.email.create' => array('email' => 'dopey@mail.com')) + ); + $this->callAPISuccessGetSingle('email', array('email' => 'dopey@mail.com')); + $diffs = $this->callAPIAndDocument('Logging', 'get', array('log_conn_id' => 'wooty wop wop'), __FUNCTION__, __FILE__); $this->assertLoggingIncludes($diffs['values'], array('to' => 'Dwarf, Dopey')); $this->assertLoggingIncludes($diffs['values'], array('to' => 'Mr. Dopey Dwarf II', 'table' => 'civicrm_contact', 'action' => 'Update', 'field' => 'display_name')); $this->assertLoggingIncludes($diffs['values'], array('to' => 'dopey@mail.com', 'table' => 'civicrm_email', 'action' => 'Insert', 'field' => 'email')); -- 2.25.1