I installed field_group module, and saw the ability to create fieldgroups on my "user" entity, but not on any of the entities I had created with ECK. I thought it must be a conflict between field_group and eck and so I went bughunting. It took me some time to find line 51 of field_group.field_ui.inc which says if (empty($form['#fields']) && empty($form['#extra'])) { and I realised the truth. I made a field, and suddenly had the ability to make field groups. No eck conflict after all.

If this is intended functionality, I hope we can make it much more obvious so nobody else wastes time like I just did. Can we just output a bit of text like "to create fieldgroups, start by adding a field".

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mark Trapp’s picture

Title: Make it clear that fieldgroups will be available once fields have been created » Existing field groups disappear and adding field groups is disabled unnecessarily when no fields are available
Version: 7.x-2.x-dev » 7.x-1.x-dev
Component: Documentation » Code
Category: task » bug
Status: Active » Needs review
FileSize
1.02 KB

Because of the empty($form['#extra']) part of the conditional, you won't notice you're Doing It Wrong™ if you have a module enabled that adds extra fields (e.g., Redirect): field_group will happily let you create field groups all you want before adding "actual" fields. Having been doing this for a few months now, I haven't seen a single side effect from the fact that field groups are created before fields.

Additionally, after spending a couple of hours debugging the Field API after a simple unset in a hook_field_extra_fields_alter() implementation, there's a major WTF moment if you happen to disable the module that's providing that extra field: when you visit the Manage Fields page again, it'll appear as if there was data loss, because all of the existing field groups disappear.

However, there isn't: re-enable the module that was providing the extra field and everything shows up again.

So the fact that it's checking for existing fields or extra fields is more of a bug than anything: there doesn't seem to be a reason at all to stop people from creating field groups before fields.

Attached is a patch that removes this check.

Stalski’s picture

This was actually by design. The check on extra was not there at first, however we got notices.
So that's when we started doing the same thing as fields do: only add "add new field/fieldgroup" when there is something to work with. As fieldgroups group (extra)fields, this makes sense ofcourse.

However there actually should not be a restriction to add fieldgroups. That said, note that fieldgroups are ment to group fields and only work in that case. So there is no reason to add a fieldgroup as a standalone data object.

So the way I see it, I would go for the text as suggested in the report rather than removing the check and thus doing altering while field is NOT doing any altering yet!

Opinions?

Mark Trapp’s picture

Title: Existing field groups disappear and adding field groups is disabled unnecessarily when no fields are available » Existing field groups disappear and Add Field Group UI is disabled when no fields are available
FileSize
1.34 KB

From a workflow perspective, it's been easier to create the field groups first, which defines the skeleton of the entity type, then add the fields afterwards. In other words, field groups effectively act as an outline for the content of the page.

But discounting that benefit, the current check is lacking: it lets field groups be added if there are any fields or extra fields, but they can go away at any time. If they do, the field group interface gets disabled entirely and all existing field groups are hidden. Re-enable any module that implements hook_field_extra_fields or add another field, and everything shows up again.

This is a weird in and of itself, but if a user unwittingly clicks "Save" on the field overview form during field group's hidden state, there's data loss as the form gets submitted without the field group data that does exist.

So if it's essential that this check still exists in some form, what about:

  1. If there are already field groups present, don't restrict the UI: show all existing field groups as normal, and show the "add field group UI".
  2. Add the notice per the OP when there are no fields or field groups present.

Attached is a patch that does this.

Mark Trapp’s picture

I was going through my local forks of contrib modules, checking to see if anything needs a reroll, and this patch still applies with latest HEAD. Is there anything else that's preventing this patch from being committed?

nils.destoop’s picture

Status: Needs review » Fixed

I committed following solution:
- on the form settings, create group is always shown, even when there are no fields or extra fields
- on the display settings, the return should stay. Because field ui does no theming on the form then. I don't show a message, because field ui already says you have to start by adding fields. People would see 2 times the same message, if field group also said that.

Mark Trapp’s picture

Works great: thanks, I appreciate it!

Status: Fixed » Closed (fixed)

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

  • Commit 7e45f10 on 7.x-1.x, 8.x-1.x authored by Mark Trapp, committed by zuuperman:
    Issue #1557258 by Mark Trapp | Chris Gillis: Fixed Existing field groups...