Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.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
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | reroll_diff_2672340-8_2672340-26.txt | 14.78 KB | yogeshmpawar |
| #26 | 2672340-26.patch | 9.06 KB | yogeshmpawar |
| #8 | interdiff-2672340-6-8.txt | 2.78 KB | therealssj |
| #8 | 2672340-8.patch | 8.69 KB | therealssj |
| #6 | issue-2672340.patch | 5.91 KB | marcingy |
Comments
Comment #2
alexpottComment #4
marcingy CreditAttribution: marcingy at Examiner.com commentedInitial pass, removes the check for sync as we no longer need it. Lets see what tests fail!
Comment #6
marcingy CreditAttribution: marcingy at Examiner.com commentedThis 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.
Comment #8
therealssj CreditAttribution: therealssj commentedSome 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.
Comment #9
therealssj CreditAttribution: therealssj commentedComment #10
alexpottThe 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.
Comment #16
fancyweb CreditAttribution: fancyweb commentedI'm having this exact same issue. It would be nice if this hook was removed.
Comment #17
idebr CreditAttribution: idebr at iO commentedIn the issue #2980237: Rebuilding routes when node.settings:use_admin_theme changes should be done in a config listener the logic was moved away from the form to make it compatible with changing config through other means such as drush/drupal console, so in a way the proposed resolution here seems a step backwards.
Comment #18
alexpott@idebr this is totally different. That issue is performing an action that HAS to occur when node.settings:use_admin_theme changes.
user_user_role_insert()is doing something that does not have to happen at all. It's a UI niceness but from an API point-of-view just gets in the way.Comment #25
larowlanComment #26
yogeshmpawarUpdated patch with reroll diff.