Merge pull request #1 from Gizra/8.x-1.x

Merger

og.module

@@ -204,7 +204,7 @@ function og_entity_create_access(AccountInterface $account, array $context, $bun
   $required = FALSE;
 
   $field_definitions = \Drupal::service('entity_field.manager')->getFieldDefinitions($entity_type_id, $bundle);
-  foreach ($field_definitions as $field_name => $field_definition) {
+  foreach ($field_definitions as $field_definition) {
     /** @var \Drupal\Core\Field\FieldDefinitionInterface $field_definition */
     if (!\Drupal::service('og.group_audience_helper')->isGroupAudienceField($field_definition)) {
       continue;
@@ -359,7 +359,7 @@ function og_invalidate_group_content_cache_tags(EntityInterface $entity) {
       }
     }
 
-    foreach ($membership_manager->getGroups($entity) as $group_entity_type_id => $groups) {
+    foreach ($membership_manager->getGroups($entity) as $groups) {
       /** @var \Drupal\Core\Entity\ContentEntityInterface $group */
       foreach ($groups as $group) {
         $tags = Cache::mergeTags($tags, $group->getCacheTagsToInvalidate());

phpcs-ruleset.xml.dist

@@ -22,10 +22,6 @@
     <rule ref="./vendor/drupal/coder/coder_sniffer/DrupalPractice">
         <!-- https://github.com/Gizra/og/issues/549 -->
         <exclude name="DrupalPractice.CodeAnalysis.VariableAnalysis.UndefinedVariable" />
-        <!-- https://github.com/Gizra/og/issues/545 -->
-        <exclude name="DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable" />
-        <!-- https://github.com/Gizra/og/issues/548 -->
-        <exclude name="DrupalPractice.Commenting.ExpectedException.TagFound" />
         <!-- https://github.com/Gizra/og/issues/550 -->
         <exclude name="DrupalPractice.Constants.GlobalDefine.GlobalConstant" />
         <!-- https://github.com/Gizra/og/issues/544 -->

src/Cache/Context/OgRoleCacheContext.php

@@ -121,9 +121,9 @@ public function getContext() {
       // Sort the memberships, so that the same key can be generated, even if
       // the memberships were defined in a different order.
       ksort($memberships);
-      foreach ($memberships as $entity_type_id => &$groups) {
+      foreach ($memberships as &$groups) {
         ksort($groups);
-        foreach ($groups as $group_id => &$role_names) {
+        foreach ($groups as &$role_names) {
           sort($role_names);
         }
       }

src/ContextProvider/OgContext.php

@@ -126,7 +126,6 @@ protected function getOgContext() {
    */
   protected function getBestCandidate() {
     $collection = new OgResolvedGroupCollection();
-    $plugins = [];
 
     // Retrieve the list of group resolvers. These are stored in config, and are
     // ordered by priority.
@@ -135,8 +134,6 @@ protected function getBestCandidate() {
     foreach ($group_resolvers as $plugin_id) {
       /** @var \Drupal\og\OgGroupResolverInterface $plugin */
       if ($plugin = $this->pluginManager->createInstance($plugin_id)) {
-        $plugins[$plugin_id] = $plugin;
-
         // Set the default vote weight according to the plugin's priority.
         $collection->setVoteWeight($priority);
 

src/Event/DefaultRoleEvent.php

@@ -73,7 +73,7 @@ public function setRole(OgRole $role) {
    * {@inheritdoc}
    */
   public function setRoles(array $roles) {
-    foreach ($roles as $name => $properties) {
+    foreach ($roles as $properties) {
       $this->setRole($properties);
     }
   }

src/GroupTypeManager.php

@@ -200,7 +200,7 @@ public function getAllGroupBundles($entity_type = NULL) {
    */
   public function getAllGroupContentBundleIds() {
     $bundles = [];
-    foreach ($this->getGroupRelationMap() as $group_entity_type_id => $group_bundle_ids) {
+    foreach ($this->getGroupRelationMap() as $group_bundle_ids) {
       foreach ($group_bundle_ids as $group_content_entity_type_ids) {
         foreach ($group_content_entity_type_ids as $group_content_entity_type_id => $group_content_bundle_ids) {
           $bundles[$group_content_entity_type_id] = array_merge(isset($bundles[$group_content_entity_type_id]) ? $bundles[$group_content_entity_type_id] : [], $group_content_bundle_ids);

src/Og.php

@@ -363,7 +363,9 @@ public static function invalidateCache() {
   protected static function getFieldBaseDefinition($plugin_id) {
     /** @var OgFieldsPluginManager $plugin_manager */
     $plugin_manager = \Drupal::service('plugin.manager.og.fields');
-    if (!$field_config = $plugin_manager->getDefinition($plugin_id)) {
+
+    $field_config = $plugin_manager->getDefinition($plugin_id);
+    if (!$field_config) {
       throw new \Exception("The Organic Groups field with plugin ID $plugin_id is not a valid plugin.");
     }
 

src/Plugin/EntityReferenceSelection/OgSelection.php

@@ -89,7 +89,7 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS')
     $ids = [];
     if (!empty($this->getConfiguration()['field_mode']) && $this->getConfiguration()['field_mode'] === 'admin') {
       // Don't include the groups, the user doesn't have create permission.
-      foreach ($user_groups as $delta => $group) {
+      foreach ($user_groups as $group) {
         $ids[] = $group->id();
       }
 

tests/src/Kernel/Access/AccessByOgMembershipTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\og\Kernel\Access;
 
+use Drupal\Core\Entity\EntityStorageException;
 use Drupal\KernelTests\KernelTestBase;
 use Drupal\Tests\node\Traits\ContentTypeCreationTrait;
 use Drupal\Tests\og\Traits\OgMembershipCreationTrait;
@@ -182,8 +183,6 @@ public function testEntityOperationAccess($user, array $expected_results) {
 
   /**
    * Tests exception is thrown when trying to save non-member role.
-   *
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    */
   public function testNonMemberRoleMembershipSave() {
     /** @var \Drupal\og\Entity\OgRole $role */
@@ -194,6 +193,7 @@ public function testNonMemberRoleMembershipSave() {
       ->save();
 
     $membership = OgMembership::create();
+    $this->expectException(EntityStorageException::class);
     $membership
       ->setOwner($this->users['non-member'])
       ->setGroup($this->group)

tests/src/Kernel/Entity/GroupMembershipManagerTest.php

@@ -305,7 +305,7 @@ public function testGetGroups($group_type_id, $group_bundle, array $expected) {
         /** @var \Drupal\Core\Entity\EntityInterface $expected_group */
         $expected_group = $this->groups[$expected_type][$expected_key];
         /** @var \Drupal\Core\Entity\EntityInterface $group */
-        foreach ($result[$expected_type] as $key => $group) {
+        foreach ($result[$expected_type] as $group) {
           if ($group->getEntityTypeId() === $expected_group->getEntityTypeId() && $group->id() === $expected_group->id()) {
             // The expected result was found. Continue the test.
             continue 2;

tests/src/Kernel/Entity/OgMembershipTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\og\Kernel\Entity;
 
+use Drupal\Core\Entity\EntityStorageException;
 use Drupal\entity_test\Entity\EntityTest;
 use Drupal\entity_test\Entity\EntityTestMul;
 use Drupal\KernelTests\KernelTestBase;
@@ -138,25 +139,25 @@ public function testGetSetOwner() {
    * Tests getting the owner of a newly created membership.
    *
    * @covers ::getOwner
-   * @expectedException \LogicException
    */
   public function testGetOwnerOnNewMembership() {
     // A brand new entity does not have an owner set yet. It should throw a
     // logic exception.
     $membership = OgMembership::create();
+    $this->expectException(\LogicException::class);
     $membership->getOwner();
   }
 
   /**
    * Tests getting the owner ID of a newly created membership.
    *
    * @covers ::getOwnerId
-   * @expectedException \LogicException
    */
   public function testGetOwnerIdOnNewMembership() {
     // A brand new entity does not have an owner set yet. It should throw a
     // logic exception.
     $membership = OgMembership::create();
+    $this->expectException(\LogicException::class);
     $membership->getOwnerId();
   }
 
@@ -225,11 +226,11 @@ public function testMembershipStaticCache() {
    * Tests exceptions are thrown when trying to save a membership with no user.
    *
    * @covers ::preSave
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    */
   public function testSetNoUserException() {
     /** @var \Drupal\og\OgMembershipInterface $membership */
     $membership = OgMembership::create(['type' => OgMembershipInterface::TYPE_DEFAULT]);
+    $this->expectException(EntityStorageException::class);
     $membership
       ->setGroup($this->group)
       ->save();
@@ -239,11 +240,11 @@ public function testSetNoUserException() {
    * Tests exceptions are thrown when trying to save a membership with no group.
    *
    * @covers ::preSave
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    */
   public function testSetNoGroupException() {
     /** @var \Drupal\og\OgMembershipInterface $membership */
     $membership = OgMembership::create();
+    $this->expectException(EntityStorageException::class);
     $membership
       ->setOwner($this->user)
       ->save();
@@ -277,7 +278,6 @@ public function testNoOwnerException() {
    * Tests saving a membership with a non group entity.
    *
    * @covers ::preSave
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    */
   public function testSetNonValidGroupException() {
     $non_group = EntityTest::create([
@@ -288,14 +288,15 @@ public function testSetNonValidGroupException() {
     $non_group->save();
     /** @var \Drupal\og\OgMembershipInterface $membership */
     $membership = Og::createMembership($non_group, $this->user);
+
+    $this->expectException(EntityStorageException::class);
     $membership->save();
   }
 
   /**
    * Tests saving an existing membership.
    *
    * @covers ::preSave
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    */
   public function testSaveExistingMembership() {
     $group = EntityTest::create([
@@ -312,14 +313,15 @@ public function testSaveExistingMembership() {
     $membership1->save();
 
     $membership2 = Og::createMembership($group, $this->user);
+
+    $this->expectException(EntityStorageException::class);
     $membership2->save();
   }
 
   /**
    * Tests saving a membership with a role with a different group type.
    *
    * @covers ::preSave
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    * @dataProvider saveRoleWithWrongGroupTypeProvider
    */
   public function testSaveRoleWithWrongGroupType($group_entity_type_id, $group_bundle_id) {
@@ -338,6 +340,7 @@ public function testSaveRoleWithWrongGroupType($group_entity_type_id, $group_bun
       ->setName(mb_strtolower($this->randomMachineName()));
     $wrong_role->save();
 
+    $this->expectException(EntityStorageException::class);
     Og::createMembership($group, $this->user)->addRole($wrong_role)->save();
   }
 
@@ -438,7 +441,6 @@ public function isRoleValidProvider() {
    *   An array of test role metadata.
    *
    * @covers ::save
-   * @expectedException \Drupal\Core\Entity\EntityStorageException
    * @dataProvider saveMembershipWithInvalidRolesProvider
    */
   public function testSaveMembershipWithInvalidRoles(array $roles_metadata): void {
@@ -486,6 +488,7 @@ public function testSaveMembershipWithInvalidRoles(array $roles_metadata): void
 
     // Create a membership with the test group and roles. This should throw an
     // exception since the roles are invalid.
+    $this->expectException(EntityStorageException::class);
     OgMembership::create()
       ->setOwner($this->user)
       ->setGroup($this->group)
@@ -567,7 +570,6 @@ public function saveMembershipWithInvalidRolesProvider(): array {
    * Tests the exception thrown if the validity of a role cannot be established.
    *
    * @covers ::isRoleValid
-   * @expectedException \LogicException
    */
   public function testIsRoleValidException() {
     $role = OgRole::create([
@@ -578,6 +580,7 @@ public function testIsRoleValidException() {
 
     // If a membership doesn't have a group yet it is not possible to determine
     // wheter a role is valid.
+    $this->expectException(\LogicException::class);
     $membership->isRoleValid($role);
   }
 
@@ -643,12 +646,12 @@ public function testGetGroup() {
    * Tests getting the group from a new membership.
    *
    * @covers ::getGroup
-   * @expectedException \LogicException
    */
   public function testGetGroupOnNewMembership() {
     $membership = OgMembership::create();
 
     // When no group has been set yet, the method should throw an assertion.
+    $this->expectException(\LogicException::class);
     $membership->getGroup();
   }
 
@@ -671,10 +674,11 @@ public function testGetGroupBundle() {
    * Tests getting the group bundle of a newly created membership.
    *
    * @covers ::getGroupBundle
-   * @expectedException \LogicException
    */
   public function testGetGroupBundleOnNewMembership() {
     $membership = OgMembership::create();
+
+    $this->expectException(\LogicException::class);
     $membership->getGroupBundle();
   }
 
@@ -693,10 +697,11 @@ public function testGetGroupEntityType() {
    * Tests getting the group entity type ID of a newly created membership.
    *
    * @covers ::getGroupEntityType
-   * @expectedException \LogicException
    */
   public function testGetGroupEntityTypeOnNewMembership() {
     $membership = OgMembership::create();
+
+    $this->expectException(\LogicException::class);
     $membership->getGroupEntityType();
   }
 
@@ -715,10 +720,11 @@ public function testGetGroupId() {
    * Tests getting the group ID of a newly created membership.
    *
    * @covers ::getGroupId
-   * @expectedException \LogicException
    */
   public function testGetGroupIdOnNewMembership() {
     $membership = OgMembership::create();
+
+    $this->expectException(\LogicException::class);
     $membership->getGroupId();
   }
 

tests/src/Unit/DefaultRoleEventTest.php

@@ -325,7 +325,7 @@ public function testAddInvalidRole(array $invalid_roles) {
     $this->expectOgRoleCreation($invalid_roles);
 
     try {
-      foreach ($invalid_roles as $name => $invalid_role) {
+      foreach ($invalid_roles as $invalid_role) {
         $this->defaultRoleEvent->addRole($invalid_role);
       }
       $this->fail('An invalid role cannot be added.');
@@ -346,11 +346,11 @@ public function testAddInvalidRole(array $invalid_roles) {
    * @covers ::addRoles
    *
    * @dataProvider invalidDefaultRoleProvider
-   * @expectedException \InvalidArgumentException
    */
   public function testAddInvalidRoles(array $invalid_roles) {
     $this->expectOgRoleCreation($invalid_roles);
 
+    $this->expectException(\InvalidArgumentException::class);
     $this->defaultRoleEvent->addRoles($invalid_roles);
     $this->fail('An array of invalid roles cannot be added.');
   }
@@ -364,12 +364,12 @@ public function testAddInvalidRoles(array $invalid_roles) {
    * @covers ::setRole
    *
    * @dataProvider invalidDefaultRoleProvider
-   * @expectedException \InvalidArgumentException
    */
   public function testSetInvalidRole(array $invalid_roles) {
     $this->expectOgRoleCreation($invalid_roles);
 
-    foreach ($invalid_roles as $name => $invalid_role) {
+    $this->expectException(\InvalidArgumentException::class);
+    foreach ($invalid_roles as $invalid_role) {
       $this->defaultRoleEvent->setRole($invalid_role);
     }
   }
@@ -383,11 +383,11 @@ public function testSetInvalidRole(array $invalid_roles) {
    * @covers ::setRoles
    *
    * @dataProvider invalidDefaultRoleProvider
-   * @expectedException \InvalidArgumentException
    */
   public function testSetInvalidRoles(array $invalid_roles) {
     $this->expectOgRoleCreation($invalid_roles);
 
+    $this->expectException(\InvalidArgumentException::class);
     $this->defaultRoleEvent->setRoles($invalid_roles);
   }
 
@@ -400,12 +400,12 @@ public function testSetInvalidRoles(array $invalid_roles) {
    * @covers ::offsetSet
    *
    * @dataProvider invalidDefaultRoleProvider
-   * @expectedException \InvalidArgumentException
    */
   public function testInvalidOffsetSet(array $invalid_roles) {
     $this->expectOgRoleCreation($invalid_roles);
 
     foreach ($invalid_roles as $name => $invalid_role) {
+      $this->expectException(\InvalidArgumentException::class);
       $this->defaultRoleEvent[$name] = $invalid_role;
     }
   }