Change createBodyField from making text_with_summary

I find it suspicious that \Drupal\Tests\node\Kernel\NodeTokenReplaceTest::testNodeTokenReplacement doesn't need a change. That probably works because it never reloads the node, so the summary is set as an undefined property and the token works.

> those who may have been relying on createBodyField to make a text_with_summary?

createBodyField was introduced in #3489266: Deprecate node_add_body_field(), as a replacement for node_add_body_field(), that's a non-issue, but you can of course ask the same question about node_add_body_field(). But that's not really a problem IMHO. There are going to be very, very few cases that specifically want a summary field. We'd need to deprecate and move this trait again to text_with_summary module, and then those contrib module would need to depend on that.

In all my contribs in my project, I see 7 calls or so to node_add_body_field(), I doubt any of them care about the summary. In contrast to that, there are 200+ calls to FieldStorageConfig::create() where tests set up their own fields explicitly. Which I think is where we want to go.

Same with core really, I don't even see any calls to createBodyField() except the one in createContentType(). Maybe there were and we refactored them away in that issue and shouldn't have introduced that trait and method at all? Core has 400+ FieldStorageConfig::create() calls where it sets up specific fields with specific configuration. We keep the concept of a "body" field being a default thing of a node type in tests while removing this on actual sites, that should IMHO be an intermediate step just to deal with the amount of tests relying on it for one reason or another.

Instead, what I think we should do is introduce an API version of \Drupal\Tests\field_ui\Traits\FieldUiTestTrait::fieldUIAddNewField(). That uses the UI, but most cases don't actually want to test the UI with this, they just want a field with a given name, type and configuration for their entity type.

Re: #12

So when we make the deprecated module text_with_summary

Just trying to get my head around the strategies described in this issue. Are you talking about creating a module named 'text_with_summary'? I suppose you mean deprecating the field type 'text_with_summary'?

Added my thoughts to the MR. Setting NW because I think the copied tests should be trimmed down in this issue.

#13 - please see the parent issue / meta, the approach can be discussed there. TL;DR a new text_with_summary module is created for the text_with_summary field type, the module is then moved to contrib and deprecated in core. Then we remove it from core in the next major. This allows sites that rely on this field type to keep using it if they want to.

@mstrelan Thank you! Appreciated.

I think this is ready, I had some concerns about the file usage test and token changes, but the current test results were replicated to the new tests and align with the discussions here and in slack.

I wonder about the utility of the file usage test as it stands now, but I can see why resolving that would be out of scope here and the current test will prevent any regressions beyond. We probably need a follow up to untangle the file usage and token tests once this is all settled. I'll ask for them to be created.

Edited to add, I'm not sure this warrants a CR on it's own, but maybe a note on one related to these changes?

Adding some thoughts, I think we should keep the new tests in the module that they're testing the code for.

See comments. both of those test code in those modules, not in text module. the job isn't done by moving the tests, we need to deal with the actual logic of the summary token and the ->summary property in editor module.

Left one more note.

Thanks for sticking with me for those nitpicks. My specific feedback has been addressed, so I'm putting this back to RTBC.

I still have some vague concerns around this and the whole change, not nothing specific to hold this up. This will break some tests, but there isn't really a way around that on our path toward stop using text_with_summary in core.

I still have some vague concerns around this and the whole change, not nothing specific to hold this up.

Yeah it's been a bit like this with every issue, had no idea text_with_summary was so deeply embedded everywhere in core until we properly tried to remove it.

MR looks good to me - agree some of those tests will eventually have to move to the text_with_summary module but that can happen later on.

Committed/pushed to 11.x, thanks!