LeftRightMinds has sponsored this patch.
It permits to nest fieldgroups defining a hierarchy.

CommentFileSizeAuthor
#360 cck-6.x-3.x-fieldgroup_unserialize-2.patch1.34 KBthekevinday
#355 cck-6.x-3.x-nestedfields_formstate-1.patch3.01 KBthekevinday
#347 cck_options-proof_of_concept_version.tgz7.99 KBthekevinday
#344 cck-6.x-3.x-nested_fieldgroups-4.patch143.73 KBthekevinday
#336 cck-6.x-3.x-fieldgroup_unserialize-1.patch1.15 KBthekevinday
#330 cck-6.x-3.x-fieldgroup_typo-1.patch726 bytesthekevinday
#329 cck-6.x-3.x-nested_fieldgroups_shared_groups_fix-1.patch1.9 KBthekevinday
#326 cck-6.x-3.x-nested_fieldgroups-3.patch58.3 KBthekevinday
#326 cck-6.x-2.x-nested_fieldgroups-3.patch47.83 KBthekevinday
#324 cck-6.x-3.x-display_subgroup_save_fixes-2.patch2.53 KBthekevinday
#320 cck-6.x-3.x-display_subgroup_save_fixes-1.patch1.79 KBthekevinday
#315 cck-6.x-2.x-nested_fieldgroup_fix_deep_nesting-1.patch1.18 KBthekevinday
#315 cck-6.x-3.x-nested_fieldgroup_fix_deep_nesting-1.patch1.18 KBthekevinday
#314 cck-6.x-x.x-tabledrag_sticky_fix-2.patch1.28 KBthekevinday
#312 cck-6.x-x.x-tabledrag_sticky_fix-1.patch834 bytesthekevinday
#303 cck-6.x-2.x-nested_fieldgroup-2.patch54.79 KBthekevinday
#303 cck-6.x-2.x-add_multigroups-2.patch110.36 KBthekevinday
#303 cck-6.x-2.x-nested_multigroup-2.patch9.7 KBthekevinday
#303 cck-6.x-2.x-workaround_multigroup_bug-2.patch1.88 KBthekevinday
#303 cck-6.x-3.x-nested_fieldgroups-2.patch55.23 KBthekevinday
#303 cck-6.x-3.x-workaround_multigroup_bug-1.patch1.88 KBthekevinday
#295 cck-6.x-2.6-nested-muligroup-all-inclusive.patch196.95 KBayalon
#293 cck-6.x-2.x-nested_fieldgroup-1.patch45.23 KBthekevinday
#293 cck-6.x-2.x-nested_multigroup-1.patch9.69 KBthekevinday
#272 300084-nested_fieldgroup_1.patch46.12 KBckng
#245 sdf.PNG9.92 KBtijeika
#234 stuck-group-1.jpg101.81 KBSc0tt
#234 stuck-group-2.jpg100.36 KBSc0tt
#234 stuck-group-3.jpg100.3 KBSc0tt
#230 cck.patch47.16 KBrconstantine
#230 cck.tar_.gz434.57 KBrconstantine
#227 cck.patch47.22 KBrconstantine
#227 cck.tar_.gz434.57 KBrconstantine
#219 Nested Field Example 4.png57.64 KBTomSherlock
#219 Nested Field Example 3.png19.32 KBTomSherlock
#219 Nested Field Example 1.png15.67 KBTomSherlock
#208 cck-nested-fieldgroup.patch46.05 KBrconstantine
#206 TwoChanges.tar_.gz23.42 KBrconstantine
#203 Right.jpg970.47 KBrconstantine
#203 Wrong.jpg972 KBrconstantine
#190 Create Product Knowledge Entry.png305.8 KBrconstantine
#190 Create Printer - Printer Inventory.png96.17 KBrconstantine
#187 cck.tar_.gz407.98 KBrconstantine
#186 cck-nested-fieldgroup.patch48.25 KBrconstantine
#178 cck-nested-fieldgroup.patch51.02 KBrconstantine
#175 cck.tar_.gz409.16 KBrconstantine
#156 Picture 74.jpeg92.75 KBsquarecandy
#137 multigroup-adding-value.png324.78 KBRoulion
#134 cck.zip364.13 KBrconstantine
#133 cck.zip731.52 KBrconstantine
#116 cck.zip466.52 KBrconstantine
#113 nestedfield-multigroup.png335.13 KBRoulion
#110 cck.patch200.09 KBrconstantine
#109 cck.zip466.5 KBrconstantine
#108 Page.png29.07 KBrconstantine
#99 cck.zip1012.45 KBrconstantine
#97 cck.zip1012.45 KBrconstantine
#85 cck.zip1011.85 KBrconstantine
#80 cck.zip1011.45 KBrconstantine
#75 Test.png37.83 KBrconstantine
#71 cck.patch44.94 KBrconstantine
#66 cck.patch44.79 KBrconstantine
#68 monster content type - incomplete.png599.05 KBrconstantine
#65 cck.patch39.98 KBrconstantine
#62 nested_fieldgroup-300084-62.patch42.16 KBmarkus_petrux
#60 nested_fieldgroup-300084-60.patch41.49 KBmarkus_petrux
#56 content.node_form.inc_.zip4.75 KBrconstantine
#49 cck.patch43.2 KBrconstantine
#48 cck.zip994.11 KBrconstantine
#42 Test application - before.png131.25 KBrconstantine
#42 Test application - after.png118.17 KBrconstantine
#36 Test1.png140.85 KBrconstantine
#35 cck.zip433.84 KBrconstantine
#32 cck.zip444.26 KBrconstantine
#22 fieldgroup.tabledrag.js_.txt1.4 KBmarkus_petrux
#20 content.admin_.inc_.diff4.85 KBrconstantine
#20 fieldgroup.module.diff20.3 KBrconstantine
#17 content.admin_.inc_.diff4.47 KBrconstantine
#17 content_copy.module.diff589 bytesrconstantine
#17 fieldgroup.install.diff20.59 KBrconstantine
#17 fieldgroup.module.diff19.39 KBrconstantine
#17 tabledrag.js_.diff709 bytesrconstantine
#17 theme.inc_.diff771 bytesrconstantine
#11 ScreenHunter_01 Dec. 17 10.13.jpg117.05 KBrconstantine
fieldgroup.module.patch11.79 KBRoberto Gerola
fieldgroup.install.patch1.29 KBRoberto Gerola
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

CHATELIER’s picture

I can't apply patch on the version 5.x-1.7 because the versions of fieldgroup.install and fieldgroup.module in it don't match.

Could you help me.

Thanks in advance.

joshuajabbour’s picture

I'm all for this, and would be glad to test and support when/if a 6.x version is created...

mansspams’s picture

+1

deekayen’s picture

Status: Needs review » Needs work

It looks like you added at least one field to the node_group table (parent_group_name). Would you mind writing a .install patch to show what at least you intended to be the db update? It'd also be nice if it was patched against the recent 5.x-1.10 version.

mitchell’s picture

mitchell’s picture

Version: 5.x-1.7 » 6.x-2.x-dev
mikeytown2’s picture

subscribing

rconstantine’s picture

I'm trying to apply this to 6. We'll see how it goes.

rconstantine’s picture

Pretty much can't use this patch except for DB addition and a couple of other things, but good idea. I'm working on modifying D6 version myself. Should take a couple of days due to trying to understand all of the new stuff in D6 version of CCK and its sub-modules. Wish me luck.

mitchell’s picture

@rconstantine: Good Luck! :)

rconstantine’s picture

For those tracking this, see the screen shot for progress. I used the extra field that Robert added to the database and then based my tree function from his but made it integral to fieldgroup_groups(). Everything else is new. So far, I assigned parents to groups by hand in the database and have figured out the display. Dragging doesn't work yet as the depths get reset to zero.

So my next step is to fix the ordering via the click-and-drag interface. Following that are validation and submission.

I don't think I have broken anything in the process by the way. The click-and-drag still works the old way, just no depth yet. Wish me more luck. I have another project I need to interleave my time with but hope to have this done by the end of the week. If not, it should be done by Christmas for sure.

rconstantine’s picture

I have successfully modified tabledrag.js to allow for nested fieldgroups. I'm not sure whether that will help or screw up other modules. I made it so that if there is no previous row, then the maxIndent is 0. If the previous row is a tabledrag-root, maxIndent is one level more than it. If the previous row is a tabledrag-leaf AND indentation of that row is more than the current row, then the maxIndent is equal to the previous row.

Now I have the validation and submission stuff to work out and also the 'Display fields' tab which I forgot about.

On that last item, I can't figure out where the $vars get set which are sent to the template_preprocess_content_display_overview_form function. Can anyone tell me?

rconstantine’s picture

I have the "Display fields" page working now. Tomorrow (or Friday if I have to shelve this), all I have left is validation/submission. I'm hoping that with javascript turned off on my browser and working out the validation/submission that way will produce the same results as with it on. That's the way I'll be doing it, taking out javascript as a factor at first.

I'm closing in on it!

My goal is to have top level fieldgroups as tabs (via the CCK Fieldgroup Tabs module) with nested ones as regular fieldgroups. Add to that the experimental Content Multigroup module and this addition I'm working on produces a ridiculous amount of form creation options! Hooray for D6 and CCK6 which make it all possible.

sproot’s picture

Really excited about the work you're doing on this, will be very useful to me.
If you'd like me to test it please let me know, I'd be more than happy to help.

rconstantine’s picture

I have two things left, issues I'm having with the 'weight' of rows. For example, given two top level items with the same weight, and a group attached to the first item, the nested stuff is showing up inside the second group. I'm not sure whether I should adjust the stored weight values or adjust once loaded. Trying both methods.

The other thing is that I simply haven't updated how this will all look on the node_form. Should be easy, I think.

rconstantine’s picture

All I have left is the node_form! Hooray! Then I'll need any of you to double check all of this.

My only worry is the change to the JavaScript from core. I don't know if it breaks anything else. I had thought about adding a third option to the leaf/root choice, but that would have touched a lot more functions and the changes would have been greater.

rconstantine’s picture

Done. Please test.

Note, I did use universal diff format, but the first two lines may need adjusting based on your own paths. Originally, the paths had my c drive stuff in it, so I changed them to reflect the head of my Drupal install. So if you're using a command line tool, you'll want to launch from your Drupal root. If you use a graphical tool, you may need to adjust the paths.

These diffs are against the latest (12/9) dev version.

Except for the fieldgroup.module file, the rest could easily be changed by hand.

Let me know about the bugs you find. Bugs should not really be reported on interactions with contrib modules as changes would have to be made on their side most likely.

rconstantine’s picture

By the way, I don't know why uploading the files introduces a bunch of underscores into the filenames.

rconstantine’s picture

Status: Needs work » Needs review

Forgot to change status.

rconstantine’s picture

Updated two files. Group deletion wasn't working before. Now does.

markus_petrux’s picture

The patch does not apply. :(

Could you please provide the patch on a single file? Also, it would help if the patch was relative to the cck module folder. Otherwise, it may not apply on someone else's installation.

In regards to the modification to the Drupal core file tabledrag.js, maybe it could be approached by providing a separate .js file that overrides the method validIndentInterval(), so that this feature does not require patching Drupal core. I'm attaching a .js file that could be loaded just after invoking drupal_add_tabledrag() in cck/theme/theme.inc:

  drupal_add_tabledrag('content-field-overview', 'order', 'sibling', 'field-weight');

  // Override methods in Drupal core tabledrag.js.
  drupal_add_js(drupal_get_path('module', 'fieldgroup') .'/fieldgroup.tabledrag.js');

Also, in fieldgroup_form_alter, there is the following:

if ((arg(0)=='node' && arg(1)=='add') || (arg(0)=='node' && is_numeric(arg(1)) && arg(2)=='edit')) {

This exposes a dependency on the menu path that may not be desired on certain situations. For example, think about drupal_execute(), which is often used to create nodes programatically.

markus_petrux’s picture

Status: Needs review » Needs work
FileSize
1.4 KB

Oops, I forgot to attach the .js file.

rconstantine’s picture

What is your suggestion for #21's form_alter issue? Admittedly, this line came from the original D5 patch and I didn't really think about whether it was necessary, only that it works. I'll give it a look.

Thanks for the js file, that is a better solution indeed.

As for the patches not applying, was it only because of the paths, in which case you can change them by editing the files, or was there some other issue? I did mention which version I patched against and I see that a new version came out today. So I'll need to do a new one.

And I'm sorry it's all broken into separate files. I will double check, but I don't think the tool I'm using allows me to create a patch for an entire directory. Sorry.

rconstantine’s picture

It looks like my original test data that I used made things look a little better than they are. Within a top level fieldgroup, sub groups aren't maintaining their weight/positioning, so I'm working on fixing that. I think what I have in mind will work.

I have updated my working copy to the latest dev (12/28) and next time I make a patch, I'll include both diffs and full files in case my diffs suck like last time.

I also undid my changes to the js file and used markus' js file and inclusion code. Thanks Markus!

markus_petrux’s picture

Glad you liked it. Hope it also worked, 'cause I didn't tried it. :o)

For the patches... if you cannot generate a single file, maybe you could simply concatenate all of them, and then just attach a single with all patches. It should work as long as patches are in unified format, use a path relative to the cck module directory and use unix line endings. http://drupal.org/patch/create

As per the stuff in form_alter... maybe those checks for arg() is not really needed?

I'll try and see how it works with the multigroup module. :-D

rconstantine’s picture

Crap! I can manually enter the 'right' weight info in the DB and get it to all come out nice on the admin page, the display settings page and the node creation form, but I'm really struggling with saving the weights properly from the admin screen. I also haven't really looked at how things come out when a node is viewed. I think perhaps I need to take a second look at the JS stuff. I think it was fine with JavaScript disabled, so I'll be looking to understand how tabledrag.js determines weights when you move rows since it doesn't seem to follow the convention I'm using manually.

In other words, this isn't working in any usable way yet. Sorry folks. This is priority #1 at work, so expect an update soon.

markus_petrux’s picture

@rconstantine: I've been playing a bit with this idea and it seems it is not really necessary to change anything in the tabledrag javascript. See how the menu administration already supports several levels of parentships.

To achive a similar thing for fieldgroups:

1) Get a fresh copy of CCK 2.x from CVS, or the latest nightly snapshot.
2) Apply the following changes in CCK sources:

2.1) File affected: includess/content.admin.inc

       'hidden_name' => array('#type' => 'hidden', '#default_value' => $group['group_name']),
-      '#root' => TRUE,
       '#row_type' => 'group',

2.2) File affected: theme/theme.inc

-  drupal_add_tabledrag('content-field-overview', 'match', 'parent', 'group-parent', 'group-parent', 'field-name', TRUE, 1);
+  drupal_add_tabledrag('content-field-overview', 'match', 'parent', 'group-parent', 'group-parent', 'field-name', TRUE, 10);

Now, the drag-n-drop interface allows you to move fieldgroups into fieldgroups. So that we only need to deal with weights and parentships properly.

The problem with multigroups is that they cannot contain standard fielgroups, so we need to figure out a method to not allow this particular case. One thing that would have to be added to the group settings is the ability to be parent of other fieldgroups. The standard fieldgroup would have this flag enabled, but the multigroup would have this disabled. Then see how to implement this restrictions to the UI.

markus_petrux’s picture

Oh, one minor suggestion for the DB schema changes. Maybe you could use 'parent' (rather than 'parent_group_name') for the new field in the {content_group} table? This way we don't get indentation differences in fieldgroup.install.

rconstantine’s picture

Yeah, I was noticing the #root thing myself just yesterday. Hadn't made the change yet. Doing so after I finish this message. I think I understand what you're talking about in regards to the multi-group issue and depending on how things go in the next several hours, I may have an idea or two for that.

I ended up changing and simplifying a bit of what I had posted before. Later today, I'll post the new stuff. I think I mentioned before that the weight changes tabledrag was making were not what I wanted. I'm adjusting things in the theme function to make sure it behaves as I expect. This will allow for a simple submit function.

Stay tuned. And thanks for the comments. I have yet to try out your patch, but need to once I'm 'done' with this.

ClayRobeson’s picture

This keeps looking better. Thanks for working on it, gang.

rconstantine’s picture

OK, I'm so close that I'm getting overly excited about finishing up. I have produced what seems to be a pretty good override of the onDrop JavaScript function which takes care of manipulating some class assignments and weight assignments in a way in which I was hoping to get them for storage. In short, each group has its own weights. So the top level will be numbered from, say, -5 and up (title is hard wired at -5?), then each "level 1" fieldgroup will start at 0 and go up, then each "level 2" will start at 0 and go up, and so on. No two rows in the same fieldgroup will have the same weight anymore. This solves a problem I was having with fields not staying in their proper groups!

What do I have left?
--I'm going to make sure that my old submit function still works with the new changes.
--I need to see whether I bothered to update the display of fields on the node/add or edit form and if not, update it
--I need to update the node/view display to properly nest as well

Those last two should be fairly trivial. I must say that the JavaScript was kicking my butt. I consider myself pretty good with PHP, Drupal, and most other languages, but JavaScript I haven't spent much time with. Whoever came up with the tabledrag stuff is crazy good - I think. It may be that I just don't know how to go about exploring Objects and debugging JS and so what I needed to learn was easier than I thought. In any case, that part is done and I think I'm in the home stretch. Finally!

@markus what indentation issues are you talking about? In any case, I was hoping to make it clear that the parent is a group. Perhaps I could just knock off the 'name' part at the end. Also, your last paragraph in #27 talks about not allowing certain fields into certain types of fieldgroups. That seems to me like it should be handled by an override of the isValidSwap function in JavaScript from the tabledrag.js file. This override would appear in your patch rather than mine I believe. Once we can verify that my JavaScript isn't crap, I'll be happy to help sort out yours.

I may finally post the latest code tonight, though it might be tomorrow.

rconstantine’s picture

FileSize
444.26 KB

Sorry I haven't made a proper patch. I'll do one as soon as I can. The only things that don't seem to be 100% straight yet are the order of the fields on the node add/edit/view pages. Oh, and I haven't actually submitted a node add form yet, so that could blow up. For now, you can play with the admin side of things.

This zip contains the patch as applied to the 12/29 version of CCK dev. Looks like a new one came out today, so I'll likely incorporate those changes as well. Once this gets close to final I'll create a patch against HEAD.

rconstantine’s picture

@#21, I thought I read somewhere that drupal_execute was fixed to work with form_alter; or is that coming in D7? I still don't have ideas around that, BTW. This is the next function I'm looking at, so perhaps I'll find something to do. I'll brush up and see if nodeapi can be used instead.

rconstantine’s picture

One last comment as I head to bed: the thought occurred to me that because I have been testing using a complex content type that I started before making this patch, I haven't actually tested adding new fields or groups lately. So that's on my list of to-dos as well.

In summary I still need to check/tune -

1) the node add/edit/view pages
2) node submission
3) adding fields and groups

I went in circles on several points if you couldn't tell, but believe I have arrived at how things should be.

As mentioned elsewhere, I think I've covered content type export OK (in fact I used it to email myself my test content type from work to home). I don't recall whether there was anything else that needs integration with that can be handled by this patch. Ideas? Am I forgetting something?

I see that KarenS has been over to the multigroup patch thread today. Will it get incorporated into dev before I finish this one? Will they be a pain in the butt to integrate together? We'll find out, but I hope not!

I think I mentioned that I'll also be using the cck fieldgroup tabs module. If it can't already, I may try to allow for nesting tabs so each top level tab could allow for a set of tabs inside. Not sure how deep that should be allowed though - it could get messy. I just don't like scrolling.

rconstantine’s picture

FileSize
433.84 KB

FYI, I submitted a test page and all data was captured correctly so far as I could tell, so my submit function doesn't seem to be broken.

Also, I finished (?) the layout of the node add/edit forms and in doing so, I think I addressed comment #21's form_alter comment. At least, I couldn't figure out why the original patch author put it in and had an 'else' to go with it.

One thing that came up that I can't seem to duplicate is an issue with the 'add another item' button. I swear it freaked out once, but I can't get it to happen again. Has worked fine every other time, including saving the data.

I still have the node view layout to update as it presently displays fields in their fieldgroups, but not the parent/grandparent fieldgroups. Should be able to finish that tomorrow. Then I will switch the status back to 'needs review' and roll proper patches. I am still on the 12/29 dev CCK version, so I'll need to incorporate any changes. Adding new fields etc. seems to work just fine as well, BTW.

There is one strange thing I can't figure out. I have the 'Journal' module enabled which allows admins to log their changes to a site. This adds a simple text area to most pages. The puzzling thing is that although the weight of this item is set to be 100, it slots in between a fieldgroup set to 0 and another set to 1. I'm really puzzled here. All other 'extra' or hook_form_altered fields/fieldgroups show up in the right place. Wait a minute. Journal is the only additional field not in a fieldgroup.

One more thing I'm bummed about is that the CCK fieldgroup tabs module seems to blow up when I press an 'add another item' button and actually duplicates more than just the wanted field. But that may be an issue there and not here. I'll have to take a look.

That's it for today's update.

[EDIT]: all issues I mentioned above seem to not be a factor on my machine at home. I'm guessing some module interaction to be the problem at work. (i.e. 'add another item' button, editing a node, etc.)

[SOLVED]: the issue was MySQL running in strict mode. It seems the default my.ini includes provisions for that. Commented it out, now all is well with editing nodes except... check out the next post.

rconstantine’s picture

FileSize
140.85 KB

See the screen shot. What is up here? Ideas? Once the page is submitted, the second set of "Application components" appears with the weight fields.

Here is what I did to get here (ignore all of the empty fieldsets which I've just been using for testing).

1) There were three items in the "Application components" list from last time, so I add two more, "Fish" and "Sticks".
2) I then move "Sticks" up to where "Fish" is, which swaps them.
3) I "Save" the page and I'm returned to the page in the screen shot. I have deactivated any modules that could be interacting badly with this including the "Vertical tabs" module you see being used.

Perhaps I simply need to go through the CCK validation routines? I'll try there, but if anyone has any other ideas, let me know.

rconstantine’s picture

I see I'm in my own little world here, but I'll provide another update anyway.

I found what was causing the issue in the screen shot, sort of. It seems that whatever I've done, I did not break multiple fields at the top level, nor at a 'standard' one group deep.

What I see, once I change the drupal_add_tabledrag in content.module's theme_content_multiple_values from this:
drupal_add_tabledrag($table_id, 'order', 'sibling', $order_class);
to this:
drupal_add_tabledrag($table_id, 'order', 'sibling', $order_class, null, null, false);

I can see that the weight fields are being treated the same when I compare top level and one-deep fields to two-deep+ fields that are now possible. What seems to be the problem is that a #options set of values is not being updated. I'm not sure exactly where this is done yet, so I can't fix it. I can't seem to find the values using the form_inspect module. I can output the $element from the form.inc file where the error occurs, but I'm not sure how to go 'up' the structure from there to find out the overall structure, and thereby find the identity of the object that needs to be changed.

I am going to take a break from this and work on the node view layout.

BTW, I've tested adding fields and groups and that all seems fine.

yched’s picture

Check out content_add_more_js() in content.node_form.inc.

This function:
a) gets the current 'offcial' definition of the $form
a) builds a new set of 'multiple widgets', with one additional empty row,
c) puts it back in the cached $form stored on the server
d) sends it back on json form for AHAH update

c) involves assumptions on the form structure : where in $form do we put the updated form elements ? Current code does

  // Add the new element at the right place in the (original, unbuilt) form.
  if (module_exists('fieldgroup') && ($group_name = _fieldgroup_field_get_group($type['type'], $field_name))) {
    $form[$group_name][$field_name] = $form_element[$field_name];
  }
  else {
    $form[$field_name] = $form_element[$field_name];
  }

So you need to update that part to account for the fact that fieldgroups can mean more than one level of nesting.

As a side note, we'll need to solve that more elegantly for 'fields in core D7', since that function will live in core, and it can't include specific code for fieldgroup, that will stay in contrib. Well, that's another story.

rconstantine’s picture

Sweet. That's what I was looking for. I thought I looked at that function, but I could have missed it, or nothing jumped out at me.

Which CCK things are going to stay contrib? Is there a list somewhere?

I've got meetings today, but should be able to get to this last(?) bug and also the node view stuff tomorrow. Look to the end of the week for a new patch.

[EDIT] One meeting got cut short, so I have finished the node view code. I tried to keep the code as similar as possible for admin/add&edit/view screens. Perhaps this could be refactored to minimize code duplication, but my head hurts already just thinking about it. Namely, I'm talking about the fieldgroup_nodeapi, fieldgroup_form_alter, and _content_overview_order functions.

rconstantine’s picture

Can anyone tell me why this code doesn't work:

// Add the new element at the right place in the (original, unbuilt) form.
  if (module_exists('fieldgroup') && ($parents = _fieldgroup_field_get_parents($type['type'], $field_name))) {
    $parents = array_reverse($parents);
    $path = "'";
    $path .= implode("']['", $parents);
    $path .= "'";
    $form[$path][$field_name] = $form_element[$field_name];
  }
  else {
    $form[$field_name] = $form_element[$field_name];
  }

$parents is an ordered array of parents such as array('parent', 'grandparent', 'greatgrandparent'), which I reverse to get a top-down path to where the field is.

I expect that I should be updating the field at $form['greatgrandparent']['grandparent']['parent'][$field_name], but is this not the case? An output to the screen of $form seems to indicate that this is right. However, assigning the $form_element[$field_name] to it wipes out what is already there.

At least it isn't duplicating the field like it was before, I guess.

BTW, I have also tried creating $path these ways as well:

$path = "'";
$path .= implode("][", $parents);
$path .= "'";
$path .= implode("][", $parents);
rconstantine’s picture

This next code also seems like it should work, but again, the existing field is being wiped out and not changed to the new values.

if (module_exists('fieldgroup') && ($parents = _fieldgroup_field_get_parents($type['type'], $field_name))) {
    $parents = array_reverse($parents);
    $reference =& $form;
    foreach ($parents as $index => $group_name) {
      $reference =& $reference[$group_name];
    }
    $reference[$field_name] = $form_element[$field_name];
  }
  else {
    $form[$field_name] = $form_element[$field_name];
  }

If I do a drupal_set_message and output the contents of $reference[$field_name] before and after it gets set, it clearly gets updated to the new form_element. I was expecting the proper $form element to be updated since I'm using references to drill down. Argh! Aggravations galore!

rconstantine’s picture

For completeness, this is what I mean by 'wiped out'.

markus_petrux’s picture

implode('][', $parents) cannot be used to point to a nested array item. Maybe using a loop like this?

    $path = array_reverse($parents);
    $form_copy = &$form;
    foreach ($path as $key) {
      if (!isset($form_copy[$key])) {
        $form_copy[$key] = array();
      }
      $form_copy = &$form_copy[$key];
    }
    $form_copy[$field_name] = $form_element[$field_name];
rconstantine’s picture

Your code seems almost the same as mine in #41. What is the difference? I'll try it, of course, but it looks essentially the same.

Anyway, why can't the implode thing be used? Isn't that how drupal_set_error's first argument works? That's where I grabbed the idea from.

Also, just to make sure I wasn't using references wrong, I did this and it works just fine:

$test1 = 'hello';
  $test2 =& $test1;
  drupal_set_message('<pre>Test2: ' . print_r($test2, TRUE) . '</pre>');
  $test2 = 'goodbye';
  drupal_set_message('<pre>Test1: ' . print_r($test1, TRUE) . '</pre>');

Thanks for the input Markus.

rconstantine’s picture

@43 - same result as before. AAAAARRRRGGGGGHHHHH!

markus_petrux’s picture

Oh, I wrote my answer after reading #40, but probably while you were writing #41. It is just something distracted me before sending the post.

why can't the implode thing be used? Isn't that how drupal_set_error's first argument works?

FAPI is not storing a nested array, but a plain one where item keys happen to look like it was nested, but they aren't. FAPI used these kind of keys because that works to build the path to the DOM element where the error class should be attached.

rconstantine’s picture

Never mind. I figured it out. I needed to look farther down in the function and do something similar with another structure as well. I'm downloading the latest dev version to integrate with and will do new patches.

Thanks anyway Markus.

rconstantine’s picture

FileSize
994.11 KB

I'm working on a patch now. For those who prefer the full module, here is a zip based on the Jan 10th dev.

I'll do two patches; one against the same dev and another against HEAD. [EDIT] Oh, I guess not HEAD since that's D7 now. 6-2 then, in which case I'll do just one patch.

rconstantine’s picture

Status: Needs review » Needs work
FileSize
43.2 KB

For some reason the patch includes all the .info files? This is against a CVS checkout of the 6.-2 branch, which I think is what dev comes from. I think the stuff in the .info is what gets inserted by Drupal when it makes a tarball.

Let me know if I screwed it up.

rconstantine’s picture

Status: Needs work » Needs review

Forgot to change status.

yched’s picture

Status: Needs work » Needs review

If you look at the D7's (work-in-progress) current version of for field_add_more_js() in http://vcs.fourkitchens.com/drupal/7-fic/annotate/head%3A/modules/field/..., you'll see that this is about exactly the same code :-)

rconstantine’s picture

What a co-inki-dink!

Is form_path a new thing? I don't recall seeing that in current forms.

Did anyone try to apply the patch? How about a review? Is it good to go?

yched’s picture

Yes, form_path is an addition for D7 : if your module form_alters a cck field to a different place in the $form, it should also alter $form['#fields'][$field_name]['form_path'] to indicate the new path to the form element, so that the 'add more' button can find it. The whole code still assumes that the *values* will be in $POST[$field_name] / $form_state['values']['$field_name], though. At some point we might want to abstract that out too. See http://groups.drupal.org/node/18019 for background.

Things might still change in this area for D7 :-), but 'form_path' is the current solution to the 'no fieldgroup-specific code'.

I *terribly* lack time to review ambitious patches right now, unfortunately :-/

jpsalter’s picture

#48 http://drupal.org/node/300084#comment-1198546 is working perfectly in my Drupal setup. Thank you for a fantastic and much needed feature.

rconstantine’s picture

@jpsalter - great! I'm glad it applied cleanly and is working for you.

I am now working on a patch for cck_fieldgroup_tabs if that's something you're interested in that. I was made a co-maintainer, so I'll be able to apply my changes quickly.

rconstantine’s picture

FileSize
4.75 KB

I have adjusted one file and introduced a new hook. This sort-of implements D7's form_path in a different way, allowing other modules to change the parentage of a field. I will be posting a patch to cck_fieldgroup_tabs which implements this new hook.

The hook is this: hook_fieldgroup_parents_alter($form, &$parents, $type, $field_name)

Presumably, a module has manipulated the location of a field and knows where to look for it and can then insert/reorder the parent list to match.

In the case of cck_fieldgroup_tabs, there are possibilities for fieldgroups to exist either below $form['fieldgroup_tabs'] or $form['fieldgroup_tabs']['fieldgroup_tabs_basic'] and so we check for these and add them to the parent list.

The parent list passed in goes from immediate parent to oldest ancestor.

I'll follow this up with a redo of the above patch, but for now, here is the changed file.

[EDIT] The cck_fieldgroup_tabs changes are here: http://drupal.org/node/325049

markus_petrux’s picture

Would it be possible to isolate the code that places a form element in the form structure into a generic function?

I mean the snippets:

  // Add the new element at the right place in the (original, unbuilt) form.
  if (module_exists('fieldgroup') && ($parents = _fieldgroup_field_get_parents($type['type'], $field_name))) {
    foreach (module_implements('fieldgroup_parents_alter') as $module) {
      $parents = call_user_func($module .'_fieldgroup_parents_alter', $form, $parents, $type['type'], $field_name);
    }
    $parents = array_reverse($parents);
    $reference =& $form;
    foreach ($parents as $index => $group_name) {
      $reference =& $reference[$group_name];
    }
    $reference[$field_name] = $form_element[$field_name];
  }
  else {
    $form[$field_name] = $form_element[$field_name];
  }
  // Render the new output.
  if (module_exists('fieldgroup') && ($parents = _fieldgroup_field_get_parents($type['type'], $field_name))) {
    foreach (module_implements('fieldgroup_parents_alter') as $module) {
      $parents = call_user_func($module .'_fieldgroup_parents_alter', $form, $parents, $type['type'], $field_name);
    }
    $parents = array_reverse($parents);
    $reference =& $form;
    foreach ($parents as $index => $group_name) {
      $reference =& $reference[$group_name];
    }
    $field_form = $reference[$field_name];
  }
  else {
    $field_form = $form[$field_name];
  }

The benefit would be that other modules could use that function as well. For example, the multigroup module, but also other fields such as filefield also need to manipulate the form structure in this way.

I hope to get soon the new version of the multigroup module, so maybe I could I could play with both features. The problem for us working on this is: which patch would enter first in the CCK CVS. I guess it would make things easier if it was this one. But we need to play with each other to make sure both features work well.

rconstantine’s picture

While I can see the benefit for this module of making the above into a function to call, I don't see how other modules could use it. There is a call inside which makes a call for modules to manipulate the parent list. Is this not good enough for other modules? If many modules use a function-ized version of the above, wouldn't that mean that every module also makes the same hook call? Is that a good thing? You mentioned that your patch could use it; could you explain how the new hook isn't enough and how you'd use it?

I figure the function signature would be something like: _fieldgroup_update_field($type, $fieldname, $target_form, $source_form)

The only problem I see to doing this is the last line before the "else". Note that in the first example, $reference[$field_name] = $form_element[$field_name]; makes an assignment differently than the next example of $field_form = $reference[$field_name];.

I don't have the code in front of me right now to determine whether that point could be changed or not. I'll take a look.

markus_petrux’s picture

Well, this is going to break, at least, filefield/imagefield (see their ahah code), so it might be interesting to think about a new API or something that CCK fields could use to know if they are compatible with the installed version of CCK, or viceversa, CCK know in advance if certain fields are compatible with CCK. I believe Views uses a similar method. This could be discussed on a separate issue if KarenS/yched agree on this.

Since there might be other modules that have a similar issue as that for filefield/imagefield (maybe the fieldgroup tabs also depends on this), it might make things easier if CCK could provide some kind of API to retrieve the form element from the form structure for a given field_name/delta. And the oposite, insert a field/delta into the form structure. Both operations depend on whether the field is inside a fieldgroup, or a nested fieldgroup.

Now that I'm more or less ready with the multigroup patch, if you could post an updated patch, I may try to play with both things, and that may require additional updates of both patches.

markus_petrux’s picture

I tried to apply the patches, but I had to fall back to manual operation. :( ...I believe I was able to compile all the changes on a single patch that is against the lastest 6.x.2.x-dev snapshot. I also removed a few whitespaces on empty lines, and at end of others.

markus_petrux’s picture

Status: Needs review » Needs work

A bug when adding a group:

1) In the Add new group section, fill in a label and a name.
2) Drag the group inside another group.
3) Save.
Result: The new group is not saved in the correct position. It is placed at the top level.

Then, there may be a few optimizations to do here. For example, part of form_alter for node edit form:

// Cover the top level fields that aren't in fieldgroups.
foreach ($field_check_off as $name) {
  $depth = 0;
  $pre_weight = $form[$name]['#weight'];
  $weight = $pre_weight * pow(10, (-3 * $depth));
  $form[$name]['#weight'] = $weight;
  $form[$name]['#depth'] = $depth;
}

Since $depth is forced to be 0, the operation can be simplified as follows:

// Cover the top level fields that aren't in fieldgroups.
foreach ($field_check_off as $name) {
  $form[$name]['#depth'] = 0;
}

This is because, (-3 * 0) = 0, and pow($x, 0) = 1, then $x * 1 = $x. Here $x really is $form[$name]['#weight'] in the loop, so the computation is really doing $form[$name]['#weight'] = $form[$name]['#weight'];.

Later, or maybe tomorrow, I'll try to post a patch with these changes.

markus_petrux’s picture

Here's an updated version of the nested fieldgroup patch. The problem mentioned in #61 when creating a group still happens, but I've more focussed on trying to clean up a little the changes.

It also implements the following APIs in content.node_form.inc, that can hopefully be reused by other modules that need to interact with the form structure, for example to implement an ahah callback to rebuild part of the form (add more values, filefield/imagefield ahah job, ...), or make things easier for the multigroup too.

/**
 * Store an element into a form.
 *
 * @param $field_name
 *   The field name.
 * @param $type
 *   The content type where the field instance belongs to.
 * @param $form
 *   The form to store this field element into.
 * @param $element
 *   The form element to store.
 */
function content_set_form_element($field_name, $type, &$form, $element) {
  if (module_exists('fieldgroup') && ($parents = _fieldgroup_field_get_parents($type, $field_name))) {
    foreach (module_implements('fieldgroup_parents_alter') as $module) {
      $parents = call_user_func($module .'_fieldgroup_parents_alter', $form, $parents, $type, $field_name);
    }
    $parents = array_reverse($parents);
    $reference = &$form;
    foreach (array_values($parents) as $group_name) {
      $reference = &$reference[$group_name];
    }
    $reference[$field_name] = $element;
  }
  else {
    $form[$field_name] = $element;
  }
}

/**
 * Retrieve an element from a form.
 *
 * @param $field_name
 *   The field name.
 * @param $type
 *   The content type where the field instance belongs to.
 * @param $form
 *   The form to retrieve this field element from.
 */
function content_get_form_element($field_name, $type, $form) {
  if (module_exists('fieldgroup') && ($parents = _fieldgroup_field_get_parents($type, $field_name))) {
    foreach (module_implements('fieldgroup_parents_alter') as $module) {
      $parents = call_user_func($module .'_fieldgroup_parents_alter', $form, $parents, $type, $field_name);
    }
    $parents = array_reverse($parents);
    $reference = &$form;
    foreach (array_values($parents) as $group_name) {
      $reference = &$reference[$group_name];
    }
    return $reference[$field_name];
  }
  return $form[$field_name];
}

The above is what I tried to mean in comment #57 above.

rconstantine’s picture

Wow Markus, you've been busy. Sorry I haven't gotten to this in the past several days. I have over a dozen projects going on at the same time and although this is a priority, I had to give the other things some time this week.

Anyway, I'm downloading your latest patch and will apply to latest dev. Thanks for your contributions.

I had noticed that bug of adding a group but hadn't gotten around to it yet. Fortunately, it's mostly an annoyance and doesn't blow up anything. Regarding #61, that's an obvious example of too much copy/paste, isn't it? How embarrassing.

Regarding #62, I would imagine you tested it and it all works, so good job. Downloading now.

Do you think we're close on this? I'm heading over to your threads after this to how those are doing. I think I'll be able to mash this all together on my end on Monday. Again, sorry for the delay. I'm glad you aren't waiting for me!

rconstantine’s picture

I've incorporated #61 and #62 and squished the bug mentioned in #61. I'm now proceeding with integration of the multigroup module changes (and dependency patch).

As a stand-alone patch, are there any outstanding issues that would keep this from being added to the D6 branch?

Also, even though I added #62, which is cleans things up a bit, I don't think most modules will need them. They can probably use the hook_fieldgroup_parents_alter instead and manipulate the tree directly like I did for cck_fieldgroups_tabs. In fact, I'd bet that filefield/imagefield may be able to use it and reduce or eliminate their existing overrides.

What I'd like to do is integrate the multigroup stuff and then look at the filefield/imagefield issues. I will be using all of this stuff myself so it won't be forgotten/ignored. It seems reasonable to me, that if we can determine that the AHAH stuff is what breaks in filefield/imagefield, with this patch and the multigroup one, and seeing as how the dev version of CCK should have new features, it makes sense that these patches should be included in dev before a complete solution is worked out in other modules. Shouldn't their official releases depend on official CCK releases and their dev releases follow development in the CCK dev anyway? Then, the CCK maintainers could wait to make the next official release until certain other modules have their dev branches caught up.

Did that make sense? I'm in a hurry and need to go to a meeting...

New patch or zip to follow soon.

rconstantine’s picture

Status: Needs work » Needs review
FileSize
39.98 KB

New patch attached - built against cvs branch. I think everything works as advertised. I did just a little refactoring to clean things up, but perhaps you all will see areas that need improvement. I'm amending my position above. After looking at both my patch and the other two, I don't think it matters which is added to the dev branch first. They don't seem to clobber each other.

Although there may be room to clean/refactor some items, since it all works, I think we're good to go.

rconstantine’s picture

FileSize
44.79 KB

I have added to the patch a fix to disallow placement of either standard fieldgroups or multigroups within multigroups. It ended up being very simple, but took me forever since I'm fairly weak on jQuery/DOM.

This should mean that I'm done integrating with the multigroup patch.

The changes I've made should not affect users that don't use the multigroup patch, nor those that don't use fieldgroups.

If anyone is out there, please test this.

Let me know if you prefer a zip of the cck folder, and if so, whether you'd like it with just this patch applied or with multigroup included.

Note: patch is based on CVS checkout which should match Jan 26th dev version.

Oops! There is a stray character in the file fieldgroup.tabledrag.js. The first character is a d, and that should not be there. So after you patch, delete that. I'll reroll a patch later.

yched’s picture

Thanks for the heroic work, Ryan (same goes for markus_petrux, if he reads this). Sorry for not being more present, things are a little hectic lately.
I'll try to take a closer look asap, but can't promise a deadline. Meanwhile, all reviews / tests welcome.

On a more operational note, I think we should release a 2.2 and then start pushing the monster patches in dev.

rconstantine’s picture

@yched I understand that things a crazy right now. I just worry that if the code drifts too far from where it is now, that it might change some of what these patches already address or depend on which would lead to breakage. I do understand that you're being pulled in other directions with D7 and all, but I think that the sooner this stuff is in D6, the sooner we can adapt them to D7.

All - see screen shot of a partially completed, and totally crazy form I'm building. I have not yet added any multigroups to it yet, but have placeholders. Several of the top level fieldgroups don't have anything in them yet. I think I'm a little less than half way done, but I wanted to show what all of this is being used for and how crazy one can get with these new patches. Note, all top level fieldgroups are used as tabs via the cck_fieldgroup_tabs module. If I had done our theme already, I'd have attached a shot of the content type's edit screen to show the tabs.

markus_petrux’s picture

Status: Needs review » Needs work

One thing I'm not sure about this patch is the overridden methods of tableDrag are really necessary, or maybe I'm missing something.

Note that taxonomy module uses tableDrag as-is, and it is able to deal with nested terms keeping separate sets of weights for each level of the tree. Isn't nested fieldgroups similar on what taxonomy module does? ...the only difference I can think of is that we may need to avoid nesting standard fielgroups inside multigroups.

rconstantine’s picture

From the top of the tabledrag.js file:

* Created tableDrag instances may be modified with custom behaviors by
* overriding the .onDrag, .onDrop, .row.onSwap, and .row.onIndent methods.
* See blocks.js for an example of adding additional functionality to tableDrag.

The first function in fieldgroup.tabledrag.js is an implementation of onDrop and deals with assigning the right classes to each row and assigning new weights. One of the things I did with this patch was to make each layer of depth have its own weight for each fieldgroup. It just made a lot of things easier. If you can't picture that, just think of a standard outline where you have something like this:

1
  a
  b

2
  a
  b
  c
    I
    II

and so on. The parent field in the DB is what keeps things separate.

Additionally, the new function I just added, is an override of validIndentInterval and prevents other groups from being included in a multigroup.

Does that clear it up? Both are quite necessary.

BTW, Markus, I found two lines of my code so far that were messing up the output from your multigroup patch, which I have fixed. I'll roll another patch after I make a bit more progress. For some reason, as I have things now, nested multigroups (and the standard group they are nested in) are disappearing. Multigroups at the top level are behaving as expected (I think), and no longer have display issues. "Add more values" works just fine.

So I'm trying to track down where in your code the part of the form containing the nested items are unset. I'm assuming it's in your code since you didn't plan for nested fieldgroups.

I'm not sure what you're marking as 'needs work'. By itself, as I mentioned in my patch post above, it all works. Throw multigroups into the mix though... But... multigroups is supposed to be an extension of fieldgroups.

rconstantine’s picture

Status: Needs work » Needs review
FileSize
44.94 KB

Hey folks. Here is another patch against the dev from the 26th. Haven't looked at the dev from yesterday yet. I fixed part of the conflict with multigroups. I think the remainder of the issue may be in the multigroups patch. Looking at that now. Any help appreciated.

yched’s picture

rconstantine: I'm also intrigued by the need for Drupal.tableDrag.prototype.onDrop.
Core's vanilla tabledrag.js should already handle different weight 'threads' per parent.
You can see that by changing the begining of theme_menu_overview_form() in modules/menu/menu.admin.inc :

- drupal_add_tabledrag('menu-overview', 'match', 'parent', 'menu-plid', 'menu-plid', 'menu-mlid', TRUE, MENU_MAX_DEPTH - 1);
+ drupal_add_tabledrag('menu-overview', 'match', 'parent', 'menu-plid', 'menu-plid', 'menu-mlid', FALSE, MENU_MAX_DEPTH - 1);
- drupal_add_tabledrag('menu-overview', 'order', 'sibling', 'menu-weight');
+ drupal_add_tabledrag('menu-overview', 'order', 'sibling', 'menu-weight', NULL, NULL, FALSE);

and visiting admin/build/menu-customize/navigation

Suggestion : maybe it has to do with this :

+    if ($element['#row_type'] == 'group') {
+      $parent_list[$element['group_name']['#value']] = strtr($element['group_name']['#value'], '_', '-');
+    }
+    if (empty($element['parent']['#value']) || !isset($element['parent']['#value'])) {
+      $element['weight']['#attributes']['class'] = 'field-weight field-weight-' . 'top';
+    }
+    else {
+      $element['weight']['#attributes']['class'] = 'field-weight field-weight-' . strtr($element['parent']['#value'], '_', '-');
+    }
+
(...)
+  drupal_add_tabledrag('content-field-overview', 'match', 'parent', 'group-parent', 'group-parent', 'field-name', TRUE, 10);
+  foreach ($parent_list as $name => $parent) {
+    drupal_add_tabledrag('content-field-overview', 'order', 'sibling', 'field-weight', 'field-weight-' . $parent, NULL, TRUE);
+  }

AFAIK, you shouldn't need to have those separate 'field-weight-' . $parent subgroups in a foreach. A tabledrag-enabled tree structure usually takes only two drupal_add_tabledrag() calls

The override of validIndentInterval is definitely needed, though, as core tabledrag doesn't handle the case of 'I accept that kind of children but not that other kind'. I'm a little surprised of how complex this turned out, but its probably rightly so after all.

markus_petrux’s picture

Here's an experiment you can try:

1) Start with a fresh installtion of CCK 6.x-2.x-dev

2) Patch cck/theme/theme.inc like this:

-  drupal_add_tabledrag('content-field-overview', 'match', 'parent', 'group-parent', 'group-parent', 'field-name', TRUE, 1);
+  drupal_add_tabledrag('content-field-overview', 'match', 'parent', 'group-parent', 'group-parent', 'field-name', TRUE, 10);

3) Patch cck/includes/content.admin.inc like this:

       'weight' => array('#type' => 'textfield', '#default_value' => $weight, '#size' => 3),
-      'parent' => array('#type' => 'hidden', '#default_value' => ''),
+      'parent' => array('#type' => 'select', '#options' => $group_options, '#default_value' => ''),
       'hidden_name' => array('#type' => 'hidden', '#default_value' => $group['group_name']),
-      '#root' => TRUE,
       '#row_type' => 'group',
       'weight' => array('#type' => 'textfield', '#default_value' => $weight, '#size' => 3),
-      'parent' => array('#type' => 'hidden', '#default_value' => ''),
+      'parent' => array('#type' => 'select', '#options' => $group_options, '#default_value' => ''),
       'hidden_name' => array('#type' => 'hidden', '#default_value' => $name),
-      '#root' => TRUE,
       '#add_new' => TRUE,
       '#row_type' => 'add_new_group',

4) Now, visit a 'Manage fields' page for a content type that already has a few fields and fieldgroups.

5) In your firebug console enter the following, that will show the parent and weight fields on the tableDrag form:

$('#content-field-overview').find('td').show();

6) Start playing with the drag-n-drop interface. You'll see how the weight and parent fields are updated automatically onDrop already.

I think that's all it's needed. Well, aside from the validIndentInterval override, which already mentioned yched above.

Edited to simplify the javascript onliner needed to bring the hidden elements, so it can be monitored how the tableDrag updates the parent and weight values on drag-n-drop operations.

Edited again to extend the mini-patch to content.admin.inc so that the 'Add new group' row get also the group selection element and its #root property is removed so that it can be a container of other fieldgroups.

rconstantine’s picture

Status: Needs review » Needs work

Are you telling me that by doing your test that I can get numbers identical to those in the screen shot attached (in next post - oops)? I really doubt that, but I'll try it. Without the onDrop, I was only getting a series of consecutive numbers. Using the rows in the screenshot, they would number from -5 to 10. That's not what I need or want here. Also, I believe I snagged the technique of looping the tabledrag function calls from another core module... http://api.drupal.org/api/file/modules/block/block-admin-display-form.tp... ... and in fact, the documentation on api.d.o talks about "more complex scenarios" and outlines the technique I used!

Also, an update. it looks like the main conflict between the two modules that I was having has to do with the #access property. I have adjusted the "nested" patch to 'hand up' the property value from an internal fieldgroup to its parent. This requires setting #access to TRUE if it shouldn't be FALSE. Previously, this hasn't been set to TRUE. Consequently, the multigroup patch is not accounting for #access to be TRUE and tests only if it is empty(). I have adjusted this on my copy and am able to nest multigroups as deep as I want. Saving the node works. The next thing to fix is the AHAH for adding another row since this doesn't take nesting into account. This will be a change to the multigroup patch's content_multigroup_add_more_js function at least. It does look, however, that the end is in sight.

[EDIT] Another core function that loops to create tabledrag stuff: http://api.drupal.org/api/function/theme_profile_admin_overview/6

rconstantine’s picture

FileSize
37.83 KB

Where did my screen shot go? I know I clicked on "attach"...

markus_petrux’s picture

Ryan, yes I mean the above mentioned changes to a fresh CCK install seem to be enough to get the tableDrag working as this patch needs (the validIndentInterval override apart). Fields and fieldgroupscan only be attached to fieldgroups, fielgroups can be nested up to 10 levels. No other change is necessary to the tableDrag behavior. It updates correctly the parent and weight attributes for each affected row in a drag-n-drop operation, at least here in my dev env.

rconstantine’s picture

I'll try it at home where I have a clean environment. I don't want to 'mess' with what I've got here. I still have doubts though. There must be something I'm doing that isn't done the other way. Hmm.

rconstantine’s picture

I'm leaving work and won't be around for a few days. The remaining issue is found in two function in the multigroup patch, in addition to the #access handling I mentioned already.

First, the function content_multigroup_add_more_js needs to call content_set_form_element and content_get_form_element, both of which need to be modified to handle either groups or fields as input. I was going to add a flag to the function to indicate which mode it should be in.

Second, because of nesting, content_multigroup_group_form cannot work on $form directly as it doesn't know where to operate. It needs to work like content_field_form and return the proper form element or optionally do so. It could build the form element and then at the end put it directly in $form in one case or return it in another case.

After this, we'll have to make sure validation and submission work. I expect they will or will only need minor changes.

See you all next week.

rconstantine’s picture

I'm back and working on the above. The stuff I mentioned in the paragraph beginning with "first" is done. Now working on "second". I'm dreading pulling out all of the changes a splitting them into the three separate patches; they interact so much with each other...

rconstantine’s picture

FileSize
1011.45 KB

I think I've got it all worked out. I'm attaching a zip of the whole cck folder. I will roll new patches against the latest dev on Monday. It's currently built against 1/29/09 I think. I made changes to both this patch and to the multigroup module. I think they play nice together now.

ec’s picture

Hi,

I'm testing the cck module found in # 80.

In my test site I have 1 content type where I'm using several (i.e. 4) multigroup groups. When in "Display fields" I want to set those multigroup from "Fieldset" to "Table - Multi columns". After saving the change, only the multigroup in the 1rst position from the top keeps its new setting, the others stay with the old one "Fieldset" for this example.

To have a new setting saved for an other multigroup, I need to place it in the 1rst position from the top and then change the display setting.

So it looks like that saving the display setting for several multigroup on the same content type only works for the multigroup on the 1rst position from the top.

Any idea concerning this? Anyone out there falling on the same issue?

Regards,
Eric

rconstantine’s picture

@eric, do you know whether the saving worked using the multigroup patch alone without the nesting patch? In any case, I'll look into this.

ec’s picture

@rconstantine, I tryed with CCK patched as explained here http://drupal.org/node/119102?page=1 #346 and used the multigroup module found in #320. So I guess that now I don't have your work concerning the nesting. With this new setting the issue I described in #81 has gone away.

rconstantine’s picture

OK. I think I got that bug it squished along with a couple of others. I'm still reviewing all validate/submit functionality on all relevant screens, but I'm almost done. I'll post another zip before leaving work for the night and as soon as my testing is more or less complete (tomorrow???), then I'll roll the real patches.

Thanks for your patience everyone.

rconstantine’s picture

FileSize
1011.85 KB

Here is an updated patch, still not against the latest dev.

Outstanding issues:

1) drag-n-drop left-right movement on admin screen seems to ignore existing function which prevents nesting groups of any kind inside a multigroup; I adjusted the validation routine to assist here, but it is not an elegant solution; up-down movement seems OK. I'll see about enhancing the existing javascript function.

2) not sure all widgets/cck field types work - ex. date module's select drop down doesn't save on first try if in a 'new' row of multigroup, but if previewed first, or if second try, it works. I'll test to see that such a field still works OK in regular fieldgroups and outside of groups to narrow the cause. This is in addition to any testing I'll have to do with filefield/imagefield modules.

Let me know if you guys find anything else.

vikramy’s picture

Great work. It works for me.

And again as you have mentioned,
"You cannot place any kind of group inside a multigroup". must be handled I guess.

Can we reorder groups??

Thanks .

rconstantine’s picture

You should be able to reorder groups. My personal copy is a mess right now, but I thought I was zipping a working copy for posting here. If that isn't the case, then let me know. I believe that it works mostly right but that there are a couple of instances where the JavaScript allows you to nest something where you shouldn't be able to. However, submission now handles this and kicks any misplaced items back to where they started. I'm still working on the JavaScript. The problem has to do with a lack of information in the function that determines the allowable range of indentation. For example, I don't have access to start and end position of the mouse movement, so I can't use that to determine start/end position of the row. There are other issues as well. I'm trying to get clever and work around them. Wish me luck. Otherwise, I may have to just pull out my function override and rely solely on the submission function, which is very inelegant.

vikramy’s picture

Good luck.

Thanks for replying back. Well I am new to cck, may not know a lot. Your cck works perfectly fine. I can reorder groups in "manage fields" easily.

But I am trying to check, is there any way of reordering groups in "creating a content"?

Thanks for wonderful work and wishing you good luck again.

rconstantine’s picture

To answer your question: yes and no. If you have a 'multiple' field (set in the field's settings when you created it), then the items can be reordered. For any single-item fields, they appear in the order specified in the 'manage fields' screen.

vikramy’s picture

Thanks again.

I understood what you are saying. But my imagination was to Reorder Groups in create content Area.

Suppose We have two Standard groups named Section1(Title,Id) and Section2(Title,Id).

Section1
title This is a test
id 11

Section 2
title Another test
id 22

Is there a way to reorder section2 and Section1?(without using manage fields)

I mean

Section2
title Another test
id 22

Section 1
title This is a test
id 11

This may be helpful if authenticated users other than admin wants to order groups based on their needs.

May be I am just too crazy. Well I know this can be handled if we use views. I mean we can sort and get what we need.

Thanks a lot for your great work.

rconstantine’s picture

Gotta use views. Otherwise, your second version is an entirely different content type definition.

vikramy’s picture

Got it.. Thanks .

Another issue. I guess you are dealing with deleting unwanted multiple values right?

I am using computed cck. I created 5 multiple values and saved it. I used computed cck to get some results.

Again I tested to delete one unwanted value. The field gets deleted but computed cck value needs to be handled here.

rconstantine’s picture

That will be something that the computed field module will have to take care of. Any "down stream" modules will have to adjust to take advantage of these changes. Naturally, I will be submitting patches to any down stream modules that I will be using. However, the following needs to take place for any patches to those modules to make it into the main releases:

1) The patches that I'll roll for these three patches to cck need to make it into cck dev
2) Downstream module maintainers need to be made aware of these changes to cck
3) It may be that downstream modules may wish to create new branches to adapt to the changes
4) Once a good number of downstream modules have dev versions which include their changes, then cck can make a new official release
5) Downstream modules can then make official releases with the changes in tact

At least, this is how I would do things. The CCK maintainers may choose another path. We'll have to see.

rconstantine’s picture

See #97 below for zip of working files...

I think I solved problem 1 from #85 above. That was tricky! I will be using the module to further construct the content types that I need, during which I'll be able to further test things out. If all is well, then I should be able to have time Friday to roll proper patches against the latest dev and we can hopefully move toward getting these babies into dev!

I will also try to find time to do the experiment from #73-75 above. I still haven't done that. In any case, I think that everything now works as advertised.

Note, the attached is again a combo of all three patches (nested fieldgroups, multigroups, and row remove) and is against an earlier dev version of cck.

Thanks to those trying these out. Thanks to Markus for the bulk of the other two patches. Thanks to the maintainers for deigning to look at all of this. I really hope that once I roll the patches that this will all get in.

BTW, I think that the order they need to go in, due to dependencies, are 1) row remove, 2) nested, 3) multigroup

vikramy’s picture

Thats great news. Good job.

Roulion’s picture

Awesome !! what a good news to have a 2.x-dev version with all of these stuff. I hope the point with filefield could be solved soon

rconstantine’s picture

FileSize
1012.45 KB

Where did my attachment go?

rconstantine’s picture

RE #96, I will have to use filefield myself soon (in a couple of weeks?) so if it isn't addressed before then, then I'll tackle that as well. Same goes for imagefield and date.

rconstantine’s picture

FileSize
1012.45 KB

Oops. I had a bug in the last post. Use this instead. Things are looking good though for those patch rolls tomorrow.

The bug had to due with multigroups more than one fieldgroup deep or inside fieldgroup tabs.

Dewi Morgan’s picture

This is fantastic. Thank you so much for making this.

IMHO this is functionality that has been missing in drupal for far too long: the "drupal way" before this seems to have been to make any compound data particle, no matter how small, into a node. In my case, it was just a course-date (or date range), and a capacity, which seems a bit silly to turn into a node. I considered doing it as a comment to a course node, but that was ugly.

I tried following Jennifer Hodgdon's article "Creating a Compound Field Module for CCK in Drupal 6.x" (http://poplarware.com/cckfieldmodule.html), but I was just getting far too many errors that were apparently only vaguely related to what I was doing: I'd squish one, and three others would pop up. Debugging my module would've taken far too long: I don't /have/ days.

While this is alpha, so was my code, so I don't lose anything in stability by using this.
And I can get a solution up and working in 5 minutes, instead of 5 days!

Thank you again.

rconstantine’s picture

Obviously, I didn't get the patches rolled last Friday like I wanted to. I'm in Jury Duty this week, though I could be done today - we'll see. My evenings have been booked too or I'd just roll them at home! Tonight I have a DUG meeting. Will try to get the new patches out by the end of this week. Sorry for the delay, but the above zip should be working just fine.

itaine’s picture

subscribing

BartK’s picture

subscribing

Roulion’s picture

i tried the #99 patch and it loks god for me (the nested multigroup works well).
I'm waiting for the multigroup to support imagefield to be able to finish my project !!! Thanks for the time you spent on this feature

Roulion’s picture

Actually when i create a content for which thre is no fieldgroup (a basic page for example), there a message : warning: Invalid argument supplied for foreach() in C:\wamp\www\afterfoot\sites\all\modules\cck\modules\fieldgroup\fieldgroup.module on line 404.

Deleting values looks good

rconstantine’s picture

Thanks for the bug report, I'll get on that first thing Monday afternoon when I get back to work. My Jury service is over.

rconstantine’s picture

#105 - I wasn't able to duplicate the problem. See my screen shot for how my page content type is setup. I tried both page creation and editing and didn't get an error. Perhaps it is fixed on accident in the latest version found below?

EDIT - Drupal seems to have implemented a disc space limit of 10MB. I couldn't upload the screen shot. Nor was I able to upload the new patch, and the new zip.

rconstantine’s picture

FileSize
29.07 KB

Here is the screen shot.

rconstantine’s picture

FileSize
466.5 KB

Here is the latest zip, built against Feb 14th dev. This contains all three patches.

rconstantine’s picture

Status: Needs work » Needs review
FileSize
200.09 KB

This is a super-patch which includes all three patches. I'll try to break out the nested into its own and the multigroup into its own. The row delete patch should be the same as it was before. I haven't made changes to that. I fear, however, that in making fieldgroups and multigroups work together that they may not apply cleanly without each other. Naturally, multigroups depends on fieldgroups since it is a type of fieldgroup.

rconstantine’s picture

Regarding the super patch: Like I said, I made changes to both this patch and the multigroup patch which are interdependent. SO, what the maintainers might want to do is apply the delete patch, then the multigroup patch as last posted by Markus and place those in the next(?) dev after the next official release. I could then roll a new patch against that which would just contain the fieldgroup/multigroup changes on top of those other patches. That might be easiest.

I know the maintainers have their attention split between this D6 version and D7. Additionally, they've mentioned that they want to make another official release before rolling these three patches into dev. While I do understand the time constraints they are operating under, I think that the longer they let this sit out here in the queue, the harder it gets to get this in and any other patches coming in threaten to break any one of these three. While it is important to address other bugs and issues, it may be that with these three patches that those other bugs go away or would need to be addressed in a different way. Knowhwatimean?

Opinions?

markus_petrux’s picture

Since the CCK maintainers said they won't apply this to dev, at least, until a new release of CCK is out... AND, I need to focus on other things, THEN I decided to not spend more time on these. At least, until CCK maintainer release a new package for CCK, and then we'll see...

Ideally, they can spend some time on this at some point in the near future.... otherwise, I guess many people that's waiting for these fearues will be affected. The situation is not easy, for example in my case, where an important part of the project I'm working depends on this. In our case, and this is something I think I posted about... we'll be using the multigroup module even if not released with CCK. It just depends on the 'remove' button thingy... then, nested fieldgroup feature is not so critical for us, so I guess we'll have to leave without it, unless it is officially included in CCK.

It's not easy for the multigroup either as it also depends on other patches to other modules. And the other module authors won't move if this is not officially included in CCK, so... too bad. It's hard to make a decission...

I suggested at some point, the multigroup could be released as a separate module. That would make things a lot easier. If CCK maintainers agree on that, then I would be glad to maintain that separate project. But trying to keep things up to date on separate issues with hundreds of follow ups... nah, it's too much already for me. I have spent much more time here than I really had.

Roulion’s picture

FileSize
335.13 KB

There is a point with nestedfieldgroup and multigroup content.
I have a multigroupe which belongs to a standard group like :
-group1
--multigroup 1

When i click to the "add more value" button, instad of getting a new line, i have the screenshot...
I the multigroup in not nested, it works correctly

jthomasbailey’s picture

subscribing

and I vote for making it a separate module if that speeds things along.

yched’s picture

@markus_petrux : I understand your frustration and would probably feel the same in your position. Rest assured that I myself have spent much more time on CCK than I really had ;-).

We intend to roll a 2.2 release in the very next few days. Not sure about Karen's agenda, but I'll try to dedicate enough time for the ginormous amount of work that went into these patches as soon as I can - which might still be another week or so.

rconstantine’s picture

FileSize
466.52 KB

Tweaked a couple of things and caught a new bug in multigroup that was caused by the latest cck dev. I'll roll a proper patch in a few days.

markus_petrux’s picture

@yched: sorry if my post looked like a rant. I believe I understand the situation. It's not the same what we wish that what we can really accomplish. So I thought it was going to be better/honest to explain my situation.

I would be glad to spend a few more endless nights working on this, but this would need a clear position from your part as CCK maintainers. The reason is, for example, one of the things that remain to address is compatibility with filefield and multigroup. I started to look at that, but it needs some kind of involvement from the filefield developers. If they don't get involved, I can do little more... reference: #367267: Compatibility issues with Content Multigroup

Another reason is that it's really hard to maintain the multigroup patch up to date within an issue that's close to reach 400 follow ups.

If there was a separate project, it could be already committed to CVS, we could provide better documentation, we could better resolve situations with other users, better interact with affected projects, etc. It would save time to everyone, IMHO. Well, unless it could be managed from the CCK project itself, but that would require commits, maybe adding a component for multihroup to properly flag cck issues, officially involve authors of other modules, etc.

In summary, from my position, I believe it's better to wait for the next official CCK release, and then try to see what we can do next.

@ryan: sorry for hijacking the issue with this discussion.

rconstantine’s picture

@markus, I think yched has said that he'd like to see this all get into CCK. I don't think that anything needs to be a separate project. In fact, it's my opinion that these features would be great additions to Drupal core for D8 and they probably have a better chance if they stay here. These patches really open up a lot of options for content type creation. I agree with you about the difficulty of continuing on with issues that have so many posts. But if yched thinks he might be able to get these patches in next month, then any issues that may come up after that should be opened as new issues and the existing ones should then be closed. Keeping these large patches up to date with dev after dev is difficult as well. It would be more convenient if small patches were put on hold until after these could be properly looked at and included.

Also, no problem hijacking. I think that these patches have become intertwined enough that this is as good a place as any for this discussion. Plus it has less comments (for now).

ellen.davis’s picture

I have the below installed.

drupal 6.10
cck from cck.zip in #116
cck_fieldgroup_tabs.zip from http://drupal.org/node/325049#comment-1204939

This works well for multigroups in field group tabs. But I get the below error message when try to create anything that does not use cck, such as a blog entry.

warning: Invalid argument supplied for foreach() in .../drupal-6.10/sites/all/modules/cck/modules/fieldgroup/fieldgroup.module on line 404.

pokadan’s picture

Got a php error while setting up a multigroup with D 6.10 and the above (patched)CCK version.

in_array() [function.in-array]: Wrong datatype for second argument in C:\wapp\apps\drupalydo\sites\drupalydo\modules\cck\modules\content_multigroup\content_multigroup.module on line 184.

Since the code around line 184 seems to be concerned with displaying error messages I'd reckon I'll just ignore it..

rconstantine’s picture

@davis4104, I now see the same error, so I'll look at it. I thought I had tried that before and couldn't duplicate the error. Now that I see it, I should be able to do something about it. For now, if it doesn't hurt, you can add a field and then hide it using the display settings.

EDIT - OK, I got it. Will include the fix in the next zip next week.

firefire’s picture

In addition, can the nested field be nested?
For instance, to the following...

group_person
├─person_name
└─group_hobby
  ├hobby_name
  ├hobby_name
  ├...

group_person
├─person_name
└─group_hobby
  ├hobby_name
  ├...

rconstantine’s picture

Let's be clear, you can't nest fields, you can nest fieldgroups which contain fields. You can also nest multigroups. You cannot nest fieldgroups inside multigroups as Markus and I decided that didn't make sense.

You can presently nest 10 levels deep + the fields inside that level. That was an arbitrary choice and I believe the spot to change that is either a tpl file or theme function. I'm really only nesting three levels deep in my content types, with the top layer fieldgroups being cck_fieldgroup_tabs, the next layer being mostly "normal" fieldgroups and then sometimes having a third layer of fieldgroups and/or multigroups. My fields are both at the third layer (equal with the third layer of fieldgroups/multigroups) and inside the third layer, making a fourth layer.

Does that answer your question?

firefire’s picture

Thank you!
There is another one question.
When are these patches formally adopted to the CCK module?
Because it is very useful, I want to use it on my site.

rconstantine’s picture

RE #124, see the rest of the comments in this issue and the other two. Short answer, hopefully within the next month.

firefire’s picture

Thank you for your comment.
I read the comment holding out though I am not good at English.
Thank you for a wonderful project.

usa2k’s picture

Subscribing

liquidcms’s picture

subscribing.. great work guys.. :)

talatnat’s picture

subscribing (nested fieldgroups)

pokadan’s picture

Hi, I have an issue with cck fields not showing in different views in Drupal 6.10, Views 2.3, patched CCK (the one at comment #116).
Interestingly enough it's not the fields contained in a multigroup that are not showing (haven't had the need to display those in my views) but the other ones.

Has anyone else had this issue? Could it come from using the pathced CCK module or perhaps not doing a proper install of the patched CCK module(uninstall previous and only then install the patched one)

Thanks in advance.

pokadan’s picture

I discovered what the issue was and it has to do with pgsql not being able to use more that 63 chars as alias names.
http://drupal.org/node/371711

rconstantine’s picture

I forgot to mention here that I accidentally posted a newer version of the zip at http://drupal.org/node/119102#comment-1338674

But I'm going to try and get a new version out today which will include changes made by Markus.

Stay tuned.

rconstantine’s picture

FileSize
731.52 KB

This is a new zip, but not against latest dev. It does have Markus' changes. It seems to work just fine.

I'm posting this because I have meetings all afternoon and didn't want to just email myself the copy to work on at home and promise a newer version that I might not actually get to. If I can't merge with the latest dev in the next hour, then it will be either tomorrow or next week that I do.

The zip has a tar inside. Too big otherwise.

rconstantine’s picture

FileSize
364.13 KB

OK folks. This is a new zip against the latest dev. I think I've merged everything correctly. Please test. This has everything from all three patches.

I did this quickly, so if there are errors, let me know!!!

Again, this is a tar in a zip.

ellen.davis’s picture

I've come across these bugs with cck_11.zip.

Add a new content type. Add a multigroup to the content type. This error appears. However, the multigroup is still added.

warning: in_array() [function.in-array]: Wrong datatype for second argument in .../drupal-6.10/sites/all/modules/cck/modules/content_multigroup/content_multigroup.module on line 183.

When creating a node with a multigroup, the edit form does not show the delete icon when there is only one grouping of a multigroup. If you click 'Add more values', the edit icons appear for all groupings.

rconstantine’s picture

As for your first error, I'll look at that. [edit] I found that error. Fixed for next release. [/edit]

As for the second, there shouldn't be a delete icon when there is only one row remaining. That's how the new delete patch thingy works for all multiple fields and multigroups.

Roulion’s picture

FileSize
324.78 KB

i tried your cck_11 version and i still have a problem with some group.

I have a nested ultigroup with
a text fild - single
an imagefield
2 text field multiline

When I add a value, instead of having a new fieldset with the fields I have a listbox with 61; 0; 1 value inside...

Thats stange because I have another multigroup (in table) (not nested), with 2 textfields (one single and one multiline) and i don't have the problem.
I gess the pb might come from the imagefield (latest released version)

portulaca’s picture

subscribing

rconstantine’s picture

imagefield is kin to fieldfield which needs to be patched for multigroups. there is a patch already for filefield for sure. i don't know about imagefield. so check the imagefield issue queue and if there isn't an issue for this already, open it. perhaps reference the latest summary by Markus in the multigroup thread. i too would like to see imagefield fixed for this.

break9’s picture

subscribing. Thanks for all the hard work

rconstantine’s picture

FYI, I'm waiting for the multigroups patch to make it into CCK officially before I roll my next real patch. It appears that it's very close to making it in! Go Markus, go!

pokadan’s picture

Way to go guys!!! If the multigroups functionality makes it into CCK officially, would the patch still need to be applied separately ? Just wondering..

markus_petrux’s picture

@poka_dan: The patches will probably get in, at least this is how we're working on them, in the following order:

- #196421: Deleting unwanted multiple values / multiple values delta issues
- #119102: Combo field - group different fields into one
- #300084: Nested fieldgroup

BTW, I'll probably update the first two issues later.

@Ryan: Yep! I believe/hope we're getting closer... also wish the time spent here can benefit a lot of people, but that will really come when it gets in.

someone_cooler’s picture

subscribe

yched’s picture

rconstantine : is there an independant patch for just the nested groups feature against current CCK 2.x-dev somewhere, so that I can start looking into this while #196421: Deleting unwanted multiple values / multiple values delta issues moves ahead ?

raystin’s picture

In my project,I need to nested fieldgroup, but I used drupal 6.10 and cck.2.2,how can I use this patch? is there any way to let the netsted fieldgroup work In cck 2.2.

nguyenquocviet’s picture

Subscribe

rconstantine’s picture

@yched - no there isn't. the reason being that I modify the modified multigroup module. so until that gets locked down, does it make sense to roll a new patch? i think i outline somewhere what the changes to that module are. primarily, the big changes are a) how the AHAH stuff works and b) the building of the form using $element instead of the reference to $form. however, those are both kinda big issues with the current implementation of multigroups. i have not been tracking the changes going on in multigroups lately. i figured i would just go through it all again once committed to dev. it will probably be a couple of weeks before i can get to this if you do need it before multigroups is committed. sorry about that. this is just not my month as my time has not been my own.

mansspams’s picture

If #300084: Nested fieldgroup (http://drupal.org/node/300084) feature is easier to implement or is more ready than #119102: Combo field - group different fields into one (http://drupal.org/node/119102) it should get in faster, since many users may find Nested fieldgroups to be good solution for their needs.

squarecandy’s picture

CCK_11.zip worked well for me in terms of getting multiple levels of fieldgroups working. Great job - this has clearly been a lot of hard work.

Unfortunately I'm unable to use the (also still experimental) CCK Fieldgroup Tabs when the fields grouped as tabs are not at the root level (the only place regular CCK would expect them to be)... They just show up as the regular expanded CCK display when they are made into "subgroups".

Probably not something you or Fieldgroup Tabs folks want to worry about until this functionality makes it into CCK or a module. Just thought I would put it out there since I tried it.

rconstantine’s picture

@squarecandy - there is also a patch for cck fieldgroup tabs in the issue queue there for this. I wrote it, so it works.

squarecandy’s picture

Cool - I will check it out and let you know how it goes.
It's this one, right?
http://drupal.org/node/325049#comment-1204939

rconstantine’s picture

Yep. That's it.

Flying Drupalist’s picture

Subscribe

Trunkhorn’s picture

subscribe

squarecandy’s picture

FileSize
92.75 KB

Hi rconstantine et al.

There seems to be a problem with filefield when you use the patched version of cck above and choose "unlimited" for the number of files (creating the "add another item" functionality).

I started with a fresh drupal install to try and isolate the problem from the more complex set up where i ran into it. So:
New default drupal 6.12 + cck 6.x-2.2 (latest stable) + filefield 6.x-3.0
Added a new file upload field to the page content type, and set the # of items to unlimited: works as expected.

Using the same but with cck_11.zip from above, I get no file upload field, just the "add another item" button. When you hit that button, you get this:

    * warning: Invalid argument supplied for foreach() in /home/.bonzai/peterwise/garage.squarecandydesign.com/drupal/sites/all/modules/cck_11/includes/content.node_form.inc on line 328.
    * warning: array_keys() [function.array-keys]: The first argument should be an array in /home/.bonzai/peterwise/garage.squarecandydesign.com/drupal/sites/all/modules/cck_11/includes/content.node_form.inc on line 336.
    * warning: Wrong parameter count for max() in /home/.bonzai/peterwise/garage.squarecandydesign.com/drupal/sites/all/modules/cck_11/includes/content.node_form.inc on line 336.

My work around for now is to just limit the user to 10 files - it seems to work fine with a set number of items.

Also note that the "unlimited/add another item" worked fine with a text field that I tried - this is specifically a problem with filefield and cck_11.zip.

I hope this is helpful in the fine tuning of this patch. I'm definitely an advocate for it being worked into future cck official releases. Let me know how else I can help with this...

Thanks!

RainbowArray’s picture

+1

jrosen’s picture

Subscribing...

Aren Cambre’s picture

subscribe

rconstantine’s picture

Looks like I need to re-roll this for the 3.x dev. I should be able to start later this week.

markus_petrux’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev
Status: Needs review » Needs work

Sure :-)

Now, probably there will be revisions on how the remove button is implemented, but I believe we can proceed to evaluate the nested fieldgroups patch against the new experimental branch.

Hopefully, it will be easier for everyone to apply the patch provided here because the other monsters are already in place.

PS: Changing issue status. This is for the new experimental branch.

rconstantine’s picture

Just downloaded latest 3.x to start looking. Wish me luck.

Flying Drupalist’s picture

Good luck rconstantine!

bfsworks’s picture

rconstantine,

How has been your luck? Looking for an update on your progress as it looks like the older patches might not mesh well with markus's newly merged dev branch.

rconstantine’s picture

I'm a bit more than halfway done porting things over to the new versions of the files. My patch touches more files than the multigroup patch does though the overall code is less IIRC. I am doing this at work, for work, but have only been able to work on it in my 'free' time. I'm expecting to get to the rest next week, at which point if it looks pretty good I'll release the patch for testing along with a bundled tar like before. I'll be glad to get this more or less off of my plate. I want this done before I begin work on V2 of an internal project which I'm to start shortly (once the feature list is finalized and prioritized). Cool?

I'm also hoping that a bug I recently found with multigroups isn't really seated in my patch and that due to changes in multigroups itself that it will just go away. The issue relates to reordering and deleting nested multigroup rows - which is why it could be a nested fieldgroup issue. I hope not. We'll see. But if that's the only bug, I'll still release the patch for review so we can all test/patch my patch.

portulaca’s picture

thank you for the update rconstantine, I'd love to test when you publish it

MGParisi’s picture

#167

not to be a bit... um... frustrated?

But issue #108753: Allow fieldgroups to be nested is active, with a active community full of people working on actual patches... yet was marked as Duplicate :( Still, with this issue having its status changed, the patches continue to flow.

I don't know what is going on (for I am not in development of this project), but it maybe a good idea to get everyone on the same page.

MGParisi’s picture

BTW #108753: Allow fieldgroups to be nested patch is for 5.x

is this a Duplicate? Can we get this ported, and also have an upgrade path?

Thanks for your hard work.
Mike

markus_petrux’s picture

@MGParisi: Yes, that issue was marked as duplicate of this one. I have associated that issue for CCK 5.x-1.x-dev, as it contains a patch for that version of CCK.

The work here, for CCK/Drupal 6, will be committed only to the experimental branch, CCK3, aka 6.x-3.x-dev. At least for the moment, there's no plan to backport this to CCK2. Also, I'm afraid, this feature won't be backported to CCK for D5, which is feature frozen. It is not possible to maintain so many different versions of CCK, when D7 is so close, and so many things yet to do.

rconstantine’s picture

Just a quick update folks. I 'finished' updating the files, then noticed a new 3.x version came out, so I have updated my copy as well. Tomorrow I'll start testing. Hopefully I merged the files correctly altogether. Things have changed a lot since my last tar I posted.

Gábor Mayer’s picture

it doesn't works with CCK Fieldgroup Tabs.

rconstantine’s picture

There's a patch for that.

@all - I'm spending the rest of the day on this. I should have a post this afternoon for you to test.

markus_petrux’s picture

Sweet, Ryan. Please, note that the fieldgroup module has been changed quite recently in its implementation of hook_nodeapi(). Here's the culprit: #495582: Panels integration for multigroups

Cheers

rconstantine’s picture

yeah, you threw me a curve ball. but i have it integrated i think. i need to finish testing. i am still testing admin screens. i had screwed up the merge of one file but figured out what i did wrong. the main admin screen (manage fields) looks and seems fine now. i'll produce a few tests there, then make sure the display tab is fine too, then i'll test node creation and editing. if all is well, i'll post. thought i'd get that done today. i'm trying for tomorrow.

by the way, i'll look at the code change you mention, but could you sum up what the changes do? sometimes it's easier to identify wrong behavior when code is described as behavior in the first place rather than trying to determine what it does. know what i mean?

rconstantine’s picture

Status: Needs work » Needs review
FileSize
409.16 KB

Testers needed!

Attached is a tar of the merged files using 3.x from the 12th of July. I'll attach a proper patch later. I have meetings ALL day!!! Argh!

Anyway, my initial testing looks good. Operations appear as I expect them, and the DB entries look good. I'll be doing more testing (hopefully) today and tomorrow, but I thought you guys could help out too!

For those that read and understand code, it would be great if you could pinpoint any problems to specific lines of code.

Thanks.

markus_petrux’s picture

Status: Needs review » Needs work

Please, provide this in patch form against current CVS code in CCK3 branch.

The tar may help others test it, but we need to evaluate the code changes before it is committed.

rconstantine’s picture

Markus, you're so impatient. I said I would post it later. ;-)

rconstantine’s picture

Status: Needs work » Needs review
FileSize
51.02 KB

Ok. I'm out of one meeting. Here's a patch. Hopefully it's correct. I checked out the 3.x branch from CVS and then copied my changed files on top, then used the 'make patch' function of TortoiseCVS.

Let me know if it's wrong.

rconstantine’s picture

Well, I found one bug. Multigroups aren't displaying as columns in the node edit screen. But view screen is OK. I'll look at this now.

markus_petrux’s picture

Status: Needs review » Needs work

Ryan, please excuse me for my post before. I a bit tired today, and I missed your comment. :(

Regarding the patch... please note the code that was in fieldgroup_nodeapi('view') is now implemented by fieldgroup_build_content(). There have been a few changes in the CVS recently.

This is so complex that we will need help testing and as much feedback as possible.

Thanks for the hard work :)

rconstantine’s picture

Status: Needs work » Needs review

Oh, crap. I added your new function but didn't change the existing. I'll look at that too. Looks like a PITA.

markus_petrux’s picture

Status: Needs review » Needs work

No problem.

In regards to possible commits related to this feature, we need to proceed with caution. The CCK3 branch is now being used by more than 1500 sites (CCK project stats), which is more than most of the modules published here at d.o., and a commit here is pretty much like a new release, so we should not generate too much noise. This is a added handicap now, but I feel happy for being able to take advantage of the official CCK repository, which should provide more stability in the end for all of us.

rconstantine’s picture

Status: Needs work » Needs review

Did you just move the contents from the one to the other? - the two functions I mean.

[EDIT] Nevermind. I see now. Hmm. Where can I add my code? I see that fieldgroup_view_group calls fieldgroup_build_content. But that doesn't work on all of the groups at once. Is build_content called from somewhere else too? The notes mention that view_group should be used by outside modules. How does CCK itself call build_content? I'm still looking...

[EDIT AGAIN] Nevermind again. I'm a dummy. Of course it's in nodeapi.

markus_petrux’s picture

Status: Needs review » Needs work

I think you should focus on fieldgroup_build_content(). This function is pretty much what it was in fieldgroup_nodeapi('view'). Please, recheck your changes there as I don't recall if I had to change/add code in there.

Now, fieldgroup_build_content() is used by fieldgroup_nodeapi('view'), and also used by fieldgroup_view_group() that can be used by other modules to get the fully rendered output for a particular group. It is the same as content_view_field(), but now able to do the job for field groups (and multigroups, of course). For example, fieldgroup_view_group() is used by the field group integration with Panels 3.

Let's go back to fieldgroup_build_content()... this function processes a single field group, which is just fine, but to deal with nested fieldgroups, it needs to become a recursive function of some sort, so it is able to build the content for the given group regardless of the nested level it is defined.

PS: Please, get a fresh page for this issue so the status can remain as "needs work". lol

rconstantine’s picture

Thanks Markus. I straightened out the code. Was easy actually. Now I just have one more item to fix, then I will post a new tar and patch.

rconstantine’s picture

FileSize
48.25 KB

Here is a new patch. I think everything works now. Continuing testing.

rconstantine’s picture

FileSize
407.98 KB

And the tar...

rconstantine’s picture

Status: Needs work » Needs review

I guess a new patch means it needs review again.

rconstantine’s picture

Side note - if you want fieldgroup tabs to work, use this patch: http://drupal.org/node/325049#comment-1204939

rconstantine’s picture

As a couple of examples of the tabs (both images) and multigroups/nested fieldgroups (one image) see the attached images.

sproot’s picture

Used the tar file in my dev site, and it's working for me.

Only three levels deep, but multiple subgroups mixed with fields, drag and drop nested groups into subgroups, delete groups with content. Tried pretty hard to break it in fact, but both edit and display are working great, going to move it into my test site now and give it a proper workout.

Thanks for your hard work on this Ryan, it's appreciated :)

EDIT: Found an issue, only a small one though: Within a group, subgroups always appear before fields in the 'view' display, they appear in the correct order in the 'edit' display though. It isn't a problem at the root level though, only within a group.

Example:
Configuration (F=field, G=Group, content of G2 and G3 not shown, F2,G2,F3,G3 are content of G1)
F1
G1
--F2
--G2
--F3
--G3
F4

When in 'view' display items will appear as:
F1
G1
--G2
--G3
--F2
--F3
F4

Note the order difference within G1.

Using D6.13 and the tar file from #187, with pre-existing fields on a pre-existing content type though. I can try with newly created ones if you think it will make a difference?

rconstantine’s picture

Status: Needs review » Needs work

Thanks for the bug report. I'll look at that. I seem to recall fixing that before, but since things have changed a lot since then I could have missed merging those changes. But if that's all anyone finds wrong with it, I'll be tickled.

I'm curious, will you be using CCK Fieldgroup Tabs like in my screen shots? I'm just wondering how many people will be.

markus_petrux’s picture

I've been trying to digest this little monster, and while I'm still trying when I have time, here's a few notes:

  • Regarding function _fieldgroup_field_get_parents():
    • This looks like something that could be used somewhere else, so maybe we can remove the first underscore?
    • Field names are prefixed by 'field_', and groups are prefixed by 'group_', so maybe we can remove the argument $is_group?
    • Where we use the variable $result, it really means a group name, so I would rename that var to $group_name.
    • Do we really need $counter? Couldn't we initialize the $parents array on top (this ensures an array is returned), and then do $parents[] = $foo;
  • Since nested fieldgroups will not be compatible with any other module that manipulates the form, hence we have introduced content_get_form_element() and content_set_form_element(). Ideally, we would have to port these to CCK2, and then find *critical* modules that would be broken in CCK3 (specially filefield) so they can be adapted to use these functions.
  • I feel somehow lost when looking at all those computations with the depth and weight of elements. Couldn't this be abstracted/optimized in some way so that we can isolate the complex stuff on a well documented function or something on that direction? I think this is the most complex part here, and we need to make the life of the people who will maintain this in the future (or port to D7) as easy as possible, IMHO.

Also, I would like to see opinions from yched/KarenS on this before it is committed. We'll have to port this to D7, most probably, so they have the final word.

rconstantine’s picture

Markus,

Glad to see you're reading it!!!

To your points:
-Feel free to remove the underscore. I'll leave that up to your discretion. You are the maintainer after all.
-I think it's easier to pass in a TRUE/FALSE rather than parse a string - a bit less overhead, no?
-I have simply seen $results and $result used all over Drupal without regard to the actual output. I thought it was a standard.
-I thought I had an issue with that at some point, but looking at it now, I can't think of what it could've been. Your way is what I typically do.

-Most modules will work if they follow the model of the 'typical' core cck modules. But you're right, some, like filefield may need to be updated. Since 3.x is not in production, I would suggest keeping it unofficial until those modules are able to catch up or something. They should at least have a patch available in their queue. I did that for the cck fieldgroup tabs module - i.e. there is a patch waiting to be committed as soon as 3.x goes official.

-I can understand that you'd think the weight computations are complex. If you do an output (drupal_set_message) just after each weight is assigned, you can see how they are built fairly easily, actually. I think I based how I built it on the weights that Drupal was assigning. You may know that although standard CCK stores ints for weights, Drupal actually assigns decimals so that those items with the same weight can be ordered. I just intercept that process and come up with the decimal numbers myself because the results from Drupal were not always correct due to the nesting and multiple nesting. Would you like me to add notes and maybe add commented-out drupal_set_message lines that would be kept there permanently?

Do you want me to make these changes or do you want to?

rconstantine’s picture

@sproot-

Add the following lines after line 833 (should be a line that says: $element[$field_name] = $node->content[$field_name];)

$element[$field_name]['#weight'] = $field['weight'];
$element[$field_name]['#depth'] = $field['depth'];

I'll include it in the next patch after Markus tells me what other changes he wants, but I figured you could test it now.

markus_petrux’s picture

In regards to the impact with other modules, I think we would have to take functions content_get_form_element() and content_set_form_element(), and deploy them in CCK2. When that happens, CCK fields that manipulate the form can use them, and this should abstract them the fact that a field can be located on different places in the form, and do it in a way that works for CCK2 and CCK3.

I think these functions are ready to go, but since this plan affects other modules and the *official* branch of CCK, then I would prefer to wait for a review from yched/KarenS. A review on these functions for CCK2, and the plan to deploy them in CCK2 to provide the same method as in CCK3 for other modules to interact with the form.

This is critical enough to keep this moving further... otherwise, we'll have to assume the nested fieldgroups may not work for some fields, and that's a problem we have to address in one way or another.

In regards to the proposed changes to current patch... I'm afraid if I spend too much time here someone is going to kill me here at the office.

rconstantine’s picture

I'll make the changes then. I'm booked through the rest of the day. Will get to them tomorrow.

markus_petrux’s picture

I was trying to figure out what happens with in _content_overview_order(). It looks to me this could be simplified a little. Here we build a pseudo form, $dummy, where each element is nested in its own level. The elements only contain their name, and when rendered, it is converted into an array that's used by the fields overview themes to render rows in a table.

I think the weights and depths do not need to be computed. This information should be ready in the field/group structure, and then used here. All we would have to do in this function is build the nested array with the proper structure, and then drupal_render() will do the rest.

If we have weight and depth in field/group structures, we don't need to compute them every time we need to render a field group.

rconstantine’s picture

I seem to recall that without doing _content_overview_order(), I was getting the crazy weights that Drupal produces automatically. You can see what I mean by starting without this patch installed. With JS enabled, move fields and groups around on a CCK type without moving fields from group to group. Save. Turn off JS and edit again. Note the weights. Turn JS back on, and reload the page. Reorder the fields and groups again and save. Turn off JS again and then edit again. Note the weights. They aren't the same. You haven't added or taken away any fields or even moved them from one group to another, yet you don't get consistent numbering.

More specifically, if you have two fields which start with weights of -1 and 0, then move the 0 above the -1, you'll get -2, -1. Do it again, you get -3, -2 and so on. Same going the other direction. Why? It's possible to have only two fields in a content type with weights in the hundreds.

That's why I elected to order things each time from scratch. I do realize that I almost duplicate the same code 3 times to do so. I'd love to pull it out as a function, but each iteration was enough different that I couldn't figure out how best to do that. Ideas?

[EDIT] Actually, it looks like this function is actually reused a few times, but the fieldgroup_form_alter version and the fieldgroup_nodeapi version are stand-alone. Perhaps those two should be made to use _content_overview_order()? It's been so long that I don't remember all of the design choices. I'm sure you can relate.

markus_petrux’s picture

How about renumbering the weights when the "Manage fields" screen is submitted?

[EDIT] The function _content_overview_order() cannot be used on node form/view. It is meant to help ordering the items when they are about to be listed on a table, which is a different use-case.

rconstantine’s picture

You lost me. What? I think I do order them at submission. There are two orderings going on, which mirror and replace the two standard orderings.

First, on the Manage Fields page, when you move fields and groups around, the weights are given integer values. Like I said before, they are pretty crazy. Indenting doesn't change the weight in regular CCK, so if an item is weight 456 and you indent it and put it in a fieldgroup, it is still weight 456. Later fields added to the same group will then be based on that number.

Instead, I only allow the outer-most set of fields and groups to have crazy positive and negative numbers, though I believe I do remove any gaps so that the results are consecutive. Anyway, each indentation results in a fresh set of weights starting with 0 and going up from there. That's what is stored in the database. So a person can actually look at the DB entries and easily re-construct the ordering by hand including parentage.

The second kind of ordering is to ensure correct display on the screen according to Drupal's format. What I found, like I said before, was that Drupal takes integer weights and then turns them into decimal weights so as to order items with the same weight correctly. Nesting and starting the weight numbers over for every group in every layer screws Drupal up. Because I (logically) start over the weight numbering for each group, Drupal, on its own, will pull fields out of groups if they share the same weight and lump them together. So, for example, given 3 groups nested inside each other, the first field in each which has a weight of 0 would be pulled out of the groups and listed at the root depth together if regular Drupal was allowed to do so. This is one reason why tracking the depths and using them to figure ordering is so important. I think I actually grabbed the algorithm I'm using from an old college text book of mine.

So what are your concerns and what would you like me to do about them? Like yched did with you and your patch, just tell me what you want in order to get it in. I'm eager.

markus_petrux’s picture

Really, I think it is not necessary. Drupal orders the elements in drupal_render(), just before executing pre_render callbacks, if any. drupal_render() is recursive function that process a particular level of the structure on each pass. When sorting, it uses uasort($elements, "element_sort"); if that level does not have the attribute #sorted enabled. This attribute is enabled by FAPI in form_builder, but disabled as soon as any element in the form has a #weight, which is the case in node edit forms. So it is drupal_render() who will order each level in the form structure independently, as per the weights assigned to form elements.

That means we can give each field/group in the form (or in node view content) the weight that was stored during a "Manage fields" screen submit, and we do not need to compute the weights again and again to build the form or node view structures. If we also store the depth, then we don't need to compute this either.

If we need to compute weights/depths to build node edit form or node view, then this means the settings we have stored for the fields/groups are missing something.

Note that the original code in _content_overview_order() does not compute weights. It just gets what there is in the field/group structures.

rconstantine’s picture

FileSize
972 KB
970.47 KB

Markus,

I'd like to see that work. What I've been saying this whole time is that drupal_render() effs it up. I certainly would not have coded all that junk if it had worked in the first place. It definitely was not the first thing I tried to do. In fact, with reasonable assurance, I'm pretty sure that the whole weight-upon-view issue was the last thing I tackled. As I recall, the first thing I did was the Manage Fields screen and getting it to store the weights and depths as I thought they should be saved. I think we agree that this is the critical area. You're suggesting that if this is being done correctly, which it is, that we shouldn't have to further process anything elsewhere. That wasn't my experience in the first place. However, I'd love to try to get it to work with just drupal_render(). How do you think we can try that out? We have identified basically three areas where I am adjusting weights and depths for display purposes. Which of those three should be looked at first?

[Side note: I actually don't calculate depth each time but I do move it from one element to another. This is supposed to mirror how Drupal itself moves around the weight. Drupal sometimes has weight, other times #weight in various locations. I try to put mine in the same spots that Drupal would have - I think.]

As for the original _content_overview_order() - I know. But the nesting messes it all up.

So let me know which function you'd like me to try to remove first - _content_overview_order(). or fieldgroup_form_alter(), or fieldgroup_nodeapi(). And also let me know if you have some idea what to change within the calling function as well (if applicable).

Thanks.

... I just swapped out the current _content_overview_order() function for the original, and if you do it, you'll see that it does not allow nesting. See the screen shots. Ideas?

BTW, the Display Fields tab looks similar to this mess when the function is swapped.

markus_petrux’s picture

... I just swapped out the current _content_overview_order() function for the original, and if you do it, you'll see that it does not allow nesting. See the screen shots. Ideas?

I think all _content_overview_order() needs is build the $dummy array with the levels defined in "Manage fields" screen.

The old function uses 2 levels, we now need to use as many levels as needed. Each group and each field in its proper place in the structure, as if it was a form. Then drupal_render() will order each level of the structure independently. There's no need to compute weights, only put the groups/fields in the correct level of the structure.

The '#depth' attribute in the $form can be set in content_field_overview_form() with the values taken from the field/group structures (that were saved from the "Manage fields" screen submit).

rconstantine’s picture

OK. So I have removed the weight stuff thus:

/**
 * Helper function to order fields and groups when theming (preprocessing)
 * overview forms.
 *
 * The $form is passed by reference because we assign depths as parenting
 * relationships are sorted out.
 */
function _content_overview_order(&$form, $field_rows, $group_rows) {
  // Put weight and parenting values into a $dummy render structure
  $dummy = array();
  // Group rows: account for weight.
  if (module_exists('fieldgroup')) {
    $max_depth = 0;
    foreach ($group_rows as $name) {
      $depth = $form[$name]['group']['#value']['depth'];
      if ($depth > $max_depth) {
        $max_depth = $depth;
      }
      $parent = $form[$name]['parent']['#value'];
      $dummy[$name] = array('#weight' => $form[$name]['weight']['#value'], '#value' => $name .' ');
      $form[$name]['#depth'] = $depth;
    }
  }
  // Field rows : account for weight and parenting.
  foreach ($field_rows as $name) {
    $dummy[$name] = array('#weight' => $form[$name]['weight']['#value'], '#value' => $name .' ');
    if (module_exists('fieldgroup')) {
      if ($parent = $form[$name]['parent']['#value']) {
        $depth = $form[$parent]['group']['#value']['depth'] + 1;
        $form[$name]['#depth'] = $depth;
        $dummy[$parent][$name] = $dummy[$name];
        unset($dummy[$name]);
      }
    }
  }
  //we have to do this at the end so as to cascade these moves bottom-up rather than use a recursive function top-down
  if (module_exists('fieldgroup')) {
    while ($max_depth >= 0) {
      foreach ($group_rows as $name) {
        if ($form[$name]['group']['#value']['depth'] == $max_depth) {
          $parent = $form[$name]['parent']['#value'];
          if (isset($parent) && $parent != '') {
            $dummy[$parent][$name] = $dummy[$name];
            unset($dummy[$name]);
          }
        }
      }
      $max_depth--;
    }
  }
  //drupal_set_message('<pre>Dummy: ' .print_r($dummy, TRUE). '</pre>');
  return $dummy ? explode(' ', trim(drupal_render($dummy))) : array();
}

And surprise! It seems to work. I'm still testing. Do you see any other changes here that need making?

rconstantine’s picture

FileSize
23.42 KB

Everybody, I'm attaching replacement files for two files from the previous tar above (#187). You'll have to install that and then overwrite with these. Please test as they contain changes that hopefully don't change anything, but reduces the code length.

@Markus, I'll try to get a patch out early next week with the changes. You may want to diff the two versions of files yourself just for your own edification. I pulled out some of the code from the form_alter and nodeapi functions that was in common and put it in a separate function. I yanked the weight stuff from there as well. It may be that those two functions can be better merged.

I must admit I was wrong (it looks like). At some point I must have mixed up weights and depth/parentage and their relationships. Thanks for your diligence in hounding me about it if indeed this works.

magnestyuk’s picture

subscribe

rconstantine’s picture

FileSize
46.05 KB

OK guys. I think I've updated the patch ton include the latest changes.

Markus, please let me know what else you want changed. I'll review the thread so far and see if I missed anything. Probably did.

dtarc’s picture

I've just enabled Content Multigroup from the 6.x-3.x branch and i'm trying to figure out how to get neted subgroups working.

I'm having problems applying the patch in #208, i think because i took the dev snapshot from today (July 21) rather than checking out from cvs. Do i also need to apply the 2 fieldgroup patchs in the original post?

I'm just finding it hard in this long thread to determine what i have to do to get the feature working.

rconstantine’s picture

Just grab #187 and overwrite with #206. The patches are for the maintainers' benefit. Run the update script that Drupal has (in the admin area) and maybe resave all your content types' "Manage fields" pages.

ellen.davis’s picture

I was at
drupal 6.12
CCK http://drupal.org/node/300084 #134 Cck_11.zip
CCK_FIELDGROUP_TABS http://drupal.org/node/325049#comment-1204939 comment #6 - cck_fieldgroup_tabs_1.zip
CCK/Tabs 6.x-1.0

Then updated to
drupal 6.13
cck http://drupal.org/node/300084#comment-1811964 #187 cck.tar.gz, with patch from http://drupal.org/node/300084#comment-1811964 (#206)

When I view a node that has mutligroups, it is fine. But when I edit, I get this error.

warning: array_keys() [function.array-keys]: The first argument should be an array in .../drupal-6.13/sites/all/modules/cck/modules/content_multigroup/content_multigroup.node_form.inc on line 143.

rconstantine’s picture

I haven't seen that one before. I too upgraded - from 6.8 to 6.13 and (I think) the same cck_11.zip.

I assume you took care to replace the files from #206 where they previously were. I didn't zip them with their paths in tact.

I'm looking now...

Oh, I see. Not sure how I missed that. Replace the beginning of the following function:

function content_multigroup_group_form(&$form, &$form_state, $group, $delta) {
  module_load_include('inc', 'content', 'includes/content.node_form');
  $element = array();
  $type_name = $group['type_name'];
  $content_type = content_types($type_name);
  $group_name = $group['group_name'];

  if (!isset($form[$group_name])) {//nested AHAH, not initial build
    $element[$group_name] = content_get_form_element($group_name, $type_name, $form, TRUE);
  }
  else {//initial build (via content_multigroup_fieldgroup_form) or non-nested AHAH
    $element[$group_name] = $form[$group_name];
  }
  if (($group['group_type'] != 'multigroup') 
      || (!(empty($element[$group['group_name']]['#access'])) && $element[$group['group_name']]['#access'] != TRUE) 
      || empty($element[$group['group_name']])) {
    return;
  }

  $group_fields = $form['#multigroups'][$group_name];
  $element[$group_name]['#fields'] = array_keys($group_fields);
  $node = $form['#node'];
  $group_multiple = $group['settings']['multigroup']['multiple'];

  foreach ($group_fields as $field_name => $field) {

I'll roll new patches and stuff on Monday. No, wait. It looks like the patch has this fix in it. I'll roll a new zip on Monday then.

zeno129’s picture

Hello,

I read most of this thread and at the moment I am unsure of which patch I should use. Could anyone please indicate me which one?

Thank you!

PS. Great work! :)

leo.ruffini’s picture

Hi,

I've installed everything as explained in #210 and all seems to go fine but then I can't nest a multigroup inside another (the GUI in Manage Fields just doesn't let me put one inside another).

What I'm exactly trying to do is to nest a multigroup called "Task" inside another multigroup called "Workpackage". This is how it would look like

Multigroup Workpackage
----Multigroup Task
----------Field Responsible
----------Field Description
----------Field Date

This way I could have

Workpackage 1
----Task 1.1
----------Responsible
----------Description
----------Field Date
----Task 1.2
----------Responsible
----------Description
----------Date

Workpackage 2
----Task 2.1
----------Responsible
----------Description
----------Date
----Task 2.2
----------Responsible
----------Description
----------Date

...

Is it possible? Am I doing something wrong?

Thanks.

TomSherlock’s picture

Hello, rconstantine.

I also got the same error as #211. When i applied your code change from #212, i got nothing but white screens.

I have Drupal 6.13 as well as your cck_fieldgroup_tabs and the latest version of admin menu (6.x-3.0-alpha2)

Subsequently, i backed out the code from #212 and shut off the multi-group module.
Then i downloaded the latest cck (6.x-3.x-dev (2009-Aug-03)
Not sure if your patch is compatible with this version of cck.

I'm going to roll back to my previous set up and start all over.

Tom

TomSherlock’s picture

rconstantine,
since you're asking how CCK Fieldgroup Tabs will be used...

My usage will be similar although not exactly as your screen shot.

I would like to use both the Fieldgroup tabs and nested fieldgroups along with content_profile on the user registration page for PTA. So the tabs would look something like this:

Account | Profile

                _________________________________________________
__________| Parent | Teacher | School Admin | School Staff | Student |___________________

Each tab would then have two or three nested fieldgroups.

i'm going live with this first week September. So hopefully there will be something available and i'll have figured out how to set things up.

Would you be able to create written or video tutorials? :)

Thanks for your efforts, time and talent.

Tom

rconstantine’s picture

OK folks, I've had server problems at work so I haven't given you a proper new patch and tar. Will do so next week.
Thanks for your patience and feedback.

TomSherlock’s picture

Thanks, rconstantine.

Looking forward to it.

TomSherlock’s picture

I realize that the combination of #187 + #206 + #212 may not be functioning the way you intended. Nevertheless, here are a few snapshots of what i've done so far. For some reason i'm only only able to get a single subgroup within a single multigroup. The only way i'm able to get more than a single subgroup within a multigroup is by clicking on the button 'Add more values'. However this creates a duplicate structure of subgroup 1.

What i was hoping to see was multiple subgroups nested within a multigroup with each nested subgroup structured differently.
For example, i would have PTA multigroup which would have an Executive Board subgroup and a Committee subgroup. The EB subgroup would have a check box and a select list. The Committee subgroup would have a checkbox and select list for committee membership and a select list for committee chairs. Different structures.

I've included screenshots.

Also got the following error for field of type content_taxonomy:

This change is not allowed. The field Test Field Ten handles multiple values differently than the Content module. Making this change could result in the loss of data.

I set the multigroup to a repetition of one to create a single instance of that subgroup. Nevertheless i am still unable to include a content_taxonomy field.

I definitely like the option of creating multiple instances of a subfieldgroup. I have a use case for this: a parent recording the name, grade and teacher of each student she has in the school. However, i also have the other described above

TomSherlock’s picture

rconstantine,
Just want to add that your right.jpg in #203 is what i was also hoping for.

I see that Drupal functions are frustrating your efforts. Hopefully you and Markus have figured something out.

Edit: Just read about a third way through the thread. Definitely a labor of love. Thank you for persevering.

Good Luck

leo.ruffini’s picture

Thanks, rconstantine.

But can I nest a multigroup inside another one, as I'm describing in #214?
I'm not sure if I now I can't because of a bug or because it is not possible at all...
Leo

rconstantine’s picture

No nested multigroups. That's an issue with multigroups. If you find the original multigroups thread, you'll see where that is discussed.

That would be something to tackle after this patch goes in.

TomSherlock’s picture

No nested multigroups

Is that different from nested fieldgroups?
Will we have nested fieldgroups soon?

zeno129’s picture

I'm having trouble with the order of the fields inside the multigroup.
I save them in the order I want and when the page refreshes they appear mixed up.

Has this happened to anyone else?

rconstantine’s picture

I'm grabbing the latest from the repository, so I'll make the new patch ASAP.

TomSherlock’s picture

Thanks, rconstantine. Looking forward to it. My alternative, CCK Fieldgroup tab, appears to be interfering with the preview and save edits process of content profile. Also being able to do nested fields just looks betters.

rconstantine’s picture

FileSize
434.57 KB
47.22 KB

I need help guys. I merged with the latest code and something is broken. I get a WSOD.

I'll setup a blank site later this week, but if anyone can spot a problem, let me know.

I carefully went line by line and it all looks good to me. Maybe I'll compare my last patch to this one.

TomSherlock’s picture

What does your error log say?

rconstantine’s picture

Thanks for the reminder to check the basics. It was the end of the day for me yesterday and I was anxious to leave. Of course, the PHP log specified the offending line. I'll fix it and retry. Then I'll repost.

rconstantine’s picture

Status: Needs work » Needs review
FileSize
434.57 KB
47.16 KB

And here we go. Should work now. Please test.

Sc0tt’s picture

Thanks very much for this. Exactly what I need. I'm not an engineer so this might be a dumb question but why not just make this a module. I think it would be easier for people to find if it were listed in the modules. It took me a long time to find this patch. Also, I think a patch scares a lot of people because they don't know if there will be continued support beyond the current version of the main module that's being patched.
Thanks again!
Scott

rconstantine’s picture

@Sc0tt, this is a patch for a module currently shipping with CCK. When you download the regular CCK module, you also get the fieldgroups module. This extends the capabilities and the goal of this thread is to get these changes tested and approved for inclusion into CCK itself - so that everyone who uses CCK will feel the warmth and love of nested fieldgroups.

As for patches, I know people are scared of them, so that's why I always include a zip of the whole CCK set of modules - just how it would be if it were already part of CCK. So open the tar.gz and play with it. Then give your feed back here.

Sc0tt’s picture

@rconstantine, Thank you. I didn't realize fieldgroups is going to be part of CCK. Thanks very much for your detailed explanation and all that you do here for Drupal.
Scott

Sc0tt’s picture

Category: feature » bug
FileSize
100.3 KB
100.36 KB
101.81 KB

I don't know if this issue has been posted before or if this is even the proper place to post this; but I'm having a problem with "a standard group that get 'stuck' within another standard group". You can see in screenshot #1 that I have the standard group called "Stock Image" a sub-group of "Slot #1". You can see in screenshot #2 that I am able to drag with group out of the sub-group of "Slot #1" however after saving changes, the "Stock Image" is still a sub-group of "Slot #1". Is this a bug? or by design? Is there a proper way to separate standard groups?
Thanks very much,
Scott

PS. I'm using the cck.tar_.gz 434.57 KB version from post #230.

ndm’s picture

Works fine, very thanks

rconstantine’s picture

Anyone else seeing this? I haven't seen this before. I'll play around myself. My live sites are all using an older 3.x version as posted higher up. I do have a test site with this version on it though.

Sc0tt’s picture

rconstantine, happy to give you access to my site to see for yourself and investigate. Will send message via contact form.
Scott

Sc0tt’s picture

rconstantine, i discovered that the problem described in my previous post (#234) only appear in IE8! Duh and Sorry. I should have thought to test this before alerting you. Works fine in the latest version of FF. Thanks for your help and your contributions to Drupal.
Scott

rconstantine’s picture

I didn't get a chance to play on your site yet but didn't forget. Do you access to another machine with IE7 on it? Or other browsers? We'd probably still need to isolate the issue - though if it's on the admin side only, developers should only be using a non-MS browser anyway!!!

TomSherlock’s picture

We'll i've finally downloaded the latest version.

I need some direction, however.

I have an existing standard group called Address. It doesn't look like i can include it within a multi-group.
I've gone into a multi-group and noticed that there are fields for subgroups.

I 've created two subgroups: Address and Contact Information.

It is not clear to me how to divide my cck fields between the two subgroups. Everything is now under Address.

suggestions?

Thanks,
Tom

mesh’s picture

found an error / bug (?) with the cck.tar_.gz package you provided.

upon clicking 'Add more values' in the multigroup on node-edit form, i received this --

warning: array_merge_recursive() [function.array-merge-recursive]: recursion detected in  modules\cck\modules\content_multigroup\content_multigroup.node_form.inc on line 768.

this is what resides on line 768 --

  $output_js = isset($javascript['setting']) ? '<script type="text/javascript">jQuery.extend(Drupal.settings, '. drupal_to_js(call_user_func_array('array_merge_recursive', $javascript['setting'])) .');</script>' : '';

i tried commenting off the line, and the error is gone. but the line must have some meaning :)
any fixes need to done here? or does it have to do with php?

-jayesh

SeanBannister’s picture

Sub

bomarmonk’s picture

Subscribe

mani.atico’s picture

I'm having some troubles too.

I installed the module posted on #230 and executed update.php.

However I already had a fieldgroup set on a content type. When I tried to edit the fields the following error was triggered:

Fatal error: Cannot use string offset as an array in C:\xxxxxxxxx\drupal\sites\all\modules\cck\modules\fieldgroup\fieldgroup.module on line 275

I disabled and uninstalled the fieldgroup module, re-enabled it but still the problem persists.

tijeika’s picture

FileSize
9.92 KB

help me! need solution as quick as possible!
i have multigroup, the standart group in it and a field in multigroup (which doesnt belongs to any other groupfield), and i cant display the standart groupfield. only field without groups shows.

all i need - is fieldset feature for text area for song lirycs:

Multigroup (it can add a group of items at onse for adding both fields "song name" and "lyrics")
  |__Field "Song name"
  |__Group (need this, becouse of its collapsable feature)
      |__Field "Lyrics"

So it must be looks like on picture below. Plz i need to hurry with site... or may be help how to do fieldset display of Text Area, made a patch for me!

rconstantine’s picture

you can't nest groups of any kind inside multigroups. that may change in the future. will take a lot of development.

kabantejay’s picture

found a bud.
from #230 release in tab Display Fields cant change and save formatters.
when i change display in multigroup with some textfields to other display (from fieldset to horisontal, or table) and save settings - there is no error or warning, but made settings are not applied in node's view.
tried this with #230 ver. on my 3 test sites.. result the same.

dixon_’s picture

subscribing

EHA’s picture

Subscribing

Nested fieldgroups is important for many custom node forms. Looking forward to a stable version or having this in core.

tijeika’s picture

look at new cck dev version... some features work correct now... feel good for me

bomarmonk’s picture

Looking at the latest dev version of cck 3.x, I cannot figure out how to add a nested fieldset or fieldgroup to a multigroup. Any tips on how this can work?

Miria’s picture

Subscribing.

Hope we can see a stable version soon.

bomarmonk’s picture

If I try to drag a fieldgroup into a multigroup, it won't let me order it under that multigroup, so I guess this isn't working at all for the moment?

bomarmonk’s picture

If I try to drag a fieldgroup into a multigroup, it won't let me order it under that multigroup, so I guess this isn't working at all for the moment?

EHA’s picture

From my testing, multigroups can be nested under standard fieldgroups but not the other way around (standard fieldgroup under multigroups). It may be by design.

Bilmar’s picture

subscribing

benone’s picture

subscribe

crea’s picture

Why would you want to nest fieldgroup inside multigroup ? Multigroup semantically is like one, big CCK field, so this kind of nesting would be like nesting group inside a "field". It really makes no sense. OTOH, nesting multigroup inside fieldgroup makes perfect sense ("field" inside group).

crea’s picture

Category: bug » feature

Btw, this is not bug report. If you find an error related to patch, that does not magically change feature request to bug, it simply means patch implementing that feature "needs work". Bug reports are for reporting errors in the code already committed to the module.

bomarmonk’s picture

How do you nest a multigroup under a standard field-group? I have the latest CCk 6.3 dev and I cannot drag the multigroup into the standard fieldgroup. Maybe I'm missing something rather basic here. Any help would be appreciated. Thanks in advance!

bomarmonk’s picture

Or is this currently included in the latest CCK 6.x-3.x-dev tarball? If not, would you mind posting a patch that applies against the current development version. My problem may be the assumption that this functionality has already been included in the multigroup version of CCK. Sorry for my ignorance here.

crea’s picture

@bomarmonk As you can see this issue is still active so it's not implemented yet.

one-five’s picture

subscribe

Aren Cambre’s picture

Status: Needs work » Needs review

Removing issue ownership. rconstantine appears to not be involved in Drupal anymore per issue 421932, and last patch here was 5 mos ago.

markus_petrux’s picture

Assigned: rconstantine » Unassigned
Status: Needs review » Needs work

Latest patch does not apply. Also, please refer to comments around #202.

bomarmonk’s picture

Doesn't the patch in #230 address your concerns about weights, Markus? I'm not sure. Either way, it would be fantastic to get this functionality implemented in the development release of cck 6.x-3.

markus_petrux’s picture

Nope. The stuff related to _content_overview_order() still applies. It should be possible to use this function to resolve part of what this patch does with no need to add more code than necessary.

I do not feel comfortable with this feature committed to CCK as-is. Sadly, I haven't had a chance to spend time here, which is something I wanted to do. :(

bomarmonk’s picture

I hope you can get this working for the latest development snapshot-- if it is possible for multigroups to be added to fieldgroups, the sky is the limit for creating flexible and powerful web applications with CCK.

dellis’s picture

Status: Needs review » Needs work

subscribing

dyke it’s picture

subscribe

stefan81’s picture

subscribe

ckng’s picture

Reroll the patch #230 against latest dev.

Changed _content_overview_order() per #267. Markus, please check if this way is ok.

Known issue:
- nested multigroup does not work

bomarmonk’s picture

I've tested this to some degree, and it seems to work rather well. Thanks!

vatavale’s picture

(subscribe)

3dloco’s picture

clashar’s picture

subscribe

clashar’s picture

would be super when multigroup + nesting will work with tabs

gsvitak’s picture

Hello all

Thanks for the great patch. I am confused.. Is the patch attached in #272 is the entire patch or do I need something else?

Thanks
G

mikeytown2’s picture

Status: Needs work » Needs review

#272 works as advertised. In case anyone is wondering, you can place multigroups inside a fieldgroup, the multigroup just can't be the parent. If you try to have a fieldgroup under a Multigroup (Multigroup is parent), bad things could happen. If this patch prevented this from happening OR if it correctly handled this, then I would say this is RTBC. It would be nice to have to stop re-rolling the patch every time cck 3 dev changes.

This is a very good UI when combined with CCK Fieldgroup Tabs.

Andrew Gorokhovets’s picture

#227 Work with field group Tabs qood for me!

okday’s picture

someone have a patch for the 2.x version?

thekevinday’s picture

The patch works fine, but with one exception.

There is a warning "strstr() expects paramater 1 to be a string, array given in site/all/modules/fieldgroup/fieldgroup.module on line 732"

I do not know why an array is being passed and whether or not that array should be processed, so my quick fix was to change line 732 of fieldgroup.module
From:

<?php
if (strstr($fields, 'field_')) {
?>

To:

<?php
if (is_string($fields) && strstr($fields, 'field_')) {
?>

such that the immediate block of code looks like:

<?php
        $fields = $node->content;
        if (!empty($fields)) {
          foreach ($fields as $name => $more) {
            if (is_string($fields) && strstr($fields, 'field_')) {
              $field_rows[] = $name;
            }
          }
        }
?>

My particular version of CCK-3 is from: http://drupal.org/node/484068

barbecue80’s picture

I see the "known issue - nested multigroup doesn't work" note. But when I try to nest a multigroup under a standard group (field group?), I get the following error:

user warning: Unknown column 'parent_group_name' in 'field list' query

Any ideas? I'm using the same cck as in #282. Thanks for putting this together.

BTW, I'm using the patch in #272.

thekevinday’s picture

I had the same issue.

This happens when you apply the patch to an already installed CCK module.

The 'parent_group_name' table column does not exist.
Run update.php to fix this.
If I remember correctly, it does not actually say that CCK has an update to make even though it needs it.

If that does not work, then you will need to manually add the column to the database via SQL.
I think the SQL command is something along the lines of:
ALTER TABLE content_group ADD COLUMN parent_group_name varchar(32) default ''

thekevinday’s picture

I have found another bug.

When any particular field or fieldgroup is dragged into or out of a group, on save, the dragged fields will be placed at the very top of that group instead of where it was placed.

Example:
Original Layout:
Group 1
- Field A
- Field B
Group 2
- Field C
- Group 3
  -- Field D
  -- Field E
- Field F

Move Field A above Field F and save.

The structure should look like this:
Group 1
- Field B
Group 2
- Field C
- Group 3
  -- Field D
  -- Field E
- Field A
- Field F

But ends up looking like this:
Group 1
- Field B
Group 2
- Field A
- Field C
- Group 3
  -- Field D
  -- Field E
- Field F

Once Field A has been moved to Group 2 and saved, you can now move field A anywhere inside Group 2 (but not Group 3), save, and the field will be properly located.

Balbo’s picture

I too have similar problems with the order of the fields. (not a real problem right now)

Aren Cambre’s picture

Status: Needs review » Needs work
thekevinday’s picture

Status: Needs work » Needs review

I am currently making an attempt to get multigroup working with nested groups.

While studying the code, I came across what may be the solution to my problem in [#282].

Looking at the code and how it gets used later on, I think the following makes more sense:

<?php
if (strstr($fields, 'field_')) {
?>

Should be:

<?php
if (strstr($name, 'field_')) {
?>

This changes $fields to $name.

Looking at the function call that immediately follows and then the functions body:

<?php
$max_depth = fieldgroup_order_fields_groups($group_rows, $groups, $field_rows);
?>

It seems to follow that $name again makes sense here.

As far as I can tell this has something to do with setting the '#depth' value and without this change the fields at the top level when never get the '#depth' value properly set.

Considering that this is about '#depth', this may be related to my issue mentioned here: [#285].
The problem is, that bug in [#285] still appears even with the mentioned changes.

Edit:
The following just popped in my mind:
I am using PHP 5.3.
It may be that in PHP 5.3 strstr() may not accept arrays whereas the older PHPs might accept arrays.
I have no idea if this is the case as the documentation at http://us3.php.net/manual/en/function.strstr.php does not mention any PHP 5.3 errata for the first argument.

thekevinday’s picture

Status: Needs review » Needs work

I apparently had my edit window open before #287 was posted.
This led to me unintentionally changing the status.

(I had this page open for a day or more and I didn't think to refresh before posting!)

This puts #287's change back to "Needs Work"

thekevinday’s picture

I have isolated the issue in [#285] to be directly related to javascript.
If I disable javascript on the node-edit page and move Field A above Field F and save, then the Field A does properly appear above Field F.
If I re-enable javascript after doing this move, the position is still correct.

barckhoff’s picture

subscribe

xkater’s picture

subscribe

thekevinday’s picture

Given the current state of CCK-3, here is a CCK-2 version of the patch for those that need it.

The CCK-3 patch does not directly apply to CCK-2, but looking at the patch itself and the CCK-2 code, the changes were all straight-forward.

Unless I made a copy+paste error (or left something out), there should only be one code difference between the patches.
This difference is the fix I mentioned in #288.

CCK-2 does not have the file: js/content.node_form.js.
Part of this patch adds the file: js/content.node_form.js.

The current CCK-2 does not have multigroup support.

Therefore, I made a separate multigroup cck-2 patch.
This patch requires one to copy the multigroup code from CCK-3 and paste it into the CCK-2 modules directory.

If you think multigroup is not working because of the nested fieldgroup patch, see this first:
http://drupal.org/node/730154

thekevinday’s picture

Another bug exists such that when a group inside of a group has its name changed, the group whose name has changed will be removed from the parent group and placed in the root group.

That is:

Group A
- Group B
-- Field 0
- Field 1

Rename "Group B" to "Group C" produces:
Group C
- Field 0
Group B
- Field 1

ayalon’s picture

Dear thekevinday

Many thanks for your work and your patch! I was able to apply everything to the current 6.x-2.6 CCK module.

Because it took me an hour to find all files and apply all patches and fixes, I made an All-Inclusive-Patch.

This is a fantastic possibility to test the multigroup and nested fieldgroup with the current CCK module within a minute.

You only have to apply one patch and activate the CCK multigroup module and you are ready to use this new cool feature.

Balbo’s picture

I didn't try this new patch from ayalon yet. But with the previous I experienced a bug when inserting an image in an image field that was in a nested group.
I mean:

MaterGroup
   |---SubGroup1
        |---ImageField1
   |---SubGroup2
        |---Others

When browsing for a picture for ImageField1 and then clicking update the field disappear and the image is not saved.

thekevinday’s picture

I have just noticed that fieldgroup.tabledrag.js is missing.

My patches and the patch mine is based on (#272) have the following statement (in the file theme/theme.inc):

<?php
  // Override methods in Drupal core tabledrag.js.
  drupal_add_js(drupal_get_path('module', 'fieldgroup') .'/fieldgroup.tabledrag.js');
?>

But there is no fieldgroup.tabledrag.js installed by the patch.

I simply commented out this line for now.

mikeytown2’s picture

interesting
http://drupalcode.org/viewvc/drupal/contributions/modules/cck/modules/fi...
do not see fieldgroup.tabledrag.js, where that go?

eltrufa’s picture

subscribe

ayalon’s picture

You are right, the file is missing.. No idea if this file ever was committed. Probably not.

perarnet’s picture

Anyone know if there is progress on nested multigroup (adding a multigroup within an existing multigroup)?


Rconstantin mentions:

#222
No nested multigroups. That's an issue with multigroups. If you find the original multigroups thread, you'll see where that is discussed.
That would be something to tackle after this patch goes in.

Anyone know where to find the original multigroups thread?

rconstantine’s picture

the tabledrag.js file is essential to the correct operation of the patch and can be found in one of my previous tars. it was what prevented the crazy behaviors which recent bug reports have been complaining about. as you all can see, after 7 months or so of work trying to get this patch in, i gave up. it has now been over 15 months with little progress. i was intending to revisit this issue once D7 and whatever accompanying CCK comes out, but i'm glad that some have taken this up where i left off and that there is still quite a bit of demand for this.

i haven't yet looked at the current state of the code, but i can assure you that with the version that i'm running, the recently mentioned bugs do not exist. moving fields from one subgroup to another, or out to the top level works as it should. changing field names works as it should, etc. in fact, the only strange behavior in my version is that you cannot move an item to the very top of the list, but you can move the top item down. there were reasons for that in the js file. i have created literally dozens of sites and dozens of content types including one with 230+ fields using fieldgroup tabs, nested fieldgroups, and multigroups - which was the reason for the work i did in the first place.

additionally, filefield and imagefield were known issues. they modify the node's data structure and expect those fields to be top-level items. the modules don't know what to do (since they use AHAH) when the fields are nested. the two new hooks i had implemented (which it sounds like may still be there but modified?) would have solved this issue. that is, IIRC. i also think that my changes to the js positively affected the AHAH from those two modules as well - i.e. they wouldn't have needed their own override functions. in fact, i might even be running an modified filefield module on that 230+ field site. i'd have to check. by the way, the hook addition also affected the multigroup module! so i hope that the new patches that have been rolled are also patching multigroup.

another thing - as someone pointed out, there IS a change to the DB that this patch should be making. when i rolled the patch in the first place, i had set the name of the update function in the .install file correctly. if 3.x has moved on, perhaps the name needs to reflect a higher number so that the change is correctly made on existing systems.

regarding nested multigroups - as quoted in #301, that was discussed here: http://drupal.org/node/119102 somewhere. the problem is both conceptual and technical. conceptually, multigroups are like fields. for example, an address should be one 'field' but made up of smaller parts. i'm not sure that people have really come up with many example use cases for having one multigroup inside of another. here is one, but it isn't necessary: if you make a multigroup called "contact" and then place "address" multigroups inside - where you would then possibly have more than one contact, each with more than one address on a given node, then i'd say there might be a problem with the site design. one way to get around this would be to create a "contact" content type which just has the "address" multigroup, and then to use nodereference with "multiple" set greater than one to have more than one contact on that node. technically, you're talking about big changes to the database and the way that cck keeps track of multiples both in storage and via both server and client code. there would be a particular headache getting the AHAH to work right.

although i haven't been able to do as much on d.o as i would like to these past months (see my neglected modules as evidence) i still very much use drupal daily. if i can ever get more help in my department, then i'm sure i'll be able to get time to again return to contributing here, which is something i very much enjoy. until then, good luck with keeping this patch going. i think it's important and enhances cck far more than some of the changes that HAVE gone in this past year. i hope these comments were helpful.

cheers

thekevinday’s picture

Okay, thanks rconstantine for letting me know where to get that file.

I have tested with the mentioned file and the bugs I mentioned do go away.
I have rebuilt the patches I previously submitted to include the missing file.

There are a number of people who may not want multigroup support added and in their favor I am still breaking up the patches.
However, to save people like ayalon time and effort, I will upload every patch I have that completely gets fieldgroups+multigroups working without flaw under CCK-2.
(The only issue multigroups has is that it is not quite views compatible. Other than views integration, multigroups seems quite stable under cck-2)

The order of the patches are important and are explained below.

Patch Order for cck-2:

  1. 2.x-nested_fieldgroup-2
  2. 2.x-add_multigroups-2
  3. 2.x-nested_multigroup-2
  4. 2.x-workaround_multigroup_bug-2

Step 1 adds nested fieldgroup support.
If you do not want multigroup support, you can stop there.

Step 2 adds the entire multigroup from cck-3-dev to cck-2.

Step 3 adds nested multigroup support to the newly added multigroup module

Step 4 adds a workaround for a bug that I found where multigroup fields don't get saved to the database

Applying nested fieldgroup patches to CCK-3 is even easier:

  1. 3.x-nested_fieldgroups-2
  2. 3.x-workaround_multigroup_bug-1
perarnet’s picture

thekevinday:

Is there a big hurdle to get nested multigroups to function under 3.x?

thekevinday’s picture

Status: Needs review » Needs work

I found a bug introduced by the tabledrag.js file.

When dragging a row to the topmost position of the root of the table, the row will not 'drop' in place.
The row gets "stuck" on the mouse and will not drop at that point unless somewhere other than the topmost position is placed.

perarnet:
That depends on what you are asking when you mentioned nested multigroups.
- I would say that multigroups current lack of (active) development is a big hurdle, but that is probably not what you are asking.
- If you simply are asking if nested fieldgroups works with multigroups, then there is no hurdle as multigroups works fine (with one major exception).
- If by nested multigroups, your are specifically talking about multigroups nested within multigroups, then yes. (this is the major exception).

I am going to assume multigroup nested multigroups is what you are asking about.
As far as I can tell, the problem seems to be that CCK was not designed in a way that will allow for this to work properly.

To properly support multigroup nested multigroups would require a significant redisgn (if not complete).
And I do not refer to what is going on in CCK-3.
This is a problem all the way down to the database level.

However, do not put too much credit to my words. I am not a CCK developer and I have only jumped into this fieldgroups/multigroups rather late in the game.
There is still quite a lot about how CCK works internally that I do not know.

perarnet’s picture

thekevinday:

Yes, I did mean multigroups nested within multigroups. I misread your last post, believing you had solved this for 2.x. Guess I'll have to figure out other ways to solve my problems.

rconstantine’s picture

@thekevinday - The row getting "stuck" is what I was talking about when I said that "in fact, the only strange behavior in my version is that you cannot move an item to the very top of the list, but you can move the top item down." I think you can also click once stuck to unstick yourself - at least if you move that item you tried to move to the top back down to at least the 2nd spot.

SO - unless someone can fix that bug, the workaround is to 1) move the item you want at the top into position #2 2) move the top item down to position #2 which will bump the desired item into position #1.

@perarnet - that feature would require rewriting many, many cck functions and would also require a significant rework of the cck database tables as well. i believe i linked to the previous discussion which touched on the difficulties. in the very least, with so much moving into core for D7, it would be a huge waste of time to do it now.

silurius’s picture

Subscribing.

miro_dietiker’s picture

subscribe

emilyf’s picture

thanks @thekevinday patch for 6.x-2 fieldgroup only seems to work great!

emilyf’s picture

hmmmm, i guess i spoke a little too soon. first patch from http://drupal.org/node/300084#comment-2930694 works well with one issue. if you drag a fieldgroup inside another fieldgroup, save the manage fields page, but THEN you edit that nested fieldgroup, it bumps it back out of the other fieldgroup. so if i save it like this:

fieldgroup 1
------fieldgroup 2

and then i edit fieldgroup2, the next page it bumps it back to this:
fieldgroup1
fieldgroup2

thekevinday’s picture

The solution to the bug mentioned in #307, #305, and on the other posts seems to be the following.

Only line 53 of fieldgroup.tabledrag.js, the following exists:
if (siblingIndent == indents) {

What I am thinking is happening here is that when at the root level siblingIndent = NULL (or 0?) and indents may be set to NULL (or 0?) as well.
The root level does not indent siblings if I remember correctly.

So I decided to try changing the mentioned line of code to the following:
if (siblingIndent == indents && newGroupName != "top") {

With this change in place, the drag+drop no longer sticks at the top level.

Please Review, I am confident that this works but not confident that this is the most correct solution nor am I confident that this does not produce any regressions.

This patch goes on top of the nested_fieldgroup patch from #303

zeno129’s picture

I used the patches in #303 for cck 3 and I also used the patch in #312.

But there seems to be a problem with the patch in #312:
After patching the js file, whenever I "manage fields" in a content type, I re-order them and click save the fields get all disorganized and placed in (what seems to me like) random positions.

Example:
Field 1
Field 2
Field 3
Field 4
Title
Menu
Comment
Revision
Path

I click save and:

Title
Field 3
Menu
Field 2
Field 1
Field 4
Comment
Revision
Path

I think the problem is with the js patch because when I reverted it and tried to do the same changes, it worked fine.

thekevinday’s picture

Thanks for the help.
I have been able to close in on the problem.

For some reason, when at the root level of the table, dragging the row to the very top will result in siblingWeightFieldUp[0] being undefined.
Because this value is undefined, the code: siblingWeightFieldUp[0].value will fail.
When javascript fails, it just stops executing the remaining code and silently exits.
Because javascript suddenly terminates, the row gets stuck on the mouse.

The solution is to prevent this case from happening by doing an is undefined check:
The code:

    var siblingWeightFieldUp = $('input.field-weight', siblingRowUp);
    var siblingWeightUp = siblingWeightFieldUp[0].value;
    var newWeight = parseInt(siblingWeightUp) + 1;
    $('input.field-weight', tableDragObject.rowObject.element).val(newWeight);

Should now be:

    var siblingWeightFieldUp = $('input.field-weight', siblingRowUp);
    if (siblingWeightFieldUp[0] != undefined){
      var siblingWeightUp = siblingWeightFieldUp[0].value;
      var newWeight = parseInt(siblingWeightUp) + 1;
      $('input.field-weight', tableDragObject.rowObject.element).val(newWeight);
    }

The attached patch does this.
Use this instead of the patch from #312

I am confident this is the correct fix and should not cause any regressions.

thekevinday’s picture

I have identified a problem with deeply nested fieldgroups.
This problem may be the same problem mentioned here: #311 and probably somewhere else amongst the 300 other posts for this issue.

The problem is that when a fieldgroup is nested within a fieldgroup, it gets pulled to the root of the table and placed at the top.
This was due to a logic flaw in the code.

The logic flaw are these lines here:

<?php
$dummy[$parent][$name] = $dummy[$name];
unset($dummy[$name]);
?>

Ultimately, this code removes a nested fieldgroup from the root of the table.
When a fieldgroup is nested within a fieldgroup, the parent fieldgroup may have already been removed from the root of the table.
The end result is unexpected behavior.

The fix is attached.

Please apply this after applying patches from:
#303 and #314

klonos’s picture

subscribing...

thekevinday’s picture

Status: Needs work » Needs review

Follow patches from #303, #314, and #315.
Please review this, I have not found any new nested fieldgroup specific bugs.

As far as the detailed information from #302 is concerned:
Modules that assume the structure of fieldgroups are bugs with that module+fieldgroup and not fieldgroup itself and should be left to the other module to correct itself (Unless otherwise proven to be caused by a fieldgroup bug).

This module seems to work fine under the 2.x series without multigroups added.

I do not see any reason why nested fieldgroups should be held back by a module that is not even released.
Nested fieldgroups does not depend on multigroups.

Please consider the patches thus far (and perhaps any needed database changes that were mentioned in #302) for applying to 2.x stable.
Considering the sheer size of this issue and the number of claimed usages in #302, nested fieldgroup is well ready to be considered to be merged into stable.

rfoster’s picture

Status: Needs review » Needs work

I found a bug so I set the status back to needs work.

On the display fields tab ( admin/content/node-type/[your-content-type-here]/display ), changes made to any drop downs in a "[Subgroup format]" row do not save after applying the patch cck-6.x-3.x-nested_fieldgroups-2.patch (from #303). This should only affect people who are using multigroups since these options are specific to multigroups.

Thanks for all of your great work thekevenday!

rfoster’s picture

(removed duplicate comment)

thekevinday’s picture

Well, I looked into and here is my explanation of the problem and the patch to fix it.

The display submit function attempts to save the subgroup settings on its own by manually calling: fieldgroup_save_group().
The fieldgroup module also calls fieldgroup_save_group() via the fieldgroup_display_overview_form_submit() function.

The end result of this is that two calls get made that write to the database.
The first call will contain the subgroup settings information.
The second call will not contain the subgroup settings information and overwrite the existing saved information.
The subgroup settings information at this point is non-existent.

There is then a second problem, this time with the display loading code.
(This may be caused by my solution to the first problem).
The display code looks for the saved subgroup data in: <?php $group['settings']['multigroup']['subgroup'] ?>
However, the subgroup data is actually stored here: <?php $group['settings']['display']['settings']['multigroup']['subgroup'] ?>

The patch solves both cases.
The patch prevents two calls from being made and so also provides a slight performance gain (probably unnoticeable).

This might be a multigroup bug with fieldgroups, but considering that this bug is reported to appear after the fieldgroup patch, I am reporting it here.

The attached patch does work against the cck-2.x version as well as the cck-3.x.

jvieille’s picture

That's a lot of patching!
(I never succeeded patching with Tortoise...)
Would it be possible to get the modified files full cooked to try this really important feature?

Thanks in advance

rfoster’s picture

Thanks for the quick response. I tried the patch in #320 but it actually made the problem more complicated, so it is better without the patch. This is what I found:

1) I can now change the "Subgroup format" settings, and the settings are saved and reloaded. However, these settings have no impact on how the form is displayed!

2) I had a "Subgroup format" that was set to display as "Table - multiple columns" (I reversed the patch in #303 to set this, and then reapplied it). After applying the patch in #320, that content type still displays the data in a table, but on the "Display fields" tab it says that the display is set to "Fieldset" (the default option). As with #1, I can change these settings any way I want, and it does not change how the fields are displayed.

So it looks like the latest patch is saving and loading extra data (in the wrong location or format), separate from the data that actually affects the display of the fields. Before applying the patch, loading the settings worked fine. It was just impossible to make any changes to those settings.

ndm’s picture

subscribe for a stable solution

thekevinday’s picture

Status: Needs work » Needs review
FileSize
2.53 KB

It turns out that the <?php fieldgroup_display_overview_form_submit() ?> function forcefully moves all fields from settings into display->settings.
That explains the second bug I mentioned in #320.

This problem can be fixed by specifically looking for the multigroup data and preventing the data from ending up in display->settings.
With this done, the fix for the second bug in #320 has been reverted/removed and the problems mentioned in #322 are gone.

Please Review and let me know if I blew anything else up with this patch.

rfoster’s picture

Appears to be working fine now with the latest patch. I'll report back if I discover anything later. Thanks!

thekevinday’s picture

Then here is a combined patch to make peoples lives easier.

This is a combined patch from the following patches:
cck-6.x-3.x-nested_fieldgroups-2.patch from http://drupal.org/comment/reply/300084?page=1#comment-2930694
cck-6.x-x.x-tabledrag_sticky_fix-2.patch from http://drupal.org/comment/reply/300084?page=1#comment-3005354
cck-6.x-3.x-nested_fieldgroup_fix_deep_nesting-1.patch from http://drupal.org/comment/reply/300084?page=1#comment-3042808
cck-6.x-3.x-display_subgroup_save_fixes-2.patch from http://drupal.org/comment/reply/300084?page=1#comment-3073766

The multigroup patch found here: http://drupal.org/node/300084?page=1#comment-2930694 still needs to be applied.

The cck-2.x patch does not provide multigroup nor does it patch multigroup.

rfoster’s picture

Status: Needs review » Needs work

OK, I found another bug that I traced back to this patch. I am getting some SQL duplicate key errors on inserts and updates to the table d_content_group_fields. I think this only affects users who are sharing groups across content types. If you aren't doing that, then you may never have this problem.

So this is what I did. I created a complex content type with lots of nested groups. Then I exported the content type, renamed the original, and then reimported it. So I have a content type called 'firewall' and an identical content type called 'xyz.' Then if I delete a group in one of these, it actually tries to make updates to the other content type too, since some of the queries are not specific enough.

One query is the last query in this function:

function fieldgroup_remove_group_submit($form, &$form_state) {
  $form_values = $form_state['values'];
  $content_type = $form['#content_type'];
  $group_name = $form['#group_name'];
  $parent_group_name = db_fetch_array(db_query("SELECT parent_group_name FROM {". fieldgroup_tablename() ."} WHERE group_name = '%s' and type_name = '%s'", $group_name, $content_type['type']));
  $result = db_query("UPDATE {". fieldgroup_tablename() ."} SET parent_group_name = '%s' WHERE parent_group_name = '%s'", $parent_group_name['parent_group_name'], $group_name);
  $result = db_query("UPDATE {". fieldgroup_fields_tablename() ."} SET group_name = '%s' WHERE group_name = '%s'", $parent_group_name['parent_group_name'], $group_name);
  fieldgroup_delete($content_type['type'], $group_name);
  drupal_set_message(t('The group %group_name has been removed.', array('%group_name' => $group_name)));
  $form_state['redirect'] = 'admin/content/node-type/'. $content_type['url_str'] .'/fields';
}

Need to add AND content_type = ... to the WHERE clause.

As for the duplicate key error on inserts I didn't make a note of the line number, so I'll have to wait until it pops up again...

rfoster’s picture

Some more notes:
I had a field 'field_other' in both content types. I deleted this field from type 'firewall.' Then I geleted the parent of that field, group 'group_other_tab' from the firewall type, and I got this error:

user warning: Duplicate entry 'xyz--field_other' for key 1 query: UPDATE d_content_group_fields SET group_name = '' WHERE group_name = 'group_other_tab' in [omitted]/sites/all/modules/cck/modules/fieldgroup/fieldgroup.module on line 216.
thekevinday’s picture

Status: Needs work » Needs review
FileSize
1.9 KB

I tried what was explained in #327.

I did not get the duplicate key error and was able to delete one group, and the other did not get deleted.
The group did have one of its entries deleted from the content_group_fields table.

What confuses me is this concept of sharing groups, I am unclear as to what you are saying.
Do you have a third party module that enables sharing groups between content types?
Or are you simply talking about groups sharing the same system name between different content types?

Either way, I have made a patch that applies your fix where I found the problems.
This patch is against cck-3, but still applies against cck-2.

thekevinday’s picture

Out of pure chance as I was looking at the cck code I noticed the following inside the fieldgroup.module in the fieldgroup_nodeapi() function:

<?php
         $fields = $node->content;
         if (!empty($fields)) {
           foreach ($fields as $name => $more) {
            if (is_string($fields) && strstr($fields, 'field_')) {
               $field_rows[] = $name;
             }
           }
?>

That seems odd to me, shouldn't <?php if (is_string($fields) && strstr($fields, 'field_')) { ?> instead be: <?php if (is_string($name) && strstr($name, 'field_')) { ?>?

I seem to have overlooked the typo at #282.

rfoster’s picture

I've been experiencing another problem that I finally discovered is related to the combination of multigroup + nested field groups.

If I have a structure like this:

Standard group
   Multigroup
      text field using select list widget

then I can't delete rows in the multigroup if a value is set in the dropdown. I can delete the rows by clearing the value of the dropdown, but I cannot delete them using the normal row delete button--when the form is saved the rows don't delete (and data in other fields can get saved in the wrong row as a result).

If I move the multigroup out of the standard group everything works fine.

I'm using a the latest available version of cck-6.x-3.x-dev with these patches applied:
cck-6.x-3.x-nested_fieldgroups-3.patch
cck-6.x-3.x-workaround_multigroup_bug-1_0.patch
cck-6.x-3.x-nested_fieldgroups_shared_groups_fix-1.patch
cck-6.x-3.x-fieldgroup_typo-1.patch

thekevinday’s picture

I can reproduce your problem in #331.

It seems that the data gets cleared out but the existence of the row does not.

I have also noticed that Required Fields do not work correctly in a similar manner.

The problem happens on line 402, which immediately follows the added code from my patch: workaround_multigroup_bug-1
Which, ironically, works around a data saving problem.

This means that either the workaround fixes the node create and breaks the node edit or there is more to this bug than I first realized.
This will take some time to debug.

EDIT:

Update on the mentioned problems:
1) Fields that violate the standard definition of "multiple values" (such as radio buttons/checkboxes and select lists) do not work properly because they do not properly use "multiple values".
- This also breaks the required fields for such problematic fields
- The solution to this is to replace the existing radio buttons/checkboxes and select list functionality with new versions that do things properly.
- I intend to do this and will open of a separate thread when I am done.
2) The delete does seem to work, but all fields passed the first do not get saved.
- Still being investigated
3) The _remove array value does not get stored twice, example:

FORM STATE group_test_00001_00002 = 
  Array (
    [0] => Array (
      [field_test_00001_00002] => Array ( [_remove] => ) [_weight] => 0 [_remove] => 0
      [field_test_00001_00003] => Array ( [value] => 3 [_error_element] => group_test_00001_00002][0][field_test_00001_00003][value )
    )
    [1] => Array (
      [field_test_00001_00002] => Array ( [_remove] => ) [_weight] => 1 [_remove] => 0
      [field_test_00001_00003] => Array ( [value] => 7 [_error_element] => group_test_00001_00002][1][field_test_00001_00003][value )
    )
    [2] => Array (
      [field_test_00001_00002] => Array ( [_remove] => ) [_weight] => 2 [_remove] => 1
      [field_test_00001_00003] => Array ( [value] => 8 [_error_element] => group_test_00001_00002][2][field_test_00001_00003][value )
    )
  ) 

- Still being investigated
4) As seen in the above example the _remove field only gets placed in the first field
- Is this a bug?

zeezhao’s picture

subscribing

thekevinday’s picture

The problem seems to be that for some reason the fields multiple value do not get set when the field is moved into the multigroup.
The end result is that the database table that handles multiple values is missing.

As a temporary fix, until a patch can be made, drag all fields out of the multigroup, set those fields to have the desired number (or unlimited), and then drag those fields back into the multigroup. Once this was done for all fields in a given multigroup, everything seemed to work fine for me.

zeezhao’s picture

Hi. Thanks for all your efforts in producing this module and its fixes.

Please, in order to get all the right/latest patches, could you post your fully patched cck 3.x with the nested fieldgroup? This will be much appreciated. It would mean that someone like me can just install the whole module. Thanks for your help.

thekevinday’s picture

I have found the proper solution to the bug mentioned at #331 and #332.

Heres what happens.
The nested fieldgroup patch manually performs a db_query instead of calling the pre-defined CCK query functions.
This is because this query is doing an INNER JOIN from its own private table with the CCK table.

Some of this queried data is serialized but fieldgroups does not unserialize the data.
What this means is that much of the field data, such as widget_settings, display_settings, etc.. is unparsable.
The result is that the field settings are never used and a bunch of weird things happen.
In particular, the mentioned fieldgroup+multigroup bug happens.

The solution is simple, unserialize the data when retrieved.
The attached patch does just that.

Now here is the thing.
The field data was never being used so expect this patch to reveal bugs that have been sitting around for a long time, unnoticed.
In particular, I have noticed that when the multigroup is set to something like 9, instead of unlimited, the fields do not properly save.
Also, I need to research if this patch is the real solution to the multigroup_workaround patch I created.

Please review this patch and report all new bugs that appear.
Once this is done, and once I have confirmed whether or not the multigroup_workaround patch can be removed, I will then make a new cumulative patch that has all of the existing patches applied.

zeezhao’s picture

Hi. Thanks for the new patch in #336. I tried it out but I am still unable to delete a row as in #331 in a multigroup. In my case the dropdown contains a list of users. It deletes the other fields but not the userlist, so the row still remains.

I am using latest cck-6.x-3.x-dev with these patches applied:
cck-6.x-3.x-nested_fieldgroups-3.patch
cck-6.x-3.x-workaround_multigroup_bug-1_0.patch
cck-6.x-3.x-nested_fieldgroups_shared_groups_fix-1.patch
cck-6.x-3.x-fieldgroup_typo-1.patch
cck-6.x-3.x-fieldgroup_unserialize-1.patch

Please let me know if I am missing anything. Thanks.

thekevinday’s picture

The way in which CCK dropdowns are designed, they will not work.

A replacement dropdown solution will have to be written.

The problem is that the developers decided that instead of creating new options for drop downs (aka select lists) they would recycle existing fields that they thought would not be used.

In this particular case, the Multiple Values feature that CCK provides is recycled to represent whether or not the drop down is a single select or multi select drop down.

Any module that re-defines the "Multiple Values" settings for its own use will likely break.

zeezhao’s picture

Thanks for your reply. Not sure I fully understand it though...

The user list I referred to is acutally a "User reference" field in cck.

Are you saying that this can also happen for any select list of any other cck field type e.g. "Text"? Not tried that yet.

Please what do you recommend I do to solve this?

Once again, thanks for you help and efforts.

edit:
Forgot to mention that the only way to delete the rows, is to set the lists to "None" option, and then remove the rows, and then save again.

zeezhao’s picture

@thekevinday - as a continuation of my earlier post #339, one way to ensure the rows get deleted is to set the values of any dropdown lists to "None" before doing the delete. Maybe if this could be done automatically in the code, then the rows can get deleted. Please let me know your thoughts on this workaround. Thanks for your help.

zeezhao’s picture

Title: Nested fieldgroup » Nested fieldgroup - order of fields when viewing the node is different from order in edit form
Category: feature » bug

Another thing I just noticed after applying the patch is that - the order of fields when viewing the node is different from order in edit form, for some of my fields... Is this a known bug, or is there a patch I am missing? Thanks.

edit: This happens after applying cck-6.x-3.x-fieldgroup_unserialize-1.patch

zeezhao’s picture

Title: Nested fieldgroup - order of fields when viewing the node is different from order in edit form » Nested fieldgroup

Sorry. Mistakenly changed title.

thekevinday’s picture

At this point the problems I am seeing are slightly different than the ones you two are seeing in: #339 and #340.

What version of cck-3-dev are you using?
Mine is: June 18, 2010 (2010-06-18), with a datestamp of 1276862557

The developer has been doing a good job of updating cck-3-dev when cck-2 gets updated and there has since been a recent update.

What specific module, fields, and widgets are you using that have this problem?
I have been doing most of my tests with text field and widget types of 'text field', 'select list', and 'check boxes / radio buttons'
(Note: text (space) field is referring to the field type that is called 'text' and 'text field' is referring to the widget type called 'text field')
I also have conditional fields-2.x module in use.

As far as #339s question/
This happens for every select list (including the cck core select list) that changes how the "Multiple Values" works.
By 'Multiple Values' I am referring to the global setting that says: 'Number of values: ' and has a number of options from 1 to 10 and also unlimited.
This should not appear for fields inside a multigroup but should appear for fields outside a multigroup.

Outside of a multigroup if the value for some text field was changed from 1 to 5 there should be 5 of those fields.
For example, a text field with a widget type of 'text field' will have 5 separate text field input boxes.
However, a text field with a widget of 'select list' will have a single select list that shows 5 options at a time.
Instead it *should* show 5 separate select field input boxes.

This is a multigroup problem that appears to expand into nested fieldgroups when use in conjunction with multigroup.
This is what I am currently thinking is causing the problem with #339, #340, and my problem.

Now, for #340.
The problem I am having is that for my select lists, they are always set to -none- when editing a node (even if set otherwise).
Do the proper values show up in the select list when you edit your node?

To try and reproduce your problem, I tried changing my select list value to a non-none- value and then deleting that row.
The delete worked, but with the following warning:
warning: Invalid argument supplied for foreach() in sites/all/modules/cck/modules/content_multigroup/content_multigroup.node_form.inc on line 401.

The challenge now is to figure out why we are having different results.
There may be a 3rd party module causing the difference.

My Full list of modules at this time is:
acl
advanced_help
autologout
auto_nodetitle
better_formats
cck
cck_accessibility (written by me)
cck_label_field (written by me)
ckeditor
conditional_fields
contemplate
content_access
content_taxonomy
ctm
custom_formatters
date
diff
email
field_indexer
form_overrides
graphstat
imce
jquery_ui
killfile
ldap_integration
login_destination
matrix
multiple_etal
multiselect
node_clone
node_export
pathauto
path_redirect
phone
preview
privileged_cck (written by me)
protect_critical_users
restrict_password_change
rules
simplemenu
sin
taxonomy_delegate
taxonomy_manager
themekey
token
type_local_nids
unique_field
vertical_tabs
views
views_bonus
view_unpublished

edit;
and i almost forgot, #341 sounds like a new problem to me.

thekevinday’s picture

At the request of a few here is a new patch will all of the ones before it rolled into one.

This includes these patches:
cck-6.x-3.x-nested_fieldgroups-3.patch
cck-6.x-3.x-workaround_multigroup_bug-1_0.patch
cck-6.x-3.x-nested_fieldgroups_shared_groups_fix-1.patch
cck-6.x-3.x-fieldgroup_typo-1.patch
cck-6.x-3.x-fieldgroup_unserialize-1.patch

bleen’s picture

subscribe

zeezhao’s picture

@thekevinday - thanks for your reply. To answer your questions in #343

- using same cck as you i.e. 6.x-3.x-dev of 2010-06-18

- the main reason for me using the nested patch, is to have 2 or more multigroups within a standard group.

- I am using "user reference" select-list, "text" select-list, "date", "text" field, all within a multigroup that sits within a standard group. In fact there are 2 multigroups within this standard group.

- The fields in the select lists show up as normal during edit, and can be selected and saved without any issues.

- Issue seen so far is with the "user reference" select list, and "text" select list, when I try to delete rows in a multigroup. Everything else gets cleared but these fields. Hence the rows remain undeleted. This is the main issue I am seeing.

- One workaround my issue, is to make all the multigroups to be of fixed "Number of repeats", so that user can not delete the rows anyway, and will always have to clear the fields. When this is done, multigroup disables its row-delete icon.

- when I add the cck-6.x-3.x-fieldgroup_unserialize-1.patch, it shows fields in random order when in node view mode, so I backed this out.

- All my "multi value" fields are within a multigroup i.e. multigroup can have unlimited sets of the data. So I do not have multi-value fields in a normal standard group or nested.

- modules - using many..... and a few custom ones. The standard ones are:

acl                        menu_editor
admin_menu                 messaging
advanced_help              nice_menus
autoload                   node_agreement
button_field               node_clone
calendar                   node_gallery
captcha                    node_import
casetracker                nodeformsettings
casetracker_notifications  nodehierarchy
cck                        notifications
cck_fieldgroup_tabs        pathauto
chart                      preview
charts_graphs              print
computed_field             privatemsg
content_access             querypath
content_taxonomy           quiz
customreports              rsvp
date                       rules
devel                      simplemenu
extended_ldapgroups        stringoverrides
faq                        swftools
filefield                  tabs
flashnode                  taxonomy_csv
format_number              taxonomy_manager
formatted_number           taxonomy_vtn
formfilter                 token
hierarchical_select        tooltips
imageapi                   transliteration
imagecache                 user_import
imagefield                 views
jquery_countdown           views_bonus
jquery_update              views_bulk_operations
ldap_integration           views_calc
ldap_provisioning          views_charts
ldaphelp                   views_groupby

Hence so far, it works well apart from the delete issue I have. Will be doing more testing over the weekend.

thekevinday’s picture

Here is a proof of concept select list replacement.
I took the entire optionswidget module and started stripping out the problems I mentioned in #338 and I also think in #343.

This module will be called cck_options.
This module is incomplete and the only widget I have working this far is "Drop Down - Single".
I am using "Drop Down" instead of "Select List" to avoid conflicting with the current Optionswidgets module. (I would rather use Select List to be consistent but that would be confusing at this time).

For any field in which you use the Select List widget, replace it with this modules "Drop Down - Single" and tell me what happens.

And as a reminder this module is woefully incomplete, use it only for testing purposes at this time.

seworthi’s picture

@thekevinday - Thanks for so much work on this.

I installed #344's patch on the 6.x-3.x-dev of 2010-06-18.

I have a std group, with 2 multi groups in side of it. In one multigroup I have file uploads, and the other has image uploads (using the latest 3.7 version's of filefield and imagefield module).

If I try to add a new item to either group, after hitting the upload button, the field disappear. The other fields remain, just the one I tried to add. The file is uploaded to the server. I can delete existing fields fine, and saving the node w/o doing anything work and the items are still there.

Has anyone else experiencing this?

klonos’s picture

@thekevinday: I've been monitoring all related issues (fieldgroup/multigroup) and I see you've been doing a great job trying to clear things up. Thank you for all the effort and time spent on this. It's really appreciated!

I like what you've done in #344 and the idea to have all related patches bundled so people apply them in one go. This will simplify the process and hopefully bring more testers, but the greatest thing about it is that all testers will have a common ground when giving feedback. That is they will have all patches applied and maintainers won't have to repeat the procedure of asking reporters to make sure and then point them to any missing (or perhaps outdated) patches.

This whole thing is great for checking any issues that might arise along the way of trying to get fieldgroup and multigroup committed and detect if one's changes get in the way of the other. But still, posting patches in issues should be limited to those patches meant for each specific issue. How about we open a new issue that simply holds only 'cumulative' patches? This issue should be updated with feedback only by people that have applied all patches in regard. I mean, right now you risk people reporting back issues that might be caused by the multigroup patch and they should be posting these kind of issues there and not here.

What say you?

thekevinday’s picture

That would make life easier.
The other reason I post the partial patches are so that any screw-ups are more easily found by people and can be more easily pointed out.

This issue was rather overwhelming when I joined in and I just had to figure things out as I went. I can only imagine how much more difficult it is to follow for any people joining 100 posts later.

You can find the cumulative patch issue here: http://drupal.org/node/849420
I re-diffed the -4 patch because I accidentally included a few .orig files.

ianchan’s picture

sub

ayalon’s picture

Hi thekevinday!

I was able to insert nested field groups into the database:

Example with 3 fields.
Title Surename Lastname
Mr Peter Black
Mrs Emily Lafontaine

But the big question is, how to displaxy these fields in a view!
It's not possible to output these fields in a logical way, f.ex. a line for each person.

Output:
Mr Peter Black
Mrs Emily Lafontaine

How did you manage this problem?

thekevinday’s picture

I believe you should look into custom_formatters: http://drupal.org/project/custom_formatters
Or contemplate: http://drupal.org/project/contemplate

With custom_formatters being the simpler of the two.

Andrew Gorokhovets’s picture

Can you to Include a patch in the dev version?

thekevinday’s picture

When nested fields get used, the location of fields and groups probably will not be at the root of the array structure given that they are nested inside of some group.

This becomes a problem with all form_set_value() function calls.
form_set_value() only works if the field is at the root of the array.

What this means is that most validate and submit functions will have some problem with fields not working properly within nested fields.
The solution is to provide a function that recursively walks through each group and returns a referential array of fields and groups.

Attached is a patch that adds the function: content_get_single_depth_array_of_fields(&$form_values, &$array_of_fields) to content.module.
This patch then uses the said function to fix a problem with multigroup values not saving.

Let me know if any of the existing problems go away with this patch.

I also suspect that this is the proper solution to what I did with the workaround_multigroup_bug patch.

rconstantine’s picture

So I suppose I have lost track of this thread, but what has happened to make it so that nested fields are not properly validated? I'm using an old version of this - i.e. I haven't updated my version since the last patch I posted - and I have a content type with over 200 fields, some nested 5 levels deep, including multigroups, which all properly validate. And I have updated the error message to show the whole path to the field - FieldGroupTab->NestedGroup1->NestedGroup2->Field - which was just a matter of listing #parents I think.

Maybe I should do a diff of the latest against mine? Could someone roll the patch and then put together a cck.zip file with all cck files including the latest patches? That would be helpful for a comparison. All I know is I definitely get error messages if nested fields don't pass validation.

Or am I talking about something other than the issue you mention because I'm that far behind in understanding what is going on here these days?

thekevinday’s picture

This is the reason why I have been submitting individual patches before merging them into a newly rolled one.

A list of the patches are: #303 + #314 + #315 + #324 + #329 + #330 + #336 + #355.

At the request of many, a new thread has been forked for the cumulative patch here: http://drupal.org/node/849420

Here are a list of reported problems:
#340 - I have not been able to reproduce myself, but I have reproduced similar..
#341 - I have not been able to reproduce myself
http://drupal.org/node/853290 - possibly related to #340
http://drupal.org/node/848758 - situation possibly caused by this module, but I do not know how to reproduce
http://drupal.org/node/758446

As far as the "validator issue", the issue is not with the individual fields validators being called.
The issue is that the multigroup validator fails to do its multigroup specific stuff given that it tries to work with fields only at the root level and does not know to search inside of groups.

Also, for #352, I just realized that you could also be talking about the "Views module" view and not "viewing a node" view.
You should look into the multigroup documentation (if any..?) as that may be a multigroup issue.

zeezhao’s picture

@thekevinday - thanks for trying to nail down these issues. To add to #357

- error reported in #341 occurs for a standard group - the fields in my case appear in reverse order once cck-6.x-3.x-fieldgroup_unserialize-1.patch is applied. So backed it out.

- #340 still appears for me too. The only way to delete the row is to clear the drop-down list. Living with it for now, as benefit of nested group outweigh the bug...

- did not have validation issue. Since I was always using multigroup before I found nested groups, I had to use recursive functions to do validations/hiding/setting required of cck fields etc. based on: http://drupal.org/node/357328 or looping through fields in a custom module.

elektrorl’s picture

hi, great innovation,

same error, in #341 for standard group created with cck-6.x-3.x-. I tried to understand if CCK fieldgroup tabs could be the problem, but i have the same without this module. Will try this on a clean install.
Thanks for all your time developping this. I subscribe ;)

thekevinday’s picture

I have the "solution" for the bug from #341.

What happens is that fieldgroup seems store store its field 'weight' in its own database table.
When I unserialize the actual field settings, the 'weight' from the actual field settings overrides the 'weight' from the fieldgroup database table.
My solution is to make sure the fieldgroup database table field 'weight' gets used instead of the fields actual weight.

Try the patch and let me know if it works.
If this patch is confirmed to work in replacement of my patch from #336, then I will rebuild the cumulative patch.

I believe the "proper" solution is to not have a separate 'weight' in the fieldgroup database table.
This probably happened because the developer must not have realized the data was serialized and in order to fix their 'weight' problems they added their own separate 'weight' values in the fieldgroup database table.

elektrorl’s picture

I will try this very soon. Thanks for your time.

Andrew Gorokhovets’s picture

#326 works perfectly for me.

CCK 2.x

mcpuddin’s picture

I tried the unseraize-1 patch but had ordering problems. However, unserialize-2 (the patch in #360) fixed the ordering.

Thanks thekevinday!

thadwheeler’s picture

This was a lifesaver for me. Thanks. I am testing it on a local dev and hope to roll it out to a live dev soon.

dspring0021’s picture

Umph...my head is spinning! To anyone who is willing to give me some guidance, I am running CCK 2.8 on Drupal 6.19. All I know is that--based on descriptions of each--I (think I) need multigroup and nestedgroup, but after spending the better part of 3 hours reading this thread and the state of the multigroup thread, I have no clue what I need to do to try to implement these. It's hard to follow the trail of archives, bugs, and patches and understand what works with which versions of the module. Can someone point me in the right direction, please? Thank you!

zeezhao’s picture

To answer #365:
1. for multigroup, first you need cck 3.x. You can get it on the release page - http://drupal.org/node/48429/release

2. Once this is installed, then for nested field patches to cck 3.x, see her for latest: http://drupal.org/node/849420#comment-3269982

That's all you need.

clashar’s picture

I can't find the file "fieldgroup.tabledrag.js" to be patched in the latest "cck-6.x-3.x-nested_fieldgroups-6.patch"
it refers to
diff -Nurp ../cck.orig/modules/fieldgroup/fieldgroup.tabledrag.js ./modules/fieldgroup/fieldgroup.tabledrag.js

where it is? I searched modules: CCK 3, fieldgroup, but didn't find there.

thekevinday’s picture

The patch adds the file.
It should not exist until after the patch is applied.

I can only assume/guess that you are not applying the patch correctly.

Normally you would so something along the lines of patch -p0 -i cck-6.x-3.x-nested_fieldgroups-6.patch.
(or alternatively change to the source directory and execute: patch -p1 -i cck-6.x-3.x-nested_fieldgroups-6.patch.

You can see the drupal tutorial on such here: http://drupal.org/patch/apply

klonos’s picture

...or he was simply patching manually. If so, create the file yourself ;)

clashar’s picture

thekevinday, thanks for link!
klonos, a little bit manually.

I run shared virtual hosting and don't have access on Linux, so I usually do patches with NetBeans IDE on my Windows PC and upload by FTP, but with this time I checked the file after applying the patch and it was not patched properly. File "content.admin.inc" had about 300 lines added, so there was a problem with either my software or the patch, I don't know exactly.

Then I divided file to different independent files for each file to be patched, that's how I couldn't find fieldgroup.tabledrag.js ))

siliconandincense’s picture

So after reading all this am I right in saying that Multigroups within multigroups are not yet possible, and probably not even feasible?
Thanks.

jvieille’s picture

Following this thread for months, expecting something great.
After doing some testing, before the patch:

Content creation
Fieldgroup F1
-field F1.1
-field F1.2
Multigroup M1
-Field M1.1
-Field M1.2

Rendering
Fieldgroup F1
-field F1.1 => value a
-field F1.2 => value b
Multigroup M1/1
-Field M1.1 => value c
-Field M1.2 => value d
Multigroup M1/d
-Field M1.1 => value e
-Field M1.2 => value f
...

After the patch
Content creation:
Fieldgroup F1
-field F1.1
-field F1.2
-Fieldgroup F2
--field F2.1
--Field F2.2
-Multigroup M1
--Field M1.1
--Field M1.2

Forbidden:
Multigroup
-Fieldgroup
and
Multigroup
-Multigroup

370 posts later, we just got the "simple" fieldgroup nesting, for which I can really figure out a practical critical use case.
Is there any chance that Multigroup could be addressed as well?

Thanks

jvieille’s picture

I think this is part of what we need to complete this work
http://drupal.org/node/520956

KarenS’s picture

Status: Needs review » Closed (duplicate)

I'm going to close this issue in favor of a newer, cleaner issue that has the latest cumulative patch on it, #849420: Cumulative Nested Fieldgroup Patches.