Merge pull request #14266 from seamuslee001/dev_core_369

dev/core#369 Prevent hard fail of API Job when SMS provider has been deleted
civicrm · May 20, 2019 · 7914f0a · 7914f0a
2 parents 5722a07 + 51c566a
commit 7914f0a
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 34 deletions.
diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php
@@ -546,6 +546,12 @@ protected static function sendReminderSms($tokenRow, $schedule, $toContactID) {
       return ["sms_phone_missing" => "Couldn't find recipient's phone number."];
     }
 
+    // dev/core#369 If an SMS provider is deleted then the relevant row in the action_schedule_table is set to NULL
+    // So we need to exclude them.
+    if (CRM_Utils_System::isNull($schedule->sms_provider_id)) {
+      return ["sms_provider_missing" => "SMS Provider is NULL in database cannot send reminder"];
+    }
+
     $messageSubject = $tokenRow->render('subject');
     $sms_body_text = $tokenRow->render('sms_body_text');
 

diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php
@@ -30,39 +30,6 @@ public function tearDown() {
     parent::tearDown();
   }
 
-  /**
-   * Setup or clean up SMS tests
-   * @param bool $teardown
-   *
-   * @throws \CiviCRM_API3_Exception
-   */
-  public function setupForSmsTests($teardown = FALSE) {
-    require_once 'CiviTest/CiviTestSMSProvider.php';
-
-    // Option value params for CiviTestSMSProvider
-    $groupID = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', 'sms_provider_name', 'id', 'name');
-    $params = array(
-      'option_group_id' => $groupID,
-      'label' => 'unittestSMS',
-      'value' => 'unit.test.sms',
-      'name'  => 'CiviTestSMSProvider',
-      'is_default' => 1,
-      'is_active'  => 1,
-      'version'    => 3,
-    );
-
-    if ($teardown) {
-      // Test completed, delete provider
-      $providerOptionValueResult = civicrm_api3('option_value', 'get', $params);
-      civicrm_api3('option_value', 'delete', array('id' => $providerOptionValueResult['id']));
-      return;
-    }
-
-    // Create an SMS provider "CiviTestSMSProvider". Civi handles "CiviTestSMSProvider" as a special case and allows it to be instantiated
-    //  in CRM/Sms/Provider.php even though it is not an extension.
-    civicrm_api3('option_value', 'create', $params);
-  }
-
   /**
    * Test case for create() method.
    */

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -3160,4 +3160,37 @@ public function enableMultilingual() {
     $dbLocale = '_en_US';
   }
 
+  /**
+   * Setup or clean up SMS tests
+   * @param bool $teardown
+   *
+   * @throws \CiviCRM_API3_Exception
+   */
+  public function setupForSmsTests($teardown = FALSE) {
+    require_once 'CiviTest/CiviTestSMSProvider.php';
+
+    // Option value params for CiviTestSMSProvider
+    $groupID = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', 'sms_provider_name', 'id', 'name');
+    $params = array(
+      'option_group_id' => $groupID,
+      'label' => 'unittestSMS',
+      'value' => 'unit.test.sms',
+      'name'  => 'CiviTestSMSProvider',
+      'is_default' => 1,
+      'is_active'  => 1,
+      'version'    => 3,
+    );
+
+    if ($teardown) {
+      // Test completed, delete provider
+      $providerOptionValueResult = civicrm_api3('option_value', 'get', $params);
+      civicrm_api3('option_value', 'delete', array('id' => $providerOptionValueResult['id']));
+      return;
+    }
+
+    // Create an SMS provider "CiviTestSMSProvider". Civi handles "CiviTestSMSProvider" as a special case and allows it to be instantiated
+    //  in CRM/Sms/Provider.php even though it is not an extension.
+    return civicrm_api3('option_value', 'create', $params);
+  }
+
 }
diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php
@@ -245,13 +245,19 @@ public function testCallSendReminderSuccessMoreThanDefaultLimit() {
    * We create 3 contacts - 1 is in our group, 1 has our membership & the chosen one has both
    * & check that only the chosen one got the reminder
    */
-  public function testCallSendReminderLimitTo() {
+  public function testCallSendReminderLimitToSMS() {
     $membershipTypeID = $this->membershipTypeCreate();
     $this->membershipStatusCreate();
     $createTotal = 3;
     $groupID = $this->groupCreate(array('name' => 'Texan drawlers', 'title' => 'a...'));
     for ($i = 1; $i <= $createTotal; $i++) {
       $contactID = $this->individualCreate();
+      $this->callAPISuccess('Phone', 'create', [
+        'contact_id' => $contactID,
+        'phone' => '555 123 1234',
+        'phone_type_id' => 'Mobile',
+        'location_type_id' => 'Billing',
+      ]);
       if ($i == 2) {
         $theChosenOneID = $contactID;
       }
@@ -271,6 +277,19 @@ public function testCallSendReminderLimitTo() {
         ));
       }
     }
+    $this->setupForSmsTests();
+    $provider = civicrm_api3('SmsProvider', 'create', array(
+      'name' => "CiviTestSMSProvider",
+      'api_type' => "1",
+      "username" => "1",
+      "password" => "1",
+      "api_type" => "1",
+      "api_url" => "1",
+      "api_params" => "a=1",
+      "is_default" => "1",
+      "is_active" => "1",
+      "domain_id" => "1",
+    ));
     $this->callAPISuccess('action_schedule', 'create', array(
       'title' => " remind all Texans",
       'subject' => "drawling renewal",
@@ -282,12 +301,16 @@ public function testCallSendReminderLimitTo() {
       'start_action_unit' => 'day',
       'group_id' => $groupID,
       'limit_to' => TRUE,
+      'sms_provider_id' => $provider['id'],
+      'mode' => 'User_Preference',
     ));
     $this->callAPISuccess('job', 'send_reminder', array());
     $successfulCronCount = CRM_Core_DAO::singleValueQuery("SELECT count(*) FROM civicrm_action_log");
     $this->assertEquals($successfulCronCount, 1);
     $sentToID = CRM_Core_DAO::singleValueQuery("SELECT contact_id FROM civicrm_action_log");
     $this->assertEquals($sentToID, $theChosenOneID);
+    $this->assertEquals(0, CRM_Core_DAO::singleValueQuery("SELECT is_error FROM civicrm_action_log"));
+    $this->setupForSmsTests(TRUE);
   }
 
   public function testCallDisableExpiredRelationships() {
@@ -314,6 +337,85 @@ public function testCallDisableExpiredRelationships() {
     $this->contactDelete($orgID);
   }
 
+  /**
+   * Test scheduled reminders respect limit to (since above identified addition_to handling issue).
+   *
+   * We create 3 contacts - 1 is in our group, 1 has our membership & the chosen one has both
+   * & check that only the chosen one got the reminder
+   *
+   * Also check no hard fail on cron job with running a reminder that has a deleted SMS provider
+   */
+  public function testCallSendReminderLimitToSMSWithDeletedProviderr() {
+    $membershipTypeID = $this->membershipTypeCreate();
+    $this->membershipStatusCreate();
+    $createTotal = 3;
+    $groupID = $this->groupCreate(array('name' => 'Texan drawlers', 'title' => 'a...'));
+    for ($i = 1; $i <= $createTotal; $i++) {
+      $contactID = $this->individualCreate();
+      $this->callAPISuccess('Phone', 'create', [
+        'contact_id' => $contactID,
+        'phone' => '555 123 1234',
+        'phone_type_id' => 'Mobile',
+        'location_type_id' => 'Billing',
+      ]);
+      if ($i == 2) {
+        $theChosenOneID = $contactID;
+      }
+      if ($i < 3) {
+        $this->callAPISuccess('group_contact', 'create', array(
+          'contact_id' => $contactID,
+          'status' => 'Added',
+          'group_id' => $groupID,
+        ));
+      }
+      if ($i > 1) {
+        $this->callAPISuccess('membership', 'create', array(
+          'contact_id' => $contactID,
+          'membership_type_id' => $membershipTypeID,
+          'join_date' => 'now',
+          'start_date' => '+ 1 day',
+        ));
+      }
+    }
+    $this->setupForSmsTests();
+    $provider = civicrm_api3('SmsProvider', 'create', array(
+      'name' => "CiviTestSMSProvider",
+      'api_type' => "1",
+      "username" => "1",
+      "password" => "1",
+      "api_type" => "1",
+      "api_url" => "1",
+      "api_params" => "a=1",
+      "is_default" => "1",
+      "is_active" => "1",
+      "domain_id" => "1",
+    ));
+    $this->callAPISuccess('action_schedule', 'create', array(
+      'title' => " remind all Texans",
+      'subject' => "drawling renewal",
+      'entity_value' => $membershipTypeID,
+      'mapping_id' => 4,
+      'start_action_date' => 'membership_start_date',
+      'start_action_offset' => 1,
+      'start_action_condition' => 'before',
+      'start_action_unit' => 'day',
+      'group_id' => $groupID,
+      'limit_to' => TRUE,
+      'sms_provider_id' => $provider['id'],
+      'mode' => 'SMS',
+    ));
+    $this->callAPISuccess('SmsProvider', 'delete', ['id' => $provider['id']]);
+    $this->callAPISuccess('job', 'send_reminder', array());
+    $cronCount = CRM_Core_DAO::singleValueQuery("SELECT count(*) FROM civicrm_action_log");
+    $this->assertEquals($cronCount, 1);
+    $sentToID = CRM_Core_DAO::singleValueQuery("SELECT contact_id FROM civicrm_action_log");
+    $this->assertEquals($sentToID, $theChosenOneID);
+    $cronlog = CRM_Core_DAO::executeQuery("SELECT * FROM civicrm_action_log")->fetchAll()[0];
+    $this->assertEquals(1, $cronlog['is_error']);
+    $this->assertEquals('SMS Provider is NULL in database cannot send reminder', $cronlog['message']);
+    $this->setupForSmsTests(TRUE);
+  }
+
   /**
    * Test the batch merge function.
    *