Issue #2015721 by dawehner: Fixed Adding a role to a user throws a php error.

core/modules/user/lib/Drupal/user/Plugin/Action/AddRoleUser.php

@@ -28,12 +28,11 @@ class AddRoleUser extends ChangeUserRoleBase {
   public function execute($account = NULL) {
     $rid = $this->configuration['rid'];
     // Skip adding the role to the user if they already have it.
-    if ($account !== FALSE && !isset($account->roles[$rid])) {
-      $roles = $account->roles + array($rid => $rid);
+    if ($account !== FALSE && !$account->hasRole($rid)) {
       // For efficiency manually save the original account before applying
       // any changes.
       $account->original = clone $account;
-      $account->roles = $roles;
+      $account->addRole($rid);
       $account->save();
     }
   }

core/modules/user/lib/Drupal/user/Plugin/Action/RemoveRoleUser.php

@@ -28,12 +28,11 @@ class RemoveRoleUser extends ChangeUserRoleBase {
   public function execute($account = NULL) {
     $rid = $this->configuration['rid'];
     // Skip removing the role from the user if they already don't have it.
-    if ($account !== FALSE && isset($account->roles[$rid])) {
-      $roles = array_diff($account->roles, array($rid => $rid));
+    if ($account !== FALSE && $account->hasRole($rid)) {
       // For efficiency manually save the original account before applying
       // any changes.
       $account->original = clone $account;
-      $account->roles = $roles;
+      $account->removeRole($rid);
       $account->save();
     }
   }

core/modules/user/lib/Drupal/user/Plugin/Core/Entity/User.php

@@ -268,4 +268,27 @@ class User extends EntityNG implements UserInterface {
     return NULL;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function hasRole($rid) {
+    return in_array($rid, $this->getRoles());
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function addRole($rid) {
+    $roles = $this->getRoles();
+    $roles[] = $rid;
+    $this->set('roles', array_unique($roles));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function removeRole($rid) {
+    $this->set('roles', array_diff($this->getRoles(), array($rid)));
+  }
+
 }

core/modules/user/lib/Drupal/user/Tests/UserEntityTest.php

@@ -0,0 +1,77 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\user\Tests\UserEntityTest.
+ */
+
+namespace Drupal\user\Tests;
+
+use Drupal\Core\Language\Language;
+use Drupal\simpletest\DrupalUnitTestBase;
+use Drupal\user\Plugin\Core\Entity\User;
+
+/**
+ * Tests the user entity class.
+ *
+ * @see \Drupal\user\Plugin\Core\Entity\User
+ */
+class UserEntityTest extends DrupalUnitTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('system', 'user');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'User entity tests',
+      'description' => 'Tests the user entity class.',
+      'group' => 'User'
+    );
+  }
+
+  /**
+   * Tests some of the methods.
+   *
+   * @see \Drupal\user\Plugin\Core\Entity\User::getRoles()
+   * @see \Drupal\user\Plugin\Core\Entity\User::addRole()
+   * @see \Drupal\user\Plugin\Core\Entity\User::removeRole()
+   */
+  public function testUserMethods() {
+    $role_storage = $this->container->get('plugin.manager.entity')->getStorageController('user_role');
+    $role_storage->create(array('id' => 'test_role_one'))->save();
+    $role_storage->create(array('id' => 'test_role_two'))->save();
+    $role_storage->create(array('id' => 'test_role_three'))->save();
+
+    $values = array('roles' => array(Language::LANGCODE_DEFAULT => array('test_role_one')));
+    $user = new User($values, 'user');
+
+    $this->assertTrue($user->hasRole('test_role_one'));
+    $this->assertFalse($user->hasRole('test_role_two'));
+    $this->assertEqual(array('test_role_one'), $user->getRoles());
+
+    $user->addRole('test_role_one');
+    $this->assertTrue($user->hasRole('test_role_one'));
+    $this->assertFalse($user->hasRole('test_role_two'));
+    $this->assertEqual(array('test_role_one'), $user->getRoles());
+
+    $user->addRole('test_role_two');
+    $this->assertTrue($user->hasRole('test_role_one'));
+    $this->assertTrue($user->hasRole('test_role_two'));
+    $this->assertEqual(array('test_role_one', 'test_role_two'), $user->getRoles());
+
+    $user->removeRole('test_role_three');
+    $this->assertTrue($user->hasRole('test_role_one'));
+    $this->assertTrue($user->hasRole('test_role_two'));
+    $this->assertEqual(array('test_role_one', 'test_role_two'), $user->getRoles());
+
+    $user->removeRole('test_role_one');
+    $this->assertFalse($user->hasRole('test_role_one'));
+    $this->assertTrue($user->hasRole('test_role_two'));
+    $this->assertEqual(array('test_role_two'), $user->getRoles());
+  }
+
+}

core/modules/user/lib/Drupal/user/UserBCDecorator.php

@@ -54,4 +54,26 @@ class UserBCDecorator extends EntityBCDecorator implements UserInterface {
   public function getSessionId() {
     return $this->decorated->getSessionId();
   }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function hasRole($rid) {
+    return $this->getBCEntity()->hasRole($rid);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function addRole($rid) {
+    $this->getBCEntity()->addRole($rid);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function removeRole($rid) {
+    $this->getBCEntity()->removeRole($rid);
+  }
+
 }

core/modules/user/lib/Drupal/user/UserInterface.php

@@ -15,4 +15,39 @@ use Drupal\Core\Session\AccountInterface;
  */
 interface UserInterface extends EntityInterface, AccountInterface {
 
+  /**
+   * Returns a list of roles.
+   *
+   * @return array
+   *   List of role IDs.
+   */
+  public function getRoles();
+
+  /**
+   * Whether a user has a certain role.
+   *
+   * @param string $rid
+   *   The role ID to check.
+   *
+   * @return bool
+   *   Returns TRUE if the user has the role, otherwise FALSE.
+   */
+  public function hasRole($rid);
+
+  /**
+   * Add a role to a user.
+   *
+   * @param string $rid
+   *   The role ID to add.
+   */
+  public function addRole($rid);
+
+  /**
+   * Remove a role from a user.
+   *
+   * @param string $rid
+   *   The role ID to remove.
+   */
+  public function removeRole($rid);
+
 }

core/modules/user/tests/Drupal/user/Tests/Plugin/Action/AddRoleUserTest.php

@@ -0,0 +1,83 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\user\Tests\Plugin\Action\AddRoleUserTest.
+ */
+
+namespace Drupal\user\Tests\Plugin\Action;
+
+use Drupal\Tests\UnitTestCase;
+use Drupal\user\Plugin\Action\AddRoleUser;
+
+/**
+ * Tests the role add plugin.
+ *
+ * @see \Drupal\user\Plugin\Action\AddRoleUser
+ */
+class AddRoleUserTest extends UnitTestCase {
+
+  /**
+   * The mocked account.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $account;
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Add user plugin',
+      'description' => 'Tests the role add plugin',
+      'group' => 'User',
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->account = $this
+      ->getMockBuilder('Drupal\user\Plugin\Core\Entity\User')
+      ->disableOriginalConstructor()
+      ->getMock();
+  }
+
+  /**
+   * Tests the execute method on a user with a role.
+   */
+  public function testExecuteAddExistingRole() {
+    $this->account->expects($this->never())
+      ->method('addRole');
+
+    $this->account->expects($this->any())
+      ->method('hasRole')
+      ->with($this->equalTo('test_role_1'))
+      ->will($this->returnValue(TRUE));
+
+    $config = array('rid' => 'test_role_1');
+    $remove_role_plugin = new AddRoleUser($config, 'user_add_role_action', array('type' => 'user'));
+
+    $remove_role_plugin->execute($this->account);
+  }
+
+  /**
+   * Tests the execute method on a user without a specific role.
+   */
+  public function testExecuteAddNonExistingRole() {
+    $this->account->expects($this->once())
+      ->method('addRole');
+
+    $this->account->expects($this->any())
+      ->method('hasRole')
+      ->with($this->equalTo('test_role_1'))
+      ->will($this->returnValue(FALSE));
+
+    $config = array('rid' => 'test_role_1');
+    $remove_role_plugin = new AddRoleUser($config, 'user_remove_role_action', array('type' => 'user'));
+
+    $remove_role_plugin->execute($this->account);
+  }
+
+}

core/modules/user/tests/Drupal/user/Tests/Plugin/Action/RemoveRoleUserTest.php

@@ -0,0 +1,83 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\user\Tests\Plugin\Action\RemoveRoleUserTest.
+ */
+
+namespace Drupal\user\Tests\Plugin\Action;
+
+use Drupal\Tests\UnitTestCase;
+use Drupal\user\Plugin\Action\RemoveRoleUser;
+
+/**
+ * Tests the role remove plugin.
+ *
+ * @see \Drupal\user\Plugin\Action\RemoveRoleUser
+ */
+class RemoveRoleUserTest extends UnitTestCase {
+
+  /**
+   * The mocked account.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $account;
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Remove user plugin',
+      'description' => 'Tests the role remove plugin',
+      'group' => 'User',
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->account = $this
+      ->getMockBuilder('Drupal\user\Plugin\Core\Entity\User')
+      ->disableOriginalConstructor()
+      ->getMock();
+  }
+
+  /**
+   * Tests the execute method on a user with a role.
+   */
+  public function testExecuteRemoveExistingRole() {
+    $this->account->expects($this->once())
+      ->method('removeRole');
+
+    $this->account->expects($this->any())
+      ->method('hasRole')
+      ->with($this->equalTo('test_role_1'))
+      ->will($this->returnValue(TRUE));
+
+    $config = array('rid' => 'test_role_1');
+    $remove_role_plugin = new RemoveRoleUser($config, 'user_remove_role_action', array('type' => 'user'));
+
+    $remove_role_plugin->execute($this->account);
+  }
+
+  /**
+   * Tests the execute method on a user without a specific role.
+   */
+  public function testExecuteRemoveNonExistingRole() {
+    $this->account->expects($this->never())
+      ->method('removeRole');
+
+    $this->account->expects($this->any())
+      ->method('hasRole')
+      ->with($this->equalTo('test_role_1'))
+      ->will($this->returnValue(FALSE));
+
+    $config = array('rid' => 'test_role_1');
+    $remove_role_plugin = new RemoveRoleUser($config, 'user_remove_role_action', array('type' => 'user'));
+
+    $remove_role_plugin->execute($this->account);
+  }
+
+}