Allow fieldgroups to be nested

I agree with you completely and made a few changes to the module to achieve nested functionality sufficiently for my project. It does not display them nested on the admin page nor does it work past one level deep at the moment.

Does this patch display them nested on content pages? I think this is great functionality, but is pointless until fieldgroups in CCK are displayed by weight and group in content anyways.

Great! Thanks!

The patch didn't patch correctly. But, I did it by hand and it works as advertised.

I did notice that if your sub-group has a sort order less than the parent group - then it does not appear when creating a node.

I don't think that this is an often needed case and it doesn't seem there is huge demand for it,
so I'll won't write that.

However, if anyone does a complete patch I would be open for this functionality.

I think this is hugely important. It allows the easy organization of information which is the main reason I choose Drupal for this project.
http://drupal.org/node/85679 I'm sure the original poster thinks its important. The number of fields you can manage in CCK without proper grouping and subgrouping is limited.
For Instance, if I was doing a product catalogue page.
Specifications (group)
--Dimensions (Subgroup)
--Power requirements(subgroup)
--Exterior Features (subgroup)
Downloads(group)

etc...

Each subgroup could have 5-15 fields making it very hard to manage if you just said Specifications and listed 75 fields, especially with the limited weight settings.

I agree that having nested fieldgroups would be nice.
Unfortunately, the patch in #1 is not satisfying. It is nice and small because it is only semi-functional, but won't allow for more than one level of nesting.
Having this done right, with multi-level nesting, _and_ ensuring a correct display on the field overview tab, will take more serious refactoring, I'm afraid.

Probably requires a new 'parent' column in {node_group} table (meaning also possibly a numerical 'group id' ?), and some code to recursively walk the groups tree.
Plus the UI part (field overview...), which will not be simple (but may come later ?)

Anyone challenged ?

worth following

This patch breaks stuff. Not worth trying unless you want to join in. Essentially, it adds a `gid`, and `pid` field to the {content_group} table and saves that information when you edit a group.

I also swapped some " to ' where the extra parsing of a "" wasn't needed. Don't know if that's appropriate for this patch or not.

Saves the nesting information to {menu_links}. Doesn't pull it back out for proper display.

Looks realy promising, thanks for this deekayen !
I only could give the patch a cursory look for now.

One thing I totally don't get is how {menu_links} and {menu_name} tables are related to this ?

I don't remember using {menu_name}. Much of it is in the same design as the book module, which uses {menu_links} to store the nesting of book nodes and grouping of which node is in what book (or in this case grouping fieldgroups so they're separated by content type). The menu system already has the mechanisms for automagically linking parent information, whether a child exists, an order of where exactly you are in relation to the top/root, and APIs to pull it back out. The fieldgroup information is still in the {content_group} table, it's just the nesting info stored in {menu_links}.

It's my third design attempt, previously making parent/child columns in {content_group} and going with the thread column design of the comments module - neither of which had an efficient way to query the data back out when taking in to the equation weighting. Now would be a good time to tell me if this implementation is a dead-end for being committed, which means I'll be happy to elaborate on any part of the implementation that's not clear, because maybe there's something I'm not thinking ahead about.

Dunno, really. My 1st move would've been "making parent/child columns in {content_group}" as you mention, of course. If you say it's hardly managable, I probably can believe you... I guess I would never have thought of hacking into menu_links. Smart :-)
I'd probably need chx or pwolanin confirming that this approach is safe (for core and for us in contrib...).

PS : the end of the patch in #9 does refer to menu_name (missing {}, btw)

menu_name is a field in the {menu_links} table, not a table itself. Like I said book.module uses {menu_links}, but I'll ask around later. This isn't an insignificant change, so I'll feel better if we have more eyes looking in on this as it goes.

menu_name : doh - sorry, it was late and i was tired...

Well, chx is perfectly fine with using {menu_links} as a free-for-all generic tree storage (pwolanin is more surprised, but basically why not)
Here's an extract of the converstaion, that might be of interest to you :

[chx] it's totally okay to use the menu table as a generic tree storage
[chx] no need to reinvent the wheel.
[chx] though, if you try to use the built in menu api
[chx] you will get access false all over
[chx] however, if you register say cck/fieldgroups and set access to true
[chx] and set the router_path to that...
[pwolanin] chx: type == 'external' ?
[chx] pwolanin: can you force-set that in the $item ?
[pwolanin] chx: I'd have to look at menu_link_save, but I think you can trigger it by having a : in the link path
(edit : nicknames got eaten)

Additionnally, I gave the code a slightly closer look and was wondering :
- shouldn't the group 'weight' be stored in {menu_links} as well ?
- Couldn't the menu_name simply be set to the content type name ? (maybe with a prefix to reduce risks of clashing ?)

While this will work, it sounds like you risk having CCK break if/when there are any changes to the menu API. Also, we may alter your data in ways you don't expect for any D6->D7 updates.

Look at the D5 book module (or menu system), there is lots of code there dealing with a tree hierarchy maintained via a single parent column. This works fine for small hierarchies, and is generally a more standard and simple methodology.

You're only going to have 10's of fieldsets, right? And you're always going to want the full tree in memory I think since you'll want to have all the fields potentially be in the form. So, the more I think about it, the les sense this approach makes. There are a number of design decisions for the menu API which are due to the fact that we want to scale to 1000's of links in a menu where only 10's may be displayed.

That being said, if you want to force the menu code to think of each of these as an "external" link, use a colon in the link_path 'link_path' => 'field:admin/content/types/'. $group['type_name'] .'/groups/'. $group['group_name'] .'/edit',

or even http:// or whatever as the first part. You can easily strip off the first part when you want to use it.

#15: I didn't even notice weight was also in {menu_links} for whatever reason, but sure, I could/should move it there.

No, I don't think the menu_name should be just the content type. Since {menu_links} is shared with other modules as part of core, I think it's proper to follow the Drupal-ism of prefixing the data with the module name. Even I think that argument is weak since there is also a module column to store that information.

I have another reason - each menu_name represents the top of another group, in this case the equivalent of the first fieldgroup in the tree. To store all the fieldsets with a content type menu_name, you'd have to have an empty record to act as the holder for all the child elements. In other words, it's my understanding that for you to just simply have two un-nested groups in a node type, they each need their own menu_name to distinguish that one is not nested under the other. Albeit, I haven't tried giving them the same content type as the menu_name and just saying a plid of 0 to prevent them from nesting - my understanding of the menus isn't that deep yet, but if someone said that would work, I'd certainly give it a whirl since the menu_name I gave it in #9 isn't all that meaningful.

#16: I'm not sure I'm understanding the penalties with stuff breaking in the future. I've had to make some sort of major change to every module I've maintained between Drupal 4.6 and 6.0 to make them work again. If menus changed again, that seems like a normal expectation. If your big picture idea of CCK is to make it cross-CMS like CiviCRM, then I could see making the tables more broken out from core magic.

I don't want to force anything, I want to do it right. I just thought since a storage system exists, a scalable one, why not use it. I thought a better argument against using the menu system would be what would happen if you wanted fieldsets nested 20 levels deep (since {menu_links} only goes to p9). I just assumed since menus can't possibly survive forever with only p9, both will evolve. Thinking we might only have 10s, repeats the same thinking of when menus thought it would only need 10s, but then it evolved. My argument is really that going back to the D5 book way seems like going backwards.

The colon sounds like a good tip. I'll add that to the next patch.

Well, anyone tryng to make 9-deep nested fieldsets needs a usability spanking...

Again, the methods used by the D5 book module were very reasonable. The suggestion with the colon is a hack (though it will work) - again, I think you're better off just coding this up with an additional table.

Subscribing

I'm obviously interested in this, but my short term efforts are being redirected.

Nothing new or different, just refreshing the patch since some other stuff got committed to fieldgroups.

subscribing

One thing I haven't learned yet is how to keep all the menu_links entries from disappearing when menu_rebuild() is called. I'm reading through book.module to try to find its method, but I'm making this issue one of my tasks for today.

I tried to install the patch by using patch -p0 < cck-diff-2008-02-11-14-42-35.patch in the module directory and I received:

patching file fieldgroup.install
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 51.
2 out of 2 hunks FAILED -- saving rejects to file fieldgroup.install.rej
patching file fieldgroup.module
Hunk #1 FAILED at 89.
Hunk #2 FAILED at 106.
Hunk #3 succeeded at 89 with fuzz 1 (offset -35 lines).
Hunk #4 FAILED at 141.
Hunk #5 FAILED at 173.
Hunk #6 FAILED at 245.
Hunk #7 FAILED at 289.
Hunk #8 FAILED at 329.
Hunk #9 succeeded at 513 (offset -43 lines).
Hunk #10 FAILED at 564.
Hunk #11 succeeded at 651 with fuzz 2 (offset -39 lines).
8 out of 11 hunks FAILED -- saving rejects to file fieldgroup.module.rej

What did I do wrong? Help please!

That means fieldgroup files were probably edited since I made the patch or you're not patching against HEAD version of CCK. It looks like the files haven't been updated since I made the patch, so I'm guessing the later.

I said before, but I'll say again, the current status of this patch will horribly break the current implementation of groups, so by installing it you're essentially volunteering to help finish the implementation, which I welcome.

The other day I added the colons to links and moved the weight to the menu table. It's not a big change from before, but if you're going to help, it's at least one more thing out of the way.

Thanks, I am going to have to wait. I am using 5.x version of Drupal. Thanks

This is just my vote that this is a desire feature. I wish my coder chops were up to the task :-(

Subscribing. This is exactly what I need for my current project, but since it's a paying gig for a doctor's office I can't use anything that's hacked so I'll have to wait until this becomes official.

Thanks,
David
http://www.FloridaPets.org