From 102279e26722a0c32a4cb2bc2673ee6ff4467ba9 Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 20 Mar 2021 10:20:09 +1300 Subject: [PATCH] Move setting of _includesSoftCredits out of getQuery We shouldn't be doing extra setting within the function --- CRM/Contribute/Form/Task.php | 14 ++++---------- CRM/Contribute/Form/Task/PDFLetterCommon.php | 4 ++-- CRM/Contribute/Form/Task/TaskTrait.php | 16 ++++++++++++---- .../Contribute/Form/Task/PDFLetterCommonTest.php | 16 +++++++++------- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/CRM/Contribute/Form/Task.php b/CRM/Contribute/Form/Task.php index 1cef21bc6c..4dd458f7af 100644 --- a/CRM/Contribute/Form/Task.php +++ b/CRM/Contribute/Form/Task.php @@ -37,13 +37,6 @@ class CRM_Contribute_Form_Task extends CRM_Core_Form_Task { */ protected $_contributionContactIds = []; - /** - * The flag to tell if there are soft credits included. - * - * @var bool - */ - public $_includesSoftCredits = FALSE; - /** * Build all the data structures needed to build the form. */ @@ -66,10 +59,11 @@ class CRM_Contribute_Form_Task extends CRM_Core_Form_Task { $ids = $form->getSelectedIDs($values); if (!$ids) { $result = $form->getSearchQueryResults(); + $form->_includesSoftCredits = $form->isQueryIncludesSoftCredits(); $contributionContactIds = $contactIds = []; while ($result->fetch()) { $ids[] = $result->contribution_id; - if ($form->_includesSoftCredits) { + if ($form->isQueryIncludesSoftCredits()) { $contactIds[$result->contact_id] = $result->contact_id; $contributionContactIds["{$result->contact_id}-{$result->contribution_id}"] = $result->contribution_id; } @@ -82,7 +76,7 @@ class CRM_Contribute_Form_Task extends CRM_Core_Form_Task { $form->assign('totalSelectedContributions', count($ids)); } - if (!empty($form->_includesSoftCredits) && !empty($contactIds)) { + if (!empty($form->isQueryIncludesSoftCredits()) && !empty($contactIds)) { $form->_contactIds = $contactIds; $form->_contributionContactIds = $contributionContactIds; } @@ -106,7 +100,7 @@ class CRM_Contribute_Form_Task extends CRM_Core_Form_Task { * since its used for things like send email */ public function setContactIDs() { - if (!$this->_includesSoftCredits) { + if (!$this->isQueryIncludesSoftCredits()) { $this->_contactIds = CRM_Core_DAO::getContactIDsFromComponent( $this->_contributionIds, 'civicrm_contribution' diff --git a/CRM/Contribute/Form/Task/PDFLetterCommon.php b/CRM/Contribute/Form/Task/PDFLetterCommon.php index eefb0cb733..957af2baf7 100644 --- a/CRM/Contribute/Form/Task/PDFLetterCommon.php +++ b/CRM/Contribute/Form/Task/PDFLetterCommon.php @@ -78,11 +78,11 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF $skipOnHold = $form->skipOnHold ?? FALSE; $skipDeceased = $form->skipDeceased ?? TRUE; $contributionIDs = $form->getVar('_contributionIds'); - if ($form->_includesSoftCredits) { + if ($form->isQueryIncludesSoftCredits()) { //@todo - comment on what is stored there $contributionIDs = $form->getVar('_contributionContactIds'); } - [$contributions, $contacts] = self::buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $form->_includesSoftCredits); + [$contributions, $contacts] = self::buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $form->isQueryIncludesSoftCredits()); $html = []; $contactHtml = $emailedHtml = []; foreach ($contributions as $contributionId => $contribution) { diff --git a/CRM/Contribute/Form/Task/TaskTrait.php b/CRM/Contribute/Form/Task/TaskTrait.php index 85d17ffe2b..56cf022d72 100644 --- a/CRM/Contribute/Form/Task/TaskTrait.php +++ b/CRM/Contribute/Form/Task/TaskTrait.php @@ -39,15 +39,14 @@ trait CRM_Contribute_Form_Task_TaskTrait { $returnProperties[$sortCol] = 1; } - $form->_includesSoftCredits = CRM_Contribute_BAO_Query::isSoftCreditOptionEnabled($queryParams); $query = new CRM_Contact_BAO_Query($queryParams, $returnProperties, NULL, FALSE, FALSE, CRM_Contact_BAO_Query::MODE_CONTRIBUTE ); // @todo the function CRM_Contribute_BAO_Query::isSoftCreditOptionEnabled should handle this // can we remove? if not why not? - if ($form->_includesSoftCredits) { - $query->_rowCountClause = " count(civicrm_contribution.id)"; - $query->_groupByComponentClause = " GROUP BY contribution_search_scredit_combined.id, contribution_search_scredit_combined.contact_id, contribution_search_scredit_combined.scredit_id "; + if ($this->isQueryIncludesSoftCredits()) { + $query->_rowCountClause = ' count(civicrm_contribution.id)'; + $query->_groupByComponentClause = ' GROUP BY contribution_search_scredit_combined.id, contribution_search_scredit_combined.contact_id, contribution_search_scredit_combined.scredit_id '; } else { $query->_distinctComponentClause = ' civicrm_contribution.id'; @@ -81,4 +80,13 @@ trait CRM_Contribute_Form_Task_TaskTrait { return $queryParams; } + /** + * Has soft credit information been requested in the query filters. + * + * @return bool + */ + public function isQueryIncludesSoftCredits(): bool { + return (bool) CRM_Contribute_BAO_Query::isSoftCreditOptionEnabled($this->getQueryParams()); + } + } diff --git a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php index c48d6e7689..758e72ba9d 100644 --- a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php +++ b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php @@ -20,7 +20,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { protected $_individualId; - protected $_docTypes = NULL; + protected $_docTypes; protected $_contactIds; @@ -33,7 +33,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { */ protected $hookTokensCalled = 0; - protected function setUp() { + protected function setUp(): void { parent::setUp(); $this->_individualId = $this->individualCreate(['first_name' => 'Anthony', 'last_name' => 'Collins']); $this->_docTypes = CRM_Core_SelectValues::documentApplicationType(); @@ -44,7 +44,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function tearDown() { + public function tearDown(): void { $this->quickCleanUpFinancialEntities(); $this->quickCleanup(['civicrm_uf_match']); CRM_Utils_Hook::singleton()->reset(); @@ -107,7 +107,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { /** * Test the buildContributionArray function. * - * @throws \CRM_Core_Exception + * @throws \CRM_Core_Exception|\CiviCRM_API3_Exception */ public function testBuildContributionArray(): void { $this->_individualId = $this->individualCreate(); @@ -175,7 +175,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { * @param array $tokens * @param string $className */ - public function hookTokenValues(&$details, $contactIDs, $jobID, $tokens, $className) { + public function hookTokenValues(&$details, $contactIDs, $jobID, $tokens, $className): void { foreach ($details as $index => $detail) { $details[$index]['favourite_emoticon'] = 'emo'; } @@ -208,7 +208,8 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { ]; $contribution = $this->callAPISuccess('Contribution', 'create', $contributionParams); $contributionId = $contribution['id']; - $form = new CRM_Contribute_Form_Task_PDFLetter(); + /* @var $form CRM_Contribute_Form_Task_PDFLetter */ + $form = $this->getFormObject('CRM_Contribute_Form_Task_PDFLetter'); $form->setContributionIds([$contributionId]); $format = Civi::settings()->get('dateformatFull'); $date = CRM_Utils_Date::getToday(); @@ -280,7 +281,8 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { ]); $contributionIDs[] = $contribution['id']; - $form = new CRM_Contribute_Form_Task_PDFLetter(); + /* @var \CRM_Contribute_Form_Task_PDFLetter $form */ + $form = $this->getFormObject('CRM_Contribute_Form_Task_PDFLetter'); $form->setContributionIds($contributionIDs); $html = CRM_Contribute_Form_Task_PDFLetterCommon::postProcess($form, $formValues); -- 2.25.1