Problem/Motivation

Current schema:

# Text with a text format.
text_format:
  type: mapping
  label: 'Text with text format'
  # We declare the entire mapping of text and text format as translatable. This
  # causes the entire mapping to be saved to the language overrides of the
  # configuration. Storing only the (to be formatted) text could result in
  # security problems in case the text format of the source text is changed.
  translatable: true
  mapping:
    value:
      type: text
      label: 'Text'
      # Mark the actual text as translatable (in addition to the entire mapping
      # being marked as translatable) so that shipped configuration with
      # formatted text can participate in the string translation system.
      translatable: true
    format:
      type: string
      label: 'Text format'
      # The text format should not be translated as part of the string
      # translation system, so this is not marked as translatable.
  • Current type of text_format.format is string; it should be machine_name.
  • There are no constraints on text_format.format; should have:
          constraints:
        ConfigExists:
          prefix: filter.format.

Proposed resolution

Adjust the schema as follows:

# Text with a text format.
text_format:
  type: mapping
  label: 'Text with text format'
  # We declare the entire mapping of text and text format as translatable. This
  # causes the entire mapping to be saved to the language overrides of the
  # configuration. Storing only the (to be formatted) text could result in
  # security problems in case the text format of the source text is changed.
  translatable: true
  mapping:
    value:
      type: text
      label: 'Text'
      # Mark the actual text as translatable (in addition to the entire mapping
      # being marked as translatable) so that shipped configuration with
      # formatted text can participate in the string translation system.
      translatable: true
    format:
      type: machine_name
      label: 'Text format'
      # The text format should not be translated as part of the string
      # translation system, so this is not marked as translatable.
      constraints:
        ConfigExists:
          prefix: filter.format.

Remaining tasks

Data model changes

Release notes snippet

Issue fork drupal-3631743

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

clayfreeman created an issue. See original summary.

clayfreeman opened merge request !17501

clayfreeman’s picture

clayfreeman
he/him
Primary language English
commented
Status: Active » Needs review
dcam’s picture

dcam commented
Status: Needs review » Needs work

There are test failures caused by this change. They need to be resolved before the issue is reviewed.

clayfreeman’s picture

clayfreeman
he/him
Primary language English
commented
Status: Needs work » Needs review

Contrib modules may need to set container_rebuild_required: true in their .info.yml when shipping config object schemas that use text_format after this change lands.

Since the module installer imports all config objects for the entire module batch before installing any config entities, the ConfigExists constraint could be violated when installing multiple modules if one module provides a new filter format and another refers to it in a config object. Dependent config entities should not be affected.

This problem presented itself for the config_translation_test module.

smustgrave’s picture

smustgrave commented

Is there an alternative solution that doesn't require contrib module having to update their .info file. If not think we need a new solution as that's not fully backwards compatible

clayfreeman’s picture

clayfreeman
he/him
Primary language English
commented

Per our discussion in Slack, I think the agreed-upon way to avoid it would be to refactor the config import process that occurs during module installation so that entities/objects are installed together for each module, rather than a full batch of objects followed by a full batch of entities.

I tend to believe that a refactor to the config importer would be a rather large change, and is probably something that won't happen soon. I also understand that the .info.yml key was probably added as a workaround for another issue, so we should ultimately try to get rid of the key anyway, but I don't think we should treat this edge case as a blocker when applying the .info.yml key is a workaround.

I believe the best way to address this is to:

  1. Create a change record that details the schema change, and describes when the .info.yml key is necessary.
  2. Update the constraint validator to include messaging related to the .info.yml key so that people know where to start looking if they encounter a problem.

We presume that most sites will be unaffected, but for those that are, the updated violation message would make the fix obvious.