Problem/Motivation

We use settings.php to override the default transport.
Admin pages that allow editing config should reflect the database config and not overrides.
But /admin/config/system/mailer/transport lists the overridden default.

Steps to reproduce

Create 2 transports and set one as default.
Add something like this to settings.php to set the other one as default:

$config['symfony_mailer.settings']['default_transport'] = 'mailhog';

Proposed resolution

MailerTransportListBuilder::buildRow() should use the non-overridden config to check if a transport is the default one.

Issue fork symfony_mailer-3391090

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

prudloff created an issue. See original summary.

prudloff’s picture

prudloff commented

Note that the label is already loaded without overrides on this page for example.

prudloff opened merge request !64

prudloff’s picture

prudloff commented
Status: Active » Needs review
adamps’s picture

adamps commented

Please let me check I understand. When mails get sent out from your site, by default they will use the override: 'mailhog'. If you look in GUI on the transport listing page, it reports that 'mailhog' is the default. However you think this is wrong, and that it should report the other transport?? Can you show any example in Core to confirm that this is correct?

prudloff’s picture

prudloff commented

The admin page allows editing config so it should always display the config that is being edited.
Here is an example of why this can be confusing: if I click "set as default" on another transport, nothing happens visually (mailhog is still displayed as default because of the override) but my config has changed. The page should reflect the config change.

It is explained here.

The core uses it for example in GDToolkit::buildConfigurationForm() or in ThemeExperimentalConfirmForm.

#2910353: Prevent saving config entities when configuration overrides are applied also talks about preventing saving the configuration when it contains overrides.
#2408549: Display status message on configuration forms when there are overridden values also discusses the fact that core philosophy is that config forms don't display overrides (and suggests displaying the overrides in a warning message, not in the form itself).

adamps’s picture

adamps commented
Status: Needs review » Needs work

Thanks for explaining, that makes sense. It would be clearer for me if we copy how it's done in the docs you link to, so like this:

// Get the default transport without overrides.
return \Drupal::config('symfony_mailer.settings')->getOriginal('default_transport') == $this->id();
prudloff’s picture

prudloff commented

The config returned is never modified so I think you are right we can use getOriginal() instead of getEditable().
I updated the MR.

prudloff’s picture

prudloff commented
Status: Needs work » Needs review
Issue tags: +DrupalCon Lille 2023

  • AdamPS committed 0ff71089 on 1.x authored by prudloff 
    Issue #3391090: Transport list should reflect non-overridden config
adamps’s picture

adamps commented
Status: Needs review » Fixed

Thanks

mishavantol’s picture

mishavantol commented

Now it is unclear the other way around. I was wondering why my local dev setting (mailhog) wasn't showing up in the transport overview. After all, the intention is to adjust the default_transport in a specific environment. Even more confusing is that the overwritten setting is used when sending the test email. So the overview says sendmail is the default, but we use mailhog as the default when sending.

mishavantol’s picture

mishavantol commented

Now it is unclear the other way around. I was wondering why my local dev setting (mailhog) wasn't showing up in the transport overview. After all, the intention is to change the default_transport in a specific environment. Even more confusing is that the overwritten setting is used when sending the test email. So the overview says sendmail is the default, but we use mailhog as the default when sending.

prudloff’s picture

prudloff commented

Well, that is the behavior of most of the Drupal back-office when using overridden config.
This is why the config_override_warn module exists (until #2408549: Display status message on configuration forms when there are overridden values is fixed).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.