If one uses fieldgroup it's hard to identify, where a field sits in $form, in particular if one shares one field across multiple content types but moves this field in different groups.
To ease customizing a field through form_alter() I propose increasing fieldgroup's module weight, so that fieldgroup's form_alter will add its groups later. So one can use form_alter and rely on $form[$field_name].

Patch attached.

CommentFileSizeAuthor
#7 fieldgroup_updates.patch1.86 KBfago
fieldgroup_weight.patch834 bytesfago

Comments

yched’s picture

yched commented

Seems reasonable, I'm all for it.
Two points, though :
- Why 9 ? :-)
- This will break existing form_alters expecting to find the elements at the current $form['group_foo']['field_bar']
Most likely, this will be found only in custom tweak modules and not in contribs, but this breaks the site all the same...
Not sure if this should stop us, just wondering.

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Fixed

@Why 9 ? :-)
Perhaps modules want to be the latest module and just set 10 :D

@This will break existing form_alters expecting to find the elements at the current $form['group_foo']['field_bar']
I also think that shouldn't stop us. Custom tweaks are breaking quick and fixing it to work again is really simple.. -> just use $form['field_name'] in every case :)

-> committed to 5.x :)
I've not committed it to 4.7.x as fieldgroup has no free update number for 4.7.x :/.
Should I commit it to HEAD too?

yched’s picture

yched commented

I think Views wants to be last and uses something like 99 or 100.
Well, there is no absolute good number I guess, 9 should be fine :-)

Custom tweaks are breaking quick and fixing it to work again is really simple
OK, agreed.

Should I commit it to HEAD too?
Yes, please. Beware that HEAD now targets D6 (well, D6 as it was 2-3 weeks ago, so It's probably broken anyway...).
Probably does not have any consequences for this patch anyway.

I've not committed it to 4.7.x as fieldgroup has no free update number for 4.7.x :/.
Ah, true. That is not good. I guess we should 'make' some space by moving the 5 branch fieldgroup updates to the 1000 series as we did for content.install. And find a way to update the installed schema versions as well.
Crap, I kind of dread this area. Gave us lots of headaches earlier on...

fago’s picture

fago
Primary language German
Location Vienna
commented

committed to HEAD to.

@views: It uses 10 ;)
@4.7: Yep, that's bad. I'll see if I can find a way...

fago’s picture

fago
Primary language German
Location Vienna
commented

damn, that's really hard to do... :/

yched’s picture

yched commented
Status: Fixed » Patch (to be ported)

I can't really remember how we managed the transition in the D5 branch of content.install.
I don't think we went to 1000 straight ahead. But the transition happened during the 'beta - pre 1.0 release' phase, so maybe we did not really bother with the installed base.

Not much time just right now, I'll try do give it a look in the next few days if you want.
Reopening to 'to be ported' as a reminder.

fago’s picture

fago
Primary language German
Location Vienna
commented
StatusFileSize
new fieldgroup_updates.patch1.86 KB

that would be great!

I've attached what I've done so far. Fieldgroup 4.7.x updates should start with 10...
But I think it totally fails because the update mechanism updates the schema version of a module after each update, so I haven't even tested this code. Perhaps this could be worked around by storing the old schema version in the session.
So perhaps it could be of some help for you, so it's attached.

Note this code is totally untested.

yched’s picture

yched commented

Actually, aside from custom _form_alter's in custom 'tweak' modules, I think we just broke 'CCK field permissions'.
Nothing that can't be fixed, obviously (and the module is already broken currently ;-) ), but i'm worried that we might step into a world of 'contrib_field 1.4 requires CCK 1.5 which works best with Views 1.6 etc...'.

Why not, but I'd like to make sure we're OK with it...

yched’s picture

yched commented
Version: 5.x-1.x-dev » 4.7.x-1.x-dev
Status: Patch (to be ported) » Closed (fixed)

4.7 won't see this now, I guess.