From 08fd4b453090fbd681c0adf533f7f52486206934 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 6 Jul 2015 12:50:37 +1200 Subject: [PATCH] CRM-16800 4.7 regression on correct line items for back office membership by credit card --- CRM/Member/BAO/Membership.php | 13 ++-- CRM/Member/Form/Membership.php | 75 +++++++++---------- CRM/Member/Form/MembershipRenewal.php | 4 +- CRM/Member/PseudoConstant.php | 2 +- api/v3/MembershipType.php | 1 + .../CRM/Member/Form/MembershipTest.php | 64 +++++++++++----- .../phpunit/CRM/Member/Form/dataset/data.xml | 13 ---- 7 files changed, 90 insertions(+), 82 deletions(-) diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index a62f68a1ee..6115e4b845 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -311,7 +311,7 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership { $params['membership_id'] = $membership->id; if (isset($ids['membership'])) { $ids['contribution'] = CRM_Core_DAO::getFieldValue('CRM_Member_DAO_MembershipPayment', - $ids['membership'], + $membership->id, 'contribution_id', 'membership_id' ); @@ -2710,14 +2710,14 @@ WHERE civicrm_membership.is_test = 0"; * @param CRM_Core_Form $qf * @param array $membershipType * Array with membership type and organization. - * @param int $priceSetId * + * @return int $priceSetId */ - public static function createLineItems(&$qf, $membershipType, &$priceSetId) { + public static function createLineItems(&$qf, $membershipType) { $qf->_priceSetId = $priceSetId = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', 'default_membership_type_amount', 'id', 'name'); - if ($priceSetId) { - $qf->_priceSet = $priceSets = current(CRM_Price_BAO_PriceSet::getSetDetail($priceSetId)); - } + $qf->_priceSet = $priceSets = current(CRM_Price_BAO_PriceSet::getSetDetail($priceSetId)); + + // The name of the price field corresponds to the membership_type organization contact. $editedFieldParams = array( 'price_set_id' => $priceSetId, 'name' => $membershipType[0], @@ -2744,6 +2744,7 @@ WHERE civicrm_membership.is_test = 0"; $fieldID = key($qf->_priceSet['fields']); $qf->_params['price_' . $fieldID] = CRM_Utils_Array::value('id', $editedResults); + return $priceSetId; } /** diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php index dca7f04c7f..ad744befb6 100644 --- a/CRM/Member/Form/Membership.php +++ b/CRM/Member/Form/Membership.php @@ -1164,6 +1164,11 @@ class CRM_Member_Form_Membership extends CRM_Member_Form { $lineItems = $this->_lineItem; } + if ($this->_id) { + $ids['membership'] = $params['id'] = $this->_id; + } + $ids['userId'] = CRM_Core_Session::singleton()->get('userID'); + // Set variables that we normally get from context. // In form mode these are set in preProcess. //TODO: set memberships, fixme @@ -1207,37 +1212,40 @@ class CRM_Member_Form_Membership extends CRM_Member_Form { } // process price set and get total amount and line items. - $lineItem = array(); - $isQuickConfig = 0; + if (!$priceSetID) { + $priceSetID = CRM_Member_BAO_Membership::createLineItems( + $this, + $formValues['membership_type_id'], + NULL + ); + } + $isQuickConfig = FALSE; + if (CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', $priceSetID, 'is_quick_config')) { + $isQuickConfig = 1; + } $termsByType = array(); - if ($priceSetID) { - CRM_Member_BAO_Membership::createLineItems($this, $formValues['membership_type_id'], $priceSetID); - - if (CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', $priceSetID, 'is_quick_config')) { - $isQuickConfig = 1; - } - CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'], - $this->_params, $lineItem[$priceSetID]); - if (CRM_Utils_Array::value('tax_amount', $this->_params)) { - $params['tax_amount'] = $this->_params['tax_amount']; - } - $params['total_amount'] = CRM_Utils_Array::value('amount', $this->_params); - $submittedFinancialType = CRM_Utils_Array::value('financial_type_id', $formValues); - if (!empty($lineItem[$priceSetID])) { - foreach ($lineItem[$priceSetID] as &$li) { - if (!empty($li['membership_type_id'])) { - if (!empty($li['membership_num_terms'])) { - $termsByType[$li['membership_type_id']] = $li['membership_num_terms']; - } + $lineItem = array(); + CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'], + $this->_params, $lineItem[$priceSetID]); + if (CRM_Utils_Array::value('tax_amount', $this->_params)) { + $params['tax_amount'] = $this->_params['tax_amount']; + } + $params['total_amount'] = CRM_Utils_Array::value('amount', $this->_params); + $submittedFinancialType = CRM_Utils_Array::value('financial_type_id', $formValues); + if (!empty($lineItem[$priceSetID])) { + foreach ($lineItem[$priceSetID] as &$li) { + if (!empty($li['membership_type_id'])) { + if (!empty($li['membership_num_terms'])) { + $termsByType[$li['membership_type_id']] = $li['membership_num_terms']; } + } - ///CRM-11529 for quick config backoffice transactions - //when financial_type_id is passed in form, update the - //lineitems with the financial type selected in form - if ($isQuickConfig && $submittedFinancialType) { - $li['financial_type_id'] = $submittedFinancialType; - } + ///CRM-11529 for quick config backoffice transactions + //when financial_type_id is passed in form, update the + //lineitems with the financial type selected in form + if ($isQuickConfig && $submittedFinancialType) { + $li['financial_type_id'] = $submittedFinancialType; } } } @@ -1303,23 +1311,10 @@ class CRM_Member_Form_Membership extends CRM_Member_Form { if (array_key_exists('max_related', $formValues)) { $membershipTypeValues[$memType]['max_related'] = CRM_Utils_Array::value('max_related', $formValues); } - } - - if ($this->_id) { - $ids['membership'] = $params['id'] = $this->_id; - } - - $ids['userId'] = CRM_Core_Session::singleton()->get('userID'); - - // membership type custom data - foreach ($this->_memTypeSelected as $memType) { $membershipTypeValues[$memType]['custom'] = CRM_Core_BAO_CustomField::postProcess($formValues, $this->_id, 'Membership' ); - } - - foreach ($this->_memTypeSelected as $memType) { $membershipTypes[$memType] = CRM_Core_DAO::getFieldValue('CRM_Member_DAO_MembershipType', $memType ); diff --git a/CRM/Member/Form/MembershipRenewal.php b/CRM/Member/Form/MembershipRenewal.php index 3f29fd4d1e..88c775f85f 100644 --- a/CRM/Member/Form/MembershipRenewal.php +++ b/CRM/Member/Form/MembershipRenewal.php @@ -633,8 +633,8 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form { //create line items $lineItem = array(); - $priceSetId = NULL; - CRM_Member_BAO_Membership::createLineItems($this, $formValues['membership_type_id'], $priceSetId); + + $priceSetId = CRM_Member_BAO_Membership::createLineItems($this, $formValues['membership_type_id']); CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'], $this->_params, $lineItem[$priceSetId] ); diff --git a/CRM/Member/PseudoConstant.php b/CRM/Member/PseudoConstant.php index 4faf4678d1..613013e423 100644 --- a/CRM/Member/PseudoConstant.php +++ b/CRM/Member/PseudoConstant.php @@ -61,7 +61,7 @@ class CRM_Member_PseudoConstant extends CRM_Core_PseudoConstant { * @return array * array reference of all membership types if any */ - public static function &membershipType($id = NULL, $force = FALSE) { + public static function membershipType($id = NULL, $force = TRUE) { if (!self::$membershipType || $force) { CRM_Core_PseudoConstant::populate(self::$membershipType, 'CRM_Member_DAO_MembershipType', diff --git a/api/v3/MembershipType.php b/api/v3/MembershipType.php index ea36118b2e..cb3ab53674 100644 --- a/api/v3/MembershipType.php +++ b/api/v3/MembershipType.php @@ -60,6 +60,7 @@ function _civicrm_api3_membership_type_create_spec(&$params) { $params['name']['api.required'] = 1; $params['duration_unit']['api.required'] = 1; $params['duration_interval']['api.required'] = 1; + $params['is_active']['api.default'] = 1; } /** diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php index e0b30cb080..9c92f40b57 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php @@ -59,6 +59,13 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { protected $_ids = array(); protected $_paymentProcessorID; + /** + * Membership type ID for annual fixed membership. + * + * @var int + */ + protected $membershipTypeAnnualFixedID; + /** * Parameters to create payment processor. * @@ -100,6 +107,19 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { dirname(__FILE__) . '/dataset/data.xml' ) ); + $membershipTypeAnnualFixed = $this->callAPISuccess('membership_type', 'create', array( + 'domain_id' => 1, + 'name' => "AnnualFixed", + 'member_of_contact_id' => 23, + 'duration_unit' => "year", + 'duration_interval' => 1, + 'period_type' => "fixed", + 'fixed_period_start_day' => "101", + 'fixed_period_rollover_day' => "1231", + 'relationship_type_id' => 20, + 'financial_type_id' => 2, + )); + $this->membershipTypeAnnualFixedID = $membershipTypeAnnualFixed['id']; $instruments = $this->callAPISuccess('contribution', 'getoptions', array('field' => 'payment_instrument_id')); $this->paymentInstruments = $instruments['values']; @@ -198,12 +218,8 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $files = array(); $obj = new CRM_Member_Form_Membership(); $rc = $obj->formRule($params, $files, $obj); - $this->assertType('array', $rc, - 'In line ' . __LINE__ - ); - $this->assertTrue(array_key_exists('end_date', $rc), - 'In line ' . __LINE__ - ); + $this->assertType('array', $rc); + $this->assertTrue(array_key_exists('end_date', $rc)); } /** @@ -223,12 +239,8 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $files = array(); $obj = new CRM_Member_Form_Membership(); $rc = $obj->formRule($params, $files, $obj); - $this->assertType('array', $rc, - 'In line ' . __LINE__ - ); - $this->assertTrue(array_key_exists('start_date', $rc), - 'In line ' . __LINE__ - ); + $this->assertType('array', $rc); + $this->assertTrue(array_key_exists('start_date', $rc)); } /** @@ -420,15 +432,16 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { * Test the submit function of the membership form. */ public function testSubmit() { - $form = new CRM_Member_Form_Membership(); + $form = $this->getForm(); + $form->_mode = 'test'; $this->createLoggedInUser(); $params = array( 'cid' => $this->_individualId, 'join_date' => date('m/d/Y', time()), 'start_date' => '', 'end_date' => '', - // This format reflects the 1 being the organisation & the 25 being the type. - 'membership_type_id' => array(1, 25), + // This format reflects the 23 being the organisation & the 25 being the type. + 'membership_type_id' => array(23, $this->membershipTypeAnnualFixedID), 'auto_renew' => '0', 'max_related' => '', 'num_terms' => '1', @@ -444,7 +457,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { 'cvv2' => '123', 'credit_card_exp_date' => array( 'M' => '9', - 'Y' => '2019', // TODO: Future proof + 'Y' => '2024', // TODO: Future proof ), 'credit_card_type' => 'Visa', 'billing_first_name' => 'Test', @@ -456,7 +469,18 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { 'billing_country_id-5' => '1228', ); $form->submit($params); - $this->callAPISuccessGetCount('Membership', array('contact_id' => $this->_individualId), 1); + $membership = $this->callAPISuccessGetSingle('Membership', array('contact_id' => $this->_individualId)); + $this->callAPISuccessGetCount('ContributionRecur', array('contact_id' => $this->_individualId), 0); + $contribution = $this->callAPISuccess('Contribution', 'get', array( + 'contact_id' => $this->_individualId, + 'is_test' => TRUE, + )); + + $this->callAPISuccessGetCount('LineItem', array( + 'entity_id' => $membership['id'], + 'entity_table' => 'civicrm_membership', + 'contribution_id' => $contribution['id'], + ), 1); } /** @@ -466,7 +490,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $form = $this->getForm(); $this->callAPISuccess('MembershipType', 'create', array( - 'id' => 25, + 'id' => $this->membershipTypeAnnualFixedID, 'duration_unit' => 'month', 'duration_interval' => 1, 'auto_renew' => TRUE, @@ -480,8 +504,8 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { 'start_date' => '', 'end_date' => '', 'campaign_id' => '', - // This format reflects the 1 being the organisation & the 25 being the type. - 'membership_type_id' => array(1, 25), + // This format reflects the 23 being the organisation & the 25 being the type. + 'membership_type_id' => array(23, $this->membershipTypeAnnualFixedID), 'auto_renew' => '1', 'is_recur' => 1, 'max_related' => 0, diff --git a/tests/phpunit/CRM/Member/Form/dataset/data.xml b/tests/phpunit/CRM/Member/Form/dataset/data.xml index 098bd0ee89..65a279e954 100644 --- a/tests/phpunit/CRM/Member/Form/dataset/data.xml +++ b/tests/phpunit/CRM/Member/Form/dataset/data.xml @@ -39,19 +39,6 @@ relationship_type_id="20" financial_type_id="2" /> -