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

+1 for this. Having the cases be mixed is not good for the recipe authoring experience.

According to the config action documentation, there are two:

simple_config_update
ensure_exists

Thanks @thejimbirch, that was my findings as well. How should changes like this be handled to communicate to the documentation there? Is there a way to link the two?

We create follow-up issues to document changes/additions in the Recipes Initiative project to the 1.0.x branch.

Needs review, its passing tests, but not entirely sure about the deprecation/backward compat implementation.

I don't see any problem with this; I think we probably just want some basic test coverage to confirm that creating an instance of the old plugins raises a deprecation message.

So it appears the change record → I was looking at is actually not yet committed. So instead, I've moved this to triggering a deprecation notice upon instance creation of the old names. Added a test for this as well.

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.

Test bot failed cause I pushed while running :S

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.

b_sharpe → changed the visibility of the branch 11.x to hidden.

b_sharpe → changed the visibility of the branch 3455113-recipe-plugins-ids to hidden.

Jeez, 11.x instead of 11.0.x...

I have observed the test failures on the MR , which indicates still some files left in which plugin ID from ensure_exists to ensureExists needs to update.

trace out those files, updated plugin id & test passed on local for me

Updated files in MR as well , test failures fixed.

Pipeline passed successfully & MR is mergeable now

Thanks for working on this. We'll still need some light test coverage, per #9, to prove that the deprecated plugin IDs are still usable, but trigger a deprecation error.

I can see already basic test coverage added to confirm that creating an instance of the old plugins throws a deprecation msg in following file:
core/lib/Drupal/Core/Config/Action/ConfigActionManager.php

This can be check in this older pipeline, deprecation msg throws:
https://git.drupalcode.org/issue/drupal-3455113/-/jobs/1897419

correct me if I 'm missing anything

You're right that the pipeline will show the deprecations, but that's not what we want. We need explicit automated test coverage of the deprecation, and we don't want the pipeline itself to show any, because that indicates that we're still using deprecated code in core.

An example of how to explicitly test for a deprecation might be seen in \Drupal\Tests\system\Functional\Module\DeprecatedTemplateTest::testDeprecatedTemplate()

(Basically -- your test method must be annotated with @group legacy, and you need to call $this->expectDeprecation(...) before you actually do the thing that will trigger the deprecation, which in this case is creating an instance of ensure_exists or simple_config_update.)

@phenaproxima, I had already added this here: https://git.drupalcode.org/project/drupal/-/merge_requests/8451/diffs#b4... is there somewhere else this is expected?

    $this->expectDeprecation('Unsilenced deprecation: The plugin ID "entity_create:ensure_exists" is deprecated. Use "entity_create:ensureExists" instead.');

Also, I've been informed by @thejimbirch that this method is changing to `createIfNotExists` which likely means this issue needs an entire rework. I will check and see what expected order of operations will be here.

this method is changing to `createIfNotExists`

Yup, that's correct. In fact, let's just directly deprecate ensure_exists and replace it with createIfNotExists here.

Oooook, rebased, changed and back to review :)

Thank you so much; this is excellent work. Marking as RTBC.

Regarding the name change. In a recipes leadership call earlier this week, we hashed out what ensure_exists really did, and want we want it used for.

1. ensure_exists currently verifies that the config exists by it's id and nothing else.
2. If the config exists it skips doing anything to the fields and moves on.
3. If the config doesn't exist, it creates the config in Drupal with the values entered.

This is a much more lenient approach to recipe dependencies vs. having config in the /config folder because it simply has to exist and not match exactly.

This is looking pretty good but the deprecation stuff needs a littel bit of work.

@alexpott updated the MR as you suggested, please review.

@alexpott updated the MR as per your suggestion.

Covered all suggestions and feedback, please review MR!8451

cc:@phenaproxima @@alexpott

I'll let the others comment if this is cause for concern or not, but previously I was only resetting the cache if the keys didn't exist in the getDefinitions() method

Do we care that now it is resetting the cache every call to this method instead of only when the replacements don't already exist?

    // This is no longer conditional.
    $this->setCachedDefinitions($definitions);

@b_sharpe great call - I think we can override a different method and not have to think about the cache.

I like that better as well, back to NR

This looks fantastic now. I would RTBC but that would mean I can't commit.

I think there are still a few small things that need fixing. Also this is going to need a dedicated change record that we link to, no? Otherwise this is an easy RTBC for me. Looks great!

Change record added.

I applying @phenaproxima's suggestions - back to needs review.

Happily marking as RTBC

+1!

All good but just pushed minor change in the Kernel test to update the url of documentation.

Backporting to 10.3.x because recipes is still experimental and there are no API breaks here to consider anyway.

Committed and pushed 9cd7babdc7 to 11.x and c867b596fe to 11.0.x and 55dd331dea to 10.4.x and 95a2848615 to 10.3.x. Thanks!

CR point to
Introduced in branch:
11.3.0
Introduced in version:
11.3.2

I think it has to be 10.3.0 and 10.3.2 respectively.

@voleger++ yep - fixed.

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