Problem/Motivation

user_user_role_insert() is a secondary write to config. Basically when you create a role through the API or UI you will always get two actions created. This entangles the role and action configuration entities from an API perspective that is wrong. When you create a role through the API the system should only create a role. At the moment configuration sync only works because we bail if the role is being created as part of a configuration sync. But whether or not it is a config sync seems irrelevant. In fact it feels as though we need different hooks of API and UI based creates.

This is a bug because if an install profile provides both the role and it's related system.action.user_add_role_action.ROLEID the install breaks horribly with the following error:

Drupal\Core\Config\ConfigDuplicateUUIDException: Attempt to save a configuration entity 'user_add_role_action.administrator' with UUID '1b27a4d1-6f94-4c33-9500-a44bd23eeab9' when this entity already exists with UUID 'b69baf55-119d-4066-8d0f-15725863cce1' in Drupal\Core\Config\Entity\ConfigEntityBase->preSave() (line 344 of core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php).

Proposed resolution

tbd. The simplest solution for user_user_role_insert() is just to move this code to the form that creates user roles.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Issue summary: View changes

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

marcingy’s picture

Status: Active » Needs review
FileSize
3.47 KB

Initial pass, removes the check for sync as we no longer need it. Lets see what tests fail!

Status: Needs review » Needs work

The last submitted patch, 4: issue-2672340.patch, failed testing.

marcingy’s picture

Status: Needs work » Needs review
FileSize
5.91 KB

This should fix some issues, but does seem like we need to re-think the DX to allow for creation programatically in a nice way. Should we introduce a $role->createAction() interface to allow those who wish to do so to create elements programatically but the default would be FALSE.

Status: Needs review » Needs work

The last submitted patch, 6: issue-2672340.patch, failed testing.

therealssj’s picture

Some more fixes.
Till we have something like $role->createAction() as marcingy suggested, i hope this test is a good workaround.

Only adding action for the first role as that is only checked.

therealssj’s picture

Status: Needs work » Needs review
alexpott’s picture

Issue tags: +Needs change record

The change looks good - I'm not sure that we need a $role->createAction() since this will have to access things about the Action entity that the role entity should not know about.

I think we need a CR and I'll discuss with the other committers about whether this is allowable in a minor release.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.