Expand Role with methods

I believe I should be able to resolve this issue soon, but I'm very new to the drupal community so I will have a chat will drupal core mentoring team to find out more before I can resolve this problem

Hm, I started work on this month ago and created patch, but haven't posted here. I think it may be helpfull. I'll try to continue work on this.

@james_kerrigan: I have reviewed your patch and it looks promising.
There are a couple of points I would like to make.

1. In the file: core/modules/user/lib/Drupal/user/RoleInterface.php

I have changed the documentation a bit to bring it in line with the drupal standard

  /**
   * Returns the role's weight.
   *
   * @return int
   *   The weight of this role.
   */
  public function getWeight();

  /**
   * Sets the role's weight.
   *
   * @param int $weight
   *   The desired weight.
   *
   * @return static
   *   The modified role.
   */
  public function setWeight($weight);

2. In the file: core/modules/user/lib/Drupal/user/Entity/Role.php

My suggestion for the function definitions would be:

  /**
   * {@inheritdoc}
   */
  public function getWeight() {
    return $this->get('weight')->value;
  }

  /**
   * {@inheritdoc}
   */
  public function setWeight($description) {
    $this->set('weight', $weight);
    return $this;
  }

Some of the public variables have to change to protected.

  protected $weight;
  protected $permissions = array();

And on line 128 you missed !isset($this->weight) to !$this->getWeight()

Added call to getWeight() line 128.

@daffie documentation seems correct to me, same as in other parts of D8.

I've got some problems with changing class properties ro protected.
First of all fails core/modules/user/lib/Drupal/user/Tests/UserRoleAdminTest.php 114 here:

$this->assertEqual($role->weight, $new_role_weights[$role->id()]);

Looks like it is something with role save. I'll try to continue work but maybe somebody else find a solution for this faster.

Just a plain reroll of this.

And again a plain re-roll.

1. +++ b/core/modules/user/lib/Drupal/user/Plugin/views/argument_validator/User.php
   @@ -10,6 +10,7 @@
   @@ -157,7 +158,7 @@ public function validateArgument($argument) {
   
   @@ -157,7 +158,7 @@ public function validateArgument($argument) {
   +    $this->argument->validated_title = String::checkPlain(user_format_name($account));
   

   If you change this you can also use $account->getUsername();

2. +++ b/core/modules/user/lib/Drupal/user/RoleInterface.php
   @@ -55,4 +55,23 @@ public function grantPermission($permission);
   +  /**
   +   * Returns the weight.
   +   *
   

   Maybe it would be a little bit helpful to explain what a weight of a role means.

Contains fix of #20.1 but for #20.2 I'm at loss. I actually do not know about the weight part of user roles.

I'm going to reroll the patch.

Reroll for patch in #21

All those check_plain => String::checkPlain changes are certainly fine, thought they seem to be a little bit out of scope.

1. +++ b/core/modules/user/lib/Drupal/user/Entity/Role.php
   @@ -150,4 +150,25 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
   +    return $this->get('weight') ? TRUE : FALSE;
   

   You could simply go with return (bool) $this->get('weight')/$this->getWeight()

2. +++ b/core/modules/user/lib/Drupal/user/RoleInterface.php
   @@ -55,4 +55,31 @@ public function grantPermission($permission);
   +   * @return \Drupal\user\RoleInterface
   +   *   The class instance this method is called on.
   

   We do use now just @return $this and no doc line below

I rerolled and removed the hasWeight function as I don't think it makes sense - something could have a weight of 0 and this method would return false. I also changed the documentation as per #25. Everything else looks good to me.

Oops

same patch, testbot isn't picking it up from #32 :|

Coding standards and spelling correction

+++ b/core/modules/user/src/RoleInterface.php
@@ -39,8 +39,7 @@ public function hasPermission($permission);
+   * @return $this

@@ -55,4 +54,23 @@ public function grantPermission($permission);
+   * @return \Drupal\user\RoleInterface
+   *   The class instance this method is called on.

These should both be @return self

These should both be @return $this: https://drupal.org/node/1354#types

I've removed the conversion of check_plain as it's already being done in #2089331: [meta] Replace calls to check_plain() with Drupal\Component\Utility\String::checkPlain()
New patch also switches the property visibility to protected.

No longer "needs followup" the check_plain has been fixed in #2089331: [meta] Replace calls to check_plain() with Drupal\Component\Utility\String::checkPlain().

Protected all the public properties and removed toArray since #2256679: Use config schema to determine which config entity properties to export regardless of whether they are public or protected has gone in.

Patch applies mostly cleanly, just a little fuzz on a couple of hunks. Attached is a re-roll moving the start lines down a bit. Tested manually manipulating user roles and the patched version works as intended.

Committed/pushed to 8.x, thanks!