Roles being stripped from users when saved if user has no access to roles field

Patch from MR at commit 7cdc8c1d

Liam Morland → made their first commit to this issue’s fork.

Updated the merge request.

Patch works for me.

Confirming RTBC here.

To note there were also efforts in 🐛 Roles being stripped from users when saved Closed: duplicate but closed it as a duplicate as this one seems to jump in earlier in where things go wrong compared to in the final presave, but maintainers may find it useful to know there are also 11 followers there,

Hiding patch

Tests are failing.

I was able to reproduce via the steps in the IS and got some test coverage going for this bug. Obviously the fix still isn't working (since it's breaking existing tests) but this'll give us something to test against (TDD :))

@acbramley i might be misunderstanding but the following section of code in the .module file:

<?php
function role_delegation_user_form_builder(string $entity_type, UserInterface $user, array &$form, FormStateInterface $form_state) {
  // If the user has no access to the "role_change" or "role" field, then the
  // form will submit an empty array for the field, which will make later
  // processing think it was intentional. Set it to the empty field value to
  // correct this.
  if (!isset($form['role_change']['#access'], $form['account']['roles']['#access']) || !$form['role_change']['#access']  ||  !$form['account']['roles']['#access']) {
    $user->set('role_change', DelegatableRoles::$emptyFieldValue);
  }
}
?>

If i am reading the text right it says that the user needs access to either the role_change field or the account roles field. However with the above code you would need access to both field for it to not submit an empty value. I believe the code should be:

<?php
function role_delegation_user_form_builder(string $entity_type, UserInterface $user, array &$form, FormStateInterface $form_state) {
  // If the user has no access to the "role_change" or "role" field, then the
  // form will submit an empty array for the field, which will make later
  // processing think it was intentional. Set it to the empty field value to
  // correct this.
  if (!isset($form['role_change']['#access'], $form['account']['roles']['#access']) || (!$form['role_change']['#access'] && !$form['account']['roles']['#access'])) {
    $user->set('role_change', DelegatableRoles::$emptyFieldValue);
  }
}
?>