Add a config action to add to an array of configuration

Created on 7 June 2023, 12 months ago
Updated 31 May 2024, 1 day ago

Problem/Motivation

We currently have the simple_config_update config action which works well for setting a configuration value. However, in some cases, we don't want to overwrite an entire config value, we only want to add a new value, e.g. with mapping and sequence data types.

Proposed resolution

Add a new config action, simple_config_add(?) that appends a value to the end of the mapping / sequence data type.

Feature request
Status

Needs review

Version

11.0

Component

Code

Created by

🇺🇸United States sonfd Portland, ME

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @sonfd
  • 🇺🇸United States thejimbirch Cape Cod, Massachusetts
  • 🇺🇸United States sonfd Portland, ME

    A couple thoughts / issues after adding this proof of concept:

    1. This shares some code with `simple_config_update` - I wonder if we should create a base class or a trait with some shared funtionality and/or just merge the two together. Perhaps merge and introduce a `method` parameter, with options like `prepend`, `append`, `set` (default to set, the current functionality)?
    2. What about things that should be sorted automatically, like `core.extension.yml`, which gets sorted alphabetically and by weight? We don't really want our additions to be prepended or appended, rather go into their correct spot.
    3. Do we need to provide a way to validate that the config action is applicable to the particular config key? E.g. appending to a boolean doesn't make any sense.
  • Issue was unassigned.
  • 🇺🇸United States sonfd Portland, ME
  • 🇨🇦Canada Laura Johnson Toronto

    I opened a MR so I could try it out as a patch. I agree with @sonfd's thoughts on this, I wonder if with #1 and #2 the normal config save event will take care of sorting.

    I also wonder about adding something to a particular subkey, eg.

        core.entity_form_display.node.page.default:
          simple_config_add:
            third_party_settings:
              field_group:
                group_content:
                  children:
                    <strong>- field_content</strong>
                    - field_text

    Is there a way we can specify that we want to add an item to the children here?

    Right now I'm using simple_config_update to replace the entire third_party_settings for this config just to get that child in there.

  • 🇺🇸United States thejimbirch Cape Cod, Massachusetts

    With your patch, does this work?

    core.entity_form_display.node.page.default:
      simple_config_add:
        third_party_settings.field_group.group_content.children.field_content
    
  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    Isn't this counter to the very intent of config actions? 🤔

    That being said, the example in #6 is for third_party_settings, and then it totally makes sense!

    Either way, the issue summary is incomplete.

    @thejimbirch: quoting the issue summary: , @ltrain wants to prepend a value.

    @ltrain I think it's a reasonable request to be able to want to insert just that one bit (a single "node in the tree") instead of overwriting the entire thing ("an entire subtree"). Because otherwise we'd reduce composability of recipes 😔 However, sequences are notoriously sensitive to this kind of problem, also for Config Translation — see 🐛 [Style] CKEditor 5 styles config storage is not compatible with config ovverides Needs work for an example where I'm the one who failed to think about this 😞

    That actually suggests that we should never allow numerically keyed sequences of primitives (strings/ints/…), and always require either named keys or ordering based on a weight key in each sequence item 🤔

  • 🇨🇦Canada Laura Johnson Toronto

    @thejimbirch I tried your suggestion and that doesn't work even to append it. It threw the exceptions you see in the action for mapping/definitions checks, but even if I comment those it doesn't seem like chaining them the way you describe works in this case. It would be cool if it did.

    I also tried

    simple_config_add:
            third_party_settings.field_group.group_content.children: field_content

    Thanks @wim-leers, This suggests that it will be more complicated... probably a separate config action if we want to keep this one 'simple'!

  • 🇮🇳India narendraR Jaipur, India

    I think #3348991: Add a method to add configurations using recipes. is also same issue. If that is the case, we can close that issue as work is already started here.

  • 🇺🇸United States phenaproxima Massachusetts

    So...I'm honestly not sure this is something we should do.

    My reasoning is that in most cases, you probably don't want to just add arbitrary values to a mapping or sequence. You most likely want to make some kind of coherent configuration change, which perhaps involves adding something to a mapping or sequence -- for example, maybe you want to an effect to an image style. In a case like that, you don't want to use simple_config_update; it's the wrong tool for the job. You need a config action that actually has a holistic understanding of what it means to an image effect, and what's needed for it to make sense. This is especially important now that config is validated after a recipe modifies it. To avoid accidentally creating bad config, you probably don't want to be messing around with the low-level simple_config_update action if you have to do anything more advanced than set a particular key to a particular value, as you can now.

    I'm not saying that the ability to append to a config array directly is never needed, but I suspect it might be rare enough that it would be okay for someone to write a dedicated config action for it, if they needed to.

  • 🇺🇸United States phenaproxima Massachusetts

    About the third-party settings use case -- I think we should aim for something like this:

    some.config.entity:
      setThirdPartySetting:
        module: some_module
        key: some_key
        # Overwrite some_key completely.
        value: some_value
    
    some.other.config.entity:
      setThirdPartySetting:
        module: some_module
        key: some_array_key
        value: foo
        # foo will be appended to the some_array_key third party setting. If the existing value is not an array, throws an exception.
        position: end
    
    yet.another.config.entity:
      setThirdPartySetting:
        module: some_module
        key: some_array_key
        value: baz
        # foo will be prepended to the some_array_key third party setting. If existing value is not an array, throws an exception.
        position: 0
    
    even.more.config.entities:
      setThirdPartySetting:
        module: some_module
        key: some_array_key
        value: pastafazoul
        # pastafazoul will be placed at position 3 of the array, replacing the previous value. If the existing value is not an array, throws an exception.
        position: 3
        replace: true
    
    another.name.i.dunno:
      setThirdPartySetting:
        module: some_module
        key: some_array_key
        value: {a: some_value, b: {some_value, another_value}}
        # value will be merged recursively into the existing value, which must be an array
        merge: true
    

    This is similar in some ways to what was implemented in #3431330: Add a dedicated config action to add a button plugin and settings into the active toolbar for a CKEditor 5 editor , where you could specify the position of the toolbar item (or it would be appended by default), and whether it should replace what was already there.

    IMHO this is something that should maybe be implemented specifically for third-party settings, since that's probably the most common need for something like this.

  • 🇺🇸United States sonfd Portland, ME

    I just want to add to the record the original use-case that I was looking at.

    The display suite module has a configuration option to define classes that can be applied to regions. It's just a list of classes, see the ds.settings.yml file.

    I do think that @narendraR is right in #10; I do think that issue is the same as this display suite settings case, though a bit more complicated because it also wants to add to the dependencies of the configuration (kind of feels to me like that should not need to be managed manually?)

  • 🇺🇸United States ultimike Florida, USA

    This is my first foray into using Recipes and I was working through an exercise where I was adding a new filter to a new text format. Where I think this issue is relevant for me is when I tried to give an existing role permission to use the new text format.

    I was able to import new config for the format and filter, but I wasn't able to give the existing role permission to use the new format because (I think) I need to add a new "dependencies:config" value and a new "dependencies.module" value to the existing list of dependencies in the existing user role configuration.

    Here's what I was hoping would work in my recipe:

    config:
      actions:
        user.role.content_editor:
          grantPermissions:
            - 'use text format ai'
          simple_config_update:
            dependencies:
              config:
                - 'filter.format.ai'
              modules:
                - 'filter'

    What ended up happening is that the existing config and module dependencies were completely replaced by the values in my recipe.

    I believe that the addition of a "simple_config_add" action is what could make this possible, no?

    In my use case, the recipe still achieves its goal, but I understand that the role configuration is a bit broken, as it won't correctly force dependencies at this point.

    thanks,
    -mike

  • 🇺🇸United States phenaproxima Massachusetts

    Sort of beside the question, but...never update the dependencies key using simple_config_update (or anything else). Config dependencies are computed when the entity is saved; they're meant to be managed by Drupal.

    Other than that, your recipe looks good. :)

  • 🇺🇸United States ultimike Florida, USA

    @phenaproxima - thanks for the quick response.

    That makes sense.

    To fully convince myself, I just checked out the role configuration on /admin/config/development/configuration/single/export and it does have all of the correct dependencies, including the ones I thought I had to add manually. Very nice.

    thanks,
    -mike

  • Status changed to Needs review 14 days ago
  • 🇺🇸United States thejimbirch Cape Cod, Massachusetts

    With the addition of the setThirdPartySetting(s) config action, is this issue still needed?

  • 🇺🇸United States thejimbirch Cape Cod, Massachusetts
  • 🇫🇷France vbouchet

    I am very new in using Recipes and config actions. My use case is to append (or prepend, the order is not very important for me) a value in the antibot.settings.form_ids to add webform_*. Right now, I have not find any other solution than copying the default config in my recipe config action.

  • 🇺🇸United States thejimbirch Cape Cod, Massachusetts

    @vbouchet I haven't used that module in a recipe. But try this:

    config:
      actions:
        antibot.settings:
          simple_config_update:
            form_ids: webform_*
    
  • 🇫🇷France vbouchet

    Thanks @thejimbirch. Config is not complaining during recipe install but accessing the module settings page is causing a fatal as form_ids is supposed to be an array.
    I also tried

    config:
      actions:
        antibot.settings:
          simple_config_update:
            form_ids:
              - 'webform_*'
    

    but as expected, it simply replace the entire config and only webform are protected. For now my solution is to duplicate the original config and add mine:

    config:
      actions:
        antibot.settings:
          simple_config_update:
            form_ids:
              - 'comment_*'
              - user_login_form
              - user_pass
              - user_register_form
              - 'contact_message_*'
              - 'webform_*'
    

    In my case, it is not a big issue as my recipe is about setting some security things. But I can imagine a "form" recipe which wants to conditionnaly integrate with the antibot module and append the config without rewritting everything.

    I think the feature request is still very valid.

Production build 0.69.0 2024