Merge pull request #10135 from seamuslee001/CRM-20410

CRM-20410 Reformat CRM_SMS_BAO_Provider to use standard create function style rather than the split
civicrm · Apr 11, 2017 · 5ce0c89 · 5ce0c89
2 parents 3ea7bd0 + 6f19290
commit 5ce0c89
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 30 deletions.
diff --git a/CRM/SMS/BAO/Provider.php b/CRM/SMS/BAO/Provider.php
@@ -88,30 +88,40 @@ public static function getProviders($selectArr = NULL, $filter = NULL, $getActiv
   }
 
   /**
-   * Save a new record into the database
-   * @todo create a create function to do this work
-   * @param $values
+   * Create or Update an SMS provider
+   * @param array $params
+   * @return array saved values
    */
-  public static function saveRecord($values) {
-    $values['domain_id'] = CRM_Utils_Array::value('domain_id', $values, CRM_Core_Config::domainID());
-    $dao = new CRM_SMS_DAO_Provider();
-    $dao->copyValues($values);
-    $dao->save();
-  }
+  public static function create(&$params) {
+    $id = CRM_Utils_Array::value('id', $params);
 
-  /**
-   * Update an SMS provider in the database.
-   * @todo combine with saveRecord in a create function
-   * @param $values
-   * @param int $providerId
-   */
-  public static function updateRecord($values, $providerId) {
-    $dao = new CRM_SMS_DAO_Provider();
-    $dao->id = $providerId;
-    if ($dao->find(TRUE)) {
-      $dao->copyValues($values);
-      $dao->save();
+    if ($id) {
+      CRM_Utils_Hook::pre('edit', 'SmsProvider', $id, $params);
+    }
+    else {
+      CRM_Utils_Hook::pre('create', 'SmsProvider', NULL, $params);
+    }
+
+    $provider = new CRM_SMS_DAO_Provider();
+    if ($id) {
+      $provider->id = $id;
+      $provider->find(TRUE);
+    }
+    if ($id) {
+      $provider->domain_id = CRM_Utils_Array::value('domain_id', $params, $provider->domain_id);
+    }
+    else {
+      $provider->domain_id = CRM_Utils_Array::value('domain_id', $params, CRM_Core_Config::domainID());
+    }
+    $provider->copyValues($params);
+    $result = $provider->save();
+    if ($id) {
+      CRM_Utils_Hook::post('edit', 'SmsProvider', $provider->id, $provider);
+    }
+    else {
+      CRM_Utils_Hook::post('create', 'SmsProvider', NULL, $provider);
     }
+    return $result;
   }
 
   /**

diff --git a/CRM/SMS/Form/Provider.php b/CRM/SMS/Form/Provider.php
@@ -171,11 +171,11 @@ public function postProcess() {
     $recData['is_active'] = CRM_Utils_Array::value('is_active', $recData, 0);
     $recData['is_default'] = CRM_Utils_Array::value('is_default', $recData, 0);
 
-    if ($this->_action & CRM_Core_Action::UPDATE) {
-      CRM_SMS_BAO_Provider::updateRecord($recData, $this->_id);
-    }
-    elseif ($this->_action & CRM_Core_Action::ADD) {
-      CRM_SMS_BAO_Provider::saveRecord($recData);
+    if ($this->_action && (CRM_Core_Action::UPDATE || CRM_Core_Action::ADD)) {
+      if ($this->_id) {
+        $recData['id'] = $this->_id;
+      }
+      civicrm_api3('SmsProvider', 'create', $recData);
     }
   }
 

diff --git a/Civi/API/Subscriber/ChainSubscriber.php b/Civi/API/Subscriber/ChainSubscriber.php
@@ -125,6 +125,17 @@ protected function callNestedApi($apiKernel, &$params, &$result, $action, $entit
         );
         $subEntity = _civicrm_api_get_entity_name_from_camel($subAPI[1]);
 
+        // Hard coded list of entitys that have fields starting api_ and shouldn't be automatically
+        // deemed to be chained API calls
+        $skipList = array(
+          'SmsProvider' => array('type', 'url', 'params'),
+          'Job' => array('prefix', 'entity', 'action'),
+          'Contact' => array('key'),
+        );
+        if (isset($skipList[$entity]) && in_array($subEntity, $skipList[$entity])) {
+          continue;
+        }
+
         foreach ($result['values'] as $idIndex => $parentAPIValues) {
 
           if ($subEntity != 'contact') {

diff --git a/tests/phpunit/CRM/SMS/BAO/ProviderTest.php b/tests/phpunit/CRM/SMS/BAO/ProviderTest.php
@@ -64,12 +64,12 @@ public function testCreateAndUpdateProvider() {
       'is_active' => 1,
       'api_type' => 1,
     );
-    CRM_SMS_BAO_Provider::saveRecord($values);
+    $this->callAPISuccess('SmsProvider', 'create', $values);
     $provider = $this->callAPISuccess('SmsProvider', 'getsingle', array('title' => 'test SMS provider'));
     $domain_id = CRM_Core_DAO::getFieldValue('CRM_SMS_DAO_Provider', $provider['id'], 'domain_id');
     $this->assertNull($domain_id);
-    $values2 = array('title' => 'Test SMS Provider2');
-    CRM_SMS_BAO_Provider::updateRecord($values2, $provider['id']);
+    $values2 = array('title' => 'Test SMS Provider2', 'id' => $provider['id']);
+    $this->callAPISuccess('SmsProvider', 'create', $values2);
     $provider = $this->callAPISuccess('SmsProvider', 'getsingle', array('id' => $provider['id']));
     $this->assertEquals('Test SMS Provider2', $provider['title']);
     $domain_id = CRM_Core_DAO::getFieldValue('CRM_SMS_DAO_Provider', $provider['id'], 'domain_id');
@@ -89,7 +89,7 @@ public function testCreateWithoutDomain() {
       'is_active' => 1,
       'api_type' => 1,
     );
-    CRM_SMS_BAO_Provider::saveRecord($values);
+    $this->callAPISuccess('SmsProvider', 'create', $values);
     $provider = $this->callAPISuccess('SmsProvider', 'getsingle', array('title' => 'test SMS provider'));
     $domain_id = CRM_Core_DAO::getFieldValue('CRM_SMS_DAO_Provider', $provider['id'], 'domain_id');
     $this->assertEquals(CRM_Core_Config::domainID(), $domain_id);