From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Mon, 3 Jul 2023 03:40:24 +0000 (+1200)
Subject: Fix ACLPermissionTrait to use EventTest create function
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=2810a487ea0bdf080876d4c46ea4fe855a086503;p=civicrm-core.git

Fix ACLPermissionTrait to use EventTest create function

Cleanup in related classes
---

diff --git a/CRM/Core/DAO/AllCoreTables.php b/CRM/Core/DAO/AllCoreTables.php
index f25bac3e93..4c22ee769b 100644
--- a/CRM/Core/DAO/AllCoreTables.php
+++ b/CRM/Core/DAO/AllCoreTables.php
@@ -360,7 +360,7 @@ class CRM_Core_DAO_AllCoreTables {
    *
    * @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
index 7ff1705e31..415a8f6aae 100644
--- a/Civi/Test/ACLPermissionTrait.php
+++ b/Civi/Test/ACLPermissionTrait.php
@@ -17,6 +17,7 @@ namespace Civi\Test;
  * 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 @@ trait ACLPermissionTrait {
    * @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 @@ trait ACLPermissionTrait {
    * @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 @@ trait ACLPermissionTrait {
    * @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 @@ trait ACLPermissionTrait {
    * @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 @@ trait ACLPermissionTrait {
    *   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 @@ trait ACLPermissionTrait {
    *   $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,18 +176,20 @@ trait ACLPermissionTrait {
   /**
    * 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');
@@ -188,8 +197,10 @@ trait ACLPermissionTrait {
 
   /**
    * 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
index b15fdd9333..407994b069 100644
--- 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
index 4f9367800b..93b711faff 100644
--- a/Civi/Test/EventTestTrait.php
+++ b/Civi/Test/EventTestTrait.php
@@ -163,7 +163,7 @@ trait EventTestTrait {
    *
    * @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
index 920cfa878d..c65d292d61 100644
--- 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
index 4efdb22044..557ce156a6 100644
--- a/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php
+++ b/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php
@@ -124,11 +124,10 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase {
    *
    * @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 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase {
 
   /**
    * 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 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase {
         '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 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase {
         '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
index 2b1e701fab..322c735be0 100644
--- 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
diff --git a/tests/phpunit/CRM/Event/BAO/EventPermissionsTest.php b/tests/phpunit/CRM/Event/BAO/EventPermissionsTest.php
index a65878bc58..a80267e57b 100644
--- a/tests/phpunit/CRM/Event/BAO/EventPermissionsTest.php
+++ b/tests/phpunit/CRM/Event/BAO/EventPermissionsTest.php
@@ -9,49 +9,40 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Test\ACLPermissionTrait;
+
 /**
  * Class CRM_Event_BAO_EventPermissionsTest
  * @group headless
  */
 class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
 
-  use Civi\Test\ACLPermissionTrait;
-
-  /**
-   * @var int
-   */
-  private $contactID;
-
-  /**
-   * @var int
-   */
-  private $ownEventID;
-
-  /**
-   * @var int
-   */
-  private $otherEventID;
+  use ACLPermissionTrait;
 
   public function setUp(): void {
     parent::setUp();
-    $this->contactID = $this->createLoggedInUser();
+    $this->createLoggedInUser();
     $this->createOwnEvent();
     $this->createOtherEvent();
   }
 
+  public function tearDown(): void {
+    $this->quickCleanUpFinancialEntities();
+    parent::tearDown();
+  }
+
   public function createOwnEvent(): void {
     $event = $this->eventCreateUnpaid([
-      'created_id' => $this->contactID,
-    ]);
-    $this->ownEventID = $event['id'];
+      'created_id' => $this->ids['Contact']['logged_in'],
+    ], 'own');
+    $this->ids['Event']['own'] = $event['id'];
   }
 
   public function createOtherEvent(): void {
-    $otherContactID = $this->contactID + 1;
     $event = $this->eventCreateUnpaid([
-      'created_id' => $otherContactID,
+      'created_id' => $this->individualCreate([], 'other'),
     ], 'other');
-    $this->otherEventID = $event['id'];
+    $this->ids['Event']['other'] = $event['id'];
   }
 
   private function setViewOwnEventPermissions(): void {
@@ -79,10 +70,10 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testViewOwnEvent(): void {
     $this->setViewOwnEventPermissions();
-    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->ownEventID, CRM_Core_Permission::VIEW));
-    // Now check that caching is actually working
-    \Civi::$statics['CRM_Event_BAO_Event']['permission']['view'][$this->ownEventID] = FALSE;
-    $permissions = CRM_Event_BAO_Event::checkPermission($this->ownEventID, CRM_Core_Permission::VIEW);
+    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['own'], CRM_Core_Permission::VIEW));
+    // Now check that caching is actually working.
+    \Civi::$statics['CRM_Event_BAO_Event']['permission']['view'][$this->ids['Event']['own']] = FALSE;
+    $permissions = CRM_Event_BAO_Event::checkPermission($this->ids['Event']['own'], CRM_Core_Permission::VIEW);
     $this->assertFalse($permissions);
   }
 
@@ -91,7 +82,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testEditOwnEvent(): void {
     $this->setViewOwnEventPermissions();
-    $permissions = CRM_Event_BAO_Event::checkPermission($this->ownEventID, CRM_Core_Permission::EDIT);
+    $permissions = CRM_Event_BAO_Event::checkPermission($this->ids['Event']['own'], CRM_Core_Permission::EDIT);
     $this->assertTrue($permissions);
   }
 
@@ -103,7 +94,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
   public function testDeleteOwnEvent(): void {
     // Check that you can't delete your own event without "Delete in CiviEvent" permission
     $this->setViewOwnEventPermissions();
-    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->ownEventID, CRM_Core_Permission::DELETE));
+    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['own'], CRM_Core_Permission::DELETE));
   }
 
   /**
@@ -111,7 +102,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testViewOtherEventDenied(): void {
     $this->setViewOwnEventPermissions();
-    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->otherEventID, CRM_Core_Permission::VIEW));
+    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['other'], CRM_Core_Permission::VIEW));
   }
 
   /**
@@ -119,7 +110,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testViewOtherEventAllowed(): void {
     $this->setViewAllEventPermissions();
-    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->otherEventID, CRM_Core_Permission::VIEW));
+    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['other'], CRM_Core_Permission::VIEW));
   }
 
   /**
@@ -136,7 +127,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testEditOtherEventDenied(): void {
     $this->setViewAllEventPermissions();
-    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->otherEventID, CRM_Core_Permission::EDIT));
+    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['other'], CRM_Core_Permission::EDIT));
   }
 
   /**
@@ -144,7 +135,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testEditOtherEventAllowed(): void {
     $this->setEditAllEventPermissions();
-    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->otherEventID, CRM_Core_Permission::EDIT));
+    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['other'], CRM_Core_Permission::EDIT));
   }
 
   /**
@@ -152,7 +143,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testDeleteOtherEventAllowed(): void {
     $this->setDeleteAllEventPermissions();
-    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->otherEventID, CRM_Core_Permission::DELETE));
+    $this->assertTrue(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['other'], CRM_Core_Permission::DELETE));
   }
 
   /**
@@ -162,7 +153,7 @@ class CRM_Event_BAO_EventPermissionsTest extends CiviUnitTestCase {
    */
   public function testDeleteOtherEventDenied(): void {
     $this->setEditAllEventPermissions();
-    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->otherEventID, CRM_Core_Permission::DELETE));
+    $this->assertFalse(CRM_Event_BAO_Event::checkPermission($this->ids['Event']['other'], CRM_Core_Permission::DELETE));
   }
 
   /**
diff --git a/tests/phpunit/CRM/Event/Form/SearchTest.php b/tests/phpunit/CRM/Event/Form/SearchTest.php
index 737f06acb6..6d7da4f262 100644
--- a/tests/phpunit/CRM/Event/Form/SearchTest.php
+++ b/tests/phpunit/CRM/Event/Form/SearchTest.php
@@ -5,11 +5,6 @@
  */
 class CRM_Event_Form_SearchTest extends CiviUnitTestCase {
 
-  /**
-   * @var int
-   */
-  private $individualID;
-
   /**
    * @var array
    */
@@ -17,7 +12,7 @@ class CRM_Event_Form_SearchTest extends CiviUnitTestCase {
 
   public function setUp(): void {
     parent::setUp();
-    $this->individualID = $this->individualCreate();
+    $individualID = $this->individualCreate();
     $event = $this->eventCreatePaid();
     $priceFieldValues = $this->createPriceSet('event', $event['id'], [
       'html_type'    => 'Radio',
@@ -34,7 +29,7 @@ class CRM_Event_Form_SearchTest extends CiviUnitTestCase {
 
     $this->participantCreate([
       'event_id'  => $event['id'],
-      'contact_id' => $this->individualID,
+      'contact_id' => $individualID,
       'status_id' => 1,
       'fee_level' => $this->participantPrice['label'],
       'fee_amount' => $this->participantPrice['amount'],
@@ -49,9 +44,12 @@ class CRM_Event_Form_SearchTest extends CiviUnitTestCase {
   }
 
   /**
-   *  Test that search form returns correct number of rows for complex regex filters.
+   * Test that search form returns correct number of rows for complex regex
+   * filters.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testSearch() {
+  public function testSearch(): void {
     $form = new CRM_Event_Form_Search();
     $form->controller = new CRM_Event_Controller_Search();
     $form->preProcess();
diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 3202d55524..a591cbed92 100644
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -1021,7 +1021,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase {
    * @noinspection PhpUnhandledExceptionInspection
    * @noinspection PhpDocMissingThrowsInspection
    */
-  public function eventCreate(array $params = [], $identifier = 0): array {
+  public function eventCreate(array $params = [], string $identifier = 'event'): array {
     // if no contact was passed, make up a dummy event creator
     if (!isset($params['contact_id'])) {
       $params['contact_id'] = $this->_contactCreate([
diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php
index f824fd346d..8c16bc1bbe 100644
--- a/tests/phpunit/api/v3/ReportTemplateTest.php
+++ b/tests/phpunit/api/v3/ReportTemplateTest.php
@@ -181,10 +181,10 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
   }
 
   /**
-   * @param \CRM_Core_DAO $entity
+   * @param string $entity
    * @param array $clauses
    */
-  public function hookSelectWhere($entity, &$clauses) {
+  public function hookSelectWhere(string $entity, array &$clauses): void {
     // Restrict access to cases by type
     if ($entity === 'Contact') {
       $clauses['contact_type'][] = " =  'Organization' ";
@@ -193,10 +193,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test getrows on contact summary report.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testReportTemplateGetRowsContactSummary() {
+  public function testReportTemplateGetRowsContactSummary(): void {
     $result = $this->callAPISuccess('report_template', 'getrows', [
       'report_id' => 'contact/summary',
       'options' => ['metadata' => ['labels', 'title']],
@@ -219,7 +217,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
   /**
    * Test getrows on Mailing Opened report.
    */
-  public function testReportTemplateGetRowsMailingUniqueOpened() {
+  public function testReportTemplateGetRowsMailingUniqueOpened(): void {
     $description = 'Retrieve rows from a mailing opened report template.';
     $this->loadXMLDataSet(__DIR__ . '/../../CRM/Mailing/BAO/queryDataset.xml');
 
@@ -254,11 +252,9 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @dataProvider getReportTemplates
    *
-   * @param $reportID
-   *
-   * @throws \CRM_Core_Exception
+   * @param string $reportID
    */
-  public function testReportTemplateGetRowsAllReports($reportID) {
+  public function testReportTemplateGetRowsAllReports(string $reportID): void {
     //$reportID = 'logging/contact/summary';
     if (stripos($reportID, 'has existing issues') !== FALSE) {
       $this->markTestIncomplete($reportID);
@@ -279,10 +275,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    * Test logging report when a custom data table has a table removed by hook.
    *
    * Here we are checking that no fatal is triggered.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testLoggingReportWithHookRemovalOfCustomDataTable() {
+  public function testLoggingReportWithHookRemovalOfCustomDataTable(): void {
     Civi::settings()->set('logging', 1);
     $group1 = $this->customGroupCreate();
     $group2 = $this->customGroupCreate(['name' => 'second_one', 'title' => 'second one', 'table_name' => 'civicrm_value_second_one']);
@@ -294,8 +288,6 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
       'report_id' => 'logging/contact/summary',
     ]);
     Civi::settings()->set('logging', 0);
-    $this->customGroupDelete($group1['id']);
-    $this->customGroupDelete($group2['id']);
   }
 
   /**
@@ -303,7 +295,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @param array $logTableSpec
    */
-  public function alterLogTablesRemoveCustom(&$logTableSpec) {
+  public function alterLogTablesRemoveCustom(array &$logTableSpec): void {
     unset($logTableSpec['civicrm_value_second_one']);
   }
 
@@ -315,11 +307,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    * @dataProvider getReportTemplates
    *
    * @param $reportID
-   *
-   * @throws \PHPUnit\Framework\IncompleteTestError
-   * @throws \CRM_Core_Exception
    */
-  public function testReportTemplateGetRowsAllReportsACL($reportID) {
+  public function testReportTemplateGetRowsAllReportsACL($reportID): void {
     if (stripos($reportID, 'has existing issues') !== FALSE) {
       $this->markTestIncomplete($reportID);
     }
@@ -340,16 +329,15 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @dataProvider getReportTemplates
    *
-   * @param $reportID
+   * @param string $reportID
    *
-   * @throws \PHPUnit\Framework\IncompleteTestError
    */
-  public function testReportTemplateGetStatisticsAllReports($reportID) {
+  public function testReportTemplateGetStatisticsAllReports(string $reportID): void {
     if (stripos($reportID, 'has existing issues') !== FALSE) {
       $this->markTestIncomplete($reportID);
     }
     if (in_array($reportID, ['contribute/softcredit', 'contribute/bookkeeping'])) {
-      $this->markTestIncomplete($reportID . ' has non enotices when calling statistics fn');
+      $this->markTestIncomplete($reportID . ' has non e-notices when calling statistics fn');
     }
     if (strpos($reportID, 'logging') === 0) {
       Civi::settings()->set('logging', 1);
@@ -368,8 +356,10 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Implements hook_civicrm_alterReportVar().
+   *
+   * @noinspection PhpParameterByRefIsNotUsedAsReferenceInspection
    */
-  public function alterReportVarHook($varType, &$var, &$object) {
+  public function alterReportVarHook($varType, &$var, &$object): void {
     if ($varType === 'sql' && $object instanceof CRM_Report_Form_Contribute_Summary) {
       $from = $var->getVar('_from');
       $from .= ' LEFT JOIN civicrm_financial_type as temp ON temp.id = contribution_civireport.financial_type_id';
@@ -412,7 +402,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * These templates require minimal data config.
    */
-  public static function getContributionReportTemplates() {
+  public static function getContributionReportTemplates(): array {
     return [['contribute/summary'], ['contribute/detail'], ['contribute/repeat'], ['topDonor' => 'contribute/topDonor']];
   }
 
@@ -420,8 +410,10 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    * Get contribution templates that work with basic filter tests.
    *
    * These templates require minimal data config.
+   *
+   * @return array
    */
-  public static function getMembershipReportTemplates() {
+  public static function getMembershipReportTemplates(): array {
     return [['member/detail']];
   }
 
@@ -430,7 +422,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @return array
    */
-  public static function getMembershipAndContributionReportTemplatesForGroupTests() {
+  public static function getMembershipAndContributionReportTemplatesForGroupTests(): array {
     $templates = array_merge(self::getContributionReportTemplates(), self::getMembershipReportTemplates());
     foreach ($templates as $key => $value) {
       if (array_key_exists('topDonor', $value)) {
@@ -448,10 +440,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test Lybunt report to check basic inclusion of a contact who gave in the year before the chosen year.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testLybuntReportWithData() {
+  public function testLybuntReportWithData(): void {
     $inInd = $this->individualCreate();
     $outInd = $this->individualCreate();
     $this->contributionCreate(['contact_id' => $inInd, 'receive_date' => '2014-03-01']);
@@ -467,10 +457,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test Lybunt report applies ACLs.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testLybuntReportWithDataAndACLFilter() {
+  public function testLybuntReportWithDataAndACLFilter(): void {
     CRM_Core_Config::singleton()->userPermissionClass->permissions = ['administer CiviCRM'];
     $inInd = $this->individualCreate();
     $outInd = $this->individualCreate();
@@ -493,10 +481,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test Lybunt report to check basic inclusion of a contact who gave in the year before the chosen year.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testLybuntReportWithFYData() {
+  public function testLybuntReportWithFYData(): void {
     $inInd = $this->individualCreate();
     $outInd = $this->individualCreate();
     $this->contributionCreate(['contact_id' => $inInd, 'receive_date' => '2014-10-01']);
@@ -529,16 +515,14 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
           AND ( contribution_civireport.contribution_status_id IN (1) )
       GROUP BY contact_civireport.id');
     // Exclude whitespace in comparison as we don't care if it changes & this allows us to make the above readable.
-    $whitespacelessSql = preg_replace('/\s+/', ' ', $rows['metadata']['sql'][0]);
-    $this->assertStringContainsString($expected, $whitespacelessSql);
+    $whitespaceFreeSql = preg_replace('/\s+/', ' ', $rows['metadata']['sql'][0]);
+    $this->assertStringContainsString($expected, $whitespaceFreeSql);
   }
 
   /**
    * Test Lybunt report to check basic inclusion of a contact who gave in the year before the chosen year.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testLybuntReportWithFYDataOrderByLastYearAmount() {
+  public function testLybuntReportWithFYDataOrderByLastYearAmount(): void {
     $inInd = $this->individualCreate();
     $outInd = $this->individualCreate();
     $this->contributionCreate(['contact_id' => $inInd, 'receive_date' => '2014-10-01']);
@@ -595,7 +579,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testContributionSummaryWithNotINSmartGroupFilter($template): void {
+  public function testContributionSummaryWithNotINSmartGroupFilter(string $template): void {
     $groupID = $this->setUpPopulatedSmartGroup();
     $rows = $this->callAPISuccess('report_template', 'getrows', [
       'report_id' => 'contribute/summary',
@@ -702,10 +686,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @param string $template
    *   Report template unique identifier.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testReportsWithNonSmartGroupFilterWithACL($template): void {
+  public function testReportsWithNonSmartGroupFilterWithACL(string $template): void {
     $this->aclGroupID = $this->setUpPopulatedGroup();
     $this->createLoggedInUser();
     CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM'];
@@ -732,7 +714,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *   Rows returned from the report.
    * @param string $template
    */
-  protected function assertNumberOfContactsInResult($numberExpected, $rows, $template) {
+  protected function assertNumberOfContactsInResult(int $numberExpected, array $rows, string $template): void {
     if (isset($rows['values'][0]['civicrm_contribution_total_amount_count'])) {
       $this->assertEquals($numberExpected, $rows['values'][0]['civicrm_contribution_total_amount_count'], 'wrong row count in ' . $template);
     }
@@ -797,10 +779,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test we don't get a fatal grouping with QUARTER frequency.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContributionSummaryGroupByYearQuarterFrequency() {
+  public function testContributionSummaryGroupByYearQuarterFrequency(): void {
     $params = [
       'report_id' => 'contribute/summary',
       'fields' => ['total_amount' => 1, 'country_id' => 1],
@@ -816,10 +796,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test we don't get a fatal grouping with QUARTER frequency.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContributionSummaryGroupByDateFrequency() {
+  public function testContributionSummaryGroupByDateFrequency(): void {
     $params = [
       'report_id' => 'contribute/summary',
       'fields' => ['total_amount' => 1, 'country_id' => 1],
@@ -835,10 +813,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test we don't get a fatal grouping with QUARTER frequency.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContributionSummaryGroupByWeekFrequency() {
+  public function testContributionSummaryGroupByWeekFrequency(): void {
     $params = [
       'report_id' => 'contribute/summary',
       'fields' => ['total_amount' => 1, 'country_id' => 1],
@@ -854,8 +830,6 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * CRM-20640: Test the group filter works on the contribution summary when a single contact in 2 groups.
-   *
-   * @throws \CRM_Core_Exception
    */
   public function testContributionSummaryWithSingleContactsInTwoGroups(): void {
     $groupID1 = $this->setUpPopulatedGroup();
@@ -982,7 +956,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @return int
    */
-  public function setUpPopulatedGroup() {
+  public function setUpPopulatedGroup(): int {
     $individual1ID = $this->individualCreate();
     $individualID = $this->ids['Contact']['primary'] = $this->individualCreate();
     $individualIDRemoved = $this->individualCreate();
@@ -1078,16 +1052,14 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test Deferred Revenue Report.
-   *
-   * @throws \CRM_Core_Exception
    */
   public function testDeferredRevenueReport(): void {
-    $indv1 = $this->individualCreate();
-    $indv2 = $this->individualCreate();
+    $this->individualCreate([], 'first');
+    $this->individualCreate([], 'second');
     Civi::settings()->set('deferred_revenue_enabled', TRUE);
     $this->contributionCreate(
       [
-        'contact_id' => $indv1,
+        'contact_id' => $this->ids['Contact']['first'],
         'receive_date' => '2016-10-01',
         'revenue_recognition_date' => date('Y-m-t', strtotime(date('ymd') . '+3 month')),
         'financial_type_id' => 2,
@@ -1095,7 +1067,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     );
     $this->contributionCreate(
       [
-        'contact_id' => $indv1,
+        'contact_id' => $this->ids['Contact']['first'],
         'revenue_recognition_date' => date('Y-m-t', strtotime(date('ymd') . '+22 month')),
         'financial_type_id' => 4,
         'trxn_id' => NULL,
@@ -1104,7 +1076,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     );
     $this->contributionCreate(
       [
-        'contact_id' => $indv2,
+        'contact_id' => $this->ids['Contact']['second'],
         'revenue_recognition_date' => date('Y-m-t', strtotime(date('ymd') . '+1 month')),
         'financial_type_id' => 4,
         'trxn_id' => NULL,
@@ -1113,7 +1085,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     );
     $this->contributionCreate(
       [
-        'contact_id' => $indv2,
+        'contact_id' => $this->ids['Contact']['second'],
         'receive_date' => '2016-03-01',
         'revenue_recognition_date' => date('Y-m-t', strtotime(date('ymd') . '+4 month')),
         'financial_type_id' => 2,
@@ -1171,10 +1143,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test we don't get a fatal grouping with various frequencies.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testActivitySummaryGroupByFrequency() {
+  public function testActivitySummaryGroupByFrequency(): void {
     $this->createContactsWithActivities();
     foreach (['MONTH', 'YEARWEEK', 'QUARTER', 'YEAR'] as $frequency) {
       $params = [
@@ -1209,10 +1179,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test activity details report - requiring all current fields to be output.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testActivityDetails() {
+  public function testActivityDetails(): void {
     $this->createContactsWithActivities();
     $fields = [
       'contact_source' => '1',
@@ -1319,10 +1287,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Activity Details report has some whack-a-mole to fix when filtering on null/not null.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testActivityDetailsNullFilters() {
+  public function testActivityDetailsNullFilters(): void {
     $this->createContactsWithActivities();
     $params = [
       'report_id' => 'activity',
@@ -1349,7 +1315,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testActivityDetailsContactFilter() {
+  public function testActivityDetailsContactFilter(): void {
     $this->createContactsWithActivities();
     $params = [
       'report_id' => 'activity',
@@ -1363,10 +1329,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Set up some activity data..... use some chars that challenge our utf handling.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function createContactsWithActivities() {
+  public function createContactsWithActivities(): void {
     $this->contactIDs[] = $this->individualCreate(['last_name' => 'Brzęczysław', 'email' => 'techo@spying.com']);
     $this->contactIDs[] = $this->individualCreate(['last_name' => 'Łąchowski-Roberts']);
     $this->contactIDs[] = $this->individualCreate(['last_name' => 'Łąchowski-Roberts']);
@@ -1387,10 +1351,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test the group filter works on the contribution summary.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContributionDetailTotalHeader() {
+  public function testContributionDetailTotalHeader(): void {
     $contactID = $this->individualCreate();
     $contactID2 = $this->individualCreate();
     $this->contributionCreate(['contact_id' => $contactID, 'api.ContributionSoft.create' => ['amount' => 5, 'contact_id' => $contactID2]]);
@@ -1414,10 +1376,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test contact subtype filter on summary report.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContactSubtypeNotNull() {
+  public function testContactSubtypeNotNull(): void {
     $this->individualCreate(['contact_sub_type' => ['Student', 'Parent']]);
     $this->individualCreate();
 
@@ -1433,10 +1393,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test contact subtype filter on summary report.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContactSubtypeNull() {
+  public function testContactSubtypeNull(): void {
     $this->individualCreate(['contact_sub_type' => ['Student', 'Parent']]);
     $this->individualCreate();
 
@@ -1452,10 +1410,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test contact subtype filter on summary report.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContactSubtypeIn() {
+  public function testContactSubtypeIn(): void {
     $this->individualCreate(['contact_sub_type' => ['Student', 'Parent']]);
     $this->individualCreate();
 
@@ -1471,10 +1427,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test contact subtype filter on summary report.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testContactSubtypeNotIn() {
+  public function testContactSubtypeNotIn(): void {
     $this->individualCreate(['contact_sub_type' => ['Student', 'Parent']]);
     $this->individualCreate();
 
@@ -1613,10 +1567,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   /**
    * Test a report that uses getAddressColumns();
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testGetAddressColumns() {
+  public function testGetAddressColumns(): void {
     $template = 'event/participantlisting';
     $this->callAPISuccess('report_template', 'getrows', [
       'report_id' => $template,
@@ -1631,7 +1583,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    * Test that the contribution aggregate by relationship report filters
    * by financial type.
    */
-  public function testContributionAggregateByRelationship() {
+  public function testContributionAggregateByRelationship(): void {
     $contact = $this->individualCreate();
     // Two contributions with different financial types.
     // We don't really care which types, just different.
@@ -1657,10 +1609,10 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
   /**
    * Basic test of the repeat contributions report.
    */
-  public function testRepeatContributions() {
+  public function testRepeatContributions(): void {
     // our sorting options are limited in this report - default is last name so let's ensure order
-    $contact1 = $this->individualCreate(['last_name' => 'aaaaa']);
-    $contact2 = $this->individualCreate(['last_name' => 'zzzzz']);
+    $contact1 = $this->individualCreate(['last_name' => 'Aardvark']);
+    $contact2 = $this->individualCreate(['last_name' => 'Zebra']);
     $this->contributionCreate(['contact_id' => $contact1, 'receive_date' => (date('Y') - 1) . '-07-01', 'financial_type_id' => 1, 'total_amount' => '10']);
     $this->contributionCreate(['contact_id' => $contact1, 'receive_date' => (date('Y') - 1) . '-08-01', 'financial_type_id' => 1, 'total_amount' => '20']);
     $this->contributionCreate(['contact_id' => $contact1, 'receive_date' => date('Y') . '-01-01', 'financial_type_id' => 1, 'total_amount' => '40']);
@@ -1702,8 +1654,10 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    * In practice, besides some setup and trigger-wrangling, the report isn't
    * useful for activities, so we're checking activity_contact records, and
    * because of how an activity update works that's actually a delete+insert.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testLoggingDetail() {
+  public function testLoggingDetail(): void {
     \Civi::settings()->set('logging', 1);
     $this->createContactsWithActivities();
     $this->doQuestionableStuffInASeparateFunctionSoNobodyNotices();
@@ -1792,6 +1746,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    *
    * On the plus side, this doesn't affect other tests since if they enable
    * logging then that'll just recreate the variable and triggers.
+   *
+   * @throws \Civi\Core\Exception\DBQueryException
    */
   private function doQuestionableStuffInASeparateFunctionSoNobodyNotices(): void {
     CRM_Core_DAO::executeQuery("DELETE FROM civicrm_setting WHERE name='logging_uniqueid_date'");
@@ -1804,13 +1760,17 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
   /**
    * Implement hook to restrict to test group 1.
    *
+   * @implements hook_aclGroups
+   *
    * @param string $type
    * @param int $contactID
    * @param string $tableName
    * @param array $allGroups
    * @param array $ids
+   *
+   * @noinspection PhpUnusedParameterInspection
    */
-  public function aclGroupOnly($type, $contactID, $tableName, $allGroups, &$ids) {
+  public function aclGroupOnly(string $type, int $contactID, string $tableName, array $allGroups, array &$ids): void {
     if ($tableName === 'civicrm_group') {
       $ids = [$this->aclGroupID];
     }
@@ -1823,9 +1783,11 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
    * @param array $tables
    * @param array $whereTables
    * @param int|null $contactID
-   * @param string $where
+   * @param string|null $where
+   *
+   * @noinspection PhpUnusedParameterInspection
    */
-  public function aclGroupContactsOnly(string $type, array &$tables, array &$whereTables, &$contactID, &$where) {
+  public function aclGroupContactsOnly(string $type, array &$tables, array &$whereTables, ?int &$contactID, ?string &$where): void {
     if (!empty($where)) {
       $where .= ' AND ';
     }