Roles no longer visible on user_form since 3.1.0 release

I've attached the patch in accordance with the proposed resolution.

I tested #5 and the problem is solved. Without the patch the list of roles on a user form is not visible, with the patch I can see and change the list of roles.

I tested #7 and it solves the problem as well.

I tested #7 and it solves the problem.

@snow_ee Thank you for your work! Actually your patch #1 already solves the issue. In my opinion #5 is an unnessessary and unrelated change which doesn't improve the code quality. And next time you improve patches, please attach an interdiff to make the differences between the patches clear (like immaculatexavier did in #5).

+1 for patch #7.

Trying to sort out the proposed patches ... all of them solve the issue at hand.

But they do it a little bit differently and some do more than just solving the issue:

• Patch 3292173-1.patch in #2 by snow_ee sets '#access' to !empty($options);
  (and thus reverts commit 97ef94b).
• Patch 3292173-1_0.patch in #4 is the same as #2.
• Patch #5 by immaculatexavier explicitly sets '#access' to TRUE if $options is not empty.
  It also adds the so called "account fix":
  • Set $account to \Drupal::currentUser(); instead of Drupal::currentUser();
  • Use $account instead of calling currentUser() twice.
• Patch #7 solves the issue the same way as #2 did.
  It also keeps the "account fix".

I agree with pminf in #10 that the "account fix" is an unrelated change - but I'm not so sure wether it's unnessessary and I do think it improves code quality because:

• core uses \Drupal::currentUser(); (with leading slash)
• we shouldn't call currentUser() twice

Maybe we should open a new issue for the "account fix" (which should also change the other two \Drupal::currentUser();) ...

That said I propose to just solve the current issue (i.e. get rid of the "account fix") and do this by by explicitly setting '#access' to TRUE if $options is not empty (like #5).
I also think it is nice to document this behaviour.
This way we reduce the risk of repeating the same error again.

What do you think?

Thanks everyone for the contributions.

Background

The commit that caused the problem was part of #3188812: Module overrides previously set #access on roles form element (I added that into the IS). If we fix this, then I believe we must undo that issue. However I now see that the OP of that issue commented after the issue was closed (hence I didn't notice) to suggest reverting. So I think it's fine. The MR on #3182353: Not possible to configure the roles on Drupal 9 was also made on an already closed issue, so this problem has remained hidden😃.

Proposal

1. I agree we should improve the comments. I like the comments from #12.
2. I prefer the single assignment like in #2 instead of the if (which effectively says "if false set to false, else set to true"). The two comment sentences of #12 could be put into a single comment before the single assignment of #2. This should reduce the risk of repeating the error without needing to make the code more verbose.
3. I agree the "account fix" is not strictly part of this issue and I also agree it's an improvement. I don't really mind whether we commit it in this issue or separately.

Drupal::currentUser() should have a slash to cancel any existing namespace but it doesn't actually cause a bug in a module file because there is no namespace.

I've tested patch #12, and it worked for us.

Here is my preferred variation. I realised that we don't need to remove existing access which could have been granted by another module. New patch only adds access, which is that same as the rest of the module.

Please can someone test, then I will commit and make a new release.

Tested patch #15, it works!

Great thanks