CRM/Logging - Fix log table exceptions

This fixes a bug that caused columns that were excluded from being
considered "log-worthy" changes to be logged anyway. That happened
because columns were extracted with backticks but compared to strings
without backticks. To preserve compatibility with exceptions set by
alterLogTables which could contain backticks, the comparison is
performed against the column name with and without backticks.

diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php
index daf6df9a050b0f4e2a1fb472cd5372e91c71b3ec..1e259ed0e7b14cc356147aa3eac651c3bbf02173 100644 (file)
--- a/CRM/Logging/Schema.php
+++ b/CRM/Logging/Schema.php
@@ -858,7 +858,11 @@ COLS;
       foreach ($columns as $column) {
         $tableExceptions = array_key_exists('exceptions', $this->logTableSpec[$table]) ? $this->logTableSpec[$table]['exceptions'] : array();
         // ignore modified_date changes
-        if ($column != 'modified_date' && !in_array($column, $tableExceptions)) {
+        $tableExceptions[] = 'modified_date';
+        // exceptions may be provided with or without backticks
+        $excludeColumn = in_array($column, $tableExceptions) ||
+          in_array(str_replace('`', '', $column), $tableExceptions);
+        if (!$excludeColumn) {
           $cond[] = "IFNULL(OLD.$column,'') <> IFNULL(NEW.$column,'')";
         }
       }

diff --git a/tests/phpunit/CRM/Logging/SchemaTest.php b/tests/phpunit/CRM/Logging/SchemaTest.php
index 2b238077e13d2f364366e935f8425535ed554711..948c61704b6d5ad519b37f7199f3e85a42492b5c 100644 (file)
--- a/tests/phpunit/CRM/Logging/SchemaTest.php
+++ b/tests/phpunit/CRM/Logging/SchemaTest.php
@@ -100,4 +100,27 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase {
     $this->assertTrue(empty($diffs['OBSOLETE']));
   }
 
+  /**
+   * Test logging trigger definition
+   */
+  public function testTriggerInfo() {
+    $info = [];
+    $schema = new CRM_Logging_Schema();
+    $schema->enableLogging();
+    $schema->triggerInfo($info, 'civicrm_group');
+    // should have 3 triggers (insert/update/delete)
+    $this->assertCount(3, $info);
+    foreach ($info as $trigger) {
+      // table for trigger should be civicrm_group
+      $this->assertEquals('civicrm_group', $trigger['table'][0]);
+      if ($trigger['event'][0] == 'UPDATE') {
+        // civicrm_group.cache_date should be an exception, i.e. not logged
+        $this->assertNotContains(
+          "IFNULL(OLD.`cache_date`,'') <> IFNULL(NEW.`cache_date`,'')",
+          $trigger['sql']
+        );
+      }
+    }
+  }
+
 }