Problem/Motivation
node_add_body_field is being deprecated and Node also stopped the automatic creation of the body field.
Although it is possible to have several node types with a body field on a simple site (e.g. article and page in standard profile), while many node types may not have a body field, it's even less likely with blocks to have more than one with a body field - either you would only have one custom block type, or you'd have several with completely different fields and no body (entity reference, media etc.).
Additionally, we definitely shouldn't be creating a text_with_summary field because blocks don't have a concept of a 'teaser' or similar view mode in any meaningful way, so there is nowhere for the summary to be shown.
Steps to reproduce
NA
Proposed resolution
Deprecate block_content_add_body_field and stop automatic creation of body field.
Remaining tasks
Do it
Review
User interface changes
NA
Introduced terminology
NA
API changes
block_content_add_body_field is deprecated
Data model changes
NA
Release notes snippet
NA
Issue fork drupal-3535526
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
smustgrave commentedComment #4
acbramley commentedWe should just wait until the node issue is done so we can reuse the trait here https://git.drupalcode.org/project/drupal/-/merge_requests/11194/diffs#8...
Comment #5
smustgrave commentedSure, going to keep assigned to me then as I hope the blocker will land soon
Comment #8
smustgrave commentedComment #9
smustgrave commentedBlocker is in, I kept as removed in 12 as that was the same as node_add_body_field
Comment #10
acbramley commentedThis is looking great, I tweaked the CR slightly and added the API changes to the IS.
Comment #11
dwwMostly looks good. 1 minor question in the MR about if this is disruptive enough to defer removal for D13.
Comment #12
dwwHah, that's what #9 said. But I think that previous removal was in 11.2 so the rules were different back then...
Comment #13
acbramley commented@dww I think the rationale for removing in D12 is explained in #9
Comment #14
smustgrave commentedYea don’t know best rule of thumb but the node-body ticket was set to 12 so just copied that
Comment #15
berdirFWIW, while this is the same concept as node types technically, the situation and arguments for removing it (or not) are a bit different here:
For node types, the majority of sites at this point use alternative means to create the content: paragraphs, layout builder, experience builder. That's less of an issue. However, unlike node types, of which most need some kind of content, you're much less likely to need a second block type that has a body field. Also, I was surprised to find out that this does in fact also use text_with_summary, which makes even less sense here, because blocks don't really have a use case for a summary/teaser.
So I think it makes sense to remove this as well, but I think the issue summary should explain this better than "because node does it too".
Also, as commented in #3447617: Stop automatic storage creation of body field for node, I don't think this should block that. Either it should have it's own issue doing that or we should do it all here, but then we'd want to postpone this on that, so that we can copy the approach and figure out what needs to be done exactly first with node.
Comment #16
smustgrave commentedTried to tweak the summary some. Going to keep the scope as is and not remove the storage file just yet as it will overly complicate this one.
Comment #17
catchUpdated the issue summary with a bit more info.
Comment #19
catchThis looks fine to me. Do we need a block-specific follow-up to also look at removing the default field config (that still uses text_with_summary) or is that covered somewhere in the general 'deprecate text_with_summary' metas somewhere?
Agreed that the deprecation should be fine for 12.x, this is only used in one place in core outside tests and the usage inside tests is pretty minimal.
Committed/pushed to 11.x, thanks!
Comment #21
catchTagging for follow-up, if there's an existing issue, would be good to add it to the related issues.
Comment #22
smustgrave commentedOpened #3539330: Convert default config to use text_long over text_with_summary in block_content config
Comment #23
acbramley commentedShould this issue be retitled as it sounds like we haven't removed the auto creation yet?one is storage, one is field all good