Fix failing settingTest and mark setting.fill as deprecated

The settingTest class was being too aggressive about creating and deleting domains, this teaches it to not delete pre-existing domains. Also marks an old unused api function deprecated.
colemanw · Jul 3, 2020 · 22a66e6 · 22a66e6
1 parent 2ca7273
commit 22a66e6
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 26 deletions.
diff --git a/api/v3/Setting.php b/api/v3/Setting.php
@@ -195,7 +195,7 @@ function _civicrm_api3_setting_revert_spec(&$params) {
  * Revert settings to defaults.
  *
  * @param array $params
- *
+ * @deprecated
  * @return array
  * @throws \CiviCRM_API3_Exception
  * @throws \Exception
@@ -240,6 +240,15 @@ function _civicrm_api3_setting_fill_spec(&$params) {
   ];
 }
 
+/**
+ * Declare deprecated api functions.
+ *
+ * @return array
+ */
+function _civicrm_api3_setting_deprecation() {
+  return ['fill' => 'Setting "fill" is no longer necessary.'];
+}
+
 /**
  * Create or update a setting.
  *

diff --git a/tests/phpunit/api/v3/SettingTest.php b/tests/phpunit/api/v3/SettingTest.php
@@ -32,21 +32,15 @@ class api_v3_SettingTest extends CiviUnitTestCase {
   public function setUp() {
     parent::setUp();
     $params = [
-      'name' => 'Default Domain Name',
-      'domain_version' => '4.7',
+      'name' => __CLASS__ . 'Second Domain',
+      'domain_version' => CRM_Utils_System::version(),
     ];
     $result = $this->callAPISuccess('domain', 'get', $params);
     if (empty($result['id'])) {
       $result = $this->callAPISuccess('domain', 'create', $params);
     }
-
-    $params['name'] = 'Second Domain';
-    $result = $this->callAPISuccess('domain', 'get', $params);
-    if (empty($result['id'])) {
-      $result = $this->callAPISuccess('domain', 'create', $params);
-    }
     $this->_domainID2 = $result['id'];
-    $params['name'] = 'A-team domain';
+    $params['name'] = __CLASS__ . 'Third domain';
     $result = $this->callAPISuccess('domain', 'get', $params);
     if (empty($result['id'])) {
       $result = $this->callAPISuccess('domain', 'create', $params);
@@ -60,7 +54,7 @@ public function tearDown() {
     CRM_Utils_Hook::singleton()->reset();
     parent::tearDown();
     $this->callAPISuccess('system', 'flush', []);
-    $this->quickCleanup(['civicrm_domain']);
+    CRM_Core_DAO::executeQuery('DELETE FROM civicrm_domain WHERE name LIKE "' . __CLASS__ . '%"');
   }
 
   /**
@@ -300,10 +294,10 @@ public function testCreateSettingMultipleDomains($version) {
     ];
     $result = $this->callAPIAndDocument('setting', 'create', $params, __FUNCTION__, __FILE__, $description, 'CreateAllDomains');
 
-    $this->assertEquals(1, $result['values'][2]['uniq_email_per_site']);
-    $this->assertEquals(1, $result['values'][1]['uniq_email_per_site']);
-    $this->assertArrayHasKey(3, $result['values'], 'Domain create probably failed Debug this IF domain test is passing');
-    $this->assertEquals(1, $result['values'][3]['uniq_email_per_site'], 'failed to set setting for domain 3.');
+    $this->assertEquals(1, $result['values'][$this->_domainID2]['uniq_email_per_site']);
+    $this->assertEquals(1, $result['values'][$this->_currentDomain]['uniq_email_per_site']);
+    $this->assertArrayHasKey($this->_domainID3, $result['values'], 'Domain create probably failed Debug this IF domain test is passing');
+    $this->assertEquals(1, $result['values'][$this->_domainID3]['uniq_email_per_site'], 'failed to set setting for domain 3.');
 
     $params = [
       'domain_id' => 'all',
@@ -313,27 +307,27 @@ public function testCreateSettingMultipleDomains($version) {
     $description = "Shows getting a variable for all domains.";
     $result = $this->callAPIAndDocument('setting', 'get', $params, __FUNCTION__, __FILE__, $description, 'GetAllDomains');
 
-    $this->assertEquals(1, $result['values'][2]['uniq_email_per_site']);
-    $this->assertEquals(1, $result['values'][1]['uniq_email_per_site']);
-    $this->assertEquals(1, $result['values'][3]['uniq_email_per_site']);
+    $this->assertEquals(1, $result['values'][$this->_domainID2]['uniq_email_per_site']);
+    $this->assertEquals(1, $result['values'][$this->_currentDomain]['uniq_email_per_site']);
+    $this->assertEquals(1, $result['values'][$this->_domainID3]['uniq_email_per_site']);
 
     $params = [
-      'domain_id' => [1, 3],
+      'domain_id' => [$this->_currentDomain, $this->_domainID3],
       'uniq_email_per_site' => 0,
     ];
     $description = "Shows setting a variable for specified domains.";
     $result = $this->callAPIAndDocument('setting', 'create', $params, __FUNCTION__, __FILE__, $description, 'CreateSpecifiedDomains');
 
-    $this->assertEquals(0, $result['values'][3]['uniq_email_per_site']);
-    $this->assertEquals(0, $result['values'][1]['uniq_email_per_site']);
+    $this->assertEquals(0, $result['values'][$this->_domainID3]['uniq_email_per_site']);
+    $this->assertEquals(0, $result['values'][$this->_currentDomain]['uniq_email_per_site']);
     $params = [
-      'domain_id' => [1, 2],
+      'domain_id' => [$this->_currentDomain, $this->_domainID2],
       'return' => ['uniq_email_per_site'],
     ];
     $description = "Shows getting a variable for specified domains.";
     $result = $this->callAPIAndDocument('setting', 'get', $params, __FUNCTION__, __FILE__, $description, 'GetSpecifiedDomains');
-    $this->assertEquals(1, $result['values'][2]['uniq_email_per_site']);
-    $this->assertEquals(0, $result['values'][1]['uniq_email_per_site']);
+    $this->assertEquals(1, $result['values'][$this->_domainID2]['uniq_email_per_site']);
+    $this->assertEquals(0, $result['values'][$this->_currentDomain]['uniq_email_per_site']);
 
   }
 
@@ -524,8 +518,8 @@ public function testRevertAll() {
    */
   public function testDefaults() {
     $domparams = [
-      'name' => 'B Team Domain',
-      'domain_version' => '4.7',
+      'name' => __CLASS__ . 'B Team Domain',
+      'domain_version' => CRM_Utils_System::version(),
     ];
     $dom = $this->callAPISuccess('domain', 'create', $domparams);
     $params = [