Add a config action to add to an array of configuration

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.

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.

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

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 🤔

@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'!

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.

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.

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.

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?)

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

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. :)

@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

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

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.

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

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

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.

Moving back to Needs work based on the comment in #21

I updated the merge request mainly to replace the + operator by a array_merge. I replaced annotation by attributes and tried to implement some tests.

Agree with the points in #3 → .

Apart from this, I think the config action name itself should be simple_config_append or something related because all it is doing is appending the values to the existing configs. simple_config_add could be confused with adding the first value as well.

Thanks!

Relating to ✨ Rename ensure_exists to createIfNotExists, and camel-case simpleConfigUpdate for consistency RTBC

I am not sure about this as proposed.

I think we should lay the foundation here for a more generalized array manipulator, since there are many operations we might want to do on arrays in simple config. I've put an idea or two in the MR.

Fantastic idea. Changing this back to needs work.

Here's an idea - what if this plugin was a wrapper around three common array functions -- array_push() (add to an array), array_unshift() (prepend to an array), and array_splice() (insert into an array at a particular position).

It could honestly be a pretty simple wrapper, where you literally just pass the arguments to the function:

'simpleConfigArray:append': # Calls array_push()
  property: path.to.property
  values: [add, these, to, the, array]

'simpleConfigArray:prepend': # Calls array_unshift()
  property: path.to.property
  values: [put, at, the, beginning, of, the, array]

'simpleConfigArray:splice': # Calls array_splice()
  property: path.to.property
  offset: 1
  length: null
  replacement: [values, to, insert]

This could truly all be implemented in a single plugin (simpleConfigArray) with a deriver that generates the three variants.

moved to Drupal Core, per https://www.drupal.org/project/distributions_recipes/issues/3513044 🌱 [meta] DrupalCon Atlanta: Triage issue queue and move any open issues to Drupal core Active

sonfd → changed the visibility of the branch 3365328-simple-config-add to hidden.

I started work on the approach proposed in #30 in the 3365328-simple-config-array branch.

It works right now, but the big thing I'm not sure about is: On line ~92, I don't like the weird special handling here to return $value['values'] if it exists. When I was returning the whole value array, array_push() was throwing an error about not allowing unrecognized named variables. My thought is to maybe define the expected parameters in the deriver and then we know exactly what we need to pass to the functions. I think I'll try that next.

Tests seem to be failing downstream, but MR !11679 takes the approach outlined in #30.