-
Notifications
You must be signed in to change notification settings - Fork 16
Convert og_is_member to Og:isMember and add test coverage #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4ac5d1e
6c24bdf
4da4d52
b6fa9a9
3b5d0a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
|
|
||
| use Drupal\Component\Render\FormattableMarkup; | ||
| use Drupal\Component\Utility\NestedArray; | ||
| use Drupal\Core\Entity\EntityInterface; | ||
| use Drupal\Core\Field\FieldConfigInterface; | ||
| use Drupal\Core\Field\FieldDefinitionInterface; | ||
| use Drupal\field\Entity\FieldConfig; | ||
|
|
@@ -103,10 +104,8 @@ public static function createField($plugin_id, $entity_type, $bundle, array $set | |
| /** | ||
| * Gets the groups an entity is associated with. | ||
| * | ||
| * @param $entity_type | ||
| * The entity type. | ||
| * @param $entity_id | ||
| * The entity ID. | ||
| * @param \Drupal\Core\Entity\EntityInterface $entity | ||
| * The entity to get groups for. | ||
| * @param $states | ||
| * (optional) Array with the state to return. Defaults to active. | ||
| * @param $field_name | ||
|
|
@@ -117,7 +116,10 @@ public static function createField($plugin_id, $entity_type, $bundle, array $set | |
| * the OG membership ID and the group ID as the value. If nothing found, | ||
| * then an empty array. | ||
| */ | ||
| public static function getEntityGroups($entity_type, $entity_id, $states = [OG_STATE_ACTIVE], $field_name = NULL) { | ||
| public static function getEntityGroups(EntityInterface $entity, $states = [OG_STATE_ACTIVE], $field_name = NULL) { | ||
| $entity_type_id = $entity->getEntityTypeId(); | ||
| $entity_id = $entity->id(); | ||
|
|
||
| // Get a string identifier of the states, so we can retrieve it from cache. | ||
| if ($states) { | ||
| sort($states); | ||
|
|
@@ -128,7 +130,7 @@ public static function getEntityGroups($entity_type, $entity_id, $states = [OG_S | |
| } | ||
|
|
||
| $identifier = [ | ||
| $entity_type, | ||
| $entity_type_id, | ||
| $entity_id, | ||
| $state_identifier, | ||
| $field_name, | ||
|
|
@@ -142,7 +144,7 @@ public static function getEntityGroups($entity_type, $entity_id, $states = [OG_S | |
|
|
||
| static::$entityGroupCache[$identifier] = []; | ||
| $query = \Drupal::entityQuery('og_membership') | ||
| ->condition('entity_type', $entity_type) | ||
| ->condition('entity_type', $entity_type_id) | ||
| ->condition('etid', $entity_id); | ||
|
|
||
| if ($states) { | ||
|
|
@@ -168,6 +170,31 @@ public static function getEntityGroups($entity_type, $entity_id, $states = [OG_S | |
| return static::$entityGroupCache[$identifier]; | ||
| } | ||
|
|
||
| /** | ||
| * Return TRUE if entity belongs to a group. | ||
| * | ||
| * @param \Drupal\Core\Entity\EntityInterface $group | ||
| * The group entity to check. | ||
| * @param \Drupal\Core\Entity\EntityInterface $entity | ||
| * The entity to get groups for. | ||
| * @param array $states | ||
| * (optional) Array with the membership states to check the membership. | ||
| * Defaults to active memberships. | ||
| * | ||
| * @return bool | ||
| * TRUE if the entity (e.g. the user) belongs to a group and is not pending | ||
| * or blocked. | ||
| */ | ||
| public static function isMember(EntityInterface $group, EntityInterface $entity, $states = [OG_STATE_ACTIVE]) { | ||
| $groups = static::getEntityGroups($entity, $states); | ||
| $group_entity_type_id = $group->getEntityTypeId(); | ||
| // We need to create a map of the group ids as Og::getEntityGroups returns a | ||
| // map of membership_id => group entity for each type. | ||
| return !empty($groups[$group_entity_type_id]) && in_array($group->id(), array_map(function($group_entity) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we don't use array_keys() because those should be having the entity IDs by keys, right?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment :) they are keyed by membership ID. Not group.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise, of course!
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, didn't knew about that, and commends are hard to read. Ship it I guess? On Mon, Dec 7, 2015 at 8:08 PM, Damian Lee notifications@github.com wrote:
|
||
| return $group_entity->id(); | ||
| }, $groups[$group_entity_type_id])); | ||
| } | ||
|
|
||
| /** | ||
| * Check if the given entity type and bundle is a group. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,216 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * @file | ||
| * Contains \Drupal\Tests\og\Kernel\Entity\GetEntityGroupsTest. | ||
| */ | ||
|
|
||
| namespace Drupal\Tests\og\Kernel\Entity; | ||
|
|
||
| use Drupal\entity_test\Entity\EntityTest; | ||
| use Drupal\KernelTests\KernelTestBase; | ||
| use Drupal\Component\Utility\Unicode; | ||
| use Drupal\og\Entity\OgMembership; | ||
| use Drupal\og\Og; | ||
| use Drupal\user\Entity\User; | ||
|
|
||
| /** | ||
| * Tests getting the memberships of an entity. | ||
| * | ||
| * @group og | ||
| */ | ||
| class GetEntityGroupsTest extends KernelTestBase { | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public static $modules = ['system', 'user', 'field', 'og', 'entity_test']; | ||
|
|
||
| /** | ||
| * @var \Drupal\user\Entity\User | ||
| */ | ||
| protected $user1; | ||
|
|
||
| /** | ||
| * @var \Drupal\user\Entity\User | ||
| */ | ||
| protected $user2; | ||
|
|
||
| /** | ||
| * @var \Drupal\user\Entity\User | ||
| */ | ||
| protected $user3; | ||
|
|
||
| /** | ||
| * @var \Drupal\entity_test\Entity\EntityTest | ||
| */ | ||
| protected $group1; | ||
|
|
||
| /** | ||
| * @var \Drupal\entity_test\Entity\EntityTest | ||
| */ | ||
| protected $group2; | ||
|
|
||
| /** | ||
| * The machine name of the group node type. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $groupBundle; | ||
|
|
||
| /** | ||
| * The machine name of the group content node type. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $groupContentBundle; | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| protected function setUp() { | ||
| parent::setUp(); | ||
|
|
||
| $this->installConfig(['og']); | ||
| $this->installEntitySchema('og_membership'); | ||
| $this->installEntitySchema('user'); | ||
| $this->installEntitySchema('entity_test'); | ||
| $this->installSchema('system', 'sequences'); | ||
|
|
||
| $this->groupBundle = Unicode::strtolower($this->randomMachineName()); | ||
| $this->groupContentBundle = Unicode::strtolower($this->randomMachineName()); | ||
|
|
||
| // Create users. | ||
| $this->user1 = User::create(['name' => $this->randomString()]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. random strings make it harder to debug, I'd just go with hardcoded strings
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. This was copied from another tests. Replacing all of them would be
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a new issue? |
||
| $this->user1->save(); | ||
|
|
||
| $this->user2 = User::create(['name' => $this->randomString()]); | ||
| $this->user2->save(); | ||
|
|
||
| $this->user3 = User::create(['name' => $this->randomString()]); | ||
| $this->user3->save(); | ||
|
|
||
| // Define the group content as group. | ||
| Og::groupManager()->addGroup('entity_test', $this->groupBundle); | ||
|
|
||
| // Create a group and associate with user 1. | ||
| $this->group1 = EntityTest::create([ | ||
| 'type' => $this->groupBundle, | ||
| 'name' => $this->randomString(), | ||
| 'user_id' => $this->user1->id(), | ||
| ]); | ||
| $this->group1->save(); | ||
|
|
||
| // Create a group and associate with user 2. | ||
| $this->group2 = EntityTest::create([ | ||
| 'type' => $this->groupBundle, | ||
| 'name' => $this->randomString(), | ||
| 'user_id' => $this->user2->id(), | ||
| ]); | ||
| $this->group2->save(); | ||
| } | ||
|
|
||
| /** | ||
| * Tests group owners have the correct groups. | ||
| */ | ||
| public function testOwnerGroupsOnly() { | ||
| $actual = Og::getEntityGroups($this->user1); | ||
|
|
||
| $this->assertCount(1, $actual['entity_test']); | ||
| $this->assertGroupExistsInResults($this->group1, $actual); | ||
|
|
||
| // Also check isMember. | ||
| $this->assertTrue(Og::isMember($this->group1, $this->user1)); | ||
| $this->assertFalse(Og::isMember($this->group1, $this->user2)); | ||
|
|
||
| $actual = Og::getEntityGroups($this->user2); | ||
|
|
||
| $this->assertCount(1, $actual['entity_test']); | ||
| $this->assertGroupExistsInResults($this->group2, $actual); | ||
|
|
||
| // Also check isMember. | ||
| $this->assertTrue(Og::isMember($this->group2, $this->user2)); | ||
| $this->assertFalse(Og::isMember($this->group2, $this->user1)); | ||
| } | ||
|
|
||
| /** | ||
| * Tests other groups users are added to. | ||
| */ | ||
| public function testOtherGroups() { | ||
| // Should be a part of no groups. | ||
| $this->assertEquals([], Og::getEntityGroups($this->user3)); | ||
| $this->assertFalse(Og::isMember($this->group1, $this->user3)); | ||
| $this->assertFalse(Og::isMember($this->group2, $this->user3)); | ||
|
|
||
| // Invalidate the caches so the static cache is cleared and group data is | ||
| // fetched again for the user. | ||
| Og::invalidateCache(); | ||
|
|
||
| // Add user to group 1 should now return that group only. | ||
| $this->createMembership($this->user3, $this->group1); | ||
|
|
||
| $actual = Og::getEntityGroups($this->user3); | ||
|
|
||
| $this->assertCount(1, $actual['entity_test']); | ||
| $this->assertGroupExistsInResults($this->group1, $actual); | ||
|
|
||
| $this->assertTrue(Og::isMember($this->group1, $this->user3)); | ||
| $this->assertFalse(Og::isMember($this->group2, $this->user3)); | ||
|
|
||
| Og::invalidateCache(); | ||
|
|
||
| // Add to group 2 should also return that. | ||
| $this->createMembership($this->user3, $this->group2); | ||
|
|
||
| $actual = Og::getEntityGroups($this->user3); | ||
|
|
||
| $this->assertCount(2, $actual['entity_test']); | ||
| $this->assertGroupExistsInResults($this->group1, $actual); | ||
| $this->assertGroupExistsInResults($this->group2, $actual); | ||
|
|
||
| $this->assertTrue(Og::isMember($this->group1, $this->user3)); | ||
| $this->assertTrue(Og::isMember($this->group2, $this->user3)); | ||
| } | ||
|
|
||
| /** | ||
| * Creates an Og membership entity. | ||
| * | ||
| * @todo This is a temp function, which will be later replaced by Og::group(). | ||
| * | ||
| * @param \Drupal\user\Entity\User $user | ||
| * @param \Drupal\Core\Entity\EntityInterface $group | ||
| * | ||
| * @return \Drupal\og\Entity|OgMembership | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should use \ instead of | |
||
| */ | ||
| protected function createMembership($user, $group) { | ||
| $membership = OgMembership::create(['type' => OG_MEMBERSHIP_TYPE_DEFAULT]) | ||
| ->setEntityId($user->id()) | ||
| ->setEntityType('user') | ||
| ->setGid($group->id()) | ||
| ->setGroupType($group->getEntityTypeId()) | ||
| ->save(); | ||
|
|
||
| return $membership; | ||
| } | ||
|
|
||
| /** | ||
| * Asserts whether a group ID exists in some results. | ||
| * | ||
| * Assumes entity_type is used. | ||
| * | ||
| * @param \Drupal\Core\Entity\EntityInterface $group_to_check | ||
| * @param array $results | ||
| */ | ||
| protected function assertGroupExistsInResults($group_to_check, array $results) { | ||
| $found = FALSE; | ||
| foreach ($results['entity_test'] as $group) { | ||
| if ($group->id() == $group_to_check->id()) { | ||
| $found = TRUE; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of this assertTrue() IMHO it would be better to throw an exception if we reach the end of loop without an early return |
||
| break; | ||
| } | ||
| } | ||
|
|
||
| $this->assertTrue($found); | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need static methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @dawehner welcome to the party! :)
What would be the alternative here? Or in other words, why not static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we were going to review this after. But just moving stuff into
services and using those services instead.
On 7 Dec 2015 18:02, "Amitai Burstein" notifications@github.com wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now we do, yes :) we don't have anywhere else to put them at the
moment. See my other comment about that. We need to see what we've got and
then how it makes sense to split that IMO.
On 7 Dec 2015 17:54, "Daniel Wehner" notifications@github.com wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean your getting involved then @da_wehner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the trivia night gang all over again! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mh, if that damian is involved I could be motivated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point regarding static methods, but yeah having kernel tests helps a lot of with refactoring those things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly! We need a proper discussion about our services I think. Now we
have a lot of the building blocks there I think soon could be a good time.
On 7 Dec 2015 18:30, "Daniel Wehner" notifications@github.com wrote: