Convert the Standard install profile into a set of recipes

phenaproxima → credited alexpott → .

phenaproxima → credited kerrymick → .

phenaproxima → credited narendraR → .

phenaproxima → credited nedjo → .

phenaproxima → credited thejimbirch → .

phenaproxima → credited Wim Leers → .

Welp, just as I submitted this issue, @alexpott gave me permission to assign credit. So, transferring credit from #3301370: Model core's standard install profile as recipes → .

Test failures ahoy!

I have a sneaking suspicion that one of the tests (StandardRecipeTest) is failing due to the config ordering bug I described in #3416287: ConfigConfigurator should be insensitive to key order when comparing active config to recipe config → .

Since I'm not certain that's the cause, I'm not going to explicitly call it a blocking issue...at least, not yet. But I'm linking it as a related one.

Re #12, Test failure is due to change in recipients email in tests from admin@example.com to simpletest@example.com. How can I fix this?

Also I have added test coverage as per https://www.drupal.org/project/distributions_recipes/issues/3301370#comm... →

Ah, I see what you mean, @narendraR; the Empty profile is creating the feedback contact form one way, and then the feedback_contact_form recipe is trying to fully replace it.

I think what I would do in this case is remove the contact form, and the code related to it, from the Empty profile entirely. That stuff only seems to be there to emulate Standard, but frankly that contradicts the purpose of the Empty profile. :)

The real problem here is that when the feedback_contact_form recipe is applied, it needs to be able to ask the user for the recipients (or at least, it needs to be able to extrapolate it from somewhere). So to really properly solve this, we should fix #3303126: Getting user input during / before applying a recipe → , but that's a job for another time.

What would be even better is if the feedback_contact_form recipe could read the value of the system.site:mail value and automatically copy it to the contact form while it was being applied! But that's currently beyond the capabilities of the recipe system.

I'll open a follow-up issue to at least raise that as a possible feature, since that's the kind of thing recipe authors will likely need from time to time.

Okay, I updated #3303126-6: Getting user input during / before applying a recipe → with our use case. We probably don't need a whole new follow-up, IMHO, since this problem falls into the scope of "collecting input while applying a recipe".

Re: #14, I had created the empty profile to have no configuration at all. Investigating what made its way in there.

Ah, I see that functionality that cannot be currently done in recipes is being configured in the empty profile.

I had called some of that out in https://www.drupal.org/project/distributions_recipes/issues/3301370#comm... →

I suggest that rather than state that only Install Profiles can do these things, that we wait until we do everything we can using recipes, and then figure out how to do the remaining items. Some of those items may bring to light additional config actions we need, or be possible with functionality on the roadmap like "default content".

I'd rather for now we leave the empty profile empty, to be used as an unadulterated clean slate for applying recipes.

Re #17, I agree with your point, but did it here in MR so that we can plan some action and then remove them. Also it seems to me that the points which you raised in https://www.drupal.org/project/distributions_recipes/issues/3301370#comm... → are the only action items to make this recipe identical to standard profile.

Makes sense, thanks @narendraR!

I copied StandardTest.php code into StandardRecipeTest.php to check how much it works.
It basically failed at 2 points:

• Validating contact feedback form Recipient. We have a separate issue for this.
• Not sure why installing responsive_image and content_moderation module through service does not worked. Added @todo for this.

#20:

1. That other issue is #3303126: Getting user input during / before applying a recipe → , right?
2. +1 for debugging those 2 problems here; that might lead to a new interesting insight into something Recipes is currently lacking! 😄

Exciting to see that we're already so close to have StandardRecipeTest passing! 🤩👏

1. I hope this has not been discussed previously 😅 If it has been, we should document the rationale in the issue summary.

   Why do recipe names start with "the subjective use case label" rather than "the objective thing"?

   Is it because then it becomes harder to spot the connection between the article_* recipes? 🤔

2. Related: why even have article_comment + article_content_type + article_tags? Why not just article? Articles have comments and tags, period. If you don't want that, use a different recipe. It feels like unnecessary granularity to me.

   Heck, we could go with standard_article. And then there could be more opinionated variants of "articles" in other recipes provided by contrib?

Re #21.1, the reason is #13. I am not sure why but the value of recipients email is not changing from admin@example.com to simpletest@example.com

#23: Are you saying that we named recipes this particular way to ensure a particular execution order of the recipes? 😰

Re #24, No. I am saying this in reference to #20.

Validating contact feedback form Recipient. We have a separate issue for this.

Re #22

1.

Why do recipe names start with "the subjective use case label" rather than "the objective thing"?

As part of https://www.drupal.org/project/distributions_recipes/issues/3284777 → , documentation was added to the 1.0.x branch. The naming convention is peppered throughout the documentation, but the definition of it is in this doc: https://git.drupalcode.org/project/distributions_recipes/-/blob/1.0.x/do...

I do agree with @wimleers though, I like bundle_uniquename better rather than uniquename_bundle.

Should we propose a different naming convention and update the documentation?

2.

Related: why even have article_comment + article_content_type + article_tags? Why not just article? Articles have comments and tags, period. If you don't want that, use a different recipe. It feels like unnecessary granularity to me.

Heck, we could go with standard_article. And then there could be more opinionated variants of "articles" in other recipes provided by contrib?

To make it as composable and reusable as possible.

Articles have comments and tags, period.

Not in any Drupal site I have built in 11 years. Most articles we build do not have comments, and have more specific taxonomy vocabularies like Topics, Category, Subject.

I believe this may have been a norm in Drupal 6 days of the web, and we should still support it in a composed recipe like standard_article (or article_standard).

But to make it usable, and extendable by all, it would be good to have the stack of

article_comment
article_content_type
article_tags

So I could create my own deliverable using the pieces.

Re #20,

I have found the reason for these 2 test failures.

Validating contact feedback form Recipient.

This is because standard.profile file's hook_form_FORM_ID_alter changes recipient's email.

Not sure why installing responsive_image and content_moderation module through service does not worked.

These is because tested configuration files exists in standard profile's config folder and only enabling module without standard profile will not help. Recipes do have these files and hence it worked.

Now as we know the reasons for test failures in StandardRecipeTest.php, what else can be done here.

All feedback addressed/answered, tests passing, IS updated.

Nice progress here! Some questions on the MR :)

Let's please document the rationale behind the current recipes that …/standard/recipe.yml depends upon.

That information will be crucial in eventually landing this in Drupal core. People will nitpick this apart and question why these particular recipes were created (because there is no objectively optimal or best choice). If that happens in 1 year from today, none of us will remember those choices.

Re #32, I have updated the issue summary to try and explain the rationale for these recipes as architected.

Whoops, HTML error.

I think I'm pretty happy with the way this looks.

All tests are passing, and the issue summary is up to date. I think we're ready for review.

Read the entire issue summary prior to re-reviewing the entire merge request. Thanks for the excellent issue summary! 😊🙏 This is definitely going to help Recipes land in core too, because we'll hopefully be able to copy/paste this verbatim for the future core inclusion issue 👍

Thoughts:

1. I made a few minor edits to the issue summary to improve clarity. Can you check and confirm they're correct, which would confirm my understanding? 🙏 Can you also add the sole missing follow-up? 🙏
2. Naming nit: empty is a fine name, but the inner nerd in me really wants this to be … punnier. Isn there something we can do that is borrowing cooking terminology?
3. Naming nit: I think comment_base is less precise than it could be. I think comment_structure would be better, because creating this stuff by hand would also fall under the subtree of the information architecture. Alternatively, I think comment_storage also is a better name. Thinking about this more, this is actually specific to Nodes, so comment_for_nodes or comment_storage_for_node probably are better still.
4. The clear outline in the issue summary combined with the recipe-based-variant of StandardTest allowed me to not check all config in detail, but instead focus on the recipes and their dependencies. This made my life as a reviewer a lot simpler 👍
5. … which made me realize that one very valuable tool in my toolbox as a Recipe author/reviewer/auditor/evaluator would be to visualize the tree structure of all recipes 🤓🙏 Can we create a follow-up issue for that, to add it as a new console command? Tagged for this.
6. Regarding restricted_html_format: that's the only text format available to the anonymous user. So as soon as a site decides to allow anonymous users to post comments, it becomes relevant. It's kind of like "optional config not if a module is installed but if one small tweak is made to other config". So I think it does make sense in the Standard install profile.
7. I spotted a few bugs, have a few questions, a few suggestions … but I think it's looking damn good, and will be comfortable RTBC'ing soon 😊👏

Great work here! 👍

Addressed feedback

Almost there!

Sorry, @narendraR, I got confused! I actually have zero remarks left — this is ready IMHO! 🤩

Congrats on and thank you for this excellent work. It's strong validation of the current state of Recipes! 🎉

Speaking of which: I think the next thing to do is #3400672: [PP-1] Robustly validate the structure of recipe.yml → , while your mind still has the encountered problems fresh, and to help avoid the next recipe author from running into the same problems 😊

Updating my attribution.

Follow-up #3421660: Add a new console command to visualize the tree structure of all recipes → and #3421666: Decide on naming convention on recipes → created.

Should this be blocked until #3400672: [PP-1] Robustly validate the structure of recipe.yml → is finished, to avoid committing something that might need to be changed soon afterwards?

No need, @DamienMcKenna. If it needs to be changed (I suspect it will pass all validation), we can just change it.

Besides, this is ready to go, and #3400672: [PP-1] Robustly validate the structure of recipe.yml → has quite a bit of work still needed. If you manually test the recipes in this issue, you'll see that they work as intended. :)

So, I see no compelling reason to block this.

Agreed with @phenaproxima in #49.

Who the hell self-hosts videos these days?

But I disagree with this 😅 My site has zero external dependencies (I want to own/control all content), with the exception of embedding remote videos for e.g. a DrupalCon recording on YouTube (those recordings I do not own).

@phenaproxima or @narendraR Can you please update the metadata + summary of this issue per the discussion in Drupal Slack? 🙏

Thanks!

I only reviewed this quickly, but at least on first glance, it looks great!

I'm curious why core_recommended_themes is a single recipe rather than splitting front-end theme (Olivero) and admin theme (Claro) into separate recipes. For example, might people want to reuse the Claro recipe with a different front-end theme, or reuse the Olivero recipe with Gin or some other admin theme? Is there discussion that led to combining them that I can read up on?

The work here always set both themes. Up until the following issue, it was in the primary standard recipe. I believe this is where it was broken out into its own, core_recommended_themes recipe:

https://www.drupal.org/project/distributions_recipes/issues/3301370#comm... →

They could be broken into their own recipes, but for simplicity's sake, the core_recommended_themes does what it states.

There are also some sticky things with themes and recipes, like blocks are created from the frontend theme for both the frontend and admin 📌 Copy block configuration from admin theme when enabling an admin theme Active , that we probably don't want to get into in this issue.

@effulgentsia in #54: That's a very good point! Let's do that. 💪 (@thejimbirch in #55: as long as we install the same theme, just split across two recipes instead of one, AFAICT everything should continue to work exactly the same.)

Marked for implementing @effulgentsia's suggestion.

1. currently 11/51 or 22%: 🌱 [meta] Add constraints to all simple configuration Active
2. currently 6/29 or 20%: 🌱 [meta] Add constraints to all config entity types Active

But even when we get that to 100%, that still doesn't mean all contrib config will overnight be validatable.

The good news is that I see a viable middle ground: … only validating config that is marked as FullyValidatable (i.e. has that as a top-level validation constraint).

Is that great? No.
Is that realistic? Yes.

We don’t want Recipes to become too limited to be useful by validating everything, including things that have not yet been made validatable. Because yes, e.g. type: label, type: config_dependencies have gotten validation constraints, which means that virtually all config will have some validatability. But that’s not a guarantee that all the concrete config using those types actually has been updated to comply.
Hence that FullyValidatable middle ground proposal.

If you agree, we should postpone this issue on a yet-to-be-created validation-refining Recipes issue. It can copy/paste the logic in \Drupal\KernelTests\Core\Config\ConfigEntityValidationTestBase::isFullyValidatable() to achieve what it needs.

(That would also solve the simple_config_update scenario that @phenaproxima refers to. To make that scenario (as well as any other config action) comply with this as well, we need to make ConfigActionManager::applyAction() also check whether the modified config is FullyValidatable.)

I bet that we'll run into the very same problem over at #3401925: After a recipe is successfully applied, validate all of the config that was imported or modified by config actions → , so proposed at #3401925-8: After a recipe is successfully applied, validate all of the config that was imported or modified by config actions → to implement #56 there. If you agree, @phenaproxima, then let's postpone this issue on that one.

Yeah, I agree with you, Wim. Postponing.

@effulgentsia's remark about core_recommended_themes in #54 has been addressed 👍

My reason for postponing this in #56 which @phenaproxima agreed with in #58. I wrote in #57/at #3401925-8: [PP-2] After a recipe is successfully applied, validate all of the config that was imported or modified by config actions → to implement the -check there, but that was then extracted into its own issue: #3425540: Only validate config which is fully validatable → .

#3425540: Only validate config which is fully validatable → has landed, which means this is unblocked! We should not block this on #3401925: [PP-2] After a recipe is successfully applied, validate all of the config that was imported or modified by config actions → , because the scope of that is bigger than the specific thing this was blocked on!

@effulgentsia's remark about core_recommended_themes in #54 has been addressed 👍

1. I think this is now truly ready, because the MR being green means more now than an February 15 in #45, since validation is much stronger now 👍
2. Thanks to #3401925: [PP-2] After a recipe is successfully applied, validate all of the config that was imported or modified by config actions → having landed, we should be able to remove the "set langcode if missing" logic.
3. Since #45/Feb 15, #3418179: [meta] Make config actions more dynamic → has also made big steps forward. #3420209: Allow config actions to be applied to multiple config entities using wildcards → has landed, and other child issues are nearly ready. So, I re-checked this entire MR to ensure that we're using #3420209 wherever possible. But … AFAICT it's not relevant for the Standard recipe, because it defines a starting point. I think the capabilities that are being added in that meta will be more relevant for recipes like .

I do think that @alexpott would prefer to not commit this until 🐛 Install profile is disabled for lots of different reasons and core doesn't allow for that Fixed lands, but … I think we could just as easily land this now, so we can get started on the Umami install profile net, and then just remove the empty install profile that this issue is adding.

Almost ready! 😄

Done #61.2

One question on the MR, plus two requested follow-ups based on the big set of changes you've just pushed, @phenaproxima.

• Can you please create a follow-up issue to ensure that "superfluous modules installed" can be a problem we automatically detect and inform the Recipe author of? Examples: https://git.drupalcode.org/project/distributions_recipes/-/merge_request..., https://git.drupalcode.org/project/distributions_recipes/-/merge_request...
• Can you please create a follow-up issue to ensure that config.import entries are listed alphabetically and inform the Recipe author that this should be the case? Example: https://git.drupalcode.org/project/distributions_recipes/-/merge_request...

Can you please create a follow-up issue to ensure that "superfluous modules installed" can be a problem we automatically detect and inform the Recipe author of?

I'm not sure we can ever reliably detect this. The simple reason being: what gives us the right to determine whether an extension is superfluous? By what metric do we decide that? In this issue, it's a judgment call I made based on my intimate knowledge of what we're trying to accomplish here.

It's entirely possible you'd want to install a module that only exposes simple config, like automatic_updates, and therefore it would only be in the install list, and not mentioned anywhere else in the recipe. But it would not be superfluous.

Can you please create a follow-up issue to ensure that config.import entries are listed alphabetically and inform the Recipe author that this should be the case?

I'd really rather not do this. Having them alphabetically is a purely stylistic thing for readability in my opinion, but it's not remotely important and not something we need to warn a recipe author about. If anything, it should be a coding standard, if we develop some coding standards specifically aimed at recipes. And that's just so minor right now that even creating a follow-up seems pointless.

RE #65: okay, so you're saying that most of what you did in the last ~dozen commits was purely subjective preference than an objective improvement. I wish you'd described that in the commit messages and/or left a comment explaining this. I was under the impression that you deemed these changes necessary, which is why I asked for those follow-ups; to ensure that future recipes are held to the same standards (pun not intended). +1 for not creating follow-ups.

Ugh, looks like GitLab failed to post my review 😞 can't access it here because it's in the localStorage of my work laptop. I'll post it in the morning… (I can't recall if it should be RTBC-blocking or if it was minor. I bet it was minor … but I also don't want to waste Alex' time, so I'd rather err on the cautious side 😅)

Overall, yes, I'd say most of what I did was subjective improvement. You could argue that removing superfluous modules was an objective improvement, but they weren't hurting anything by their presence. They were just cruft. It did not occur to me to use my commit messages (which can be awfully short and callous - I'm particularly proud of things like "GET F**KED, PHPSTAN") to explain my reasoning, but I should have left a comment here. My bad.

All good, no worries! 't Was but a mere suggestion to allow me (or whomever is reviewing your indeed often delightful commit messages 😆) to understand your rationale and which level of rigor is appropriate :)

Posted the comment that failed to post: https://git.drupalcode.org/project/distributions_recipes/-/merge_request...

This made me wonder why some of those subjective changes are even accurate. And that is why I requested those follow-ups.

AFAICT the oversight would've been caught by #3423526: Validate that every config entity listed in the config.actions section of recipe.yml belongs to a module that was installed by the recipe, or one of its dependencies → , so let's postpone on that? 😊

The core blocker landed!

Our next step here is to remove the Empty profile, and convert our test coverage to apply the Standard recipe suite on top of Minimal, since I imagine that's how most people will actually use these rightnow (it wouldn't make much sense to apply the Standard recipes on top of Standard).

We'll also need an issue summary update to remove the mention of the Empty profile.

Updated the IS to remove mentions of Empty.

🤩

P.S.: there's a single line whitespace-only change in Recipe.php that could be reverted.

I...I don't think it need be blocked on that. Not sure why it was. Fixing.

So rather than painstakingly review all the config the standard recipes create and check against the standard profile I thought I'd make the test do that for us. This will ensure if there are change in core's Standard profile we'll find them.

This gives us some things to fix:

1) Drupal\FunctionalTests\Core\Recipe\StandardRecipeTest::testStandard
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
-    'create' => Array &1 ()
+    'create' => Array &1 (
+        0 => 'claro.settings'
+        1 => 'core.entity_view_mode.media.full'
+        2 => 'image.style.media_library'
+        3 => 'shortcut.set.default'
+        4 => 'system.action.media_delete_action'
+        5 => 'system.action.media_publish_action'
+        6 => 'system.action.media_save_action'
+        7 => 'system.action.media_unpublish_action'
+        8 => 'views.view.media'
+        9 => 'views.view.media_library'
+    )
     'update' => Array &2 (
         0 => 'core.extension'
+        1 => 'field.field.media.image.field_media_image'
+        2 => 'core.entity_view_display.media.remote_video.default'
+        3 => 'node.settings'
+        4 => 'core.entity_view_display.node.article.teaser'
+        5 => 'user.role.authenticated'
     )
     'delete' => Array &3 ()
     'rename' => Array &4 ()
 )

We need to fix it so that the only thing changing is core.extension. Or we agree to not do something... ie. the shortcuts. It seems we're not creating the media views. I think we should be doing that. The missing claro.settings is interesting too.

Okay we're green and found two kinda config bugs in Standard FTW. Let's do this!

Woot

Amazing! Great work everyone!

🥳🥳🥳

Best thing to happen all week!

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