Config form: Display override values

I don't understand the problem. How else would you do it? This is a configuration form used to set and change the SMTP module settings - it HAS to get the editable smtp.settings configuration in order to be able to change the settings. This is the way all core modules do it. These days it's more common to implement getEditableConfigNames(), but that's just a convenience function which does the EXACT SAME THING as getEditable()^*. This convenience function didn't exist back in the early days of D8 when the SMTP module was ported which I assume is why the SMTP module doesn't currently use it.

^*Specifically, for classes that extend ConfigFormBase, $this->configFactory->getEditable('smtp.settings') will return the exact same thing as the combination of protected function getEditableConfigNames() { return ['smtp.settings']; } and $this->config('smtp.settings').

IMO, in this matter the SMTP module operates exactly as documented and expected for core and contributed modules. See https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-... for a description of configuration overrides and how they work.

Because overrides are a core feature which apply to all configuration entities. any extension of this core mechanism to, for example, display the overridden values should be handled by core. There are links in the above documentation that point to core issues/discussions about doing this on a general basis and not just for one specific module. For example #2408549: Display status message on configuration forms when there are overridden values.

Perhaps I've misunderstood or missed some use case, but if not I am in favor of marking this as "Closed, works as designed", or at the very lease, "Closed, duplicate" (duplicate of #2408549: Display status message on configuration forms when there are overridden values).

I realized this behavior today and ran into troubles, when trying to send a test mail from /admin/config/system/smtp .

In settings.php I'm using this line to override the smtp server's host:

$config['smtp.settings']['smtp_host'] = 'some.host.xyz';

With older versions (ie. Core 8.7.x and SMTP Auth. Support 8.x-1.0-beta4) when I open /admin/config/system/smtp I can see the disabled field and it contains the overrideen address of the smtp host.

With a newer setup (ie. Core 8.9.x and smtp 8.x-1.0-rc4) when I open /admin/config/system/smtp I can still see the disabled field, but it contains the value originally entered in the config form not the one overridden in settings.php.

In my case I never entered a smtp hostname in the config form, only in settings.php. So the field is disabled and empty. When I then try to send a test mail from the config form at /admin/config/system/smtp I receive the error: You must enter an SMTP server address. Obviously because the field is empty.

I didn't dare trying it out, but I wonder wether the SEND TEST E-MAIL function uses the overridden value from settings.php, or the one from the config form. Other mails (ie. password reset) are correctly using the overridden hostname from settings.php.

I have exact same issue.

1. configure smtp module via settings.php
2. go to /admin/config/system/smtp
3. send test email
4. Receive "You must enter an SMTP server address" validation error.

Two potential solutions:
1. As you mention, move test form to a different tab/page.
2. Change form validation so it recognizes fields that are set via config and do not validate them.

Since the form element is disabled if the value is overridden, and since overridden values are not saved, @see SMTPConfigForm::submitForm

I update buildForm to show the overridden values.

I also added more tests.

I want to make improvements to how this is handled.

I removed the custom methods and used built-in methods.

This commit created some BC issues and needs to be partially reverted. Setting to NW.

What are the bc?

Since the form is not 'final' the constructor signature is different. While its a minor change, we cannot guarantee that no one has extended this form, without making a major change.

Also, unit tests are.. ehh not the greatest. Converted it into a Kernel test. It'd be even better if it was doing request based kernel tests, but this at least confirms the values entered should be returning correctly when submitted.

I made a MR that uses a constructor, but maintains backwards compatibility.