Problem/Motivation

Recent core changes added strictness to schema. Now it is impossible to add new items to the system.mail:interface mapping.

Proposed resolution

Change type of system.mail:interface to sequence.

Remaining tasks

There might be other configs that have the same problem?

User interface changes

None

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Arla’s picture

Status: Active » Needs review
FileSize
1.46 KB
2.08 KB

A test to prove the point, and a fix that changes the type to sequence.

Gábor Hojtsy’s picture

So what is stored there? Are all of those settings for sure?

Gábor Hojtsy’s picture

@berdir pointed to MailManager::getInstance():

   * The selection of a particular implementation is controlled via the config
   * 'system.mail.interface', which is a keyed array.  The default
   * implementation is the mail plugin whose ID is the value of 'default' key. A
   * more specific match first to key and then to module will be used in
   * preference to the default. To specify a different plugin for all mail sent
   * by one module, set the plugin ID as the value for the key corresponding to
   * the module name. To specify a plugin for a particular message sent by one
   * module, set the plugin ID as the value for the array key that is the
   * message ID, which is "${module}_${key}".
   *
   * For example to debug all mail sent by the user module by logging it to a
   * file, you might set the variable as something like:
   *
   * @code
   * array(
   *   'default' => 'php_mail',
   *   'user' => 'devel_mail_log',
   *   'contact_page_autoreply' => 'null_mail',
   * );

So it is indeed a list of arbitrary strings. The patch seems to be good. The reason it was not caught before is because we only had a phpunit test for this, no integration test.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
Issue tags: +Configuration system
+++ b/core/modules/system/config/schema/system.schema.yml
@@ -333,12 +333,11 @@ system.mail:
       label: 'Interface'
...
+          label: 'Individual interface'

I would call them 'Interfaces' and 'Interface', that is what we do elsewhere. We don't need to change the key names, but the labels we have more free control over :)

Arla’s picture

Status: Needs work » Needs review
FileSize
2.1 KB
563 bytes

Aha, I got "Individual interface" from your schema cheat-sheet ;)

Berdir’s picture

Interface is a pretty weird thing anyway, not sure what that means exactly, in the past, we stored class names (implementations, not interfaces) and what we are storing there now are plugin IDs.

Not sure what to do about that with the labels, having something completely different in the labels is also weird.

Gábor Hojtsy’s picture

LOL I will fix that individual thing in a new version later :D It will need 1 line moved...

The last submitted patch, 1: mail-interface-schema-2392427-1-TEST_ONLY.patch, failed testing.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

I think this is good to go.

Not sure if it's just normal, it's at breaking the ability to use this in tests, which means it's blocking contrib module tests.

alexpott’s picture

Priority: Normal » Major

I think this is a major bug.

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 2a7aaa0 and pushed to 8.0.x. Thanks!

  • alexpott committed 2a7aaa0 on 8.0.x
    Issue #2392427 by Arla: Too strict schema for system.mail:interface
    
tstoeckler’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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