Fix trigger rebuild to avoid deprecated function
authorEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 1 Jun 2021 05:43:44 +0000 (17:43 +1200)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 1 Jun 2021 08:02:25 +0000 (20:02 +1200)
[Ref] Remove function parameter only used from test

The test passes a function parameter to rebuild triggers but the only other
call to this function in the civi-verse doesn't. We shouldn't make this function
more complex for just the test.

Note the function also uses the force param which I didn't move to the
test call - it's possible when the tests all run together I'll need
to force cache clearing but this seems like a dumb way

CRM/Logging/Schema.php
tests/phpunit/api/v3/LoggingTest.php

index 6fc36509a4b968db3727143e462775f55d98c714..4c6cb192b9a1b5a3d916bc6cacce6012e50b6ca8 100644 (file)
@@ -518,10 +518,8 @@ AND    (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString )
 
   /**
    * Fix schema differences.
-   *
-   * @param bool $rebuildTrigger
    */
-  public function fixSchemaDifferencesForAll($rebuildTrigger = FALSE) {
+  public function fixSchemaDifferencesForAll(): void {
     $diffs = [];
     $this->resetTableColumnsCache();
 
@@ -537,10 +535,6 @@ AND    (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString )
     foreach ($diffs as $table => $cols) {
       $this->fixSchemaDifferencesFor($table, $cols);
     }
-    if ($rebuildTrigger) {
-      // invoke the meta trigger creation call
-      CRM_Core_DAO::triggerRebuild(NULL, TRUE);
-    }
   }
 
   /**
index 5e467d118dcb43893aa87bea2c75298f96637986..0b0f47894f3a34800f9f54a2745518ece453ab1e 100644 (file)
@@ -29,6 +29,10 @@ class api_v3_LoggingTest extends CiviUnitTestCase {
 
   /**
    * Clean up log tables.
+   *
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
   protected function tearDown(): void {
     $this->quickCleanup(['civicrm_email', 'civicrm_address']);
@@ -41,8 +45,10 @@ class api_v3_LoggingTest extends CiviUnitTestCase {
 
   /**
    * Test that logging is successfully enabled and disabled.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testEnableDisableLogging() {
+  public function testEnableDisableLogging(): void {
     $this->assertEquals(0, $this->callAPISuccessGetValue('Setting', ['name' => 'logging']));
     $this->assertLoggingEnabled(FALSE);
 
@@ -150,16 +156,20 @@ class api_v3_LoggingTest extends CiviUnitTestCase {
 
   /**
    * Check that if a field is added then the trigger is updated on refresh.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testRebuildTriggerAfterSchemaChange() {
+  public function testRebuildTriggerAfterSchemaChange(): void {
     $this->callAPISuccess('Setting', 'create', ['logging' => TRUE]);
     $tables = ['civicrm_acl', 'civicrm_website'];
     foreach ($tables as $table) {
       CRM_Core_DAO::executeQuery("ALTER TABLE $table ADD column temp_col INT(10)");
     }
+    unset(\Civi::$statics['CRM_Logging_Schema']);
 
     $schema = new CRM_Logging_Schema();
-    $schema->fixSchemaDifferencesForAll(TRUE);
+    $schema->fixSchemaDifferencesForAll();
+    Civi::service('sql_triggers')->rebuild();
 
     foreach ($tables as $table) {
       $this->assertTrue($this->checkColumnExistsInTable('log_' . $table, 'temp_col'), 'log_' . $table . ' has temp_col');
@@ -168,8 +178,8 @@ class api_v3_LoggingTest extends CiviUnitTestCase {
         $this->assertStringContainsString('temp_col', $dao->Statement);
       }
     }
-    CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_acl DROP column temp_col");
-    CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_website DROP column temp_col");
+    CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_acl DROP column temp_col');
+    CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_website DROP column temp_col');
   }
 
   /**