From cebff547e44151661362e001d01007b09f727d31 Mon Sep 17 00:00:00 2001 From: Pradeep Nayak Date: Tue, 21 Feb 2017 18:49:53 +0530 Subject: [PATCH] CRM-20385 : IIDA-72 oversensitive deferred revenue account validation --- CRM/Admin/Form/Preferences/Contribute.php | 28 ----- CRM/Contribute/BAO/Contribution.php | 2 - CRM/Event/BAO/Event.php | 5 - CRM/Event/Form/ManageEvent/Fee.php | 12 +- CRM/Financial/BAO/FinancialAccount.php | 108 +----------------- CRM/Member/BAO/MembershipType.php | 7 -- CRM/Member/Form/MembershipType.php | 13 +-- CRM/Price/BAO/PriceFieldValue.php | 7 -- CRM/Price/BAO/PriceSet.php | 7 -- CRM/Price/Form/Field.php | 16 --- CRM/Price/Form/Option.php | 7 -- CRM/Price/Form/Set.php | 12 -- templates/CRM/Event/Form/ManageEvent/Fee.tpl | 3 +- templates/CRM/Member/Form/MembershipType.tpl | 1 + .../CRM/common/deferredFinancialType.tpl | 47 ++++++++ .../Financial/BAO/FinancialAccountTest.php | 70 +----------- 16 files changed, 61 insertions(+), 284 deletions(-) create mode 100644 templates/CRM/common/deferredFinancialType.tpl diff --git a/CRM/Admin/Form/Preferences/Contribute.php b/CRM/Admin/Form/Preferences/Contribute.php index e07b876a3f..1c26bc51a2 100644 --- a/CRM/Admin/Form/Preferences/Contribute.php +++ b/CRM/Admin/Form/Preferences/Contribute.php @@ -166,34 +166,6 @@ class CRM_Admin_Form_Preferences_Contribute extends CRM_Admin_Form_Preferences { } $this->assign('htmlFields', $htmlFields); parent::buildQuickForm(); - $this->addFormRule(array('CRM_Admin_Form_Preferences_Contribute', 'formRule'), $this); - } - - /** - * Global validation rules for the form. - * - * @param array $values - * posted values of the form - * @param $files - * @param $self - * - * @return array - * list of errors to be posted back to the form - */ - public static function formRule($values, $files, $self) { - $errors = array(); - if (CRM_Utils_Array::value('deferred_revenue_enabled', $values)) { - $errorMessage = CRM_Financial_BAO_FinancialAccount::validateTogglingDeferredRevenue(); - if ($errorMessage) { - // Since the error msg is too long and - // takes the whole space to display inline - // therefore setting blank text to highlight the field - // setting actual error msg to _qf_default to show in pop-up screen - $errors['deferred_revenue_enabled'] = ' '; - $errors['_qf_default'] = $errorMessage; - } - } - return $errors; } /** diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index a259fb270c..04abf8642a 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -193,8 +193,6 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { $params['prevContribution'] = self::getOriginalContribution($contributionID); } - // CRM-16189 - CRM_Financial_BAO_FinancialAccount::checkFinancialTypeHasDeferred($params, $contributionID); if ($contributionID && !empty($params['revenue_recognition_date']) && !empty($params['prevContribution']) && !($contributionStatus[$params['prevContribution']->contribution_status_id] == 'Pending') && !self::allowUpdateRevenueRecognitionDate($contributionID) diff --git a/CRM/Event/BAO/Event.php b/CRM/Event/BAO/Event.php index cd9d8294db..3affafcfd5 100644 --- a/CRM/Event/BAO/Event.php +++ b/CRM/Event/BAO/Event.php @@ -95,11 +95,6 @@ class CRM_Event_BAO_Event extends CRM_Event_DAO_Event { CRM_Utils_Hook::pre('create', 'Event', NULL, $params); } - // CRM-16189 - if (!empty($params['financial_type_id'])) { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($params['financial_type_id']); - } - $event = new CRM_Event_DAO_Event(); $event->copyValues($params); diff --git a/CRM/Event/Form/ManageEvent/Fee.php b/CRM/Event/Form/ManageEvent/Fee.php index 4ab1176e7f..beaeb885bf 100644 --- a/CRM/Event/Form/ManageEvent/Fee.php +++ b/CRM/Event/Form/ManageEvent/Fee.php @@ -386,7 +386,10 @@ class CRM_Event_Form_ManageEvent_Fee extends CRM_Event_Form_ManageEvent { $this->addElement('submit', $this->getButtonName('submit'), ts('Add Discount Set to Fee Table'), array('class' => 'crm-form-submit cancel') ); - + if (CRM_Contribute_BAO_Contribution::checkContributeSettings('deferred_revenue_enabled')) { + $deferredFinancialType = CRM_Financial_BAO_FinancialAccount::getDeferredFinancialType(); + $this->assign('deferredFinancialType', array_keys($deferredFinancialType)); + } $this->buildAmountLabel(); parent::buildQuickForm(); } @@ -524,13 +527,6 @@ class CRM_Event_Form_ManageEvent_Fee extends CRM_Event_Form_ManageEvent { } } } - // CRM-16189 - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($values['financial_type_id']); - } - catch (CRM_Core_Exception $e) { - $errors['financial_type_id'] = $e->getMessage(); - } return empty($errors) ? TRUE : $errors; } diff --git a/CRM/Financial/BAO/FinancialAccount.php b/CRM/Financial/BAO/FinancialAccount.php index 182865e66c..089d225a89 100644 --- a/CRM/Financial/BAO/FinancialAccount.php +++ b/CRM/Financial/BAO/FinancialAccount.php @@ -426,118 +426,12 @@ LIMIT 1"; } if ($isError) { - $error = ts('Revenue recognition date can only be specified if the financial type selected has a deferred revenue account configured. Please have an administrator set up the deferred revenue account at Administer > CiviContribute > Financial Accounts, then configure it for financial types at Administer > CiviContribution > Financial Types, Accounts'); + $error = ts('Revenue Recognition Date cannot be processed unless there is a Deferred Revenue account setup for the Financial Type. Please remove Revenue Recognition Date, select a different Financial Type with a Deferred Revenue account setup for it, or setup a Deferred Revenue account for this Financial Type.'); throw new CRM_Core_Exception($error); } return $isError; } - /** - * Check if financial type has Deferred Revenue Account is relationship - * with Financial Account. - * - * @param int $financialTypeId - * Financial Type Id. - * - * @param int $entityID - * Holds id for PriceSet/PriceField/PriceFieldValue. - * - * @param string $entity - * Entity like PriceSet/PriceField/PriceFieldValue. - * - * @return bool - * - */ - public static function validateFinancialType($financialTypeId, $entityID = NULL, $entity = NULL) { - if (!CRM_Contribute_BAO_Contribution::checkContributeSettings('deferred_revenue_enabled')) { - return FALSE; - } - if ($entityID) { - $query = ' SELECT ps.extends FROM civicrm_price_set ps'; - $params = array( - 1 => array('ps', 'Text'), - 2 => array($entityID, 'Integer'), - ); - if ($entity == 'PriceField') { - $params[1] = array('pf', 'Text'); - $query .= ' INNER JOIN civicrm_price_field pf ON pf.price_set_id = ps.id '; - } - $query .= ' WHERE %1.id = %2'; - $extends = CRM_Core_DAO::singleValueQuery($query, $params); - $extends = explode('', $extends); - if (!(in_array(CRM_Core_Component::getComponentID('CiviEvent'), $extends) - || in_array(CRM_Core_Component::getComponentID('CiviMember'), $extends)) - ) { - return FALSE; - } - } - $deferredFinancialType = self::getDeferredFinancialType(); - if (!array_key_exists($financialTypeId, $deferredFinancialType)) { - throw new CRM_Core_Exception(ts('Deferred revenue account is not configured for selected financial type. Please have an administrator set up the deferred revenue account at Administer > CiviContribute > Financial Accounts, then configure it for financial types at Administer > CiviContribution > Financial Types, Accounts')); - } - return FALSE; - } - - /** - * Validate if Deferred Account is set for Financial Type - * when Deferred Revenue is enabled - * - * @return string - * - */ - public static function validateTogglingDeferredRevenue() { - $deferredFinancialType = self::getDeferredFinancialType(); - $message = ts('Before Deferred Revenue can be enabled, a Deferred Revenue Account relationship must be defined for all financial types currently used for Memberships and Events, including - -In other words, please create deferred revenue accounts at Administer > CiviContribute > Financial Accounts, then configure them for the following financial types at Administer > CiviContribute > Financial Types, accounts:'); - $tables = array( - 'civicrm_membership_type', - 'civicrm_event', - 'civicrm_price_set', - 'civicrm_price_field_value', - ); - $params[2] = array('', 'Text'); - if (!empty($deferredFinancialType)) { - $params[2] = array(' AND financial_type_id NOT IN (' . implode(',', array_keys($deferredFinancialType)) . ') ', 'Text'); - } - $query_1 = 'SELECT %5.id FROM %4 WHERE %5.is_active = 1'; - $query_2 = $query_1 . ' %2'; - foreach ($tables as $table) { - $params[4] = array($table, 'Text'); - $params[5] = array($table, 'Text'); - $dao = CRM_Core_DAO::executeQuery($query_1, $params); - if ($dao->N) { - if (in_array($table, array('civicrm_price_set', 'civicrm_price_field_value'))) { - $query_2 .= " AND civicrm_price_set.name NOT IN ('default_contribution_amount', 'default_membership_type_amount') AND (civicrm_price_set.extends LIKE '%1%' OR civicrm_price_set.extends like '3')"; - if ($table == 'civicrm_price_field_value') { - $string = $table . ' INNER JOIN civicrm_price_field ON civicrm_price_field.id = civicrm_price_field_value.price_field_id INNER JOIN civicrm_price_set ON civicrm_price_set.id = civicrm_price_field.price_set_id '; - $params[4] = array($string, 'Text'); - $params[2][0] = str_replace('financial_type_id', "{$table}.financial_type_id", $params[2][0]); - } - } - $dao = CRM_Core_DAO::executeQuery($query_2, $params); - if ($dao->N) { - $message .= ''; - return $message; - } - } - } - return NULL; - } - /** * Retrieve all Deferred Financial Accounts. * diff --git a/CRM/Member/BAO/MembershipType.php b/CRM/Member/BAO/MembershipType.php index 74887ed0ac..d65d2fe209 100644 --- a/CRM/Member/BAO/MembershipType.php +++ b/CRM/Member/BAO/MembershipType.php @@ -106,13 +106,6 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType { } } - // CRM-16189 - if (!empty($params['financial_type_id'])) { - CRM_Financial_BAO_FinancialAccount::validateFinancialType( - $params['financial_type_id'] - ); - } - // action is taken depending upon the mode $membershipType = new CRM_Member_DAO_MembershipType(); $membershipType->copyValues($params); diff --git a/CRM/Member/Form/MembershipType.php b/CRM/Member/Form/MembershipType.php index 067f1b3c25..026f7ab70e 100644 --- a/CRM/Member/Form/MembershipType.php +++ b/CRM/Member/Form/MembershipType.php @@ -204,6 +204,11 @@ class CRM_Member_Form_MembershipType extends CRM_Member_Form_MembershipConfig { $this->addFormRule(array('CRM_Member_Form_MembershipType', 'formRule')); $this->assign('membershipTypeId', $this->_id); + + if (CRM_Contribute_BAO_Contribution::checkContributeSettings('deferred_revenue_enabled')) { + $deferredFinancialType = CRM_Financial_BAO_FinancialAccount::getDeferredFinancialType(); + $this->assign('deferredFinancialType', array_keys($deferredFinancialType)); + } } /** @@ -282,14 +287,6 @@ class CRM_Member_Form_MembershipType extends CRM_Member_Form_MembershipConfig { } } - // CRM-16189 - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($params['financial_type_id']); - } - catch (CRM_Core_Exception $e) { - $errors['financial_type_id'] = $e->getMessage(); - } - return empty($errors) ? TRUE : $errors; } diff --git a/CRM/Price/BAO/PriceFieldValue.php b/CRM/Price/BAO/PriceFieldValue.php index f6b8b3687b..186c4326c8 100644 --- a/CRM/Price/BAO/PriceFieldValue.php +++ b/CRM/Price/BAO/PriceFieldValue.php @@ -64,13 +64,6 @@ class CRM_Price_BAO_PriceFieldValue extends CRM_Price_DAO_PriceFieldValue { if (!$priceFieldID) { $priceFieldID = CRM_Core_DAO::getFieldValue('CRM_Price_BAO_PriceFieldValue', $id, 'price_field_id'); } - if (!empty($params['financial_type_id'])) { - CRM_Financial_BAO_FinancialAccount::validateFinancialType( - $params['financial_type_id'], - $priceFieldID, - 'PriceField' - ); - } if (!empty($params['is_default'])) { $query = 'UPDATE civicrm_price_field_value SET is_default = 0 WHERE price_field_id = %1'; $p = array(1 => array($params['price_field_id'], 'Integer')); diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php index 531372bc6f..d59c50faca 100644 --- a/CRM/Price/BAO/PriceSet.php +++ b/CRM/Price/BAO/PriceSet.php @@ -76,13 +76,6 @@ class CRM_Price_BAO_PriceSet extends CRM_Price_DAO_PriceSet { else { $priceSetID = CRM_Utils_Array::value('id', $params); } - // CRM-16189 - if ($validatePriceSet && !empty($params['financial_type_id'])) { - CRM_Financial_BAO_FinancialAccount::validateFinancialType( - $params['financial_type_id'], - $priceSetID - ); - } $priceSetBAO = new CRM_Price_BAO_PriceSet(); $priceSetBAO->copyValues($params); if (self::eventPriceSetDomainID()) { diff --git a/CRM/Price/Form/Field.php b/CRM/Price/Form/Field.php index 51f7d3fd05..f17e11ad16 100644 --- a/CRM/Price/Form/Field.php +++ b/CRM/Price/Form/Field.php @@ -404,15 +404,6 @@ class CRM_Price_Form_Field extends CRM_Core_Form { if ($fields['financial_type_id'] == '') { $errors['financial_type_id'] = ts('Financial Type is a required field'); } - else { - // CRM-16189 - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($fields['financial_type_id'], $form->_sid); - } - catch (CRM_Core_Exception $e) { - $errors['financial_type_id'] = $e->getMessage(); - } - } } //avoid the same price field label in Within PriceSet @@ -529,13 +520,6 @@ class CRM_Price_Form_Field extends CRM_Core_Form { } $_flagOption = $_emptyRow = 0; - // CRM-16189 - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($fields['option_financial_type_id'][$index], $form->_fid, 'PriceField'); - } - catch(CRM_Core_Exception $e) { - $errors["option_financial_type_id[{$index}]"] = $e->getMessage(); - } } if (!empty($memTypesIDS)) { diff --git a/CRM/Price/Form/Option.php b/CRM/Price/Form/Option.php index 99126381c8..0bd160b25c 100644 --- a/CRM/Price/Form/Option.php +++ b/CRM/Price/Form/Option.php @@ -288,13 +288,6 @@ class CRM_Price_Form_Option extends CRM_Core_Form { ) { $errors['count'] = ts('Participant count can not be greater than max participants.'); } - // CRM-16189 - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($fields['financial_type_id'], $form->_fid, 'PriceField'); - } - catch (CRM_Core_Exception $e) { - $errors['financial_type_id'] = $e->getMessage(); - } return empty($errors) ? TRUE : $errors; } diff --git a/CRM/Price/Form/Set.php b/CRM/Price/Form/Set.php index a212750250..0169689aa7 100644 --- a/CRM/Price/Form/Set.php +++ b/CRM/Price/Form/Set.php @@ -102,18 +102,6 @@ class CRM_Price_Form_Set extends CRM_Core_Form { if ($asciiValue >= 48 && $asciiValue <= 57) { $errors['title'] = ts("Name cannot not start with a digit"); } - // CRM-16189 - if (!empty($fields['extends']) - && (array_key_exists(CRM_Core_Component::getComponentID('CiviEvent'), $fields['extends']) - || array_key_exists(CRM_Core_Component::getComponentID('CiviMember'), $fields['extends'])) - ) { - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($fields['financial_type_id']); - } - catch (CRM_Core_Exception $e) { - $errors['financial_type_id'] = $e->getMessage(); - } - } return empty($errors) ? TRUE : $errors; } diff --git a/templates/CRM/Event/Form/ManageEvent/Fee.tpl b/templates/CRM/Event/Form/ManageEvent/Fee.tpl index b3307236db..6ba2f04cf6 100644 --- a/templates/CRM/Event/Form/ManageEvent/Fee.tpl +++ b/templates/CRM/Event/Form/ManageEvent/Fee.tpl @@ -247,6 +247,7 @@ +{include file="CRM/common/deferredFinancialType.tpl" context='Event'} {include file="CRM/common/showHide.tpl"} {/literal} -{/if} +{/if} \ No newline at end of file diff --git a/templates/CRM/Member/Form/MembershipType.tpl b/templates/CRM/Member/Form/MembershipType.tpl index ed4c88818c..a01c610759 100644 --- a/templates/CRM/Member/Form/MembershipType.tpl +++ b/templates/CRM/Member/Form/MembershipType.tpl @@ -156,6 +156,7 @@ +{include file="CRM/common/deferredFinancialType.tpl" context='MembershipType'} {literal} +{/literal} +{/if} diff --git a/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php b/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php index c1b6a2416a..ae06581e4d 100644 --- a/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php +++ b/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php @@ -331,78 +331,10 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase { $this->fail("Missed expected exception"); } catch (CRM_Core_Exception $e) { - $this->assertEquals('Revenue recognition date can only be specified if the financial type selected has a deferred revenue account configured. Please have an administrator set up the deferred revenue account at Administer > CiviContribute > Financial Accounts, then configure it for financial types at Administer > CiviContribution > Financial Types, Accounts', $e->getMessage()); + $this->assertEquals('Revenue Recognition Date cannot be processed unless there is a Deferred Revenue account setup for the Financial Type. Please remove Revenue Recognition Date, select a different Financial Type with a Deferred Revenue account setup for it, or setup a Deferred Revenue account for this Financial Type.', $e->getMessage()); } } - /** - * Test if financial type has Deferred Revenue Account is relationship with Financial Account. - * - */ - public function testValidateFinancialType() { - Civi::settings()->set('contribution_invoice_settings', array('deferred_revenue_enabled' => '1')); - $financialTypes = CRM_Contribute_PseudoConstant::financialType(); - foreach ($financialTypes as $key => $value) { - try { - CRM_Financial_BAO_FinancialAccount::validateFinancialType($key); - if (!in_array($value, array('Member Dues', 'Event Fee'))) { - $this->fail("Missed expected exception"); - } - } - catch (CRM_Core_Exception $e) { - if (in_array($value, array('Member Dues', 'Event Fees'))) { - $this->fail("Should not call exception"); - } - else { - $this->assertEquals('Deferred revenue account is not configured for selected financial type. Please have an administrator set up the deferred revenue account at Administer > CiviContribute > Financial Accounts, then configure it for financial types at Administer > CiviContribution > Financial Types, Accounts', $e->getMessage()); - } - } - } - } - - /** - * Test Validate if Deferred Account is set for Financial Type. - */ - public function testValidateTogglingDeferredRevenue() { - $orgContactID = $this->organizationCreate(); - - //create relationship - $params = array( - 'name_a_b' => 'Relation 1', - 'name_b_a' => 'Relation 2', - 'contact_type_a' => 'Individual', - 'contact_type_b' => 'Organization', - 'is_reserved' => 1, - 'is_active' => 1, - ); - $relationshipTypeId = $this->relationshipTypeCreate($params); - $ids = array(); - $params = array( - 'name' => 'test type', - 'domain_id' => 1, - 'description' => NULL, - 'minimum_fee' => 10, - 'duration_unit' => 'year', - 'member_of_contact_id' => $orgContactID, - 'relationship_type_id' => $relationshipTypeId, - 'period_type' => 'fixed', - 'duration_interval' => 1, - 'financial_type_id' => 1, - 'visibility' => 'Public', - 'is_active' => 1, - ); - - $membershipType = CRM_Member_BAO_MembershipType::add($params, $ids); - - $membership = $this->assertDBNotNull('CRM_Member_BAO_MembershipType', $orgContactID, - 'name', 'member_of_contact_id', - 'Database check on updated membership record.' - ); - $error = CRM_Financial_BAO_FinancialAccount::validateTogglingDeferredRevenue(); - $this->assertTrue(!empty($error), "Error message did not appear"); - $this->membershipTypeDelete(array('id' => $membershipType->id)); - } - /** * Test testGetAllDeferredFinancialAccount. */ -- 2.25.1