Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion og.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ services:
arguments: ['@config.factory', '@current_user', '@module_handler']
og.event_subscriber:
class: Drupal\og\EventSubscriber\OgEventSubscriber
arguments: ['@og.permission_manager']
arguments: ['@og.permission_manager', '@entity_type.manager']
tags:
- { name: 'event_subscriber' }
og.group.manager:
Expand Down
72 changes: 42 additions & 30 deletions src/Entity/OgRole.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public function setGroupBundle($group_bundle) {
* OgRoleInterface::ROLE_TYPE_STANDARD.
*/
public function getRoleType() {
return $this->get('role_type');
return $this->get('role_type') ?: OgRoleInterface::ROLE_TYPE_STANDARD;
}

/**
Expand All @@ -174,11 +174,46 @@ public function setRoleType($role_type) {
return $this->set('role_type', $role_type);
}

/**
* {@inheritdoc}
*/
public function getName() {
// If the name is not set yet, try to derive it from the ID.
if (empty($this->name) && !empty($this->id()) && !empty($this->getGroupType()) && !empty($this->getGroupBundle())) {
// Check if the ID matches the pattern '{entity type}-{bundle}-{name}'.
$pattern = preg_quote("{$this->getGroupType()}-{$this->getGroupBundle()}-");
preg_match("/$pattern(.+)/", $this->id(), $matches);
if (!empty($matches[1])) {
$this->setName($matches[1]);
}
}
return $this->get('name');
}

/**
* {@inheritdoc}
*/
public function setName($name) {
$this->name = $name;
return $this;
}

/**
* {@inheritdoc}
*/
public function save() {
if ($this->isNew()) {
// The ID of a new OgRole has to consist of the entity type ID, bundle ID

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it also be good to be able to have more generic roles? rather than always tied to a group? I am currently trying to use something similar. I have an admin type role, and this is available for on on all groups I create.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an admin type role, and this is available for on on all groups I create.

Not sure I understand, but to clarify the logic in OG7 that should be ported to OG8:

OgRoles could be either perBundle or perGroupId.

  • per bundle means the group's roles are "read-only" for the group admins, and are controlled by the site admin.
  • per group Id means the group's roles are being handled by the group admins. They can add/ change existing roles - apart of course of the non-member and member.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thanks for clarifying @amitaibu. I think I am just using per bundle in that case... :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, ignore me please ;)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the per-group ID roles are not in yet, these are going to be added in #209. I'm not worrying about them here, that's the responsibility of that ticket.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damiankloip if you want to provide a role that is available for all groups, you can provide an event subscriber that simply ignores the passed in group entity type and bundle and returns the role every time, eg:

class MyEventSubscriber implements EventSubscriberInterface {

  public static function getSubscribedEvents() {
    return [
      DefaultRoleEventInterface::EVENT_NAME => [['provideDefaultRoles']],
    ];
  }

  public function provideDefaultRoles(DefaultRoleEventInterface $event) {
    // Just ignore $event->getEntityTypeId() and $event->getBundleId(),
    // this role applies to all groups.
    $event->addRole([
      'name' => 'moderator',
      'label' => 'Moderator',
      'is_admin' => TRUE,
    ]);
  }

}

// and role name, separated by dashes.
if ($this->isNew() && !empty($this->id())) {
list($entity_type_id, $bundle_id, $name) = explode('-', $this->id());
if ($entity_type_id !== $this->getGroupType() || $bundle_id !== $this->getGroupBundle() || $name !== $this->getName()) {
throw new ConfigValueException('The ID should consist of the group entity type ID, group bundle ID and role name, separated by dashes.');
}
}

// If a new OgRole is saved and the ID is not set, construct the ID from
// the entity type ID, bundle ID and role name.
if ($this->isNew() && empty($this->id())) {
if (empty($this->getGroupType())) {
throw new ConfigValueException('The group type can not be empty.');
}
Expand All @@ -187,6 +222,10 @@ public function save() {
throw new ConfigValueException('The group bundle can not be empty.');
}

if (empty($this->getName())) {
throw new ConfigValueException('The role name can not be empty.');

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we seem to have a similar check in validate, however that one throws a different exception. Maybe consolidate it, and call validate() from here instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look at this but it is not really possible I think. The DefaultRoleEvent::validate() method is used to validate only the data that is necessary for the event listener to gather the default roles. This one is in OgRole::save() and validates everything required to successfully store the entity.

These are different use cases, and calling validate() from here will make OgRole depend on DefaultRoleEvent.

}

// When assigning a role to group we need to add a prefix to the ID in
// order to prevent duplicate IDs.
$prefix = $this->getGroupType() . '-' . $this->getGroupBundle() . '-';
Expand All @@ -195,7 +234,7 @@ public function save() {
$prefix .= $this->getGroupId() . '-';
}

$this->id = $prefix . $this->id();
$this->id = $prefix . $this->getName();
}

parent::save();
Expand Down Expand Up @@ -233,33 +272,6 @@ public function delete() {
parent::delete();
}

/**
* Returns default properties for the default OG roles.
*
* These are the two roles that are required by every group: the 'member' and
* 'non-member' roles.
*
* All other default roles are provided by DefaultRoleEvent.
*
* @return array
* An array of properties, keyed by OG role.
*
* @see \Drupal\og\Event\DefaultRoleEventInterface
* @see \Drupal\og\GroupManager::getDefaultRoles()
*/
public static function getDefaultRoles() {
return [
self::ANONYMOUS => [
'role_type' => OgRoleInterface::ROLE_TYPE_REQUIRED,
'label' => 'Non-member',
],
self::AUTHENTICATED => [
'role_type' => OgRoleInterface::ROLE_TYPE_REQUIRED,
'label' => 'Member',
],
];
}

/**
* Maps role names to role types.
*
Expand Down
66 changes: 27 additions & 39 deletions src/Event/DefaultRoleEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Drupal\og\Event;

use Drupal\og\OgRoleInterface;
use Drupal\og\Entity\OgRole;
use Symfony\Component\EventDispatcher\Event;

/**
Expand Down Expand Up @@ -41,43 +41,40 @@ public function getRoles() {
/**
* {@inheritdoc}
*/
public function addRole($name, array $properties) {
if (array_key_exists($name, $this->roles)) {
throw new \InvalidArgumentException("The '$name' role already exists.");
}
$this->validate($name, $properties);
public function addRole(OgRole $role) {
$this->validate($role);

// Provide default value for the role type.
if (empty($properties['role_type'])) {
$properties['role_type'] = OgRoleInterface::ROLE_TYPE_STANDARD;
if (array_key_exists($role->getName(), $this->roles)) {
throw new \InvalidArgumentException("The '{$role->getName()}' role already exists.");
}

$this->roles[$name] = $properties;
$this->roles[$role->getName()] = $role;
}

/**
* {@inheritdoc}
*/
public function addRoles(array $roles) {
foreach ($roles as $role => $properties) {
$this->addRole($role, $properties);
foreach ($roles as $role) {
$this->addRole($role);
}
}

/**
* {@inheritdoc}
*/
public function setRole($name, array $properties) {
$this->deleteRole($name);
$this->addRole($name, $properties);
public function setRole(OgRole $role) {
$this->validate($role);
$this->deleteRole($role->getName());
$this->addRole($role);
}

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

Expand Down Expand Up @@ -105,8 +102,12 @@ public function offsetGet($key) {
/**
* {@inheritdoc}
*/
public function offsetSet($key, $value) {
$this->setRole($key, $value);
public function offsetSet($key, $role) {
$this->validate($role);
if ($role->getName() !== $key) {
throw new \InvalidArgumentException('The key and the "name" property of the role should be identical.');
}
$this->setRole($role);
}

/**
Expand All @@ -131,33 +132,20 @@ public function getIterator() {
}

/**
* Validates a role that is about to be set or added.
* Validates that a role that is about to be set or added has a name.
*
* @param string $name
* The name of the role to add or set.
* @param array $properties
* The role properties to validate.
* The roles are stored locally keyed by role name.
*
* @param \Drupal\og\Entity\OgRole $role
* The role to validate.
*
* @throws \InvalidArgumentException
* Thrown when the role name is empty, the 'label' property is missing, or
* the 'role_type' property is invalid.
* Thrown when the role name is empty.
*/
protected function validate($name, $properties) {
if (empty($name)) {
protected function validate(OgRole $role) {
if (empty($role->getName())) {
throw new \InvalidArgumentException('Role name is required.');
}

if (empty($properties['label'])) {
throw new \InvalidArgumentException('The label property is required.');
}

$valid_role_types = [
OgRoleInterface::ROLE_TYPE_STANDARD,
OgRoleInterface::ROLE_TYPE_REQUIRED,
];
if (!empty($properties['role_type']) && !in_array($properties['role_type'], $valid_role_types)) {
throw new \InvalidArgumentException('The role type is invalid.');
}
}

}
44 changes: 22 additions & 22 deletions src/Event/DefaultRoleEventInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Drupal\og\Event;

use Drupal\og\Entity\OgRole;

/**
* Interface for DefaultRoleEvent classes.
*
Expand All @@ -21,6 +23,10 @@ interface DefaultRoleEventInterface extends \ArrayAccess, \IteratorAggregate {
* @param $name
* The name of the role to return.
*
* @return \Drupal\og\Entity\OgRole
* The OgRole entity. Note that we cannot specify OgRoleInterface here
* because of limitations in interface inheritance in PHP 5.
*
* @throws \InvalidArgumentException
* Thrown when the role with the given name does not exist.
*/
Expand All @@ -37,49 +43,43 @@ public function getRoles();
/**
* Adds a default role.
*
* @param string $name
* The name of the role to add.
* @param array $properties
* An associative array of role properties, keyed by the following:
* - 'label': The human readable label.
* - 'role_type': Either OgRoleInterface::ROLE_TYPE_STANDARD or
* OgRoleInterface::ROLE_TYPE_REQUIRED. Defaults to
* OgRoleInterface::ROLE_TYPE_STANDARD.
* @param \Drupal\og\Entity\OgRole $role
* The OgRole entity to add. This should be an unsaved entity that doesn't
* have the group entity type and bundle IDs set.
*
* @throws \InvalidArgumentException
* Thrown when the role that is added already exists, when the role name is
* empty, or when the 'label' property is missing.
* Thrown when the role that is added already exists.
*/
public function addRole($name, array $properties);
public function addRole(OgRole $role);

/**
* Adds multiple default roles.
*
* @param array $roles
* An associative array of default role properties, keyed by role name.
* @param \Drupal\og\Entity\OgRole[] $roles
* An array of OgRole entities to add. These should be unsaved entities that
* don't have the group entity type and bundle IDs set.
*/
public function addRoles(array $roles);

/**
* Sets a default roles.
*
* @param string $name
* The name of the role to set.
* @param array $properties
* An associative array of role properties to set, keyed by the following:
* - 'label': The human readable label.
* @param \Drupal\og\Entity\OgRole $role
* The OgRole entity to set. This should be an unsaved entity that doesn't
* have the group entity type and bundle IDs set.
*
* @throws \InvalidArgumentException
* Thrown when the role name is empty, or when the 'label' property is
* missing.
*/
public function setRole($name, array $properties);
public function setRole(OgRole $role);

/**
* Sets multiple default roles.
*
* @param array $roles
* An associative array of default role properties, keyed by role name.
* @param \Drupal\og\Entity\OgRole[] $roles
* An array of OgRole entities to set. These should be unsaved entities that
* don't have the group entity type and bundle IDs set.
*/
public function setRoles(array $roles);

Expand All @@ -95,7 +95,7 @@ public function deleteRole($name);
* Returns whether or not the given role exists.
*
* @param string $name
* The name of the role for which to verify the existance.
* The name of the role for which to verify the existence.
*
* @return bool
* TRUE if the role exists, FALSE otherwise.
Expand Down
20 changes: 18 additions & 2 deletions src/EventSubscriber/OgEventSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Drupal\og\EventSubscriber;

use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\og\Event\DefaultRoleEventInterface;
use Drupal\og\Event\PermissionEventInterface;
use Drupal\og\OgRoleInterface;
Expand All @@ -20,14 +21,24 @@ class OgEventSubscriber implements EventSubscriberInterface {
*/
protected $permissionManager;

/**
* The storage handler for OgRole entities.
*
* @var \Drupal\core\Entity\EntityStorageInterface
*/
protected $ogRoleStorage;

/**
* Constructs an OgEventSubscriber object.
*
* @param \Drupal\og\PermissionManagerInterface $permission_manager
* The OG permission manager.
* @param \Drupal\core\Entity\EntityTypeManagerInterface $entity_type_manager
* The entity type manager.
*/
public function __construct(PermissionManagerInterface $permission_manager) {
public function __construct(PermissionManagerInterface $permission_manager, EntityTypeManagerInterface $entity_type_manager) {
$this->permissionManager = $permission_manager;
$this->ogRoleStorage = $entity_type_manager->getStorage('og_role');
}

/**
Expand Down Expand Up @@ -69,7 +80,12 @@ public function provideDefaultOgPermissions(PermissionEventInterface $event) {
* The default role event.
*/
public function provideDefaultRoles(DefaultRoleEventInterface $event) {
$event->addRole(OgRoleInterface::ADMINISTRATOR, ['label' => 'Administrator']);
$role = $this->ogRoleStorage->create([

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

'name' => OgRoleInterface::ADMINISTRATOR,
'label' => 'Administrator',
'is_admin' => TRUE,
]);
$event->addRole($role);
}

}
Loading