From ce6cca3007aa034a446eeebd6c4c4d5ce527afd7 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Thu, 31 Aug 2023 23:36:41 +1200 Subject: [PATCH] Add weights to membership type links This includes moving the permissioning of those links to the financialacls extension (since there was already an affected test in that extension too). --- CRM/Member/Page/MembershipType.php | 14 +++++------ CRM/PCP/BAO/PCP.php | 7 ++++-- CRM/Utils/Hook.php | 2 +- ext/financialacls/financialacls.php | 19 +++++++++++++++ .../Financialacls/MembershipTypesTest.php | 23 +++++++++++++++++-- 5 files changed, 52 insertions(+), 13 deletions(-) diff --git a/CRM/Member/Page/MembershipType.php b/CRM/Member/Page/MembershipType.php index ab117fef18..f68ca25da6 100644 --- a/CRM/Member/Page/MembershipType.php +++ b/CRM/Member/Page/MembershipType.php @@ -45,22 +45,26 @@ class CRM_Member_Page_MembershipType extends CRM_Core_Page { 'url' => 'civicrm/admin/member/membershipType/add', 'qs' => 'action=update&id=%%id%%&reset=1', 'title' => ts('Edit Membership Type'), + 'weight' => CRM_Core_Action::getWeight(CRM_Core_Action::UPDATE), ], CRM_Core_Action::DISABLE => [ 'name' => ts('Disable'), 'ref' => 'crm-enable-disable', 'title' => ts('Disable Membership Type'), + 'weight' => CRM_Core_Action::getWeight(CRM_Core_Action::DISABLE), ], CRM_Core_Action::ENABLE => [ 'name' => ts('Enable'), 'ref' => 'crm-enable-disable', 'title' => ts('Enable Membership Type'), + 'weight' => CRM_Core_Action::getWeight(CRM_Core_Action::ENABLE), ], CRM_Core_Action::DELETE => [ 'name' => ts('Delete'), 'url' => 'civicrm/admin/member/membershipType/add', 'qs' => 'action=delete&id=%%id%%', 'title' => ts('Delete Membership Type'), + 'weight' => CRM_Core_Action::getWeight(CRM_Core_Action::DELETE), ], ]; } @@ -86,9 +90,9 @@ class CRM_Member_Page_MembershipType extends CRM_Core_Page { /** * Browse all membership types. * - * @return void + * @throws \CRM_Core_Exception */ - public function browse() { + public function browse(): void { // Ensure an action is assigned, even null - since this page is overloaded for other uses // we need to avoid e-notices. $this->assign('action'); @@ -137,12 +141,6 @@ class CRM_Member_Page_MembershipType extends CRM_Core_Page { ); } } - if (CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus() && !CRM_Core_Permission::check('edit contributions of type ' . CRM_Contribute_PseudoConstant::financialType($type['financial_type_id']))) { - unset($links[CRM_Core_Action::UPDATE], $links[CRM_Core_Action::ENABLE], $links[CRM_Core_Action::DISABLE]); - } - if (CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus() && !CRM_Core_Permission::check('delete contributions of type ' . CRM_Contribute_PseudoConstant::financialType($type['financial_type_id']))) { - unset($links[CRM_Core_Action::DELETE]); - } // form all action links $action = array_sum(array_keys($this->links())); diff --git a/CRM/PCP/BAO/PCP.php b/CRM/PCP/BAO/PCP.php index 973fb104ed..72a059a708 100644 --- a/CRM/PCP/BAO/PCP.php +++ b/CRM/PCP/BAO/PCP.php @@ -93,13 +93,13 @@ ORDER BY page_type, page_id'; $params = [1 => [$contactId, 'Integer']]; $pcpInfoDao = CRM_Core_DAO::executeQuery($query, $params); - $links = self::pcpLinks(); - $hide = $mask = array_sum(array_keys($links['all'])); $approved = CRM_Core_PseudoConstant::getKey('CRM_PCP_BAO_PCP', 'status_id', 'Approved'); $contactPCPPages = []; $pcpInfo = []; while ($pcpInfoDao->fetch()) { + $links = self::pcpLinks($pcpInfoDao->id); + $hide = $mask = array_sum(array_keys($links['all'])); $mask = $hide; if ($links) { $replace = [ @@ -274,6 +274,9 @@ WHERE pcp.id = %1 AND cc.contribution_status_id = %2 AND cc.is_test = 0"; * (reference) of action links */ public static function &pcpLinks($pcpId = NULL) { + if (!$pcpId) { + CRM_Core_Error::deprecatedWarning('pcpId should be provided to render links'); + } if (!(self::$_pcpLinks)) { $deleteExtra = ts('Are you sure you want to delete this Personal Campaign Page?') . '\n' . ts('This action cannot be undone.'); diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index d3b4417f60..a7ab628d48 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -414,7 +414,7 @@ abstract class CRM_Utils_Hook { * The unique identifier for the object. * @param array $links * (optional) the links array (introduced in v3.2). - * @param int $mask + * @param int|null $mask * (optional) the bitmask to show/hide links. * @param array $values * (optional) the values to fill the links. diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php index 3265558fed..41992b8a5a 100644 --- a/ext/financialacls/financialacls.php +++ b/ext/financialacls/financialacls.php @@ -383,3 +383,22 @@ function financialacls_civicrm_alterMenu(array &$menu): void { } $menu['civicrm/admin/financial/financialType']['access_arguments'] = [['administer CiviCRM Financial Types']]; } + +function financialacls_civicrm_links(string $op, ?string $objectName, ?int $objectID, array &$links, ?int &$mask, array &$values) { + if ($objectName === 'MembershipType') { + $financialType = CRM_Core_PseudoConstant::getName('CRM_Member_BAO_MembershipType', 'financial_type_id', CRM_Member_BAO_MembershipType::getMembershipType($objectID)['financial_type_id']); + $hasEditPermission = CRM_Core_Permission::check('edit contributions of type ' . $financialType); + $hasDeletePermission = CRM_Core_Permission::check('delete contributions of type ' . $financialType); + if (!$hasDeletePermission || !$hasEditPermission) { + foreach ($links as $index => $link) { + if (!$hasEditPermission && in_array($link['name'], ['Edit', 'Enable', 'Disable'], TRUE)) { + unset($links[$index]); + } + if (!$hasDeletePermission && $link['name'] === 'Delete') { + unset($links[$index]); + } + } + } + } + +} diff --git a/ext/financialacls/tests/phpunit/Civi/Financialacls/MembershipTypesTest.php b/ext/financialacls/tests/phpunit/Civi/Financialacls/MembershipTypesTest.php index 0d45587232..6b1370d07a 100644 --- a/ext/financialacls/tests/phpunit/Civi/Financialacls/MembershipTypesTest.php +++ b/ext/financialacls/tests/phpunit/Civi/Financialacls/MembershipTypesTest.php @@ -32,12 +32,31 @@ class MembershipTypesTest extends BaseTestClass { $assigned = \CRM_Core_Smarty::singleton()->get_template_vars(); $this->assertArrayNotHasKey($types['Forbidden']['id'], $assigned['rows']); $this->assertArrayHasKey($types['Go for it']['id'], $assigned['rows']); + $links = $assigned['rows'][$types['Go for it']['id']]['action']; + $this->assertStringContainsString("title='Edit Membership Type' ", $links); + $this->assertStringContainsString("title='Disable Membership Type' ", $links); + $this->assertStringContainsString("title='Delete Membership Type' ", $links); + + // Now check that the edit & delete links are removed if we remove those permissions. + $permissions = \CRM_Core_Config::singleton()->userPermissionClass->permissions; + foreach ($permissions as $index => $permission) { + if (in_array($permission, ['edit contributions of type Donation', 'delete contributions of type Donation'], TRUE)) { + unset($permissions[$index]); + } + } + $this->setPermissions($permissions); + $page->browse(); + $assigned = \CRM_Core_Smarty::singleton()->get_template_vars(); + $this->assertEquals('', $assigned['rows'][$types['Go for it']['id']]['action']); } /** + * Set up a membership scenario where the user can access one type but not the other. + * * @return \Civi\Api4\Generic\Result - * @throws \CRM_Core_Exception - * @throws \Civi\API\Exception\UnauthorizedException + * + * @noinspection PhpDocMissingThrowsInspection + * @noinspection PhpUnhandledExceptionInspection */ protected function setUpMembershipTypesACLLimited(): Result { $types = MembershipType::save(FALSE) -- 2.25.1