From 8cb12ff15b5b001d8cf66e8998e91596d7a7c4f1 Mon Sep 17 00:00:00 2001 From: eileenmcnaugton Date: Sun, 4 Oct 2015 21:28:45 +1300 Subject: [PATCH] CRM-17329 fix over-zealous setting of recurring and add unit test --- CRM/Contribute/Form/Contribution/Main.php | 49 +++++-- CRM/Contribute/Form/ContributionBase.php | 17 ++- .../Contribute/Form/Contribution/MainTest.php | 131 ++++++++++++++++++ 3 files changed, 179 insertions(+), 18 deletions(-) create mode 100644 tests/phpunit/CRM/Contribute/Form/Contribution/MainTest.php diff --git a/CRM/Contribute/Form/Contribution/Main.php b/CRM/Contribute/Form/Contribution/Main.php index ca53823def..efe88d646a 100644 --- a/CRM/Contribute/Form/Contribution/Main.php +++ b/CRM/Contribute/Form/Contribution/Main.php @@ -961,19 +961,34 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu * Process the form submission. */ public function postProcess() { - $config = CRM_Core_Config::singleton(); // we first reset the confirm page so it accepts new values $this->controller->resetPage('Confirm'); // get the submitted form values. $params = $this->controller->exportValues($this->_name); + $this->submit($params); + + if (empty($this->_values['is_confirm_enabled'])) { + $this->skipToThankYouPage(); + } + + } + /** + * Submit function. + * + * This is the guts of the postProcess made also accessible to the test suite. + * + * @param array $params + * Submitted values. + */ + public function submit($params) { //carry campaign from profile. if (array_key_exists('contribution_campaign_id', $params)) { $params['campaign_id'] = $params['contribution_campaign_id']; } - $params['currencyID'] = $config->defaultCurrency; + $params['currencyID'] = CRM_Core_Config::singleton()->defaultCurrency; if (!empty($params['priceSetId'])) { $is_quick_config = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', $this->_priceSetId, 'is_quick_config'); @@ -1025,6 +1040,7 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $params['is_pay_later'] = 0; } + // Would be nice to someday understand the point of this set. $this->set('is_pay_later', $params['is_pay_later']); // assign pay later stuff $this->_params['is_pay_later'] = CRM_Utils_Array::value('is_pay_later', $params, FALSE); @@ -1039,14 +1055,12 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $params['separate_amount'] = $params['amount']; $memFee = NULL; if (!empty($params['selectMembership'])) { - if (!empty($this->_membershipTypeValues)) { - $membershipTypeValues = $this->_membershipTypeValues[$params['selectMembership']]; - } - else { - $membershipTypeValues = CRM_Member_BAO_Membership::buildMembershipTypeValues($this, + if (empty($this->_membershipTypeValues)) { + $this->_membershipTypeValues = CRM_Member_BAO_Membership::buildMembershipTypeValues($this, (array) $params['selectMembership'] ); } + $membershipTypeValues = $this->_membershipTypeValues[$params['selectMembership']]; $memFee = $membershipTypeValues['minimum_fee']; if (!$params['amount'] && !$this->_separateMembershipPayment) { $params['amount'] = $memFee ? $memFee : 0; @@ -1140,9 +1154,12 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $invoiceID = md5(uniqid(rand(), TRUE)); $this->set('invoiceID', $invoiceID); $params['invoiceID'] = $invoiceID; - $params['description'] = ts('Online Contribution') . ': ' . (($this->_pcpInfo['title']) ? $this->_pcpInfo['title'] : $this->_values['title']); + $params['description'] = ts('Online Contribution') . ': ' . ((!empty($this->_pcpInfo['title']) ? $this->_pcpInfo['title'] : $this->_values['title'])); $params['button'] = $this->controller->getButtonName(); // required only if is_monetary and valid positive amount + // @todo it seems impossible for $memFee to be greater than 0 & $params['amount'] not to + // be & by requiring $memFee down here we make it harder to do a sensible refactoring of the function + // above (ie. extract the amount in a small function). if ($this->_values['is_monetary'] && is_array($this->_paymentProcessor) && ((float ) $params['amount'] > 0.0 || $memFee > 0.0) @@ -1162,11 +1179,6 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $this->handlePreApproval($this->_params); } } - - if (empty($this->_values['is_confirm_enabled'])) { - $this->skipToThankYouPage(); - } - } /** @@ -1218,4 +1230,15 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contribute/transact', "_qf_ThankYou_display=1&qfKey=$qfKey", TRUE, NULL, FALSE)); } + /** + * Function for unit tests on the postProcess function. + * + * @param array $params + */ + public function testSubmit($params) { + $_SERVER['REQUEST_METHOD'] = 'GET'; + $this->controller = new CRM_Contribute_Controller_Contribution(); + $this->submit($params); + } + } diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php index 5f1984b3d0..a70f427ee8 100644 --- a/CRM/Contribute/Form/ContributionBase.php +++ b/CRM/Contribute/Form/ContributionBase.php @@ -1198,6 +1198,7 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { /** * Determine if recurring parameters need to be added to the form parameters. + * * - is_recur * - frequency_interval * - frequency_unit @@ -1209,18 +1210,24 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { * Arguably the form should start to build $this->_params in the pre-process main page & use that array consistently throughout. */ protected function setRecurringMembershipParams() { - if (!empty($this->_params['priceSetId']) && !empty($this->_params['selectMembership'])) { + $selectedMembershipTypeID = CRM_Utils_Array::value('selectMembership', $this->_params); + if ($selectedMembershipTypeID) { // @todo the price_x fields will ALWAYS allow us to determine the membership - so we should ignore // 'selectMembership' and calculate from the price_x fields so we have one method that always works // this is lazy & only catches when selectMembership is set, but the worst of all worlds would be to fix // this with an else (calculate for price set). - $membershipTypes = CRM_Price_BAO_PriceSet::getMembershipTypesFromPriceSet($this->_params['priceSetId']); - if (in_array($this->_params['selectMembership'], $membershipTypes['autorenew'])) { + $membershipTypes = CRM_Price_BAO_PriceSet::getMembershipTypesFromPriceSet($this->_priceSetId); + if (in_array($selectedMembershipTypeID, $membershipTypes['autorenew_required']) + || (in_array($selectedMembershipTypeID, $membershipTypes['autorenew_optional']) && + !empty($this->_params['is_recur'])) + ) { $this->_params['auto_renew'] = TRUE; } } - if ((!empty($this->_params['selectMembership']) || !empty($this->_params['priceSetId'])) && !empty($this->_paymentProcessor['is_recur']) && - CRM_Utils_Array::value('auto_renew', $this->_params) && empty($this->_params['is_recur']) && empty($this->_params['frequency_interval']) + if ((!empty($this->_params['selectMembership']) || !empty($this->_params['priceSetId'])) + && !empty($this->_paymentProcessor['is_recur']) && + CRM_Utils_Array::value('auto_renew', $this->_params) + && empty($this->_params['is_recur']) && empty($this->_params['frequency_interval']) ) { $this->_params['is_recur'] = $this->_values['is_recur'] = 1; diff --git a/tests/phpunit/CRM/Contribute/Form/Contribution/MainTest.php b/tests/phpunit/CRM/Contribute/Form/Contribution/MainTest.php new file mode 100644 index 0000000000..cc59f2356e --- /dev/null +++ b/tests/phpunit/CRM/Contribute/Form/Contribution/MainTest.php @@ -0,0 +1,131 @@ +quickCleanUpFinancialEntities(); + } + + /** + * Test that the membership is set to recurring if the membership type is always autorenew. + */ + public function testSetRecurFunction() { + $membershipTypeID = $this->membershipTypeCreate(array('auto_renew' => 2, 'minimum_fee' => 80)); + $form = $this->getContributionForm(); + $form->testSubmit(array( + 'selectMembership' => $membershipTypeID, + )); + $this->assertEquals(1, $form->_params['is_recur']); + } + + /** + * Test that the membership is set to recurring if the membership type is always autorenew. + */ + public function testSetRecurFunctionOptionalYes() { + $membershipTypeID = $this->membershipTypeCreate(array('auto_renew' => 1, 'minimum_fee' => 80)); + $form = $this->getContributionForm(); + $form->testSubmit(array( + 'selectMembership' => $membershipTypeID, + 'is_recur' => 1, + )); + $this->assertEquals(1, $form->_params['is_recur']); + } + + /** + * Test that the membership is set to recurring if the membership type is always autorenew. + */ + public function testSetRecurFunctionOptionalNo() { + $membershipTypeID = $this->membershipTypeCreate(array('auto_renew' => 1, 'minimum_fee' => 80)); + $form = $this->getContributionForm(); + $form->testSubmit(array( + 'selectMembership' => $membershipTypeID, + 'is_recur' => 0, + )); + $this->assertEquals(0, $form->_params['is_recur']); + } + + /** + * Test that the membership is set to recurring if the membership type is always autorenew. + */ + public function testSetRecurFunctionNotAvailable() { + $membershipTypeID = $this->membershipTypeCreate(array('auto_renew' => 0, 'minimum_fee' => 80)); + $form = $this->getContributionForm(); + $form->testSubmit(array( + 'selectMembership' => $membershipTypeID, + )); + $this->assertArrayNotHasKey('is_recur', $form->_params); + } + + /** + * Get a contribution form object for testing. + * + * @return \CRM_Contribute_Form_Contribution_Main + */ + protected function getContributionForm() { + $form = new CRM_Contribute_Form_Contribution_Main(); + $form->_values['is_monetary'] = 1; + $form->_values['is_pay_later'] = 0; + $form->_priceSetId = $this->callAPISuccessGetValue('PriceSet', array( + 'name' => 'default_membership_type_amount', + 'return' => 'id', + )); + $priceFields = $this->callAPISuccess('PriceField', 'get', array('id' => $form->_priceSetId)); + $form->_priceSet['fields'] = $priceFields['values']; + $paymentProcessorID = $this->paymentProcessorCreate(array('payment_processor_type_id' => 'Dummy')); + $form->_paymentProcessor = array( + 'billing_mode' => CRM_Core_Payment::BILLING_MODE_FORM, + 'object' => Civi\Payment\System::singleton()->getById($paymentProcessorID), + 'is_recur' => TRUE, + ); + $form->_values = array( + 'title' => "Test Contribution Page", + 'financial_type_id' => 1, + 'currency' => 'NZD', + 'goal_amount' => 6000, + 'is_pay_later' => 1, + 'is_monetary' => TRUE, + 'pay_later_text' => 'Front up', + 'pay_later_receipt' => 'Ta', + ); + return $form; + } + +} -- 2.25.1