Deprecate node_add_body_field()

Created on 22 November 2024, 6 months ago

Problem/Motivation

After 📌 Remove node_add_body_field from NodeTypeForm Active node_add_body_field() is only called in tests. Let's remove all the calls and deprecate it.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

node system

Created by

🇬🇧United Kingdom catch

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

Merge Requests

Comments & Activities

  • Issue created by @catch
  • 🇨🇭Switzerland berdir Switzerland

    Would we also deprecate/remove the body field config itself then ((or more likely, move to standard/recipes)) or keep it for manually adding the body field?

  • 🇬🇧United Kingdom catch

    @berdir we're trying to change it to text_long in 📌 Change automatic body field creation to use formatted text field instead of text_with_summary Active (which turns out to be hard) but ultimately I think this is something that recipes should handle - shipping a content type (that needs a body field) is recipe-ish already, and recipes can also handle the create-if-not-exists logic that removing the default config would require.

  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    Total: 105s
    #423071
  • Pipeline finished with Success
    3 months ago
    Total: 429s
    #423089
  • First commit to issue fork.
  • Pipeline finished with Failed
    about 1 month ago
    Total: 261s
    #463001
  • Pipeline finished with Failed
    about 1 month ago
    #463005
  • 🇦🇺Australia acbramley

    Rebased and tidied up a lot of duplication in tests by using ContentTypeCreationTrait, also added a CR.

    There were 2 spots in runtime code that are still using this function:
    - NodeTypeForm - only using for the testing profile, this should definitely just be removed. We don't want test code in runtime code.
    - The EntityNodeType migration destination plugin - I've replaced this with the contents of node_add_body_field

    So the outcome here is a little bit more duplication (between the test trait and migration plugin) but I think that's ok.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 527s
    #463008
  • 🇦🇺Australia acbramley

    Looks like only 6 tests relied on the NodeTypeForm bits https://git.drupalcode.org/issue/drupal-3489266/-/pipelines/463009/test_...

    Other failures in that pipeline are random afaict

  • Pipeline finished with Failed
    about 1 month ago
    Total: 122s
    #463018
  • Pipeline finished with Failed
    about 1 month ago
    Total: 778s
    #463022
  • 🇦🇺Australia acbramley

    Last failure is in Nightwatch and for once doesn't look like a random fail, I'm assuming we need to add a step to it to create the body field

    https://git.drupalcode.org/issue/drupal-3489266/-/jobs/4842498

  • 🇺🇸United States smustgrave

    Whatever we decide here we should do the same for block_content as pretty sure we got copy and paste code with the same stuff.

    Maybe we need some default config in the testing profile for nightwatch? Or is there a utility call to add a field?

  • 🇦🇺Australia acbramley

    Postponing on converting those tests to WebDriver tests.

  • 🇫🇷France spooky063

    #13 📌 Deprecate node_add_body_field() Active So you've decided to drop the Nightwatch test?

  • 🇦🇺Australia acbramley

    @spooky063 that's right, it's probably the most straight forward and best overall solution IMO. Nightwatch tests are already on the chopping block, and since it's only 2 tests that can both be more easily tested in PHP (WebDriver) tests it makes sense to move those anyway.

    Otherwise we need to either
    a) copy the same field creation code into NodeTypeForm again, we don't want to tie that form to the testing profile anyway so this wouldn't do, or
    b) add more Nightwatch steps to the 2 tests, further extending the already lengthy test case. That would then need to be ported again when Nightwatch is eventually replaced.

    I looked at adding default config to the testing profile but the profile is specifically very barebones so that didn't feel right either.

    One of the issues is already up for review with more detail so feel free to review if you can :)

  • 🇦🇺Australia acbramley

    Whatever we decide here we should do the same for block_content as pretty sure we got copy and paste code with the same stuff.

    We should probably also make the body field creation reusable in that case and use it in the NodeTypeCreationTrait

Production build 0.71.5 2024