Convert user roles to entity_reference_field

"Roles" implies multiple values and base fields cannot have multiple values.

base fields cannot have multiple values.

Do we have to create new issue for this? Or is there any existing issue? If we can't convert roles to reference field then what is the purpose of #1757452: Support config entities in entity reference fields please explain.

We don't have to create a new issue, this is simply not possible :)

The purpose of the issue you linked is to be able to reference bundles, for example. Roles are not bundles for the user entity type..

Okay. Thanks for the clarification.

"Roles" implies multiple values and base fields cannot have multiple values.

Not necessarily.

Our general, default storage approach handles single valued base fields only, that doesn't mean it could work with multiple valued base fields though. It works, but the storage controller would have to take care of loading/saving the values as appropriate (what I guess it does already).

I think, that's best solved by extending the configurable entity reference class to a UserRolesReference class which then implements the respective insert/save logic. Loading is more problematic though, as there is no respective method to handle multiple loading on the field. One variant would be to use getValue() to lazily load + return the roles for caching, what would work fine for cached entities but single-load on cache fails.
Alternatively, we'd have to deal with loading (or evering) in the respective entity class method.

Not to mention EntityQuery..

I mean, of course it's doable if you bend the system in every possible way, but do we really want to go there?

Also, why do we need this one to be a base field?

I figured, moving it's storage logic into the field isn't related to this at all - thus should be its own issue.

But moreover being a multi-valued base field - it already is.

I mean, of course it's doable if you bend the system in every possible way, but do we really want to go there?

Not sure why it would bend it if we use an existing configurable entity reference field type for a configurable entity reference.

EntityQuery

Yes, afaik it's not queryable right now - no changes here?

Also, why do we need this one to be a base field?

Not sure what it should be else? It's provided with the user entity, so it's a base field.

But moreover being a multi-valued base field - it already is.

So.. what part of this definition makes it a multi-valued base field?

    $properties['roles'] = array(
      'label' => t('Roles'),
      'description' => t('The roles the user has.'),
      // @todo Convert this to entity_reference_field, see
      // https://drupal.org/node/2044859
      'type' => 'string_field',
    );

Like I said above, the fact that we split a string in the user storage controller to make it *look* like it contains actual references doesn't make it a multi-valued field. Edit: Removed snarky remark :)

Am I missing something? This has no reason to stay a crappy semi-field. Why not making it a full blown configurable field and be done with it?

User roles are converted to ER in #2192259-29: Remove ConfigField.. Item and List classes + interfaces
So probably this one could be closed after commit

I don't think user roles should be converted to ER in [#8557525].

I'd like to see this revived, mainly because I think getRoles() should be returning proper role objects rather than strings.

This leads to issues like we have in hasPermission on User and UserSession, where they both have to convert the roles themselves to find the permission:

   $roles = \Drupal::entityManager()->getStorage('user_role')->loadMultiple($this->getRoles());

I doubt we will be allowed to do this after beta...

@andypost - Looks like #8557525 landed, which you explained could accomplish this issue. Are we at least closer now?

There's issue #8557525, and roles still string with some complicated logic behind.
I'm +1 to #12 - configurable field like a signature

#2337927: SqlContentEntityStorage::onFieldStorageDefinition(Create|Update|Delete)() is broken for base fields will provide support for multi-value base fields and I already tried to convert user roles storage in #2248977: Complete support for multi-value base fields in ContentEntitySchemaHandler and use it for the user.roles field, which I will pick up when the first issue lands.

Note that converting to an entity reference field itself would be easy at this point, entity reference supports config entities, the only difference would be that ->value would now be ->target_id. The interesting part is converting custom storage to default storage, string or entity_reference.

New patch posted in #2248977: Complete support for multi-value base fields in ContentEntitySchemaHandler and use it for the user.roles field, please review.

To clarify what I meant above, "converting to entity reference field" would be this patch.

I always forget the target_type setting. Fixed the tests.

Looks good to me.

Me too. Committed e3e12db and pushed to 8.0.x. Thanks!

Nice!

Awesome !