From 32f89cf39c780d4d755657be22500f1fae72c37a Mon Sep 17 00:00:00 2001 From: colemanw Date: Tue, 22 Aug 2023 22:11:38 -0400 Subject: [PATCH] PCP - Update BAO to use writeRecord/deleteRecord - Ensures hooks consistently called and custom data properly handled - Fixed incorrect entity name passed to pre/post hooks by delete function - Defaults set by pre hook instead of inside create function --- CRM/PCP/BAO/PCP.php | 60 ++++++++------------ CRM/PCP/BAO/PCPBlock.php | 11 +--- CRM/PCP/Form/Campaign.php | 2 +- CRM/PCP/Form/Contribute.php | 2 +- CRM/PCP/Form/Event.php | 2 +- CRM/PCP/Form/PCP.php | 4 +- tests/phpunit/CRM/PCP/BAO/PCPTest.php | 14 ++--- tests/phpunit/CRMTraits/PCP/PCPTestTrait.php | 2 +- tests/phpunit/api/v3/ReportTemplateTest.php | 6 +- 9 files changed, 42 insertions(+), 61 deletions(-) diff --git a/CRM/PCP/BAO/PCP.php b/CRM/PCP/BAO/PCP.php index 9ceeb01f4b..7eeb141560 100644 --- a/CRM/PCP/BAO/PCP.php +++ b/CRM/PCP/BAO/PCP.php @@ -14,7 +14,7 @@ * @package CRM * @copyright CiviCRM LLC https://civicrm.org/licensing */ -class CRM_PCP_BAO_PCP extends CRM_PCP_DAO_PCP { +class CRM_PCP_BAO_PCP extends CRM_PCP_DAO_PCP implements \Civi\Core\HookInterface { /** * The action links that we need to display for the browse screen. @@ -24,31 +24,29 @@ class CRM_PCP_BAO_PCP extends CRM_PCP_DAO_PCP { public static $_pcpLinks = NULL; /** - * Add or update either a Personal Campaign Page OR a PCP Block. - * - * @param array $params - * Values to create the pcp. - * - * @return object + * @deprecated + * @return CRM_PCP_DAO_PCP */ public static function create($params) { + CRM_Core_Error::deprecatedFunctionWarning('writeRecord'); + return self::writeRecord($params); + } - $dao = new CRM_PCP_DAO_PCP(); - $dao->copyValues($params); - - // ensure we set status_id since it is a not null field - // we should change the schema and allow this to be null - if (!$dao->id && !isset($dao->status_id)) { - $dao->status_id = 0; - } - - // set currency for CRM-1496 - if (!isset($dao->currency)) { - $dao->currency = CRM_Core_Config::singleton()->defaultCurrency; + /** + * Callback for hook_civicrm_pre(). + * + * @param \Civi\Core\Event\PreEvent $event + * + * @throws \CRM_Core_Exception + */ + public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event): void { + if ($event->action === 'create') { + // For some reason `status_id` is allowed to be empty + // FIXME: Why? + $event->params['status_id'] = $event->params['status_id'] ?? 0; + // Supply default for `currency` + $event->params['currency'] = $event->params['currency'] ?? Civi::settings()->get('defaultCurrency'); } - - $dao->save(); - return $dao; } /** @@ -341,24 +339,12 @@ WHERE pcp.id = %1 AND cc.contribution_status_id = %2 AND cc.is_test = 0"; } /** - * Delete the campaign page. - * + * @deprecated * @param int $id - * Campaign page id. */ public static function deleteById($id) { - CRM_Utils_Hook::pre('delete', 'Campaign', $id); - - $transaction = new CRM_Core_Transaction(); - - // delete from pcp table - $pcp = new CRM_PCP_DAO_PCP(); - $pcp->id = $id; - $pcp->delete(); - - $transaction->commit(); - - CRM_Utils_Hook::post('delete', 'Campaign', $id, $pcp); + CRM_Core_Error::deprecatedFunctionWarning('deleteRecord'); + return self::deleteRecord(['id' => $id]); } /** diff --git a/CRM/PCP/BAO/PCPBlock.php b/CRM/PCP/BAO/PCPBlock.php index de88b23318..41b07aa03c 100644 --- a/CRM/PCP/BAO/PCPBlock.php +++ b/CRM/PCP/BAO/PCPBlock.php @@ -17,17 +17,12 @@ class CRM_PCP_BAO_PCPBlock extends CRM_PCP_DAO_PCPBlock { /** - * Create or update either a Personal Campaign Page OR a PCP Block. - * - * @param array $params - * + * @deprecated * @return CRM_PCP_DAO_PCPBlock */ public static function create($params) { - $dao = new CRM_PCP_DAO_PCPBlock(); - $dao->copyValues($params); - $dao->save(); - return $dao; + CRM_Core_Error::deprecatedFunctionWarning('writeRecord'); + return self::writeRecord($params); } } diff --git a/CRM/PCP/Form/Campaign.php b/CRM/PCP/Form/Campaign.php index a4b9ff69a4..59fc07b62a 100644 --- a/CRM/PCP/Form/Campaign.php +++ b/CRM/PCP/Form/Campaign.php @@ -227,7 +227,7 @@ class CRM_PCP_Form_Campaign extends CRM_Core_Form { $params['id'] = $this->_pageId; - $pcp = CRM_PCP_BAO_PCP::create($params); + $pcp = CRM_PCP_BAO_PCP::writeRecord($params); // add attachments as needed CRM_Core_BAO_File::formatAttachment($params, diff --git a/CRM/PCP/Form/Contribute.php b/CRM/PCP/Form/Contribute.php index c4c58270e3..19f4840996 100644 --- a/CRM/PCP/Form/Contribute.php +++ b/CRM/PCP/Form/Contribute.php @@ -151,7 +151,7 @@ class CRM_PCP_Form_Contribute extends CRM_Contribute_Form_ContributionPage { $params['is_approval_needed'] = CRM_Utils_Array::value('is_approval_needed', $params, FALSE); $params['is_tellfriend_enabled'] = CRM_Utils_Array::value('is_tellfriend_enabled', $params, FALSE); - CRM_PCP_BAO_PCPBlock::create($params); + CRM_PCP_BAO_PCPBlock::writeRecord($params); parent::endPostProcess(); } diff --git a/CRM/PCP/Form/Event.php b/CRM/PCP/Form/Event.php index 158e3ecf34..636968f8a0 100644 --- a/CRM/PCP/Form/Event.php +++ b/CRM/PCP/Form/Event.php @@ -185,7 +185,7 @@ class CRM_PCP_Form_Event extends CRM_Event_Form_ManageEvent { $params['is_approval_needed'] = CRM_Utils_Array::value('is_approval_needed', $params, FALSE); $params['is_tellfriend_enabled'] = CRM_Utils_Array::value('is_tellfriend_enabled', $params, FALSE); - CRM_PCP_BAO_PCPBlock::create($params); + CRM_PCP_BAO_PCPBlock::writeRecord($params); // Update tab "disabled" css class $this->ajaxResponse['tabValid'] = !empty($params['is_active']); diff --git a/CRM/PCP/Form/PCP.php b/CRM/PCP/Form/PCP.php index 06e48873fb..5da4e043ac 100644 --- a/CRM/PCP/Form/PCP.php +++ b/CRM/PCP/Form/PCP.php @@ -75,7 +75,7 @@ class CRM_PCP_Form_PCP extends CRM_Core_Form { switch ($this->_action) { case CRM_Core_Action::DELETE: case 'delete': - CRM_PCP_BAO_PCP::deleteById($this->_id); + CRM_PCP_BAO_PCP::deleteRecord(['id' => $this->_id]); CRM_Core_Session::setStatus(ts("The Campaign Page '%1' has been deleted.", [1 => $this->_title]), ts('Page Deleted'), 'success'); break; @@ -178,7 +178,7 @@ class CRM_PCP_Form_PCP extends CRM_Core_Form { */ public function postProcess() { if ($this->_action & CRM_Core_Action::DELETE) { - CRM_PCP_BAO_PCP::deleteById($this->_id); + CRM_PCP_BAO_PCP::deleteRecord(['id' => $this->_id]); CRM_Core_Session::setStatus(ts("The Campaign Page '%1' has been deleted.", [1 => $this->_title]), ts('Page Deleted'), 'success'); } else { diff --git a/tests/phpunit/CRM/PCP/BAO/PCPTest.php b/tests/phpunit/CRM/PCP/BAO/PCPTest.php index aac73bb4af..7954a4e45a 100644 --- a/tests/phpunit/CRM/PCP/BAO/PCPTest.php +++ b/tests/phpunit/CRM/PCP/BAO/PCPTest.php @@ -32,7 +32,7 @@ class CRM_PCP_BAO_PCPTest extends CiviUnitTestCase { public function testAddPCPBlock() { $params = $this->pcpBlockParams(); - $pcpBlock = CRM_PCP_BAO_PCPBlock::create($params); + $pcpBlock = CRM_PCP_BAO_PCPBlock::writeRecord($params); $this->assertInstanceOf('CRM_PCP_DAO_PCPBlock', $pcpBlock, 'Check for created object'); $this->assertEquals($params['entity_table'], $pcpBlock->entity_table, 'Check for entity table.'); @@ -58,13 +58,13 @@ class CRM_PCP_BAO_PCPTest extends CiviUnitTestCase { public function testAddPCPNoStatus() { $blockParams = $this->pcpBlockParams(); - $pcpBlock = CRM_PCP_BAO_PCPBlock::create($blockParams); + $pcpBlock = CRM_PCP_BAO_PCPBlock::writeRecord($blockParams); $params = $this->pcpParams(); $params['pcp_block_id'] = $pcpBlock->id; unset($params['status_id']); - $pcp = CRM_PCP_BAO_PCP::create($params); + $pcp = CRM_PCP_BAO_PCP::writeRecord($params); $this->assertInstanceOf('CRM_PCP_DAO_PCP', $pcp, 'Check for created object'); $this->assertEquals($params['contact_id'], $pcp->contact_id, 'Check for entity table.'); @@ -83,7 +83,7 @@ class CRM_PCP_BAO_PCPTest extends CiviUnitTestCase { $pcp = CRM_Core_DAO::createTestObject('CRM_PCP_DAO_PCP'); $pcpId = $pcp->id; - CRM_PCP_BAO_PCP::deleteById($pcpId); + CRM_PCP_BAO_PCP::deleteRecord(['id' => $pcpId]); $this->assertDBRowNotExist('CRM_PCP_DAO_PCP', $pcpId, 'Database check PCP deleted successfully.'); } @@ -93,7 +93,7 @@ class CRM_PCP_BAO_PCPTest extends CiviUnitTestCase { * @throws \CRM_Core_Exception */ public function testGetPcpDashboardInfo() { - $block = CRM_PCP_BAO_PCPBlock::create($this->pcpBlockParams()); + $block = CRM_PCP_BAO_PCPBlock::writeRecord($this->pcpBlockParams()); $contactID = $this->individualCreate(); $submitParams = [ 'id' => 1, @@ -129,7 +129,7 @@ class CRM_PCP_BAO_PCPTest extends CiviUnitTestCase { // Reset the cache otherwise our hook will not be called CRM_PCP_BAO_PCP::$_pcpLinks = NULL; - $block = CRM_PCP_BAO_PCPBlock::create($this->pcpBlockParams()); + $block = CRM_PCP_BAO_PCPBlock::writeRecord($this->pcpBlockParams()); $contactID = $this->individualCreate(); $contributionPage = $this->callAPISuccessGetSingle('ContributionPage', []); $pcp = $this->callAPISuccess('Pcp', 'create', ['contact_id' => $contactID, 'title' => 'pcp', 'page_id' => $contributionPage['id'], 'pcp_block_id' => $block->id, 'is_active' => TRUE, 'status_id' => 'Approved']); @@ -172,7 +172,7 @@ class CRM_PCP_BAO_PCPTest extends CiviUnitTestCase { */ public function testGatherMessageValuesForPCP() { // set up a pcp page - $block = CRM_PCP_BAO_PCPBlock::create($this->pcpBlockParams()); + $block = CRM_PCP_BAO_PCPBlock::writeRecord($this->pcpBlockParams()); // The owner of the pcp, who gets the soft credit $contact_owner = $this->individualCreate([], 0, TRUE); $contributionPage = $this->callAPISuccessGetSingle('ContributionPage', []); diff --git a/tests/phpunit/CRMTraits/PCP/PCPTestTrait.php b/tests/phpunit/CRMTraits/PCP/PCPTestTrait.php index 6887d40ca5..8822bd8915 100644 --- a/tests/phpunit/CRMTraits/PCP/PCPTestTrait.php +++ b/tests/phpunit/CRMTraits/PCP/PCPTestTrait.php @@ -89,7 +89,7 @@ trait CRMTraits_PCP_PCPTestTrait { $params = array_merge($this->pcpParams(), $params); $params['pcp_block_id'] = PCPBlock::create()->setValues($blockParams)->execute()->first()['id']; - $pcp = CRM_PCP_BAO_PCP::create($params); + $pcp = CRM_PCP_BAO_PCP::writeRecord($params); return (int) $pcp->id; } diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index b87a7dced1..7db1a6889e 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -1444,7 +1444,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { // pcpBLockParams creates a contribution page and returns the parameters // necessary to create a PBP Block. $blockParams = $this->pcpBlockParams(); - $pcpBlock = CRM_PCP_BAO_PCPBlock::create($blockParams); + $pcpBlock = CRM_PCP_BAO_PCPBlock::writeRecord($blockParams); // Keep track of the contribution page id created. We will use this // contribution page id for all the PCP pages. @@ -1458,7 +1458,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { $pcpParams['pcp_block_id'] = $pcpBlock->id; $pcpParams['page_id'] = $contribution_page_id; $pcpParams['page_type'] = 'contribute'; - $pcp1 = CRM_PCP_BAO_PCP::create($pcpParams); + $pcp1 = CRM_PCP_BAO_PCP::writeRecord($pcpParams); // Nice work. Now, let's create a second PCP page. $pcpParams = $this->pcpParams(); @@ -1469,7 +1469,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { $pcpParams['pcp_block_id'] = $pcpBlock->id; $pcpParams['page_id'] = $contribution_page_id; $pcpParams['page_type'] = 'contribute'; - $pcp2 = CRM_PCP_BAO_PCP::create($pcpParams); + $pcp2 = CRM_PCP_BAO_PCP::writeRecord($pcpParams); // Get soft credit types, with the name column as the key. $soft_credit_types = CRM_Core_PseudoConstant::get('CRM_Contribute_BAO_ContributionSoft', 'soft_credit_type_id', ['flip' => TRUE, 'labelColumn' => 'name']); -- 2.25.1