Problem/Motivation

In core/modules/node/src/NodeForm.php
$form['status']['#group'] = 'footer';
That has been introduced in: #2068063: Change "Save and keep un-/published" buttons to a "Published" checkbox and an included "Save" button
is setting #group for the status field regardless whether the field is exist or not.

Steps to reproduce

- Go to manage form display for a content type.
- Move status field to "Disabled" section,
- Debug/Dump the form.
You see:

  "status" => array:1 [▼
    "#group" => "footer"
  ]

Proposed resolution

Set #group for status field if it is exist.

Remaining tasks

Add MR

User interface changes

No

API changes

No

Data model changes

No

CommentFileSizeAuthor
#2 drupal_3450792_2.patch580 bytesmsnassar

Issue fork drupal-3450792

Command icon 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

msnassar created an issue. See original summary.

msnassar’s picture

StatusFileSize
new580 bytes
msnassar’s picture

Assigned: msnassar » Unassigned
Status: Active » Needs review

msnassar’s picture

Issue summary: View changes
smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Issue summary appears to be missing some sections if those could be filled in please. Specifically proposed solution.

MR should be against 11.x not 11.0.x also

msnassar’s picture

Version: 11.0.x-dev » 11.x-dev
msnassar’s picture

Issue summary: View changes
msnassar’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update +Needs Review Queue Initiative

Thanks

Change seems small enough I don't think it needs a test under the new policy.

quietone’s picture

Title: Do not set #group for not existing status field » Only set #group is status field exists in NodeForm
Status: Reviewed & tested by the community » Needs work

@msnassar, thanks for making the MR.

Nice to see an issue referring to the updated section of the Testing core gate. But this should still have an explanation as to why tests are not needed. I know that the size of the change is not in the heuristics. So, what is there that supports no tests?

acbramley’s picture

I'm also curious, does this actually have any end user effect?

acbramley’s picture

Status: Needs work » Needs review

I would say this falls into this: "usually only for trivial bugs where testing would be convoluted" from that section.

To test this we would need to setup a content type with the field disabled, build the form, and check the array key doesn't exist. I don't think it's worth the hassle or overhead.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @acbramley!

longwave’s picture

Version: 11.x-dev » 10.6.x-dev
Status: Reviewed & tested by the community » Fixed

Backported down to 10.6.x as an eligible bug fix.

Committed and pushed f2ee3e47285 to main and 6d2e5f4b4c1 to 11.x and 8a115bd5759 to 11.3.x and dcc6b0e7b34 to 10.6.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • longwave committed dcc6b0e7 on 10.6.x
    fix: #3450792 Only set #group is status field exists in NodeForm
    
    By:...

  • longwave committed 8a115bd5 on 11.3.x
    fix: #3450792 Only set #group is status field exists in NodeForm
    
    By:...

  • longwave committed 6d2e5f4b on 11.x
    fix: #3450792 Only set #group is status field exists in NodeForm
    
    By:...

  • longwave committed f2ee3e47 on main
    fix: #3450792 Only set #group is status field exists in NodeForm
    
    By:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.