Fix ACLPermissionTrait to use EventTest create function

Cleanup in related classes
civicrm · Jul 3, 2023 · 2810a48 · 2810a48
1 parent a500d82
commit 2810a48
Show file tree

Hide file tree

Showing 11 changed files with 156 additions and 197 deletions.
diff --git a/CRM/Core/DAO/AllCoreTables.php b/CRM/Core/DAO/AllCoreTables.php
@@ -360,7 +360,7 @@ public static function getTableForClass($className) {
    *
    * @return FALSE|string
    */
-  public static function getTableForEntityName($briefName) {
+  public static function getTableForEntityName($briefName): string {
     self::init();
     return self::$entityTypes[$briefName]['table'];
   }

diff --git a/Civi/Test/ACLPermissionTrait.php b/Civi/Test/ACLPermissionTrait.php
@@ -17,6 +17,7 @@
  * Trait for working with ACLs in tests
  */
 trait ACLPermissionTrait {
+  use EventTestTrait;
 
   /**
    * ContactID of allowed Contact
@@ -46,10 +47,12 @@ trait ACLPermissionTrait {
    * @param array $tables
    * @param array $whereTables
    * @param int $contactID
-   * @param string $where
+   * @param string|null $where
+   *
+   * @noinspection PhpUnusedParameterInspection
    */
-  public function aclWhereHookAllResults($type, &$tables, &$whereTables, &$contactID, &$where) {
-    $where = " (1) ";
+  public function aclWhereHookAllResults(string $type, array &$tables, array &$whereTables, int &$contactID, ?string &$where): void {
+    $where = ' (1) ';
   }
 
   /**
@@ -61,9 +64,9 @@ public function aclWhereHookAllResults($type, &$tables, &$whereTables, &$contact
    * @param array $tables
    * @param array $whereTables
    * @param int $contactID
-   * @param string $where
+   * @param string|null $where
    */
-  public function aclWhereHookNoResults($type, &$tables, &$whereTables, &$contactID, &$where) {
+  public function aclWhereHookNoResults(string $type, array &$tables, array &$whereTables, int &$contactID, ?string &$where): void {
   }
 
   /**
@@ -75,10 +78,12 @@ public function aclWhereHookNoResults($type, &$tables, &$whereTables, &$contactI
    * @param array $tables
    * @param array $whereTables
    * @param int $contactID
-   * @param string $where
+   * @param string|null $where
+   *
+   * @noinspection PhpUnusedParameterInspection
    */
-  public function aclWhereOnlySecond($type, &$tables, &$whereTables, &$contactID, &$where) {
-    $where = " contact_a.id > 1";
+  public function aclWhereOnlySecond(string $type, array &$tables, array &$whereTables, int &$contactID, ?string &$where): void {
+    $where = ' contact_a.id > 1';
   }
 
   /**
@@ -90,10 +95,12 @@ public function aclWhereOnlySecond($type, &$tables, &$whereTables, &$contactID,
    * @param array $tables
    * @param array $whereTables
    * @param int $contactID
-   * @param string $where
+   * @param string|null $where
+   *
+   * @noinspection PhpUnusedParameterInspection
    */
-  public function aclWhereOnlyOne($type, &$tables, &$whereTables, &$contactID, &$where) {
-    $where = " contact_a.id = " . $this->allowedContactId;
+  public function aclWhereOnlyOne(string $type, array &$tables, array &$whereTables, int &$contactID, ?string &$where): void {
+    $where = ' contact_a.id = ' . $this->allowedContactId;
   }
 
   /**
@@ -105,10 +112,12 @@ public function aclWhereOnlyOne($type, &$tables, &$whereTables, &$contactID, &$w
    * @param array $tables
    * @param array $whereTables
    * @param int $contactID
-   * @param string $where
+   * @param string|null $where
+   *
+   * @noinspection PhpUnusedParameterInspection
    */
-  public function aclWhereGreaterThan($type, &$tables, &$whereTables, &$contactID, &$where) {
-    $where = " contact_a.id > " . $this->allowedContactId;
+  public function aclWhereGreaterThan(string $type, array &$tables, array &$whereTables, int &$contactID, ?string &$where): void {
+    $where = ' contact_a.id > ' . $this->allowedContactId;
   }
 
   /**
@@ -121,21 +130,19 @@ public function aclWhereGreaterThan($type, &$tables, &$whereTables, &$contactID,
    *   An ID of 0 means that 'Everyone' can access the group.
    * @param string $operation View|Edit|Create|Delete|Search|All
    * @param string $entity Group|CustomGroup|Profile|Event
-   *
-   * @throws CRM_Core_Exception
    */
-  public function setupCoreACLPermittedAcl($permissionedEntities = [], $groupAllowedAccess = 'Everyone', $operation = 'View', $entity = 'Group') {
+  public function setupCoreACLPermittedAcl(array $permissionedEntities = [], $groupAllowedAccess = 'Everyone', string $operation = 'View', string $entity = 'Group'): void {
     $tableMap = ['Group' => 'civicrm_group', 'CustomGroup' => 'civicrm_custom_group', 'Profile' => 'civicrm_uf_group', 'Event' => 'civicrm_event'];
     $entityTable = $tableMap[$entity];
 
     $permittedRoleID = ($groupAllowedAccess === 'Everyone') ? 0 : $groupAllowedAccess;
     if ($permittedRoleID !== 0) {
-      throw new \CRM_Core_Exception('only handling everyone group as yet');
+      $this->fail('only handling everyone group as yet');
     }
 
     foreach ($permissionedEntities as $permissionedEntityID) {
-      $this->callAPISuccess('Acl', 'create', [
-        'name' => uniqid(),
+      $this->createTestEntity('ACL', [
+        'name' => 'test acl' . $permissionedEntityID,
         'operation' => $operation,
         'entity_id' => $permittedRoleID,
         'object_id' => $permissionedEntityID,
@@ -156,11 +163,11 @@ public function setupCoreACLPermittedAcl($permissionedEntities = [], $groupAllow
    *   $this->scenarioIDs['Contact'] = ['permitted_contact' => x, 'non_permitted_contact' => y]
    *   $this->scenarioIDs['Group'] = ['permitted_group' => x]
    */
-  public function setupScenarioCoreACLEveryonePermittedToGroup() {
+  public function setupScenarioCoreACLEveryonePermittedToGroup(): void {
     $this->quickCleanup(['civicrm_acl_cache', 'civicrm_acl_contact_cache']);
     $this->scenarioIDs['Group']['permitted_group'] = $this->groupCreate();
     $this->scenarioIDs['Contact']['permitted_contact'] = $this->individualCreate();
-    $result = $this->callAPISuccess('GroupContact', 'create', ['group_id' => $this->scenarioIDs['Group']['permitted_group'], 'contact_id' => $this->scenarioIDs['Contact']['permitted_contact'], 'status' => 'Added']);
+    $this->callAPISuccess('GroupContact', 'create', ['group_id' => $this->scenarioIDs['Group']['permitted_group'], 'contact_id' => $this->scenarioIDs['Contact']['permitted_contact'], 'status' => 'Added']);
     $this->scenarioIDs['Contact']['non_permitted_contact'] = $this->individualCreate();
     \CRM_Core_Config::singleton()->userPermissionClass->permissions = [];
     $this->setupCoreACLPermittedAcl([$this->scenarioIDs['Group']['permitted_group']]);
@@ -169,27 +176,31 @@ public function setupScenarioCoreACLEveryonePermittedToGroup() {
   /**
    * Set up a scenario where everyone can access the permissioned group.
    *
-   * A scenario in this class involves multiple defined assets. In this case we create
+   * A scenario in this class involves multiple defined assets. In this case we
+   * create
    * - a group to which the everyone has permission
    * - a contact in the group
    * - a contact not in the group
    *
    * These are arrayed as follows
-   *   $this->scenarioIDs['Contact'] = ['permitted_contact' => x, 'non_permitted_contact' => y]
+   *   $this->scenarioIDs['Contact'] = ['permitted_contact' => x,
+   * 'non_permitted_contact' => y]
    *   $this->scenarioIDs['Group'] = ['permitted_group' => x]
    */
-  public function setupScenarioCoreACLEveryonePermittedToEvent() {
+  public function setupScenarioCoreACLEveryonePermittedToEvent(): void {
     $this->quickCleanup(['civicrm_acl_cache', 'civicrm_acl_contact_cache']);
-    $this->scenarioIDs['Event']['permitted_event'] = $this->eventCreate()['id'];
+    $this->scenarioIDs['Event']['permitted_event'] = $this->eventCreateUnpaid()['id'];
     $this->scenarioIDs['Contact']['permitted_contact'] = $this->individualCreate();
     \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['view event info'];
     $this->setupCoreACLPermittedAcl([$this->scenarioIDs['Event']['permitted_event']], 'Everyone', 'View', 'Event');
   }
 
   /**
    * Clean up places where permissions get cached.
+   *
+   * @noinspection PhpUnhandledExceptionInspection
    */
-  protected function cleanupCachedPermissions() {
+  protected function cleanupCachedPermissions(): void {
     if (isset(\Civi::$statics['CRM_Contact_BAO_Contact_Permission'])) {
       unset(\Civi::$statics['CRM_Contact_BAO_Contact_Permission']);
     }

diff --git a/Civi/Test/ContactTestTrait.php b/Civi/Test/ContactTestTrait.php
@@ -34,11 +34,11 @@ trait ContactTestTrait {
   public function createLoggedInUser(): int {
     $params = [
       'first_name' => 'Logged In',
-      'last_name' => 'User ' . rand(),
+      'last_name' => 'User ' . mt_rand(),
       'contact_type' => 'Individual',
       'domain_id' => \CRM_Core_Config::domainID(),
     ];
-    $contactID = $this->individualCreate($params);
+    $contactID = $this->individualCreate($params, 'logged_in');
     $this->callAPISuccess('UFMatch', 'get', ['uf_id' => 6, 'api.UFMatch.delete' => []]);
     $this->callAPISuccess('UFMatch', 'create', [
       'contact_id' => $contactID,

diff --git a/Civi/Test/EventTestTrait.php b/Civi/Test/EventTestTrait.php
@@ -163,7 +163,7 @@ protected function getEvent(string $identifier): array {
    *
    * @return array
    */
-  protected function eventCreate(array $params = [], string $identifier = 'event'): array {
+  public function eventCreate(array $params = [], string $identifier = 'event'): array {
     try {
       $event = Event::create(FALSE)->setValues($params)->execute()->first();
       $this->setTestEntity('Event', $event, $identifier);

diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
@@ -50,8 +50,6 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase {
 
   /**
    * Setup for tests.
-   *
-   * @throws CRM_Core_Exception
    */
   public function setUp(): void {
     parent::setUp();

diff --git a/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php b/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php
@@ -124,11 +124,10 @@ public function testGetTotalPaymentsParticipantOrder(): void {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testCreateDeferredTrxn() {
+  public function testCreateDeferredTrxn(): void {
     Civi::settings()->set('deferred_revenue_enabled', TRUE);
-    $cid = $this->individualCreate();
     $params = [
-      'contact_id' => $cid,
+      'contact_id' => $this->individualCreate(),
       'receive_date' => '2016-01-20',
       'total_amount' => 622,
       'financial_type_id' => 4,
@@ -153,13 +152,10 @@ public function testCreateDeferredTrxn() {
 
   /**
    * Test for updateCreditCardDetails().
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testUpdateCreditCardDetailsUsingContributionAPI() {
-    $cid = $this->individualCreate();
+  public function testUpdateCreditCardDetailsUsingContributionAPI(): void {
     $params = [
-      'contact_id' => $cid,
+      'contact_id' => $this->individualCreate(),
       'receive_date' => '2016-01-20',
       'total_amount' => 100,
       'financial_type_id' => 1,
@@ -173,8 +169,8 @@ public function testUpdateCreditCardDetailsUsingContributionAPI() {
         'return' => ['card_type_id', 'pan_truncation'],
       ]
     );
-    $this->assertEquals(CRM_Utils_Array::value('card_type_id', $financialTrxn), NULL);
-    $this->assertEquals(CRM_Utils_Array::value('pan_truncation', $financialTrxn), NULL);
+    $this->assertArrayNotHasKey('card_type_id', $financialTrxn);
+    $this->assertEquals(NULL, CRM_Utils_Array::value('pan_truncation', $financialTrxn));
     $params = [
       'card_type_id' => 2,
       'pan_truncation' => 4567,
@@ -188,8 +184,8 @@ public function testUpdateCreditCardDetailsUsingContributionAPI() {
         'return' => ['card_type_id', 'pan_truncation'],
       ]
     );
-    $this->assertEquals($financialTrxn['card_type_id'], 2);
-    $this->assertEquals($financialTrxn['pan_truncation'], 4567);
+    $this->assertEquals(2, $financialTrxn['card_type_id']);
+    $this->assertEquals(4567, $financialTrxn['pan_truncation']);
   }
 
   /**

diff --git a/tests/phpunit/CRM/Core/Permission/BaseTest.php b/tests/phpunit/CRM/Core/Permission/BaseTest.php
@@ -1,12 +1,15 @@
 <?php
 
+use Civi\Test\ACLPermissionTrait;
+
 /**
  * Class CRM_Core_Permission_BaseTest
+ *
  * @group headless
  */
 class CRM_Core_Permission_BaseTest extends CiviUnitTestCase {
 
-  use Civi\Test\ACLPermissionTrait;
+  use ACLPermissionTrait;
 
   /**
    * @return array