Determine which core config entity methods should be config actions

Added a child issue for Config::set().

What are the "core provided config entity methods"?

Would someone be able to list, or link to a list?

@thejimbirch

Here are some quick pointers that I hope may be enough to get you started.

Config entity types are defined via the ConfigEntityType annotation, see the relevant documentation page. To list core-provided config entity types, search api.drupal.org for ConfigEntityType and click on the link for the annotation class with the description "Defines a config entity type annotation object." On the resulting page, click to expand "33 classes are annotated with ConfigEntityType" and then click "See full list". Then click on each of the listed types: Action, Block, and so on--though we can presumably skip anything with "Test" in its name.

How do we start to decide which methods should be config actions? Here are some thoughts and suggestions.

For starters, it has to be something that modifies the config entity. Often, that's reflected in a verb that starts the method name. The most common such verb is "set", but there are others.

Once you bring up the class for a config entity type - let's take Block as an example - scroll down to the "Members" section. For "Type" enter "Function" and click "Filter". In the resulting list, you can ignore any that say "protected" under "Modifiers", and also any function starting with "get" ("getPlugin" and so on), since by definition those methods won't alter anything. Conversely, anything starting with "set" is probably a strong candidate for a config action. For Block, that starts with "Block::setRegion".

In doing some research, I discovered another thing we'd want to grant in some situations - the ability to change the configuration options of a plugin that powers a config entity.

A good example here is adding an additional entity type or bundle to a Content Moderation workflow. In this case, the workflow itself is not really the thing to update - we need to get the workflow's type plugin, and tell it to make the change as needed. How could this be represented in config actions?

Media types are another example of a plugin-backed config entity that would need similar handling.

Another example: changing filter plugin settings for an enabled filter plugin in a text format.

And AFAIK the most complex example possible: changing the CKEditor 5 plugin settings for a text editor that happens to be using the CKEditor 5 plugin — this is AFAIK the only config entity with multiple layers of indirection.

If we can make config actions work there, then we can make them work everywhere AFAICT.

From #3417835-14: Convert the Standard install profile into a set of recipes - it would make sense to have the setter methods of \Drupal\contact\ContactFormInterface be config actions:

• \Drupal\contact\ContactFormInterface::setMessage()
• \Drupal\contact\ContactFormInterface::setRecipients() -- there should also be an addRecipient action (and a pluralized version of that), which adds a single recipient to an existing list.
• \Drupal\contact\ContactFormInterface::setRedirectPath()
• \Drupal\contact\ContactFormInterface::setReply()
• \Drupal\contact\ContactFormInterface::setWeight()

#7: excellent info — that's exactly the kind of real-world research we need to move this forward IMO 🤩

OK, I ended up taking a whirlwind tour through core and marking a lot of entity methods as action methods.

Curiously, most of them don't make sense in pluralized form (things like setting the weight, or changing the name, of an entity -- you only do that once in any given receipe, they're not really methods that take multiple values). So these all got pluralize: FALSE in their annotation.

Okay, I think this is ready for a look.

I took a pretty broad approach to exposing config entity methods as actions, and added test coverage for every one of them. Some methods I omitted on purpose because they couldn't work from an API perspective, despite being useful (like \Drupal\image\Entity\ImageStyle::deleteImageEffect); others I skipped because their usefulness is dubious (like \Drupal\search\Entity\SearchPage::setPlugin).

Would be curious what people think.

This looks good to me, except that I find it a little odd that in some cases we label it in agreement with the method verb:
setRequired() label = 'Set whether field is required'

...but other times instead of 'set' it's 'change'
setSettings() label = 'Change field settings'

I think I would always go with 'Set' (or whatever the verb of the method is).

I'll test all of these in a recipe at contrib day tomorrow.

Am I thinking of this correctly? Based on these changes we can update following:

## Field instances:

config:
  action:
    field.field.*.*.ymal
      # Change field label
      setLabel: 'words'
      # Change field description
      setDescription: 'words'
      # Set whether field is translatable
      setTranslatable: true
      # Change field settings
      setSettings:
        on_label: 'Yes'
        off_label: 'No'
      # Set whether field is required
      setRequired: true

### And we can't update:

uuid: ...
langcode: en
dependencies: ...
id: entity.bundle.field_bar
field_name: field_bar
entity_type: bar
bundle: bar
default_value: {  }
default_value_callback: ''
field_type: entity_reference

Correct.

## Block

config:
  action:
    block.block.*.yml
      # Set block region
      setRegion: content
      # Set block weight
      setWeight: 10

### And we can't update:

uuid: ...
langcode: en
status: true
dependencies: ...
id: unique_id
theme: themw_name
provider: null
plugin: system_main_block
settings: ...
visibility: ...

All correct.

It's pretty much a subjective judgment call, but I only exposed methods that I think are likely to be useful, but not cause additional complications. Complicated things need their own issues and/or dedicated config actions, so we can think through and address their implications.

That makes perfect sense. Thank you for your patience as I work through this.

It is helping my brain understand better making a list of what we can and cannot do per config type. I feel like it will make a good documentation resource also.

## Contact form

config:
  action:
    contact.form.*.yml
      # Set contact form message
      setMessage: 'words'
      # Set recipients
      setRecipients: 'admin@example.com'
      # Set redirect path
      setRedirectPath: '/path/to/page'
      # Set auto-reply message
      setReply: 'admin@example.com'
      # Set weight
      setWeight: 10
      # We can use the `set` action to update the label
      set:
        key: label
        value: Personal contact form

### And we can't update:

langcode: en
status: true
dependencies: {  }
id: personal

So about that...

It's not really true that you can't update label...it's just that there's no dedicated method for it on the ContactForm class. But this MR adds generalized access to ConfigEntityBase::set(), so you could do this:

contact.form.feedback:
  set:
    key: label
    value: Personal contact form

...but that's sort of a lower-level action, so maybe a bit of an advanced use case.

That's great! Updating comment #22, thanks!

Updated the labels based on the comment in #16 and phenaproxima's thumbs up. I can't resolve the comments, but they should be closed.

I believe this is everything we can do with these additions.

Block

config:
  action:
    block.block.*.yml
      # Set entity status
      setStatus: true
      # Set block region
      setRegion: content
      # Set block weight
      setWeight: 10

And we can't update

uuid: ...
langcode: en
dependencies: ...
id: unique_id
theme: theme_name
provider: null
plugin: system_main_block
settings: ...
visibility: ...

Contact form

config:
  action:
    contact.form.*.yml
      # Set entity status
      setStatus: true
      # Set contact form message
      setMessage: 'words'
      # Set recipients
      setRecipients: 'admin@example.com'
      # Set redirect path
      setRedirectPath: '/path/to/page'
      # Set auto-reply message
      setReply: 'admin@example.com'
      # Set weight
      setWeight: 10
      # We can use the `set` action to update the label
      set:
        key: label
        value: Personal contact form

And we can't update

langcode: en
dependencies: {  }
id: personal

Field instances

config:
  action:
    field.field.*.*.yml
      # Change field label
      setLabel: 'words'
      # Change field description
      setDescription: 'words'
      # Set whether field is translatable
      setTranslatable: true
      # Change field settings
      setSettings:
        on_label: 'Yes'
        off_label: 'No'
      # Set whether field is required
      setRequired: true

And we can't update

uuid: ...
langcode: en
dependencies: ...
id: entity.bundle.field_bar
field_name: field_bar
entity_type: bar
bundle: bar
default_value: {  }
default_value_callback: ''
field_type: entity_reference

Image

config:
  action:
    image.style.*.yml
      # Use `setMultiple` to update name and label
      setMultiple:
        -
          key: name
          value: machine_name
        -
          key: label
          value: Human readable name
      # Add an image effect
      addImageEffect:
        id: image_scale
        weight: 1
        data:
        width: 1300
        height: 1300
        upscale: false

And we can't update

langcode: en
dependencies:

Language

config:
  action:
    language.entity.*.yml
      # Set entity status
      setStatus: true
      # Set Language name
      setName: foo
      # Set weight
      setWeight: 1
      # Use `setMultiple` to update direction and locked
      setMultiple:
        -
          key: direction
          value: rtl
        -
          key: locked
          value: true

And we can't update

langcode: en
dependencies: {  }
id: es
direction: ltr
locked: false

Node

config:
  actions:
    node.type.*.yml
      # We can use the `setMultiple` action to update the name and description
      setMultiple:
        -
          key: name
          value: Article
        -
          key: description
          value: 'Use <em>articles</em> for time-sensitive content like foo, bar'
      # Set entity status
      setStatus: true
      # Set revisions or not
      setNewRevision: false
      # Change the Preview mode
      setPreviewMode: 2
      # Change the Display Submitted.
      setDisplaySubmitted: false

And we can't update

langcode: en
type: article
help: null

Media

config:
  actions:
    media.type.*.yml
      # We can use the `setMultiple` action to update things
      setMultiple:
        -
          key: label
          value: New name
        -
          # Queue thumbnail downloads
          key: queue_thumbnail_downloads
          value: false
      # Set entity status
      setStatus: true
      # Set Description
      setDescription: "words"
      # Set revisions or not
      setNewRevision: false
      # Set field mapping
      setFieldMap:
        name: name

And we can't update

langcode: en
dependencies: {  }
id: image
source: image
source_configuration:
  source_field: field_media_image

This MR is awesome. It will really help push config actions forward. I have some questions and comments.

1. The tests are testing the additions here and helped me understand the options. However, they are far from complete. Could we update these test to cover all of the possible changes you can do per config type? Or would that be better suited for a new issue?

2. If we can do things with just the set config action, do we need to have all the specificity?

For example, on Contact form we can set the label name with:

      set:
        key: label
        value: Personal contact form

Yet on Language we have setName: foo.

And on Field instances we have setLabel: 'words'

3. In the example above, we have setName and setLabel. Can we decide on one for consistency?

4. I made some assumptions in the documentation I put in the comment above this that we could use set: on things. Could someone validate those will work?

#27.3: I think the current MR is consistent with the naming of each config entity type. Sadly, not all config entity types are consistent.

I don't think it makes sense to change that here — if you want that consistency, we should be enforcing it at the config entity type level instead, and provide update paths for those config entities. That'd be a separate issue IMHO.

Thanks @Wim Leers. That makes sense.

Also need to document:

set:
setMultiple:
removeComponent:

#27.2:

If we can do things with just the set config action, do we need to have all the specificity?

Yes, yes we do. set() is an extremely dumb method; it just updates a value on the object, regardless of whether it makes any sense as passed, or needs additional massaging or processing to be valid/useful. It's handy for filling in incomplete entity APIs, but it's fundamentally a shim. The more specific methods are the ones that should be used wherever possible.

#27.3:

In the example above, we have setName and setLabel. Can we decide on one for consistency?

Not with the ActionMethod annotation, we can't. Using that annotation, the config action is named for the entity method. Renaming it would (I think) require a whole separate config action, or at least some kind of alter hook.

#27.4:

I made some assumptions in the documentation I put in the comment above this that we could use set: on things. Could someone validate those will work?

Yes, we have set: and setMultiple:. They are both explicitly tested in EntityMethodConfigActionsTest::testSet(). However, in your comment, the syntax is wrong. To call set() a single time, it's like this:

      set:
        key: label
        value: New name

To call it multiple times:

      setMultiple:
        -
          key: label
          value: New name
        -
          # Queue thumbnail downloads
          key: queue_thumbnail_downloads
          value: false

Thanks for answering my questions! I updated my examples in #26 to use setMultiple.

This is a great step forward for Actions. I am marking as RTBC.

Based on @alexpott's review, I think the next steps are:

Move the following to their own issues/MRs
* core/lib/Drupal/Core/Field/FieldConfigBase.php - Remove component
* core/lib/Drupal/Core/Field/FieldConfigBase.php - Set default value

"This needs validation" - does that mean in this MR, or break this one out also?
* core/modules/block/src/Entity/Block.php - Set block region

Slack convo:
https://drupal.slack.com/archives/C2THUBAVA/p1719847206878749

TLDR: Keep, but move into its own Issue/MR

phenaproxima
1 day ago
I don’t think this can be considered a truly “destructive” operation in the sense that it can break the site.

alexpott
1 day ago
Well it could break a site - what happens if you remove a required field’s widget?

alexpott
1 day ago
But I do get the point. What component are you wanting to remove?

phenaproxima
1 day ago
Well, one example in Starshot’s prototype is, say, a geofield that stores latitude and longitude.

phenaproxima
1 day ago
If I’m geocoding from an address field, then I want to hide the geofield.

alexpott
1 day ago
So you want to add the field and remove the components straight away?

phenaproxima
1 day ago
Well, in my case, I don’t want the field to show up at all, it’s just a data store.

phenaproxima
1 day ago
So, yes.

phenaproxima
1 day ago
I am, to be clear, totally fine with deferring removeComponent to a follow-up.

phenaproxima
1 day ago
But I’m about 99% sure we are going to want it.

alexpott
1 day ago
Sure let’s leave it in. Maybe add some docs about trying to only use it in recipes that add the field that is being removed. At the end of the day we’re going to hit things like this where the pragmatic solution is to allow and indicate how it should be used.

phenaproxima
1 day ago
Agreed.

phenaproxima
1 day ago
I have overall been totally on board with not adding destructive things, but I favor usefulness. Removing components is something I have to do quite often (edited)

thejimbirch
1 day ago
Would a better word be unSetComponent(s)? (edited)

mikelutz (he/him)
:4k: 23 hours ago
Just call it iDontKnowWhatYouAreTalkingAboutINeverSawAComponentThere()
:stuck_out_tongue_closed_eyes:
1

mandclu
21 hours ago
In the geofield example, wouldn't you be hiding the field instead of removing it?

mandclu
21 hours ago
That looks like it's exactly what the code does. Maybe calling it hideComponent instead would make it more clear that it's nondestructive?

thejimbirch
20 hours ago
Thats better than my suggestion.

phenaproxima
20 hours ago
@mandclu
removeComponent is the name of the method. ActionMethod actions use the name of the method AFAIK. Not sure if there’s a way to override that
:+1:
1

alexpott
8 hours ago
There is no way to override it (yet). We could add that but it decouples us from the API which could get confusing. Tricky.

mandclu
6 hours ago
I mean, we did recently rename ensure_exists to createIfNotExists so it's not like renaming things is impossible

alexpott
6 hours ago
@mandclu
totally. But that is it’s own action so less tied to the config entity’s existing API.

Commented on the EntityDisplay. Re: the "needs validation", I'm wondering what the approach should be here. For example, there's technically nothing stopping someone from just doing the following on their own:

$block->setRegion('foo');
$block->save();

Why should recipes be any different? It's up to the recipe author IMO to ensure validity of the data they pass to a config action.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Assigning to myself to do some cleanup here.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Updating the issue summary with an partial list of every action added by this merge request, with examples for documentation.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Tests are passing, Confirmed aliasing works as expected, noted other concerns, all others addressed. Looks great!

Fixed @alexpott's feedback. Since it was literally just renaming a parameter, I don't think we need to shuffle through the whole needs review process for this. Restoring RTBC.

Tests pass and feedback addressed. Marking as RTBC.