From 936a2029a377652c609061f28604e98b69d86556 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 16 May 2023 12:57:23 +1200 Subject: [PATCH] Use payment create rather than transitionComponents --- CRM/Contribute/BAO/Contribution.php | 30 +++++++++---- CRM/Contribute/Form/Contribution.php | 45 ++++++++++--------- .../CRM/Member/Form/MembershipTest.php | 2 +- tests/phpunit/api/v3/ContributionTest.php | 21 ++++----- 4 files changed, 55 insertions(+), 43 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 2d3175cb5e2c..82b0ff93349f 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -4212,11 +4212,7 @@ public static function updateMembershipBasedOnCompletionOfContribution($contribu } // @todo remove all this stuff in favour of letting the api call further down handle in // (it is a duplication of what the api does). - $dates = array_fill_keys([ - 'join_date', - 'start_date', - 'end_date', - ], NULL); + $dates = []; if ($currentMembership) { /* * Fixed FOR CRM-4433 @@ -4239,9 +4235,9 @@ public static function updateMembershipBasedOnCompletionOfContribution($contribu } else { //get the status for membership. - $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($dates['start_date'], - $dates['end_date'], - $dates['join_date'], + $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($dates['start_date'] ?? NULL, + $dates['end_date'] ?? NULL, + $dates['join_date'] ?? NULL, 'now', TRUE, $membershipParams['membership_type_id'], @@ -4255,7 +4251,23 @@ public static function updateMembershipBasedOnCompletionOfContribution($contribu //so make status override false. $membershipParams['is_override'] = FALSE; $membershipParams['status_override_end_date'] = 'null'; - civicrm_api3('Membership', 'create', $membershipParams); + $membership = civicrm_api3('Membership', 'create', $membershipParams); + $membership = $membership['values'][$membership['id']]; + // Update activity to Completed. + // Perhaps this should be in Membership::create? Test cover in + // api_v3_ContributionTest.testPendingToCompleteContribution. + $priorMembershipStatus = $memberships[$membership['id']]['status_id'] ?? NULL; + Activity::update(FALSE)->setValues([ + 'status_id:name' => 'Completed', + 'subject' => ts('Status changed from %1 to %2'), [ + 1 => CRM_Core_PseudoConstant::getLabel('CRM_Member_BAO_Membership', 'status_id', $priorMembershipStatus), + 2 => CRM_Core_PseudoConstant::getLabel('CRM_Member_BAO_Membership', 'status_id', $membership['status_id']), + ], + + ])->addWhere('source_record_id', '=', $membership['id']) + ->addWhere('status_id:name', '=', 'Scheduled') + ->addWhere('activity_type_id:name', 'IN', ['Membership Signup', 'Membership Renewal']) + ->execute(); } } diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php index f7a935aed89d..8a343718bb90 100644 --- a/CRM/Contribute/Form/Contribution.php +++ b/CRM/Contribute/Form/Contribution.php @@ -1974,7 +1974,6 @@ protected function submit($submittedValues, $action, $pledgePaymentID) { $fields = [ 'financial_type_id', - 'contribution_status_id', 'payment_instrument_id', 'cancel_reason', 'source', @@ -1985,6 +1984,21 @@ protected function submit($submittedValues, $action, $pledgePaymentID) { foreach ($fields as $f) { $params[$f] = $formValues[$f] ?? NULL; } + if ($this->_id && $action & CRM_Core_Action::UPDATE) { + // Can only be updated to contribution which is handled via Payment.create + $params['contribution_status_id'] = $this->getSubmittedValue('contribution_status_id'); + + // Set is_pay_later flag for back-office offline Pending status contributions CRM-8996 + // else if contribution_status is changed to Completed is_pay_later flag is changed to 0, CRM-15041 + if ($params['contribution_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending')) { + $params['is_pay_later'] = 1; + } + elseif ($params['contribution_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed')) { + // @todo - if the contribution is new then it should be Pending status & then we use + // Payment.create to update to Completed. + $params['is_pay_later'] = 0; + } + } $params['revenue_recognition_date'] = NULL; if (!empty($formValues['revenue_recognition_date'])) { @@ -2005,17 +2019,6 @@ protected function submit($submittedValues, $action, $pledgePaymentID) { $params['cancel_date'] = $params['cancel_reason'] = 'null'; } - // Set is_pay_later flag for back-office offline Pending status contributions CRM-8996 - // else if contribution_status is changed to Completed is_pay_later flag is changed to 0, CRM-15041 - if ($params['contribution_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending')) { - $params['is_pay_later'] = 1; - } - elseif ($params['contribution_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed')) { - // @todo - if the contribution is new then it should be Pending status & then we use - // Payment.create to update to Completed. - $params['is_pay_later'] = 0; - } - // Add Additional common information to formatted params. CRM_Contribute_Form_AdditionalInfo::postProcessCommon($formValues, $params, $this); if ($pId) { @@ -2036,21 +2039,21 @@ protected function submit($submittedValues, $action, $pledgePaymentID) { if (!empty($params['note']) && !empty($submittedValues['note'])) { unset($params['note']); } - $contribution = CRM_Contribute_BAO_Contribution::create($params); - $previousStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $this->_values['contribution_status_id'] ?? NULL); // process associated membership / participant, CRM-4395 - if ($contribution->id && $action & CRM_Core_Action::UPDATE + if ($this->getContributionID() && $this->getAction() & CRM_Core_Action::UPDATE && in_array($previousStatus, ['Pending', 'Partially paid'], TRUE) && 'Completed' === CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $this->getSubmittedValue('contribution_status_id'))) { - // @todo use Payment.create to do this, remove transitioncomponents function - // if contribution is being created with a completed status it should be - // created pending & then Payment.create adds the payment - CRM_Contribute_BAO_Contribution::transitionComponents([ - 'contribution_id' => $contribution->id, - 'receive_date' => $contribution->receive_date, + // @todo make users use add payment form. + civicrm_api3('Payment', 'create', [ + 'contribution_id' => $this->getContributionID(), + 'total_amount' => $this->getContributionValue('balance_amount'), + 'currency' => $this->getSubmittedValue('currency'), + 'payment_instrument_id' => $this->getSubmittedValue('payment_instrument_id'), + 'check_number' => $this->getSubmittedValue('check_number'), ]); } + $contribution = CRM_Contribute_BAO_Contribution::create($params); array_unshift($this->statusMessage, ts('The contribution record has been saved.')); diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php index 88c2aed05095..7d54a2d066df 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php @@ -1109,7 +1109,7 @@ public function testSubmitUpdateMembershipFromPartiallyPaid(): void { // Check if Membership is updated to New. $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]); - $this->assertEquals($membership['status_id'], array_search('New', $memStatus)); + $this->assertEquals('New', CRM_Core_PseudoConstant::getName('CRM_Member_BAO_Membership', 'status_id', $membership['status_id'])); } /** diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index c4fd664940f6..0bb1e080bd68 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -3443,9 +3443,9 @@ public function testCompleteTransactionMembershipPriceSet(): void { 'membership_id' => $this->getMembershipID(), ]); $this->assertEquals(4, $logs['count']); - //Assert only three activities are created. + //Assert activities are created. $activityNames = (array) ActivityContact::get(FALSE) - ->addWhere('contact_id', '=', $this->_ids['contact']) + ->addWhere('contact_id', '=', $membership['contact_id']) ->addSelect('activity_id.activity_type_id:name')->execute()->indexBy('activity_id.activity_type_id:name'); $this->assertArrayHasKey('Contribution', $activityNames); $this->assertArrayHasKey('Membership Signup', $activityNames); @@ -3458,7 +3458,7 @@ public function testCompleteTransactionMembershipPriceSet(): void { */ public function testPendingToCompleteContribution(): void { $this->createPriceSetWithPage('membership'); - $this->setUpPendingContribution($this->_ids['price_field_value'][0]); + $this->setUpPendingContribution($this->_ids['price_field_value'][0], 'new'); $this->callAPISuccess('membership', 'getsingle', ['id' => $this->_ids['membership']]); // Case 1: Assert that Membership Signup Activity is created on Pending to Completed Contribution via backoffice $activity = $this->callAPISuccess('Activity', 'get', [ @@ -3575,11 +3575,6 @@ public function testCompleteTransactionMembershipPriceSetTwoTerms(): void { $this->assertEquals(date('Y-m-d', strtotime('yesterday + 5 years')), $membership['end_date']); } - public function cleanUpAfterPriceSets(): void { - $this->quickCleanUpFinancialEntities(); - $this->contactDelete($this->_ids['contact']); - } - /** * Set up a pending transaction with a specific price field id. * @@ -3590,7 +3585,7 @@ public function cleanUpAfterPriceSets(): void { * @param array $membershipParams */ public function setUpPendingContribution(int $priceFieldValueID, string $key = 'first', array $contributionParams = [], array $lineParams = [], array $membershipParams = []): void { - $contactID = $this->individualCreate(); + $contactID = $this->ids['Contact']['individual_0'] ?? $this->individualCreate(); $membershipParams = array_merge([ 'contact_id' => $contactID, 'membership_type_id' => $this->_ids['membership_type'], @@ -3629,8 +3624,7 @@ public function setUpPendingContribution(int $priceFieldValueID, string $key = ' ], ], $contributionParams)); - $this->_ids['contact'] = $contactID; - $this->ids['contribution'][$key] = $contribution['id']; + $this->ids['Contribution'][$key] = $contribution['id']; $this->_ids['membership'] = $this->callAPISuccessGetValue('MembershipPayment', ['return' => 'membership_id', 'contribution_id' => $contribution['id']]); } @@ -5113,7 +5107,10 @@ private function createQuickConfigContributionPage(array $contributionPageParams * @return int */ protected function getContributionID(string $key = 'first'): int { - return (int) $this->ids['contribution'][$key]; + if (count($this->ids['Contribution']) === 1) { + return reset($this->ids['Contribution']); + } + return (int) $this->ids['Contribution'][$key]; } /**