From bc999cd161407772b24b8799ca27508697436061 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 25 Aug 2014 20:58:32 +0100 Subject: [PATCH] CRM-15172 - Add centralized state/county validation --- CRM/Contact/Form/Edit/Address.php | 43 ----------------- .../Form/Contribution/OnBehalfOf.php | 11 +---- CRM/Contribute/Form/ContributionBase.php | 22 --------- CRM/Core/BAO/Location.php | 2 +- CRM/Core/Form.php | 33 +++++++++++-- CRM/Core/Payment/Form.php | 46 +------------------ CRM/Profile/Form.php | 15 ------ 7 files changed, 34 insertions(+), 138 deletions(-) diff --git a/CRM/Contact/Form/Edit/Address.php b/CRM/Contact/Form/Edit/Address.php index 11d47f9cfb..a367854d30 100644 --- a/CRM/Contact/Form/Edit/Address.php +++ b/CRM/Contact/Form/Edit/Address.php @@ -276,7 +276,6 @@ class CRM_Contact_Form_Edit_Address { $customDataRequiredFields = explode(',', $self->_addressRequireOmission); } - // check for state/county match if not report error to user. if (!empty($fields['address']) && is_array($fields['address'])) { foreach ($fields['address'] as $instance => $addressValues) { @@ -307,48 +306,6 @@ class CRM_Contact_Form_Edit_Address { } } - $countryId = CRM_Utils_Array::value('country_id', $addressValues); - - $stateProvinceId = CRM_Utils_Array::value('state_province_id', $addressValues); - - //do check for mismatch countries - if ($stateProvinceId && $countryId) { - $stateProvinceDAO = new CRM_Core_DAO_StateProvince(); - $stateProvinceDAO->id = $stateProvinceId; - $stateProvinceDAO->find(TRUE); - if ($stateProvinceDAO->country_id != $countryId) { - // countries mismatch hence display error - $stateProvinces = CRM_Core_PseudoConstant::stateProvince(); - $countries = CRM_Core_PseudoConstant::country(); - $errors["address[$instance][state_province_id]"] = ts('State/Province %1 is not part of %2. It belongs to %3.', - array( - 1 => $stateProvinces[$stateProvinceId], - 2 => $countries[$countryId], - 3 => $countries[$stateProvinceDAO->country_id] - ) - ); - } - } - - $countyId = CRM_Utils_Array::value('county_id', $addressValues); - - //state county validation - if ($stateProvinceId && $countyId) { - $countyDAO = new CRM_Core_DAO_County(); - $countyDAO->id = $countyId; - $countyDAO->find(TRUE); - if ($countyDAO->state_province_id != $stateProvinceId) { - $counties = CRM_Core_PseudoConstant::county(); - $errors["address[$instance][county_id]"] = ts('County %1 is not part of %2. It belongs to %3.', - array( - 1 => $counties[$countyId], - 2 => $stateProvinces[$stateProvinceId], - 3 => $stateProvinces[$countyDAO->state_province_id] - ) - ); - } - } - if (!empty($addressValues['use_shared_address']) && empty($addressValues['master_id'])) { $errors["address[$instance][use_shared_address]"] = ts('Please select valid shared contact or a contact with valid address.'); } diff --git a/CRM/Contribute/Form/Contribution/OnBehalfOf.php b/CRM/Contribute/Form/Contribution/OnBehalfOf.php index a34c0692aa..ea2d992564 100644 --- a/CRM/Contribute/Form/Contribution/OnBehalfOf.php +++ b/CRM/Contribute/Form/Contribution/OnBehalfOf.php @@ -169,16 +169,7 @@ class CRM_Contribute_Form_Contribution_OnBehalfOf { foreach ($profileFields as $name => $field) { if (in_array($field['field_type'], $fieldTypes)) { list($prefixName, $index) = CRM_Utils_System::explode('-', $name, 2); - if (in_array($prefixName, array('state_province', 'country', 'county'))) { - if (count($form->_submitValues)) { - $locationTypeId = $field['location_type_id']; - if (!empty($form->_submitValues['onbehalf']["country-{$locationTypeId}"]) && - $prefixName == "state_province") { - $field['is_required'] = CRM_Core_Payment_Form::checkRequiredStateProvince($form, "country-{$locationTypeId}", TRUE); - } - } - } - elseif (in_array($prefixName, array('organization_name', 'email')) && empty($field['is_required'])) { + if (in_array($prefixName, array('organization_name', 'email')) && empty($field['is_required'])) { $field['is_required'] = 1; } diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php index 8bc1716aef..cb9e64d40a 100644 --- a/CRM/Contribute/Form/ContributionBase.php +++ b/CRM/Contribute/Form/ContributionBase.php @@ -737,28 +737,6 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { continue; } - list($prefixName, $index) = CRM_Utils_System::explode('-', $key, 2); - if ($prefixName == 'state_province' || $prefixName == 'country' || $prefixName == 'county') { - if ($prefixName == "state_province") { - if ($profileContactType == 'onbehalf') { - //CRM-11881: Bypass required-ness check for state/province on Contribution Confirm page - //as already done during Contribution registration via onBehalf's quickForm - $field['is_required'] = FALSE; - } - else { - if (count($this->_submitValues)) { - $locationTypeId = $field['location_type_id']; - if (array_key_exists("country-{$locationTypeId}", $fields) && - array_key_exists("state_province-{$locationTypeId}", $fields) && - !empty($this->_submitValues["country-{$locationTypeId}"])) { - $field['is_required'] = - CRM_Core_Payment_Form::checkRequiredStateProvince($this, "country-{$locationTypeId}"); - } - } - } - } - } - if ($profileContactType) { //Since we are showing honoree name separately so we are removing it from honoree profile just for display $honoreeNamefields = array('prefix_id', 'first_name', 'last_name', 'suffix_id', 'organization_name', 'household_name'); diff --git a/CRM/Core/BAO/Location.php b/CRM/Core/BAO/Location.php index e7108aaea6..2c054aed64 100644 --- a/CRM/Core/BAO/Location.php +++ b/CRM/Core/BAO/Location.php @@ -409,7 +409,7 @@ WHERE e.id = %1"; if (!$values) { return array(); } - $values = (array) $values; + $values = array_filter((array) $values); $elements = array(); $list = &$elements; $method = $valueType == 'country' ? 'stateProvinceForCountry' : 'countyForState'; diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index 76d4a6c3d3..67077004e6 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -394,6 +394,8 @@ class CRM_Core_Form extends HTML_QuickForm_Page { function validate() { $error = parent::validate(); + $this->validateChainSelectFields(); + $hookErrors = CRM_Utils_Hook::validate( get_class($this), $this->_submitValues, @@ -1759,16 +1761,17 @@ class CRM_Core_Form extends HTML_QuickForm_Page { foreach ($this->_chainSelectFields as $control => $target) { $controlField = $this->getElement($control); $targetField = $this->getElement($target); + $controlType = $targetField->getAttribute('data-callback') == 'civicrm/ajax/jqCounty' ? 'stateProvince' : 'country'; $css = (string) $controlField->getAttribute('class'); $controlField->updateAttributes(array( 'class' => ($css ? "$css " : 'crm-select2 ') . 'crm-chain-select-control', 'data-target' => $target, )); - $selection = $controlField->getValue(); + $controlValue = $controlField->getValue(); $options = array(); - if ($selection) { - $options = CRM_Core_BAO_Location::getChainSelectValues($selection, strpos($control, 'rovince') ? 'stateProvince' : 'country', TRUE); + if ($controlValue) { + $options = CRM_Core_BAO_Location::getChainSelectValues($controlValue, $controlType, TRUE); if (!$options) { $targetField->setAttribute('placeholder', $targetField->getAttribute('data-none-prompt')); } @@ -1784,5 +1787,29 @@ class CRM_Core_Form extends HTML_QuickForm_Page { $targetField->loadArray($options); } } + + /** + * Validate country / state / county match and suppress unwanted "required" errors + */ + private function validateChainSelectFields() { + foreach ($this->_chainSelectFields as $control => $target) { + $controlValue = (array) $this->getElementValue($control); + $targetField = $this->getElement($target); + $controlType = $targetField->getAttribute('data-callback') == 'civicrm/ajax/jqCounty' ? 'stateProvince' : 'country'; + $targetValue = array_filter((array) $targetField->getValue()); + if ($targetValue || $this->getElementError($target)) { + $options = CRM_Core_BAO_Location::getChainSelectValues($controlValue, $controlType, TRUE); + if ($targetValue) { + if (!array_intersect($targetValue, array_keys($options))) { + $this->setElementError($target, $controlType == 'country' ? ts('State/Province does not match the selected Country') : ts('County does not match the selected State/Province')); + } + } + // Suppress "required" error for field if it has no options + elseif (!$options) { + $this->setElementError($target, NULL); + } + } + } + } } diff --git a/CRM/Core/Payment/Form.php b/CRM/Core/Payment/Form.php index 91a119b989..3ec3edd03b 100644 --- a/CRM/Core/Payment/Form.php +++ b/CRM/Core/Payment/Form.php @@ -107,9 +107,10 @@ class CRM_Core_Payment_Form { $form->_paymentFields["billing_state_province_id-{$bltID}"] = array( 'htmlType' => 'chainSelect', + 'title' => ts('State / Province'), 'name' => "billing_state_province_id-{$bltID}", 'cc_field' => TRUE, - 'is_required' => self::checkRequiredStateProvince($form, "billing_country_id-{$bltID}"), + 'is_required' => TRUE, ); $form->_paymentFields["billing_postal_code-{$bltID}"] = array( @@ -446,48 +447,5 @@ class CRM_Core_Payment_Form { return CRM_Utils_Array::value('Y', $src['credit_card_exp_date']); } - /** - * function to return state/province is_required = true/false - * - * @param obj $form: Form object - * @param string $name: Country index name on $_submitValues array - * @param bool $onBehalf: Is 'On Behalf Of' profile? - * - * @return bool - * TRUE/FALSE for is_required if country consist/not consist of state/province respectively - * @static - */ - static function checkRequiredStateProvince($form, $name, $onBehalf = FALSE) { - // If selected country has possible values for state/province mark the - // state/province field as required. - $config = CRM_Core_Config::singleton(); - $stateProvince = new CRM_Core_DAO_StateProvince(); - - if ($onBehalf) { - $stateProvince->country_id = CRM_Utils_Array::value($name, $form->_submitValues['onbehalf']); - } - else { - $stateProvince->country_id = CRM_Utils_Array::value($name, $form->_submitValues); - } - - $limitCountryId = $stateProvince->country_id; - - if ($stateProvince->count() > 0) { - // check that the state/province data is not excluded by a - // limitation in the localisation settings. - $countryIsoCodes = CRM_Core_PseudoConstant::countryIsoCode(); - $limitCodes = $config->provinceLimit(); - $limitIds = array(); - foreach ($limitCodes as $code) { - $limitIds = array_merge($limitIds, array_keys($countryIsoCodes, $code)); - } - - if ($limitCountryId && in_array($limitCountryId, $limitIds)) { - return TRUE; - } - return FALSE; - } - return FALSE; - } } diff --git a/CRM/Profile/Form.php b/CRM/Profile/Form.php index 3fcc264132..d0d229fa17 100644 --- a/CRM/Profile/Form.php +++ b/CRM/Profile/Form.php @@ -683,21 +683,6 @@ class CRM_Profile_Form extends CRM_Core_Form { return FALSE; } - if (count($this->_submitValues)) { - $locationTypeId = null; - foreach ($this->_fields as $field) { - if (!empty($field['location_type_id'])) { - $locationTypeId = $field['location_type_id']; - } - if (array_key_exists("country-{$locationTypeId}", $this->_fields) && - array_key_exists("state_province-{$locationTypeId}", $this->_fields) && - !empty($this->_submitValues["country-{$locationTypeId}"])) { - $this->_fields["state_province-{$locationTypeId}"]['is_required'] = - CRM_Core_Payment_Form::checkRequiredStateProvince($this, "country-{$locationTypeId}"); - } - } - } - $this->assign('id', $this->_id); $this->assign('mode', $this->_mode); $this->assign('action', $this->_action); -- 2.25.1