From: eileen Date: Sat, 5 Sep 2020 01:30:43 +0000 (+1200) Subject: [REF] Code cleanup on membership renewal & test X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=0c1281235ead0ba3767780c13d159199b26b2d3d;p=civicrm-core.git [REF] Code cleanup on membership renewal & test This wound up being mainly a clean up of the test class & the addition of a test on changing membership type. However, there is a minor code consolidation which makes 3 actual changes 1) if we are doing a test renewal we shouldn't make the membership test 2) even when renewing a Pending membership we should save any submitted custom data 3) modified id is also set for pending (although future test work might indicate this can go --- diff --git a/CRM/Member/Form/MembershipRenewal.php b/CRM/Member/Form/MembershipRenewal.php index 162ca1cc61..e7ea294229 100644 --- a/CRM/Member/Form/MembershipRenewal.php +++ b/CRM/Member/Form/MembershipRenewal.php @@ -750,7 +750,19 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form { $allStatus = CRM_Member_PseudoConstant::membershipStatus(); $ids = []; $currentMembership = civicrm_api3('Membership', 'getsingle', ['id' => $membershipID]); - + $memParams = [ + 'id' => $currentMembership['id'], + 'membership_type_id' => $membershipTypeID, + 'join_date' => $currentMembership['join_date'], + 'modified_id' => $contactID, + 'custom' => $customFieldsFormatted, + 'membership_activity_status' => ($pending || $isPayLater) ? 'Scheduled' : 'Completed', + // Since we are renewing, make status override false. + 'is_override' => FALSE, + ]; + if ($contributionRecurID) { + $memParams['contribution_recur_id'] = $contributionRecurID; + } // Do NOT do anything. //1. membership with status : PENDING/CANCELLED (CRM-2395) //2. Paylater/IPN renew. CRM-4556. @@ -759,18 +771,11 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form { // CRM-15475 array_search('Cancelled', CRM_Member_PseudoConstant::membershipStatus(NULL, " name = 'Cancelled' ", 'name', FALSE, TRUE)), ])) { - $memParams = [ - 'id' => $currentMembership['id'], + $memParams = array_merge($memParams, [ 'status_id' => $currentMembership['status_id'], 'start_date' => $currentMembership['start_date'], 'end_date' => $currentMembership['end_date'], - 'join_date' => $currentMembership['join_date'], - 'membership_type_id' => $membershipTypeID, - 'membership_activity_status' => ($pending || $isPayLater) ? 'Scheduled' : 'Completed', - ]; - if ($contributionRecurID) { - $memParams['contribution_recur_id'] = $contributionRecurID; - } + ]); return CRM_Member_BAO_Membership::create($memParams); } @@ -785,18 +790,11 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form { $membershipTypeID, $numRenewTerms ); - $memParams = [ - 'membership_type_id' => $membershipTypeID, + $memParams = array_merge($memParams, [ 'end_date' => $dates['end_date'] ?? NULL, - 'join_date' => $currentMembership['join_date'], 'start_date' => $isMembershipCurrent ? $currentMembership['start_date'] : ($dates['start_date'] ?? NULL), - 'id' => $currentMembership['id'], - 'is_test' => $is_test, - // Since we are renewing, make status override false. - 'is_override' => FALSE, - 'modified_id' => $contactID, 'log_start_date' => $dates['log_start_date'], - ]; + ]); // Now Renew the membership if ($isMembershipCurrent) { @@ -804,16 +802,8 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form { if (!empty($currentMembership['id'])) { $ids['membership'] = $currentMembership['id']; } - $memParams['membership_activity_status'] = ($pending || $isPayLater) ? 'Scheduled' : 'Completed'; - } - - // Putting this in an IF is precautionary as it seems likely that it would be ignored if empty, but - // perhaps shouldn't be? - if ($contributionRecurID) { - $memParams['contribution_recur_id'] = $contributionRecurID; } - $memParams['custom'] = $customFieldsFormatted; // @todo stop passing $ids (membership and userId may be set by this point) $membership = CRM_Member_BAO_Membership::create($memParams, $ids); diff --git a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php index 1b8fe6b3ce..f13fdf4c52 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php @@ -77,12 +77,11 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $this->_individualId = $this->individualCreate(); $this->_paymentProcessorID = $this->processorCreate(); $this->financialTypeID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Member Dues'); - - $this->loadXMLDataSet(dirname(__FILE__) . '/dataset/data.xml'); + $this->ids['contact']['organization'] = $this->organizationCreate(); $this->membershipTypeAnnualFixedID = $this->callAPISuccess('membership_type', 'create', [ 'domain_id' => 1, 'name' => 'AnnualFixed', - 'member_of_contact_id' => 23, + 'member_of_contact_id' => $this->ids['contact']['organization'], 'duration_unit' => 'year', 'duration_interval' => 1, 'period_type' => 'fixed', @@ -116,16 +115,13 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $this->quickCleanup( [ 'civicrm_relationship', - 'civicrm_membership_type', - 'civicrm_membership', 'civicrm_uf_match', 'civicrm_address', ] ); - foreach ([17, 18, 23, 32] as $contactID) { + foreach ($this->ids['contact'] as $contactID) { $this->callAPISuccess('contact', 'delete', ['id' => $contactID, 'skip_undelete' => TRUE]); } - $this->callAPISuccess('relationship_type', 'delete', ['id' => 20]); } /** @@ -137,43 +133,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { public function testSubmit() { $form = $this->getForm(); $this->createLoggedInUser(); - $params = [ - 'cid' => $this->_individualId, - 'join_date' => date('m/d/Y', time()), - 'start_date' => '', - 'end_date' => '', - // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], - 'auto_renew' => '0', - 'num_terms' => '1', - 'source' => '', - 'total_amount' => '50.00', - //Member dues, see data.xml - 'financial_type_id' => '2', - 'soft_credit_type_id' => '', - 'soft_credit_contact_id' => '', - 'from_email_address' => '"Demonstrators Anonymous" ', - 'receipt_text_signup' => 'Thank you text', - 'payment_processor_id' => $this->_paymentProcessorID, - 'credit_card_number' => '4111111111111111', - 'cvv2' => '123', - 'credit_card_exp_date' => [ - 'M' => '9', - // TODO: Future proof - 'Y' => '2024', - ], - 'credit_card_type' => 'Visa', - 'billing_first_name' => 'Test', - 'billing_middlename' => 'Last', - 'billing_street_address-5' => '10 Test St', - 'billing_city-5' => 'Test', - 'billing_state_province_id-5' => '1003', - 'billing_postal_code-5' => '90210', - 'billing_country_id-5' => '1228', - ]; + $params = $this->getBaseSubmitParams(); $form->_contactID = $this->_individualId; - $form->testSubmit($params); + $form->testSubmit(array_merge($params, ['total_amount' => 50])); $form->setRenewalMessage(); $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]); $this->callAPISuccessGetCount('ContributionRecur', ['contact_id' => $this->_individualId], 0); @@ -216,42 +179,41 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $this->enableTaxAndInvoicing(); $this->relationForFinancialTypeWithFinancialAccount($this->financialTypeID); $form = $this->getForm(); - - $form->testSubmit([ - 'cid' => $this->_individualId, - 'join_date' => date('Y-m-d'), - 'start_date' => '', - 'end_date' => '', - // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], - 'auto_renew' => '0', - 'num_terms' => '1', - 'source' => '', + $form->testSubmit(array_merge($this->getBaseSubmitParams(), [ 'total_amount' => '50.00', - 'financial_type_id' => $this->financialTypeID, - 'from_email_address' => '"Demonstrators Anonymous" ', - 'receipt_text_signup' => 'Thank you text', - 'payment_processor_id' => $this->_paymentProcessorID, - 'credit_card_number' => '4111111111111111', - 'cvv2' => '123', - 'credit_card_exp_date' => [ - 'M' => '9', - 'Y' => date('Y') + 2, - ], - 'credit_card_type' => 'Visa', - 'billing_first_name' => 'Test', - 'billing_middle_name' => 'Last', - 'billing_street_address-5' => '10 Test St', - 'billing_city-5' => 'Test', - 'billing_state_province_id-5' => '1003', - 'billing_postal_code-5' => '90210', - 'billing_country_id-5' => '1228', - ]); + ])); $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $this->_individualId, 'is_test' => TRUE, 'return' => ['total_amount', 'tax_amount']]); $this->assertEquals(50, $contribution['total_amount']); $this->assertEquals(4.55, $contribution['tax_amount']); } + /** + * Test the submit function of the membership form. + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testSubmitChangeType() { + $form = $this->getForm(); + $this->createLoggedInUser(); + $membershipBefore = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]); + $newMembershipTypeID = $this->callAPISuccess('MembershipType', 'create', [ + 'name' => 'Monthly', + 'member_of_contact_id' => $this->ids['contact']['organization'], + 'financial_type_id' => $this->financialTypeID, + 'duration_unit' => 'month', + 'duration_interval' => 2, + 'period_type' => 'rolling', + ])['id']; + $form->_contactID = $this->_individualId; + + $form->testSubmit(array_merge($this->getBaseSubmitParams(), ['membership_type_id' => [$this->ids['contact']['organization'], $newMembershipTypeID]])); + $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]); + $this->assertEquals($newMembershipTypeID, $membership['membership_type_id']); + // The date (31 Dec this year) should be progressed by 2 months to 28 Dec next year. + $this->assertEquals(date('Y', strtotime($membershipBefore['end_date'])) + 1 . '-02-28', $membership['end_date']); + } + /** * Test the submit function of the membership form. * @@ -277,7 +239,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { 'end_date' => '', 'campaign_id' => '', // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], + 'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID], 'auto_renew' => '1', 'is_recur' => 1, 'num_terms' => '1', @@ -285,8 +247,6 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { 'total_amount' => '77.00', //Member dues, see data.xml 'financial_type_id' => '2', - 'soft_credit_type_id' => 11, - 'soft_credit_contact_id' => '', 'from_email_address' => '"Demonstrators Anonymous" ', 'receipt_text' => 'Thank you text', 'payment_processor_id' => $this->_paymentProcessorID, @@ -373,7 +333,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $form->preProcess(); $form->_contactID = $this->_individualId; - $params = $this->getBaseSubmitParams(); + $params = array_merge($this->getBaseSubmitParams(), ['is_recur' => 1, 'auto_renew' => '1']); $form->_mode = 'test'; $form->testSubmit($params); @@ -459,11 +419,9 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $form->preProcess(); $form->_contactID = $this->_individualId; - $params = $this->getBaseSubmitParams(); - $params['send_receipt'] = 1; $form->_mode = 'test'; - $form->testSubmit($params); + $form->testSubmit(array_merge($this->getBaseSubmitParams(), ['is_recur' => 1, 'send_receipt' => 1, 'auto_renew' => 1])); $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', ['contact_id' => $this->_individualId]); $this->assertEquals(1, $contributionRecur['is_email_receipt']); $this->mut->checkMailLog([ @@ -489,14 +447,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { 'start_date' => '', 'end_date' => '', // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], + 'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID], 'auto_renew' => '0', 'num_terms' => '2', 'total_amount' => '50.00', //Member dues, see data.xml 'financial_type_id' => '2', - 'soft_credit_type_id' => '', - 'soft_credit_contact_id' => '', 'payment_instrument_id' => 4, 'from_email_address' => '"Demonstrators Anonymous" ', 'receipt_text_signup' => 'Thank you text', @@ -537,18 +493,15 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $originalMembership = $this->callAPISuccessGetSingle('membership', []); $params = [ 'cid' => $this->_individualId, - 'join_date' => date('m/d/Y', time()), 'start_date' => '', 'end_date' => '', - // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], + // This format reflects the first value being the organisation & the second being the type. + 'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID], 'auto_renew' => '0', 'num_terms' => '2', 'total_amount' => '50.00', //Member dues, see data.xml 'financial_type_id' => '2', - 'soft_credit_type_id' => '', - 'soft_credit_contact_id' => '', 'payment_instrument_id' => 4, 'from_email_address' => '"Demonstrators Anonymous" ', 'receipt_text_signup' => 'Thank you text', @@ -605,14 +558,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { 'start_date' => '', 'end_date' => '', // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], + 'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID], 'auto_renew' => '0', 'num_terms' => '2', 'total_amount' => '50.00', //Member dues, see data.xml 'financial_type_id' => '2', - 'soft_credit_type_id' => '', - 'soft_credit_contact_id' => '', 'payment_instrument_id' => 4, 'from_email_address' => '"Demonstrators Anonymous" ', 'receipt_text_signup' => 'Thank you text', @@ -673,21 +624,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { protected function getBaseSubmitParams() { return [ 'cid' => $this->_individualId, - 'price_set_id' => 0, - 'join_date' => date('Y-m-d'), - 'start_date' => '', - 'end_date' => '', - 'campaign_id' => '', - // This format reflects the 23 being the organisation & the 25 being the type. - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], - 'auto_renew' => '1', - 'is_recur' => 1, + // This format reflects the key being the organisation & the value being the type. + 'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID], 'num_terms' => '1', 'total_amount' => $this->formatMoneyInput('7800.90'), //Member dues, see data.xml 'financial_type_id' => '2', - 'soft_credit_type_id' => 11, - 'soft_credit_contact_id' => '', 'from_email_address' => '"Demonstrators Anonymous" ', 'receipt_text' => 'Thank you text', 'payment_processor_id' => $this->_paymentProcessorID, @@ -729,7 +671,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { $params = [ 'contact_id' => $this->_individualId, - 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], + 'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID], 'renewal_date' => '2020-06-10', 'financial_type_id' => '2', 'num_terms' => '1',