From 2f6c641a1b502567f65104cad01c5a8e5377f878 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 1 Nov 2018 15:02:39 +1300 Subject: [PATCH] Finish fixing contribute preferences & preferences form to isolate hacks into one form --- CRM/Admin/Form/Preferences.php | 4 + CRM/Admin/Form/Preferences/Contribute.php | 234 +++++++++--------- settings/Contribute.setting.php | 3 + .../CRM/Admin/Form/Preferences/Contribute.tpl | 19 +- templates/CRM/Form/basicFormFields.tpl | 3 +- .../phpunit/CRMTraits/ACL/PermissionTrait.php | 83 +++++++ .../CRMTraits/ACL/PermissionTrait.phpx | 83 +++++++ 7 files changed, 303 insertions(+), 126 deletions(-) create mode 100644 tests/phpunit/CRMTraits/ACL/PermissionTrait.php create mode 100644 tests/phpunit/CRMTraits/ACL/PermissionTrait.phpx diff --git a/CRM/Admin/Form/Preferences.php b/CRM/Admin/Form/Preferences.php index 710623cef0..ad9053ccba 100644 --- a/CRM/Admin/Form/Preferences.php +++ b/CRM/Admin/Form/Preferences.php @@ -92,6 +92,7 @@ class CRM_Admin_Form_Preferences extends CRM_Core_Form { $settings = Civi::settings(); // @todo replace this by defining all in settings. foreach ($this->_varNames as $groupName => $settingNames) { + CRM_Core_Error::deprecatedFunctionWarning('deprecated use of preferences form. This will be removed from core soon'); foreach ($settingNames as $settingName => $options) { $this->_config->$settingName = $settings->get($settingName); } @@ -107,6 +108,7 @@ class CRM_Admin_Form_Preferences extends CRM_Core_Form { $this->setDefaultsForMetadataDefinedFields(); foreach ($this->_varNames as $groupName => $settings) { + CRM_Core_Error::deprecatedFunctionWarning('deprecated use of preferences form. This will be removed from core soon'); foreach ($settings as $settingName => $settingDetails) { $this->_defaults[$settingName] = isset($this->_config->$settingName) ? $this->_config->$settingName : CRM_Utils_Array::value('default', $settingDetails, NULL); } @@ -123,6 +125,7 @@ class CRM_Admin_Form_Preferences extends CRM_Core_Form { public function cbsDefaultValues(&$defaults) { foreach ($this->_varNames as $groupName => $groupValues) { + CRM_Core_Error::deprecatedFunctionWarning('deprecated use of preferences form. This will be removed from core soon'); foreach ($groupValues as $settingName => $fieldValue) { if ($fieldValue['html_type'] == 'checkboxes') { if (isset($this->_config->$settingName) && @@ -150,6 +153,7 @@ class CRM_Admin_Form_Preferences extends CRM_Core_Form { parent::buildQuickForm(); if (!empty($this->_varNames)) { + CRM_Core_Error::deprecatedFunctionWarning('deprecated use of preferences form. This will be removed from core soon'); foreach ($this->_varNames as $groupName => $groupValues) { $formName = CRM_Utils_String::titleToVar($groupName); $this->assign('formName', $formName); diff --git a/CRM/Admin/Form/Preferences/Contribute.php b/CRM/Admin/Form/Preferences/Contribute.php index 8e51270b0d..7efb2d2e1e 100644 --- a/CRM/Admin/Form/Preferences/Contribute.php +++ b/CRM/Admin/Form/Preferences/Contribute.php @@ -46,112 +46,127 @@ class CRM_Admin_Form_Preferences_Contribute extends CRM_Admin_Form_Preferences { ); /** - * Process the form submission. + * Our standards for settings are to have a setting per value with defined metadata. + * + * Unfortunately the 'contribution_invoice_settings' has been added in non-compliance. + * We use this array to hack-handle. + * + * I think the best way forwards would be to covert to multiple individual settings. + * + * @var array */ - public function preProcess() { - $config = CRM_Core_Config::singleton(); - CRM_Utils_System::setTitle(ts('CiviContribute Component Settings')); - $this->_varNames = array( - CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME => array( - 'invoice_prefix' => array( - 'html_type' => 'text', - 'title' => ts('Invoice Prefix'), - 'weight' => 1, - 'description' => ts('Enter prefix to be display on PDF for invoice'), - ), - 'credit_notes_prefix' => array( - 'html_type' => 'text', - 'title' => ts('Credit Notes Prefix'), - 'weight' => 2, - 'description' => ts('Enter prefix to be display on PDF for credit notes.'), - ), - 'due_date' => array( - 'html_type' => 'text', - 'title' => ts('Due Date'), - 'weight' => 3, - ), - 'due_date_period' => array( - 'html_type' => 'select', - 'title' => ts('For transmission'), - 'weight' => 4, - 'description' => ts('Select the interval for due date.'), - 'option_values' => array( - 'select' => ts('- select -'), - 'days' => ts('Days'), - 'months' => ts('Months'), - 'years' => ts('Years'), - ), - ), - 'notes' => array( - 'html_type' => 'wysiwyg', - 'title' => ts('Notes or Standard Terms'), - 'weight' => 5, - 'description' => ts('Enter note or message to be displayed on PDF invoice or credit notes '), - 'attributes' => array('rows' => 2, 'cols' => 40), - ), - 'is_email_pdf' => array( - 'html_type' => 'checkbox', - 'title' => ts('Automatically email invoice when user purchases online'), - 'weight' => 6, - ), - 'tax_term' => array( - 'html_type' => 'text', - 'title' => ts('Tax Term'), - 'weight' => 7, - ), - 'tax_display_settings' => array( - 'html_type' => 'select', - 'title' => ts('Tax Display Settings'), - 'weight' => 8, - 'option_values' => array( - 'Do_not_show' => ts('Do not show breakdown, only show total -i.e ' . - $config->defaultCurrencySymbol . '120.00'), - 'Inclusive' => ts('Show [tax term] inclusive price - i.e. ' . - $config->defaultCurrencySymbol . - '120.00 (includes [tax term] of ' . - $config->defaultCurrencySymbol . '20.00)'), - 'Exclusive' => ts('Show [tax term] exclusive price - i.e. ' . - $config->defaultCurrencySymbol . '100.00 + ' . - $config->defaultCurrencySymbol . '20.00 [tax term]'), - ), - ), - ), - ); - parent::preProcess(); - } + protected $invoiceSettings = []; /** * Build the form object. */ public function buildQuickForm() { - $htmlFields = array(); - foreach ($this->_settings as $setting => $group) { - // @todo - remove this whole loop! The parent form does this - it's just because of the werid handling - // of $htmlFields for this form that needs to be unwound that we have it atm. - // The 'basicform' has been contaminated with processing $htlFields - // to cater to this form - probably due to the way invoicing settings were handled as - // an array not a bunch of keys. - $settingMetaData = civicrm_api3('setting', 'getfields', array('name' => $setting)); - $props = $settingMetaData['values'][$setting]; - if (isset($props['quick_form_type'])) { - $add = 'add' . $props['quick_form_type']; - if ($add == 'addElement') { - if (in_array($props['html_type'], array('checkbox', 'textarea'))) { - } - else { - if ($props['html_type'] == 'select') { - $functionName = CRM_Utils_Array::value('name', CRM_Utils_Array::value('pseudoconstant', $props)); - if ($functionName) { - $props['option_values'] = array('' => ts('- select -')) + CRM_Contribute_PseudoConstant::$functionName(); - } - } - } - } + parent::buildQuickForm(); + $config = CRM_Core_Config::singleton(); + $this->invoiceSettings = [ + 'invoice_prefix' => [ + 'html_type' => 'text', + 'title' => ts('Invoice Prefix'), + 'weight' => 1, + 'description' => ts('Enter prefix to be display on PDF for invoice'), + ], + 'credit_notes_prefix' => [ + 'html_type' => 'text', + 'title' => ts('Credit Notes Prefix'), + 'weight' => 2, + 'description' => ts('Enter prefix to be display on PDF for credit notes.'), + ], + 'due_date' => [ + 'html_type' => 'text', + 'title' => ts('Due Date'), + 'weight' => 3, + ], + 'due_date_period' => [ + 'html_type' => 'select', + 'title' => ts('For transmission'), + 'weight' => 4, + 'description' => ts('Select the interval for due date.'), + 'option_values' => [ + 'select' => ts('- select -'), + 'days' => ts('Days'), + 'months' => ts('Months'), + 'years' => ts('Years'), + ], + ], + 'notes' => [ + 'html_type' => 'wysiwyg', + 'title' => ts('Notes or Standard Terms'), + 'weight' => 5, + 'description' => ts('Enter note or message to be displayed on PDF invoice or credit notes '), + 'attributes' => ['rows' => 2, 'cols' => 40], + ], + 'is_email_pdf' => [ + 'html_type' => 'checkbox', + 'title' => ts('Automatically email invoice when user purchases online'), + 'weight' => 6, + 'description' => ts('Should a pdf invoice be emailed automatically?'), + ], + 'tax_term' => [ + 'html_type' => 'text', + 'title' => ts('Tax Term'), + 'weight' => 7, + ], + 'tax_display_settings' => [ + 'html_type' => 'select', + 'title' => ts('Tax Display Settings'), + 'weight' => 8, + 'option_values' => [ + 'Do_not_show' => ts('Do not show breakdown, only show total -i.e ' . + $config->defaultCurrencySymbol . '120.00'), + 'Inclusive' => ts('Show [tax term] inclusive price - i.e. ' . + $config->defaultCurrencySymbol . + '120.00 (includes [tax term] of ' . + $config->defaultCurrencySymbol . '20.00)'), + 'Exclusive' => ts('Show [tax term] exclusive price - i.e. ' . + $config->defaultCurrencySymbol . '100.00 + ' . + $config->defaultCurrencySymbol . '20.00 [tax term]'), + ], + ], + ]; + + // @todo this is a faux metadata approach - we should be honest & add them correctly or find a way to make this + // compatible with our settings standards. + foreach ($this->invoiceSettings as $fieldName => $fieldValue) { + switch ($fieldValue['html_type']) { + case 'text': + $this->addElement('text', + $fieldName, + $fieldValue['title'], + [ + 'maxlength' => 64, + 'size' => 32, + ] + ); + break; + + case 'checkbox': + $this->add($fieldValue['html_type'], + $fieldName, + $fieldValue['title'] + ); + break; + + case 'select': + $this->addElement('select', + $fieldName, + $fieldValue['title'], + $fieldValue['option_values'], + CRM_Utils_Array::value('attributes', $fieldValue) + ); + break; + + case 'wysiwyg': + $this->add('wysiwyg', $fieldName, $fieldValue['title'], $fieldValue['attributes']); + break; } - $htmlFields[$setting] = ts($props['description']); } - $this->assign('htmlFields', $htmlFields); - parent::buildQuickForm(); + + $this->assign('htmlFields', $this->invoiceSettings); } /** @@ -160,16 +175,8 @@ class CRM_Admin_Form_Preferences_Contribute extends CRM_Admin_Form_Preferences { * default values are retrieved from the database */ public function setDefaultValues() { - $defaults = Civi::settings()->get('contribution_invoice_settings'); - //CRM-16691: Changes made related to settings of 'CVV'. - foreach (array('cvv_backoffice_required') as $setting) { - $defaults[$setting] = civicrm_api3('setting', 'getvalue', - array( - 'name' => $setting, - 'group' => CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME, - ) - ); - } + $defaults = parent::setDefaultValues(); + $defaults = array_merge($defaults, Civi::settings()->get('contribution_invoice_settings')); return $defaults; } @@ -179,10 +186,11 @@ class CRM_Admin_Form_Preferences_Contribute extends CRM_Admin_Form_Preferences { public function postProcess() { // store the submitted values in an array $params = $this->controller->exportValues($this->_name); - unset($params['qfKey']); - unset($params['entryURL']); - Civi::settings()->set('contribution_invoice_settings', $params); + $invoiceParams = array_intersect_key($params, $this->invoiceSettings); + Civi::settings()->set('contribution_invoice_settings', $invoiceParams); + parent::postProcess(); + // @todo - all this should be handled by defining an on change action in the metadata. // to set default value for 'Invoices / Credit Notes' checkbox on display preferences $values = CRM_Core_BAO_Setting::getItem("CiviCRM Preferences"); $optionValues = CRM_Core_OptionGroup::values('user_dashboard_options', FALSE, FALSE, FALSE, NULL, 'name'); @@ -206,10 +214,6 @@ class CRM_Admin_Form_Preferences_Contribute extends CRM_Admin_Form_Preferences { CRM_Core_DAO::VALUE_SEPARATOR; Civi::settings()->set('user_dashboard_options', $settingName); } - //CRM-16691: Changes made related to settings of 'CVV'. - $settings = array_intersect_key($params, array('cvv_backoffice_required' => 1)); - $result = civicrm_api3('setting', 'create', $settings); - CRM_Core_Session::setStatus(ts('Your changes have been saved.'), ts('Changes Saved'), "success"); } } diff --git a/settings/Contribute.setting.php b/settings/Contribute.setting.php index 41b9cc8e12..22bdd134fd 100644 --- a/settings/Contribute.setting.php +++ b/settings/Contribute.setting.php @@ -39,6 +39,7 @@ return array( 'group' => 'contribute', 'name' => 'cvv_backoffice_required', 'type' => 'Boolean', + 'html_type' => 'radio', 'quick_form_type' => 'YesNo', 'default' => '1', 'add' => '4.1', @@ -49,6 +50,8 @@ return array( 'help_text' => 'If set it back-office credit card transactions will required a cvv code. Leave as required unless you have a very strong reason to change', ), 'contribution_invoice_settings' => array( + // @todo our standard is to have a setting per item not to hide settings in an array with + // no useful metadata. Undo this setting. 'group_name' => 'Contribute Preferences', 'group' => 'contribute', 'name' => 'contribution_invoice_settings', diff --git a/templates/CRM/Admin/Form/Preferences/Contribute.tpl b/templates/CRM/Admin/Form/Preferences/Contribute.tpl index 2629d66f60..42139e9d9a 100644 --- a/templates/CRM/Admin/Form/Preferences/Contribute.tpl +++ b/templates/CRM/Admin/Form/Preferences/Contribute.tpl @@ -25,25 +25,27 @@ *}
{include file="CRM/common/formButtons.tpl" location="top"}
- - {foreach from=$htmlFields item=desc key=htmlField} + {include file="CRM/Form/basicFormFields.tpl"} + +
+ {foreach from=$htmlFields item=fieldSpec key=htmlField} {if $form.$htmlField} {assign var=n value=$htmlField|cat:'_description'} - {if $form.$htmlField.html_type EQ 'checkbox'|| $form.$htmlField.html_type EQ 'checkboxes'} + {if $fieldSpec.html_type EQ 'checkbox'|| $fieldSpec.html_type EQ 'checkboxes'} {else} {/if} @@ -51,14 +53,13 @@ {/if} {/foreach}
{$form.$htmlField.html} {$form.$htmlField.label} {if $desc} -
{$desc} +
{$fieldSpec.description} {/if} 		{$form.$htmlField.label} {if $htmlField eq 'acl_financial_type'}{help id="$htmlField"}{/if} {$form.$htmlField.html} - {if $desc} -
{$desc} + {if $fieldSpec.description} +
{$fieldSpec.description} {/if}
- {include file="CRM/Form/basicFormFields.tpl"}
{include file="CRM/common/formButtons.tpl" location="bottom"}
{literal}