Stop automatic storage creation of body field

Per discussion with @catch make this a starshot blocker

I've updated the issue summary clarifying that the creation of the body field should be prevented for content types and block types and added the Needs product manager review per discussion on Slack.

@sime could you provide a bit more info about what makes this a Starshot blocker?

The change in the merge request and what the title says doesn't really correspond. the body field *storage* is regular default config, what this changes and removes is the automatic addition of the field itself, aka the per-bundle auto-configuration.

@pameela it's not a blocker to starshot contrib efforts, but I think it's at least a soft blocker to core integration. We originally decided to remove text with summary about eight years ago in 🌱 Deprecate text_with_summary Closed: duplicate but it didn't happen at the time.

To fully deprecate install profiles in core and provide recipe selection in the installer, we need to convert Umami and Standard profiles to recipes. When we convert them to recipes, the constituent parts of those recipes may/should also be re-usable recipes, but then they need to make sense.

Umami was using the text + summary field type despite not using the summary anywhere (as in, none of the content populates the filed, and the special formatter for it is never used), recently changed in 🐛 Don't use text_with_summary in Umami Needs review .

Standard also uses text with summary, but it's not a good basis for building a site from (IMO, I always make a completely separate summary field if needed). So if we want the current core install profiles to work nicely together based on common recipes for an article content type etc., we need to clean this up.

I've been meeting to revisit but I've tried to remove that body creation feature and default config and it is definitely tricky. Hit failures everywhere.

@catch so wonder if this MR is on the right track?

I've been trying to figure out how to best tackle the deprecation in parts but each seem to open a whole can of worms.

That's how comment module keeping storage but not a field for comment #2422353: Comment module should check that comment body field exists →

Wondering if we really want this, because if we just remove this outright then nodes and block content will only have a title by default? I might be wrong but I think users expect both a title and body field if they are creating these from the UI. This is already only done on form submit so if you programmatically create an entity type it doesn't automatically get added.

If we want to stop using "text with summary" can't we just change the default in config/install/field.storage.node.body.yml?

I'm actually not opposed to that idea. 9/10 I'm going to just be adding field_body myself anyway.

@longwave would you mind discussing with other framework managers if that's the desired route? If so that might make this step easier.

So question

Do we want to get the field name has just "body" or "field_body" like we did for Umami? Do we want to change persist_with_no_fields to false while we are at it so those who want can delete the body field.

I think I have misunderstood the issue. This is about preventing the storage from being created automatically, but does not request changes to the UI. So what we probably need to happen is remove core/modules/block_content/config/install/field.storage.block_content.body.yml and core/modules/node/config/install/field.storage.node.body.yml but then fix up the *_add_body_field() methods so that they are capable of creating the storage if it doesn't exist?

This means that minimal installs and recipes will be responsible for managing the storage as required, but manually created block content and content entity types will function exactly the same as before.

So kinda the track the MR is on?

I don't think it's very clear right now what the goal of this issue is. There's a bunch of options with quite different requirements.

1. Deprecation of text with summary field type. As mentioned, we can switch to just a text field without summary and nothing else needs to change. We might want to add a separate teaser/summary field in standard profile for example for article as a replacement though.
2. Starshot is an entirely different situation however. Because " I might be wrong but I think users expect both a title and body field if they are creating these from the UI" is no longer true if you are using an alternative to the body field, for example paragraphs/layout builder/experience builder. At that point, you likely want a body field anymore when you create a node type through the UI, because you won't use it. We remove the body field in almost all cases in our projects as we replace it with a paragraph field. So the answer to that would probably be to move the body fields to standard/umami install profiles and deprecate and no longer call the _add_body_field() functions. Adding new types would then imply that you have to add the body field manually. I assume the field would then also no longer be delete-protected, so if you remove manually from article and page in standard install profile, the storage will be deleted too.
3. comment #20 is something else again and a very different approach than 2.

I agree that we need a clear decision what exactly this issue should accomplish before we start on it. My guess is the focus is Starshot, so that would mean option 2, that is the only option that enables a workflow where creating a new content type in the UI allows to default to experience builder IMHO. But that's a non-trivial shift in behavior that doesn't just affect Starshot but also all existing users of Drupal standard install profile as the UI behavior changes as well as all other distributions that rely on the field.storage.node.body field config being in node module, because if that's moved to standard then their install config is going to be broken.

Because Starshot is using recipes to create content types, we can specify what fields we want. So I don't understand how this affects Starshot at all?

@pameela it's related to starshot in two ways, one is very long term, one not so much.

In the longer term, Drupal core install profile needs to move from install profiles to project browser + recipes. To do this, we need to convert existing install profiles in core to recipes - and those should be based on re-usable components that make sense. e.g. umami should not ship with a text_and_summary field then not use the summary anywhere, which is what it did for years.

Hopefully Umami can be partially re-built on shared foundations with Drupal CMS. Standard might go altogether eventually since it won't really make sense if there is Drupal CMS. Minimal also doesn't really make sense in recipe world either.

In the short term, once Drupal CMS ships with experience builder. If someone creates a new content type, that content type should not come with a 'body' field using text + summary because that would be very confusing if XB is also enabled by default (or even if it's not, because you'd have to both enable XB and delete a field).

@berdir fwiw I think both the reasons in #22 are good:

1. I don't like the text and summary field and I think that is quite common, it could live in contrib.
2. The automatic creation of the body field also seems anachronistic.

However we can and should separate those things if it makes it easier to get done.

Ah yes the XB part makes sense, I was only thinking of Starshot v1 which probably won’t be affected.

Created 📌 Change automatic body field creation to use formatted text field instead of text_with_summary Active to unblock Starshot while we figure this out.

Maybe we don't need this part since 🌱 [Research] Landing page integration Active is the plan for XB at the moment? Or we can postpone it to see what happens there?

Thinking more about it, I guess it’s unlikely that any node type using XB would want a body field specifically so I don’t think it does change things.

No longer a blocker since 📌 Remove node_add_body_field from NodeTypeForm Active landed.

smustgrave → closed merge request !8160

Closing as a dup since 📌 Remove node_add_body_field from NodeTypeForm Active landed

Actually nvm think they just overlapped, but lets postpone on 📌 Deprecate node_add_body_field() Active

📌 Deprecate node_add_body_field() Active is in.

📌 Remove node_add_body_field from NodeTypeForm Active was the user-facing change, so removing the product manager review tag from this one.

So what should the scope of this issue be? Config cleanup?

Removing the default config file. And then adjust the places that do rely on it, such as the deprecated but still expected to function node_add_body_field() to create the storage on the fly if missing.

Also, when removing it from node module, it needs to be added to standard/umami and recipes that create node types with a body field.

You mean remove the storage.body file?

Yes, but also moving it to places that need it like profiles and recipes

Going to give it a shot!

Actually another question do we want to change the storage body file to be text formatted not text formatted with summary since the goal is to deprecate that plugin?

I don't think so, not in this issue. lets move it out of node.module first I' say. going to be tricky enough a it is.

Actually postponed on 📌 Stop automatic storage creation of body field Active which I already have included in this MR.

Could use some eyes though

I'm really confused by #43, where this issue appears to be postponed upon itself. :recursion: 🤣

Maybe you meant 📌 Deprecate node_add_body_field() Active ? But that one is already committed + fixed. So I believe this only needs a rebase to latest 11.x branch.

Oops I meant postponed on https://www.drupal.org/project/drupal/issues/3535526 📌 Deprecate block_content_add_body_field Active

I don't think it needs to be deprecated on that. There is no connection between the two other than being the same concept.

I'd do the config removal either there or in a separate issue for that.

If the block content add body ticket doesn't land in the next day or two I'll untangle these two.

smustgrave → closed merge request !12813

Okay have pulled out the storage for block_content and just focusing on node now.

Failure is unrelated 🐛 Incorrect warning for system requirements for APCu memory Active

Posted a review. And because my comments surely weren't annoying enough yet, I'll finish up with a bonus: The node:body and node:summary tokens. What on earth are we going to do with those? Deprecate them? See also 🐛 "summary" token for text+summary field is empty when using anything other than the core "body" field Active , metatag relies on this as default configuration.

Regardless if we keep or not node:summary would have to be deprecated I imagine. If we kept the storage.body and checked it to a storage not text_with_summary.

So my current question is this which is preferred

1. We keep storage.body in the node module and change the type to not use summary.
2. We remove it from node, which current MR is doing

The node:body and node:summary tokens. What on earth are we going to do with those? Deprecate them?

I think that we can and should deprecate these. Not sure exactly how though. We might need to leave the tokens in and trigger a deprecation message for a long time? We already render an empty string if the field isn't on the node type per #1671420: If no body field, [node:summary] [node:body] etc tokens are not replaced → and metatag relying on this seems like a bug, it could accept a special view mode (like search indexing does) or similar maybe?

We keep storage.body in the node module and change the type to not use summary.

I personally think the end point should be that node module doesn't ship the storage field any more.

pipeline is green but not sure how to use node_storage_body_field

And with regards to the summary token what if we move that to the text_with_summary module we eventually create?

not sure how to use node_storage_body_field

We shouldn't be relying on it at all in core. The most we could do is add a test to make sure it works as a bc layer - e.g. a test module that ships a body field that depends on field storage body and also the module install that, make sure the field is created, uninstall field_storage_body, make sure the fields are still there after install - because we want people to uninstall the deprecated module but also keep their fields.

Metatag already depends on the token module so maybe the summary token could be moved there? Token module should also already provide a direct replacement for node:body with the entity field chaining stuff it has.

Believe this one is ready now

With regards to the token. Think after this one lands I’ll start the text_with_summary module and maybe we can move the summary token there?

Drafted a CR https://www.drupal.org/node/3540814 → and made the storage module hidden.

I think the module will need to hook_system_info_alter() to mark itself as un-hidden when it's installed, so that people can uninstall it. That hopefully should work pretty well though.

Added!

I think this is much closer than the number of comments I added world normally imply.

A couple are just too make sure i doesn't miss something.
Several are related to persisting storage (I think we want to keep the profile one, but the rest I think should be false)
One potentially complicated related to migrate, but I think it's handled already.

CR I think should include the file name and content and a brief explanation on when you would choose persist (or at least a link to docs on that option)

Tried to address some of the feedback let me know your thoughts!

Reverted the change to the testing profile and article recipe as it broke tests. Probably could be a follow up to stop persisting the storage?