ScheduledJob - Clean up form code & improve validation

CRM/Admin/Form/Job.php

@@ -19,7 +19,6 @@
  * Class for configuring jobs.
  */
 class CRM_Admin_Form_Job extends CRM_Admin_Form {
-  public $_id = NULL;
 
   /**
    * @var bool
@@ -146,23 +145,20 @@ public function buildQuickForm($check = FALSE) {
    * @throws CRM_Core_Exception
    */
   public static function formRule($fields) {
-
     $errors = [];
 
-    require_once 'api/api.php';
-
-    /** @var \Civi\API\Kernel $apiKernel */
-    $apiKernel = \Civi::service('civi_api_kernel');
-    $apiRequest = \Civi\API\Request::create($fields['api_entity'], $fields['api_action'], ['version' => 3]);
     try {
+      $apiParams = CRM_Core_BAO_Job::parseParameters($fields['parameters']);
+      /** @var \Civi\API\Kernel $apiKernel */
+      $apiKernel = \Civi::service('civi_api_kernel');
+      $apiRequest = \Civi\API\Request::create($fields['api_entity'], $fields['api_action'], $apiParams);
       $apiKernel->resolve($apiRequest);
     }
     catch (\Civi\API\Exception\NotImplementedException $e) {
       $errors['api_action'] = ts('Given API command is not defined.');
     }
-
-    if (!empty($errors)) {
-      return $errors;
+    catch (CRM_Core_Exception $e) {
+      $errors['parameters'] = ts('Parameters must be formatted as key=value on separate lines');
     }
 
     return empty($errors) ? TRUE : $errors;
@@ -249,7 +245,7 @@ public function postProcess() {
     $dao->api_entity = $values['api_entity'];
     $dao->api_action = $values['api_action'];
     $dao->description = $values['description'];
-    $dao->is_active = CRM_Utils_Array::value('is_active', $values, 0);
+    $dao->is_active = $values['is_active'] ?? 0;
 
     // CRM-17686
     $ts = strtotime($values['scheduled_run_date']);

CRM/Admin/Form/PaymentProcessor.php

@@ -248,10 +248,6 @@ public static function formRule($fields) {
       $errors['_qf_default'] = ts('You must have at least the test or live section filled');
     }
 
-    if (!empty($errors)) {
-      return $errors;
-    }
-
     return empty($errors) ? TRUE : $errors;
   }
 

CRM/Core/BAO/Job.php

@@ -117,4 +117,25 @@ public static function copy($id, $params = []) {
     return $copy;
   }
 
+  /**
+   * Parse multi-line `$parameters` string into an array
+   *
+   * @param string|null $parameters
+   * @return array
+   * @throws CRM_Core_Exception
+   */
+  public static function parseParameters(?string $parameters): array {
+    $result = ['version' => 3];
+    $lines = $parameters ? explode("\n", $parameters) : [];
+
+    foreach ($lines as $line) {
+      $pair = explode("=", $line);
+      if ($pair === FALSE || count($pair) !== 2 || !trim($pair[0]) || trim($pair[1]) === '') {
+        throw new CRM_Core_Exception('Malformed API parameters in scheduled job');
+      }
+      $result[trim($pair[0])] = trim($pair[1]);
+    }
+    return $result;
+  }
+
 }

CRM/Core/ScheduledJob.php

@@ -18,10 +18,24 @@
  */
 class CRM_Core_ScheduledJob {
 
+  /**
+   * @var int
+   * @deprecated
+   */
   public $version = 3;
 
+  /**
+   * @var int
+   */
+  public $id;
+
   public $name = NULL;
 
+  /**
+   * @var string
+   */
+  public $parameters = '';
+
   public $apiParams = [];
 
   public $remarks = [];
@@ -30,31 +44,16 @@ class CRM_Core_ScheduledJob {
    * @param array $params
    */
   public function __construct($params) {
+    // Fixme - setting undeclared class properties!
     foreach ($params as $name => $param) {
       $this->$name = $param;
     }
 
-    // version is set to 3 by default - if different number
-    // defined in params, it's replaced later on, however,
-    // it's practically useles, since it seems none of api v2
-    // will work properly in cron job setup. It might become
-    // useful when/if api v4 starts to emerge and will need
-    // testing in the cron job setup. To permanenty require
-    // hardcoded api version, it's enough to move below line
-    // under following if block.
-    $this->apiParams = ['version' => $this->version];
-
-    if (!empty($this->parameters)) {
-      $lines = explode("\n", $this->parameters);
-
-      foreach ($lines as $line) {
-        $pair = explode("=", $line);
-        if ($pair === FALSE || count($pair) != 2 || trim($pair[0]) == '' || trim($pair[1]) == '') {
-          $this->remarks[] .= 'Malformed parameters!';
-          break;
-        }
-        $this->apiParams[trim($pair[0])] = trim($pair[1]);
-      }
+    try {
+      $this->apiParams = CRM_Core_BAO_Job::parseParameters($this->parameters);
+    }
+    catch (CRM_Core_Exception $e) {
+      $this->remarks[] = $e->getMessage();
     }
   }
 
@@ -137,7 +136,4 @@ public function needsRunning() {
     return ($now >= $nextTime);
   }
 
-  public function __destruct() {
-  }
-
 }