Skip to content

Commit

Permalink
Fix trigger rebuild to avoid deprecated function
Browse files Browse the repository at this point in the history
[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
  • Loading branch information
eileenmcnaughton committed Jun 1, 2021
1 parent 3f27686 commit 753bc83
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
8 changes: 1 addition & 7 deletions CRM/Logging/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -518,10 +518,8 @@ private function _getColumnQuery($col, $createQuery) {

/**
* Fix schema differences.
*
* @param bool $rebuildTrigger
*/
public function fixSchemaDifferencesForAll($rebuildTrigger = FALSE) {
public function fixSchemaDifferencesForAll(): void {
$diffs = [];
$this->resetTableColumnsCache();

Expand All @@ -537,10 +535,6 @@ public function fixSchemaDifferencesForAll($rebuildTrigger = FALSE) {
foreach ($diffs as $table => $cols) {
$this->fixSchemaDifferencesFor($table, $cols);
}
if ($rebuildTrigger) {
// invoke the meta trigger creation call
CRM_Core_DAO::triggerRebuild(NULL, TRUE);
}
}

/**
Expand Down
20 changes: 15 additions & 5 deletions tests/phpunit/api/v3/LoggingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ protected function setUp(): void {

/**
* 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']);
Expand All @@ -41,8 +45,10 @@ protected function tearDown(): void {

/**
* 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);

Expand Down Expand Up @@ -150,16 +156,20 @@ public function testUpdateLogTableHookINNODB() {

/**
* 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');
Expand All @@ -168,8 +178,8 @@ public function testRebuildTriggerAfterSchemaChange() {
$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');
}

/**
Expand Down

0 comments on commit 753bc83

Please sign in to comment.