While the fieldgroup.module is awesome - it would be awesomer if groups could be placed inside other groups.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sarvab’s picture

Status: Active » Needs work
FileSize
2.91 KB

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.

littleprince’s picture

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.

jpsalter’s picture

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.

fago’s picture

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.

littleprince’s picture

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.

yched’s picture

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 ?

nathanraft’s picture

worth following

deekayen’s picture

Version: 4.7.x-1.x-dev » 6.x-1.x-dev
Assigned: Unassigned » deekayen
FileSize
15.06 KB

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.

deekayen’s picture

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

yched’s picture

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 ?

deekayen’s picture

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.

yched’s picture

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)

deekayen’s picture

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.

yched’s picture

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

yched’s picture

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 ?)

pwolanin’s picture

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.

deekayen’s picture

#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.

pwolanin’s picture

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.

yhager’s picture

Subscribing

deekayen’s picture

Assigned: deekayen » Unassigned

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

deekayen’s picture

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

robertgarrigos’s picture

subscribing

deekayen’s picture

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.

oscarp’s picture

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!

deekayen’s picture

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.

oscarp’s picture

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

okeedoak’s picture

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

tknospdr’s picture

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

antipix’s picture

patching gives me

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 255.
Hunk #8 FAILED at 290.
Hunk #9 FAILED at 330.
Hunk #10 FAILED at 411.
Hunk #11 succeeded at 514 (offset -43 lines).
Hunk #12 FAILED at 565.
Hunk #13 succeeded at 653 with fuzz 2 (offset -39 lines).
10 out of 13 hunks FAILED -- saving rejects to file fieldgroup.module.rej

and I see an empty "parent group" select box on the new group form...

This functionnality would be great, subscribing ^^

MGParisi’s picture

I have this working on my one site, but I cant seem to get it working on this new site. What is up with that!

Shane Birley’s picture

Subscribing.

tecto’s picture

subscribing

ifoundthetao’s picture

What is the status on this? I'm down for helping out where I can help out. I haven't done much programming with Drupal specifically, but I will do what I can to get the ball rolling a bit more with this.

deekayen’s picture

I still want to see it happen, but I work on it on and off. Right now I'm doing some things with workflow_fields, db_maintenance, and user_readonly. If someone wants to help get me back working on this bug, test, debug, and patch user_readonly 6.x-dev so it'll knock one thing off my list of things in the way of working on this.

dopry’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev
Moonshine’s picture

I'd love to see this functionality added. At first glace it looks as though it would also allow for fieldgroups to be nested under CCK Fieldgroup Tab sets, which would be priceless for me.

It doesn't look like there was a final consensus on how the hierarchy should be stored yet? Would that need to be cleared up first, or would a fully functional patch using {menu_links} be acceptable?

yched’s picture

#300084: Nested fieldgroup has a different, more 'orthodox' take on this. Testers welcome.
If we can go without plugging into core menu tables, I'd prefer that.

deekayen’s picture

Status: Needs work » Closed (duplicate)

I'm willing to defer to #37.

stella’s picture

FileSize
5.52 KB

I realise the patch in comment #1 is unlikely to be used, but also realise this feature may not be backported to Drupal 5, so here's a re-roll of the first patch for Drupal 5 in case anyone needs a partial solution.

Cheers,
Stella

stella’s picture

FileSize
6.47 KB
stella’s picture

FileSize
6.88 KB

updated patch for managing visible / invisible fieldgroups, both parents and children.

MGParisi’s picture

Great thread, #42 posts (with patches) and the feature seems to be ignored (probably because its marked as "Duplicate").

Too bad all this work is going unknown.

Also tags seem to be funky in IE. Can the patch include a CSS change that will fix this (maybe include it in Drupal 7 core). I personally would love to know why my image and text placed within a fieldgroup is not contained within the box when displayed with IE. (any answers)

markus_petrux’s picture

Version: 6.x-2.x-dev » 5.x-1.x-dev

As per stella's comment in #39, the latest patches are for Drupal 5, so changing the version associated to the issue.

However, I don't think this will be committed to D5, which is feature frozen, and also no upgrade path exists to anything working in D6.

The issue to follow for D6 is: #300084: Nested fieldgroup.

MGParisi’s picture

So unfortunately, the D5 is frozen. So if we want to include upgrades...

  1. A new maintainer for d5 has to step up...
  2. ychad refuses a D5 upgrade and a fork would need to be made.
  3. We dont add a patch

Personally I too am working on porting to D6, but there are still some bugs in D6 modules that prevent me from doing so. I am stuck (as with allot of people) with D5. Ychad is very busy with D7 (I talked to him on IRC) and it is understandable why D5 is not being upgraded by him. The next step is up to the individual who is willing to take on one of the above (very difficult) options.

Can we at least change the status from Duplicated to wont fix.

MGParisi’s picture

Maybe me and Stella can create a second module "fieldgroup" for D5, that simply contains files to replace in CCK's version 5.

I would hate to see these patches go unused and completely hidden (since duplicate and wont fix gets hidden).

KarenS’s picture

Status: Closed (duplicate) » Closed (won't fix)

There is nothing to prevent anyone from creating separate D5-only module from these files, so if someone wants to do so they are welcome to do it. If you do, let us know and we can mark this 'fixed' and link to that project from this issue. You would create your own version of Fieldgroup, with whatever changes you want, and tell people to disable the original module and enable yours instead. I have no idea what the upgrade path would be from that to the D6 version, so your module would have to provide that too.

KarenS’s picture

I should say, your module would have to provide a D6 version that consists only of an install file that performs any necessary upgrades so that people can use the regular version of fieldgroup in D6.

markus_petrux’s picture

@MGParisi: I would also like to suggest to spend the time on helping fix the bugs that prevent you from step up to D6. At this point in time, the benefits of spending time developing for D5 are very small, IMHO. Think about the time you may need to fork the fieldgroup, provide an upgrade path to whatever is done for D6 and up, support and maintain it, and so on, compared to the time you will need to spend to upgrade sooner or later.

yched’s picture

+1 on what both Karen and Markus said.

Stomper’s picture

I really would appreciate the ability to nest fieldgroups, is this now supported in the D6 version? I am running CCK 6x.2.9 and it doesn't seem to allow me at least using the GUI drag and drop interface.

-Mania-’s picture

It doesn't look like this has been implemented in Drupal 6. :(