From 03c5ceba7e09286479dcc37920387ef84d8c01ec Mon Sep 17 00:00:00 2001 From: "deb.monish" Date: Tue, 29 Aug 2017 13:06:41 +0530 Subject: [PATCH] QA fixes --- CRM/Admin/Form/Setting.php | 26 ++---------- CRM/Admin/Form/Setting/Debugging.php | 25 ++--------- CRM/Core/BAO/Setting.php | 42 ++++++++++++++++++- CRM/Utils/Mail.php | 4 +- Civi/Core/SettingsBag.php | 4 +- api/v3/Job.php | 7 ++++ api/v3/Setting.php | 6 +++ settings/Developer.setting.php | 3 ++ .../CRM/Mailing/BaseMailingSystemTest.php | 2 +- 9 files changed, 70 insertions(+), 49 deletions(-) diff --git a/CRM/Admin/Form/Setting.php b/CRM/Admin/Form/Setting.php index c378b53d80..89e2574084 100644 --- a/CRM/Admin/Form/Setting.php +++ b/CRM/Admin/Form/Setting.php @@ -98,23 +98,9 @@ class CRM_Admin_Form_Setting extends CRM_Core_Form { foreach ($settingMetaData as $setting => $props) { if (isset($props['quick_form_type'])) { if (isset($props['pseudoconstant'])) { - if (array_key_exists('optionGroupName', $props['pseudoconstant'])) { - $optionValues = civicrm_api3('OptionValue', 'get', array( - 'return' => array("label", "value"), - 'option_group_id' => $setting, - )); - if ($optionValues['count'] > 0) { - foreach ($optionValues['values'] as $key => $values) { - $vals[$values['value']] = $values['label']; - } - $options['values'] = $vals; - } - } - else { - $options = civicrm_api3('Setting', 'getoptions', array( - 'field' => $setting, - )); - } + $options = civicrm_api3('Setting', 'getoptions', array( + 'field' => $setting, + )); } else { $options = NULL; @@ -131,11 +117,7 @@ class CRM_Admin_Form_Setting extends CRM_Core_Form { ); } elseif ($add == 'addSelect') { - $element = $this->addElement('select', $setting, ts($props['title']), $options['values'], CRM_Utils_Array::value('html_attributes', $props)); - if (defined('CIVICRM_ENVIRONMENT')) { - $element->freeze(); - CRM_Core_Session::setStatus(ts('The environment settings have been disabled because it has been overridden in the settings file.'), ts('Environment settings'), 'info'); - } + $this->addElement('select', $setting, ts($props['title']), $options['values'], CRM_Utils_Array::value('html_attributes', $props)); } elseif ($add == 'addCheckBox') { $this->addCheckBox($setting, ts($props['title']), $options['values'], NULL, CRM_Utils_Array::value('html_attributes', $props), NULL, NULL, array('  ')); diff --git a/CRM/Admin/Form/Setting/Debugging.php b/CRM/Admin/Form/Setting/Debugging.php index 0d7219a2f3..5a065c567a 100644 --- a/CRM/Admin/Form/Setting/Debugging.php +++ b/CRM/Admin/Form/Setting/Debugging.php @@ -54,29 +54,12 @@ class CRM_Admin_Form_Setting_Debugging extends CRM_Admin_Form_Setting { } parent::buildQuickForm(); - } - /** - * Process the form submission. - */ - public function postProcess() { - $params = $this->controller->exportValues($this->_name); - - if ($params['environment'] != 'Production') { - $mailing = Civi::settings()->get('mailing_backend'); - if ($mailing['outBound_option'] != 2) { - Civi::settings()->set('mailing_backend_store', $mailing); - } - Civi::settings()->set('mailing_backend', array('outBound_option' => CRM_Mailing_Config::OUTBOUND_OPTION_DISABLED)); - CRM_Core_Session::setStatus(ts('Outbound emails have been disabled. Scheduled jobs will not run unless runInNonProductionEnvironment=TRUE is added as a parameter for a specific job'), ts("Non-production environment set"), "success"); - } - else { - $mailing = Civi::settings()->get('mailing_backend_store'); - if ($mailing) { - Civi::settings()->set('mailing_backend', $mailing); - } + if (defined('CIVICRM_ENVIRONMENT')) { + $element = $this->getElement('environment'); + $element->freeze(); + CRM_Core_Session::setStatus(ts('The environment settings have been disabled because it has been overridden in the settings file.'), ts('Environment settings'), 'info'); } - parent::postProcess(); } } diff --git a/CRM/Core/BAO/Setting.php b/CRM/Core/BAO/Setting.php index 9b45f11aa8..7997c9e172 100644 --- a/CRM/Core/BAO/Setting.php +++ b/CRM/Core/BAO/Setting.php @@ -509,8 +509,46 @@ class CRM_Core_BAO_Setting extends CRM_Core_DAO_Setting { * @throws API_Exception */ public static function isAPIJobAllowedToRun($params) { - if (CRM_Core_Config::environment() != 'Production' && !CRM_Utils_Array::value('runInNonProductionEnvironment', $params)) { - throw new Exception("Job has not been executed as it is a non-production environment."); + $environment = CRM_Core_Config::environment(NULL, TRUE); + if ($environment != 'Production') { + if (CRM_Utils_Array::value('runInNonProductionEnvironment', $params)) { + $mailing = Civi::settings()->get('mailing_backend_store'); + if ($mailing) { + Civi::settings()->set('mailing_backend', $mailing); + } + } + else { + throw new Exception(ts("Job has not been executed as it is a %1 (non-production) environment.", array(1 => $environment))); + } + } + } + + /** + * Setting Callback - On Change. + * + * Respond to changes in the "environment" setting. + * + * @param array $oldValue + * Value of old environment mode. + * @param array $newValue + * Value of new environment mode. + * @param array $metadata + * Specification of the setting (per *.settings.php). + */ + public static function onChangeEnvironmentSetting($oldValue, $newValue, $metadata) { + if ($newValue != 'Production') { + $mailing = Civi::settings()->get('mailing_backend'); + if ($mailing['outBound_option'] != 2) { + Civi::settings()->set('mailing_backend_store', $mailing); + } + Civi::settings()->set('mailing_backend', array('outBound_option' => CRM_Mailing_Config::OUTBOUND_OPTION_DISABLED)); + CRM_Core_Session::setStatus(ts('Outbound emails have been disabled. Scheduled jobs will not run unless runInNonProductionEnvironment=TRUE is added as a parameter for a specific job'), ts("Non-production environment set"), "success"); + } + else { + $mailing = Civi::settings()->get('mailing_backend_store'); + if ($mailing) { + Civi::settings()->set('mailing_backend', $mailing); + } } } diff --git a/CRM/Utils/Mail.php b/CRM/Utils/Mail.php index 78dc6676e7..87e62fb0f0 100644 --- a/CRM/Utils/Mail.php +++ b/CRM/Utils/Mail.php @@ -100,12 +100,12 @@ class CRM_Utils_Mail { } elseif ($mailingInfo['outBound_option'] == CRM_Mailing_Config::OUTBOUND_OPTION_DISABLED) { CRM_Core_Error::debug_log_message(ts('Outbound mail has been disabled. Click Administer >> System Setting >> Outbound Email to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1')))); - CRM_Core_Session::setStatus(ts('Outbound mail has been disabled. Click Administer >> System Setting >> Outbound Email to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1')))); + CRM_Core_Error::statusBounce(ts('Outbound mail has been disabled. Click Administer >> System Setting >> Outbound Email to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1')))); } else { CRM_Core_Error::debug_log_message(ts('There is no valid SMTP server Setting Or SendMail path setting. Click Administer >> System Setting >> Outbound Email to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1')))); - CRM_Core_Session::setStatus(ts('There is no valid SMTP server Setting Or sendMail path setting. Click Administer >> System Setting >> Outbound Email to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1')))); CRM_Core_Error::debug_var('mailing_info', $mailingInfo); + CRM_Core_Error::statusBounce(ts('There is no valid SMTP server Setting Or sendMail path setting. Click Administer >> System Setting >> Outbound Email to set the OutBound Email.', array(1 => CRM_Utils_System::url('civicrm/admin/setting/smtp', 'reset=1')))); } return $mailer; } diff --git a/Civi/Core/SettingsBag.php b/Civi/Core/SettingsBag.php index 1428949924..2aee1eb1c2 100644 --- a/Civi/Core/SettingsBag.php +++ b/Civi/Core/SettingsBag.php @@ -352,7 +352,9 @@ class SettingsBag { } $dao->find(TRUE); - if (isset($metadata['on_change']) && !($value == 0 && ($dao->value === NULL || unserialize($dao->value) == 0))) { + // string comparison with 0 always return true, so to be ensure the type use === + // ref - https://stackoverflow.com/questions/8671942/php-string-comparasion-to-0-integer-returns-true + if (isset($metadata['on_change']) && !($value === 0 && ($dao->value === NULL || unserialize($dao->value) == 0))) { foreach ($metadata['on_change'] as $callback) { call_user_func( \Civi\Core\Resolver::singleton()->get($callback), diff --git a/api/v3/Job.php b/api/v3/Job.php index bbc62b8819..1d21b34965 100644 --- a/api/v3/Job.php +++ b/api/v3/Job.php @@ -314,6 +314,13 @@ function civicrm_api3_job_process_pledge($params) { function civicrm_api3_job_process_mailing($params) { $mailsProcessedOrig = CRM_Mailing_BAO_MailingJob::$mailsProcessed; + try { + CRM_Core_BAO_Setting::isAPIJobAllowedToRun($params); + } + catch (Exception $e) { + return civicrm_api3_create_error($e->getMessage()); + } + if (!CRM_Mailing_BAO_Mailing::processQueue()) { return civicrm_api3_create_error('Process Queue failed'); } diff --git a/api/v3/Setting.php b/api/v3/Setting.php index e8d85801e1..0677d2ffa0 100644 --- a/api/v3/Setting.php +++ b/api/v3/Setting.php @@ -160,6 +160,12 @@ function civicrm_api3_setting_getoptions($params) { $values = Civi\Core\Resolver::singleton()->call($pseudoconstant['callback'], array()); return civicrm_api3_create_success($values, $params, 'Setting', 'getoptions'); } + elseif (!empty($pseudoconstant['optionGroupName'])) { + return civicrm_api3_create_success( + CRM_Core_OptionGroup::values($pseudoconstant['optionGroupName'], FALSE, FALSE, TRUE), + $params, 'Setting', 'getoptions' + ); + } throw new API_Exception("The field '" . $params['field'] . "' uses an unsupported option list."); } diff --git a/settings/Developer.setting.php b/settings/Developer.setting.php index 2c0902ba6a..9a47e7916e 100644 --- a/settings/Developer.setting.php +++ b/settings/Developer.setting.php @@ -116,6 +116,9 @@ return array( 'is_domain' => 1, 'is_contact' => 0, 'description' => "Setting to define the environment in which this CiviCRM instance is running.", + 'on_change' => array( + 'CRM_Core_BAO_Setting::onChangeEnvironmentSetting', + ), ), 'fatalErrorHandler' => array( 'group_name' => 'Developer Preferences', diff --git a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php index 4fec7e9506..37bf968c14 100644 --- a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php +++ b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php @@ -395,7 +395,7 @@ abstract class CRM_Mailing_BaseMailingSystemTest extends CiviUnitTestCase { $mailingParams = array_merge($this->defaultParams, $params); $this->callAPISuccess('mailing', 'create', $mailingParams); $this->_mut->assertRecipients(array()); - $this->callAPISuccess('job', 'process_mailing', array()); + $this->callAPISuccess('job', 'process_mailing', array('runInNonProductionEnvironment' => TRUE)); $allMessages = $this->_mut->getAllMessages('ezc'); // There are exactly two contacts produced by setUp(). -- 2.25.1