Stop automatic storage creation of body field

Created on 16 May 2024, 12 months ago

Problem/Motivation

There's a discussion at #3427095: [Meta] Deprecate text_with_summary about actions to remove autocreation of text_summary fields. This has come up again looking at Starshot.

In this ticket we prevent the creation of the body field, which is automatic even if you use the minimal profile.

Steps to reproduce

Proposed resolution

* Prevent creating the body field storage automatically.
* Fix tests

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component
FieldΒ  β†’

Last updated about 20 hours ago

Created by

πŸ‡¦πŸ‡ΊAustralia sime Melbourne

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @sime
  • πŸ‡¦πŸ‡ΊAustralia sime Melbourne

    Per discussion with @catch make this a starshot blocker

  • πŸ‡©πŸ‡ͺGermany rkoller NΓΌrnberg, Germany

    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.

  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States cainaru Norwood, NY, USA
  • First commit to issue fork.
  • Merge request !8160Resolve #3447617 "Stop automatic storage" β†’ (Open) created by smustgrave
  • Pipeline finished with Failed
    12 months ago
    #179838
  • Pipeline finished with Failed
    12 months ago
    Total: 205s
    #179851
  • Assigned to smustgrave
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave
  • Pipeline finished with Failed
    12 months ago
    Total: 617s
    #179860
  • Pipeline finished with Failed
    12 months ago
    Total: 510s
    #179862
  • πŸ‡¦πŸ‡ΊAustralia pameeela
  • πŸ‡¦πŸ‡ΊAustralia pameeela

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

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    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.

  • πŸ‡¬πŸ‡§United Kingdom catch

    @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.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    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.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    @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.

  • πŸ‡«πŸ‡·France andypost

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

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    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?

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

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

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

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

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    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.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    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.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    So kinda the track the MR is on?

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    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.

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    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?

  • πŸ‡¬πŸ‡§United Kingdom catch

    @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.

  • πŸ‡¦πŸ‡ΊAustralia pameeela

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

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    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.

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    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?

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    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.

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    No longer a blocker since πŸ“Œ Remove node_add_body_field from NodeTypeForm Active landed.

Production build 0.71.5 2024