From d51ea753b36a8260823cdc11a2920a2d511a9446 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Fri, 23 Sep 2022 11:00:44 +1200 Subject: [PATCH] Remove another call to deprecated contributionStatus --- CRM/Contribute/BAO/Contribution.php | 34 +++++++++-------------- CRM/Contribute/Form/Contribution.php | 7 ++++- api/v3/Contribution.php | 7 ++--- tests/phpunit/api/v3/ContributionTest.php | 4 ++- 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 714c8811ad..75e10f36d6 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -3367,25 +3367,18 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac /** * Check status validation on update of a contribution. * - * @param array $values + * @param array $oldContributionValues * Previous form values before submit. * - * @param array $fields + * @param array $newContributionValues * The input form values. * - * @param array $errors - * List of errors. - * - * @return bool + * @throws \CRM_Core_Exception */ - public static function checkStatusValidation($values, &$fields, &$errors) { - if (CRM_Utils_System::isNull($values) && !empty($fields['id'])) { - $values['contribution_status_id'] = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $fields['id'], 'contribution_status_id'); - if ($values['contribution_status_id'] == $fields['contribution_status_id']) { - return FALSE; - } - } - $contributionStatuses = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'); + public static function checkStatusValidation(array $oldContributionValues, array $newContributionValues): void { + $newContributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $newContributionValues['contribution_status_id']); + $oldContributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $oldContributionValues['contribution_status_id']); + $checkStatus = [ 'Cancelled' => ['Completed', 'Refunded'], 'Completed' => ['Cancelled', 'Refunded', 'Chargeback'], @@ -3396,14 +3389,13 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac 'Pending refund' => ['Completed', 'Refunded'], 'Failed' => ['Pending'], ]; + $validNewStatues = $checkStatus[$oldContributionStatus] ?? []; - if (!in_array($contributionStatuses[$fields['contribution_status_id']], - CRM_Utils_Array::value($contributionStatuses[$values['contribution_status_id']], $checkStatus, [])) - ) { - $errors['contribution_status_id'] = ts("Cannot change contribution status from %1 to %2.", [ - 1 => $contributionStatuses[$values['contribution_status_id']], - 2 => $contributionStatuses[$fields['contribution_status_id']], - ]); + if (!in_array($newContributionStatus, $validNewStatues, TRUE)) { + throw new CRM_Core_Exception(ts('Cannot change contribution status from %1 to %2.', [ + 1 => $oldContributionStatus, + 2 => $newContributionStatus, + ])); } } diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php index 60cc091581..0ae0b3a0b8 100644 --- a/CRM/Contribute/Form/Contribution.php +++ b/CRM/Contribute/Form/Contribution.php @@ -893,7 +893,12 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP && $self->_values['contribution_status_id'] != $fields['contribution_status_id'] && $self->_values['is_template'] != 1 ) { - CRM_Contribute_BAO_Contribution::checkStatusValidation($self->_values, $fields, $errors); + try { + CRM_Contribute_BAO_Contribution::checkStatusValidation($self->_values, $fields); + } + catch (CRM_Core_Exception $e) { + $errors['contribution_status_id'] = $e->getMessage(); + } } // CRM-16015, add form-rule to restrict change of financial type if using price field of different financial type if (($self->_action & CRM_Core_Action::UPDATE) diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php index 06fae12ada..9c00fe34c8 100644 --- a/api/v3/Contribution.php +++ b/api/v3/Contribution.php @@ -54,13 +54,12 @@ function civicrm_api3_contribution_create($params) { } } if (!empty($params['id']) && !empty($params['contribution_status_id'])) { - $error = []; //throw error for invalid status change such as setting completed back to pending //@todo this sort of validation belongs in the BAO not the API - if it is not an OK // action it needs to be blocked there. If it is Ok through a form it needs to be OK through the api - CRM_Contribute_BAO_Contribution::checkStatusValidation(NULL, $params, $error); - if (array_key_exists('contribution_status_id', $error)) { - throw new CRM_Core_Exception($error['contribution_status_id']); + $values = ['contribution_status_id' => (int) CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $params['id'], 'contribution_status_id')]; + if ($values['contribution_status_id'] !== (int) $params['contribution_status_id']) { + CRM_Contribute_BAO_Contribution::checkStatusValidation($values, $params); } } if (!empty($params['id']) && !empty($params['financial_type_id'])) { diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 547e7f472d..8c23a22d1e 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -2639,9 +2639,11 @@ class api_v3_ContributionTest extends CiviUnitTestCase { * * @dataProvider contributionStatusProvider * + * @param array $contributionStatus + * * @throws \CRM_Core_Exception */ - public function testRepeatTransactionMembershipRenewContributionNotCompleted($contributionStatus): void { + public function testRepeatTransactionMembershipRenewContributionNotCompleted(array $contributionStatus): void { // Completed status should renew so we don't test that here // In Progress status was never actually intended to be available for contributions. // Partially paid is not valid. -- 2.25.1