Fix bug where log hook is ignored on custom field create

It is possible to specify that various tables should not
result in a log table via the `alterLogTables` hook.

This is useful for excluding tables of low information
value or tables like summary fields that are calculated
values. However, when altering custom fields this
hook's output was being ignored as it was going through
the specific path of
which did not have awareness of this hook.

This code path is the primary use for this function.

It's also called via createMissingLogTables - but in
that case the calculation is already done. It is not
elsewhere in git universe and the return value
is never used, so I removed it

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index 73944a1e7280881e155d8ace80112d268c39a14a..c5421a604ba5cd70ddb33b36d5c2a1662d76b920 100644 (file)
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -134,9 +134,8 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
    * @param array $records
    * @return CRM_Core_DAO_CustomField[]
    * @throws CRM_Core_Exception
-   * @throws CiviCRM_API3_Exception
    */
-  public static function writeRecords(array $records) {
+  public static function writeRecords(array $records): array {
     $addedColumns = $sql = $customFields = $pre = $post = [];
     foreach ($records as $index => $params) {
       CRM_Utils_Hook::pre(empty($params['id']) ? 'create' : 'edit', 'CustomField', $params['id'] ?? NULL, $params);

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 774b3548b60e7997fe85c759ebf4a46b728c03bd..2a375fbca14d621081e35ac2f5f895258d39d1c7 100644 (file)
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -932,7 +932,7 @@ class CRM_Core_DAO extends DB_DataObject {
    * @return static[]
    * @throws CRM_Core_Exception
    */
-  public static function writeRecords(array $records) {
+  public static function writeRecords(array $records): array {
     $results = [];
     foreach ($records as $record) {
       $results[] = static::writeRecord($record);

diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php
index ad09e3a32c20431bbc7266fee8c543f441424377..9d09e62d20fd7c76b0ba65791a4e5163d4276002 100644 (file)
--- a/CRM/Logging/Schema.php
+++ b/CRM/Logging/Schema.php
@@ -429,16 +429,18 @@ AND    (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString )
    *   name of the relevant table.
    * @param array $cols
    *   Mixed array of columns to add or null (to check for the missing columns).
-   *
-   * @return bool
    */
-  public function fixSchemaDifferencesFor($table, $cols = []) {
-    if (empty($table)) {
-      return FALSE;
+  public function fixSchemaDifferencesFor(string $table, array $cols = []): void {
+    if (!in_array($table, $this->tables, TRUE)) {
+      // Create the table if the log table does not exist and
+      // the table is in 'this->tables'. This latter array
+      // could have been altered by a hook if the site does not
+      // want to log a specific table.
+      return;
     }
     if (empty($this->logs[$table])) {
       $this->createLogTableFor($table);
-      return TRUE;
+      return;
     }
 
     if (empty($cols)) {
@@ -480,8 +482,6 @@ AND    (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString )
     }
 
     $this->resetSchemaCacheForTable("log_$table");
-
-    return TRUE;
   }
 
   /**

diff --git a/tests/phpunit/CRM/Logging/LoggingTest.php b/tests/phpunit/CRM/Logging/LoggingTest.php
index 050fce542a3d83edddb02a9c563186f94488c4b6..58ba85a0021b8a605c9f11f15f2175b2565a992d 100644 (file)
--- a/tests/phpunit/CRM/Logging/LoggingTest.php
+++ b/tests/phpunit/CRM/Logging/LoggingTest.php
@@ -38,6 +38,28 @@ class CRM_Logging_LoggingTest extends CiviUnitTestCase {
     $this->assertNotEmpty(CRM_Core_DAO::singleValueQuery("SHOW tables LIKE 'log_abcd'"));
   }
 
+  /**
+   * Test that hooks removing tables from logging are respected during custom field add.
+   *
+   * During custom field save logging is only handled for the affected table.
+   * We need to make sure this respects hooks to remove from the logging set.
+   */
+  public function testLoggingHookIgnore(): void {
+    $this->hookClass->setHook('civicrm_alterLogTables', [$this, 'ignoreSillyName']);
+    Civi::settings()->set('logging', TRUE);
+    $this->createCustomGroupWithFieldOfType(['table_name' => 'silly_name']);
+    $this->assertEmpty(CRM_Core_DAO::singleValueQuery("SHOW tables LIKE 'log_silly_name'"));
+  }
+
+  /**
+   * Implement hook to cause our log table to be ignored.
+   *
+   * @param array $logTableSpec
+   */
+  public function ignoreSillyName(array &$logTableSpec): void {
+    unset($logTableSpec['silly_name']);
+  }
+
   /**
    * Test creating logging schema when database is in multilingual mode.
    */