AccountForm clean-up

I rebased the latest patch and tests are passing but I did not test manually.

Think the issue summary needs some love. The motivation section mentions performance? But anything to support that?

Was also mentioned as a remaining task

The 8.1 I think is out of date haha

I tried to rewrite the summary.

Debated turning to a feature request but think we should have some test coverage that altering is possible.

What test coverage is this missing?

We don't need to test every form allows altering, that's not specific for any specific form but the form API, which is already covered.

If the scenario is already covered then why make the change? Assuming this change is to solve a problem that maybe wasn’t covered before.

Can put back into review if you disagree though

Clean up. The existing tests passing is the test.

This was suggested by @alexpott in #1998648-52: "Administration pages language" user account setting should be hidden if not used → . Let's land this 12 years later.

Alright going to leave for others

Rebased 221 commits.

Test-only test passes:

ℹ️ Changes from faec1421c65a694b320f6c807c9546b4dde9482d
core/modules/user/src/AccountForm.php
If this list contains more files than what you changed, then you need to rebase your branch.
1️⃣ Reverting non test changes
grep: warning: * at start of expression
grep: warning: * at start of expression
↩️ Reverting core/modules/user/src/AccountForm.php
grep: warning: * at start of expression
2️⃣ Running test changes for this branch
Exiting with EXIT_CODE=0
Running after_script 00:01
Running after script...
$ sed -i "s#$CI_PROJECT_DIR/##" ./sites/default/files/simpletest/phpunit-*.xml || true
sed: can't read ./sites/default/files/simpletest/phpunit-*.xml: No such file or directory
Uploading artifacts for successful job 00:00
Uploading artifacts...
WARNING: ./sites/default/files/simpletest/phpunit-*.xml: no matching files. Ensure that the artifact path is relative to the working directory (/builds/issue/drupal-2058319) 
WARNING: ./sites/simpletest/browser_output: no matching files. Ensure that the artifact path is relative to the working directory (/builds/issue/drupal-2058319) 
ERROR: No files to upload                          
Uploading artifacts...
WARNING: ./sites/default/files/simpletest/phpunit-*.xml: no matching files. Ensure that the artifact path is relative to the working directory (/builds/issue/drupal-2058319) 
ERROR: No files to upload                          
Cleaning up project directory and file based variables 00:01
Job succeeded

Just fyi rebases typically aren’t needed unless the MR no longer applies or if testing is randomly failing for a bug fix

RE #86 Yes, after running the test-only test seems pointless since no test coverage so far.

Why would we need a new MR? The current MR shows the diff. And the diff fits two-scrolls.

The re-arrangement of the code is exactly what we want, that's clear in the issue summary.

No lenghty comments are added or deleted, they are just moved.

This just needs someone willing to RTBC it.

RE: #88
I edited #87 quite a lot before I saw #88 mainly in agreement with #88. Starting to get my head around it. I tracked down your comment in the related issue. So these optional form elements will now be always present in the render array but as 'hidden' fields when not needed? It sounds an enhancement if it is likely to be a restricted list of specific fields for the foreseeable future. If it were likely to grow to more than a few fields might not be an enhancement..

I see this in the UserHooks.php file in the user core module.

  /**
   * Implements hook_entity_extra_field_info().
   */
  #[Hook('entity_extra_field_info')]
  public function entityExtraFieldInfo(): array {
    $fields['user']['user']['form']['account'] = [
      'label' => $this->t('User name and password'),
      'description' => $this->t('User module account form elements.'),
      'weight' => -10,
    ];
    $fields['user']['user']['form']['language'] = [
      'label' => $this->t('Language settings'),
      'description' => $this->t('User module form element.'),
      'weight' => 0,
    ];
    if (\Drupal::config('system.date')->get('timezone.user.configurable')) {
      $fields['user']['user']['form']['timezone'] = [
        'label' => $this->t('Timezone'),
        'description' => $this->t('System module form element.'),
        'weight' => 6,
      ];
    }
    $fields['user']['user']['display']['member_for'] = [
      'label' => $this->t('Member for'),
      'description' => $this->t("User module 'member for' view element."),
      'weight' => 5,
    ];
    return $fields;
  }

This new hook could have a bearing on this issue.

RE: #90 The new hook seems to be adding entity fields ie nothing to do with the account form fields..