Problem

If a field can have unlimited values, Drupal adds an empty field after the last completed field on the node or entity edit page.

But if you are using the field collection embedded widget with a field collection that has unlimited values and contains a field with a default value, the field collection thinks that a user has entered content and will add a new field collection on entity save. So each time you edit a node that contains an unlimited field collection that contains a field with a default value, a new field collection will be added to the node.

Field collection needs a way to contain fields with default values and not create new collections unless the user wants it to.

To reproduce:

  1. Make a content type playlist.
    -- Add a field collection field_track
    -- -- with field_artist
    -- -- and field_song_name.
  2. Set field_track to use the embedded widget, and contain unlimited values.
    Give field_artist a default value of "Mozart".
  3. Create a new playlist.
    Give field_artist and field_song_name values (for example, "Beethoven" and "Für Elise")
    Save the node.
  4. Now edit the node, and save it without changing anything.

Expected result:

A playlist with one track: "Beethoven" and "Für Elise".

Actual result

A playlist with two tracks: "Beethoven" and "Für Elise", and a second with the default artist "Mozart" and no song name.

Cause

field_default_form() checks to see if the entity is new before populating the default values. However, even when editing an existing playlist, each new track is considered a new entity, and its fields are populated with default values.

Proposed resolution

Find a way to trick field_default_form() into not firing for field collection items, at least when using the embedded widget.

Remaining tasks

Fix the problem :).

User interface changes

None yet.

API changes

None yet.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Priority: Normal » Major
nils.destoop’s picture

Same problem without the default values.
When using unlimited collections. New, empty collections will be created when saving the node.

mh86’s picture

Status: Active » Needs review
FileSize
680 bytes

serializing the host entity fixed the problem for me (this was changed in #1208514: Entity object serialization is unnecessarily fat)

mh86’s picture

I have to note that I'm using the hidden widget and add the field collections after the entity has been created. Still, I think it's the same problem.

fago’s picture

Title: Field collection items with a default value result in new items on save » Embedded field collection items with a default value result in new items on save
Status: Needs review » Needs work

I had a short look at this. The problem is that we now 2 ways of defining default values.
1) with an embedded widget, you can specify a default value for the whole field-collection. This default works for me with the embedded widget, thus it doesn't create duplicates. However, it's not applied when a collection is added via the separate form.
2) you can specify defaults on a per-field basis for all fields in a collection. This defaults are applied regardless of how field-collections are added, but led to the described behaviour of duplicated entries as field-collection items with those values are not recognized as empty any more.

Proposed solution:
* Only support one way of setting defaults, option 2) as it works regardless of the widget.
* So, we need to remove option 1)
* We need to to fix the is_empty() handling. E.g. if a new field-collection item has some values we need to store them. But we cannot know whether the user intends to save the defaults or wants to ignore them.

Thus we have two ways to proceed:
a) Always display a new "empty/default" item when editing a node - but leave it totally empty regardless of the defaults. So the defaults only apply to the first item.
b) Never display a new empty/default item when editing a node. Only add it on demand, so we can apply the defaults and store them if they aren't changed. And yes, a remove button would be nice too.

mh86’s picture

I'm in favour for option 2 (default values) and b for new items

fago’s picture

I just were able to reproduce the issue mh86 described in #4 and committed a fix for it.

westbywest’s picture

Using field_collection v7.x-1.x-dev (downloaded last week), it looks like another symptom of this problem is that form submission will bounce back to the user if you have a required field (in my case a text field) in the collection with no default value. Note this happens with editing an existing node, not with adding a node.

That is, the empty item appended by default to the end of the field_collection gets treated as a genuine time to add, and the form submission fails on a missing value for the required field. The patch in comment #4 seems to have no effect on this problem.

westbywest’s picture

My work-around to this issue was to add a custom module with form_alter() code that removes the blank item appended to be bottom of the field_collection table. In this instance, the collection is "field_items".

/**
 * Implements hook_form_FORM_ID_alter().
 */
function custom_form_nodetype_node_form_alter(&$form, &$form_state, $form_id) {

  //remove the blank item field_collections appends to bottom of collection table in node edit form
  if (arg(2) == 'edit') {
    $max = 1;
    foreach ($form['field_items'][LANGUAGE_NONE] as $key => $item) {
      if (is_int($key) && $key > $max) {
        $max = $key;
      }
    }
    unset($form['field_items'][LANGUAGE_NONE][$max]);
  }
}

liquidcms’s picture

sorry.. i don't see how #9 is checking to see if field is empty... think solution is generally ok; but need to actually look much deeper into group to see if it has a default value set

liquidcms’s picture

this partially works:

function project_images_form_alter(&$form, &$form_state, $form_id) {
  //remove the blank item field_collections appends to bottom of collection table in node edit form
  if (arg(2) == 'edit') {
    $max = 1;
    foreach ($form['field_imagegroup'][LANGUAGE_NONE] as $key => $item) {
      if (is_int($key)) {
        if (!$item['field_projectimages'][LANGUAGE_NONE][0]['#default_value']['fid']) unset($form['field_imagegroup'][LANGUAGE_NONE][$key]);
      }
    }
  }
}

but doesn't solve the case when someone Adds New Item and saves with it empty.

jfinkel’s picture

This may or may not be germane to this. I have a Field, nameaddressfield, with two elements: Name and Email. I attach this Field to the Basic Page content type and set it to have unlimited values.

When editing content, the elements are displayed as Name -- Email in the first instance. However, if more are added (by clicking on Add new item button), then are displayed as Email -- Name; that is, in reversed order.

If this is not germane, then perhaps this can be established as a separate issue. Thanks.

xrampage16’s picture

I've gone ahead and written a small patch, that while not being the best solution, will work for the needs that I think are presented. I am personally having the issue that default blank data is adding another field collection each time that node is saved. This patch does the following...

It checks that there are at least one item in the field collection (otherwise, adding a node or attempting to add the first item will not be possible.)

If there exists at least one item, if checks to see if the value of $delta is the same value as the number of items, which equates to the $delta of the tail item that is displayed when you edit a node. If it is, it returns FALSE, not rendering the $element object.

If you click "Add another item", the $delta will be different than the count, so it will display normally.

This is quick fix for the current situation, and is working fine for my needs. Hopefully there will be a proper fix in the future.

Also, this patch is for the September 26th dev release that is currently available on drupal.org

mishac’s picture

The way I solved this was putting this in a hook_form_alter:

//remove the blank item field_collections appends to bottom of collection table in node edit form
  if (arg(2) == 'edit') {
    $field_name = $form['field_collection_item'][LANGUAGE_NONE]['#field_name'];
    $langcode = $form['field_collection_item'][LANGUAGE_NONE]['#language'];
    $parents = $form['field_collection_item'][LANGUAGE_NONE]['#field_parents'];
    $field_state = field_form_get_state($parents, $field_name, $langcode, $form_state);
    unset($form['field_collection_item'][LANGUAGE_NONE][$field_state['items_count']]);
    // decrement the items count.
    $field_state['items_count']--;
    field_form_set_state($parents, $field_name, $langcode, $form_state, $field_state);
  } 

HnLn’s picture

sub

Fidelix’s picture

Subscribing...

liquidcms’s picture

the patch in #13 sure helps this out a lot...i no longer get additional items with each save

comments on this:

- initial create of a node still adds 1 blank collection (it is visible on the initial node create form)
- if someone hits add another; but doesn't fill it in; still get 1 empty item - this likely can be solved simply by adding a remove collection item button (another issue posted for this)

jox’s picture

Status: Needs work » Needs review
FileSize
4.68 KB

Here is a patch that allows to hide the blank items of field collection forms. It adds an option named 'Hide blank items' to the settings of any field collection field. With this you can configure exactly which field collections should have no blank items.

It addresses both problems with default values and required fields in field_collections.

Per default this option is disabled so the default functionality is not touched. To enable it, go to 'Manage fields' (either at content type or field collection editing) and edit the field. Down at the bottom of the page you find the option 'Hide blank items'.

I tested it also with nested field collections.

After patching a cache flush is required.

A help message is added to the 'Manage fields' page to give a hint about this option.

Some technical info: It still just removes the blank item that always gets added by the field form api. I used hook_field_attach_form() to modify the form and form_state in a hopefully proper way.

It builds upon and works well together with the only just added delete button (#1234574: Add a delete button on the field-collection item edit form).

Chipie’s picture

The Patch in #18 works great for me. Thank you for your work, jox.

xcono’s picture

xcono’s picture

Thanks for the patch.
If field collection is prohibited to view, edit or create, the field does not contain properties ['#max_delta'].

if ($field_info['type'] == 'field_collection' && $field_info['settings']['hide_blank_items'] && $form[$field_name][$lang]['#max_delta'] >= 0) {
          // Here we modify the $form and $form_state to remove the blank item form elements.
		  $last_index = $form[$field_name][$lang]['#max_delta']--;
		  unset($form[$field_name][$lang][$last_index]);
          unset($form_state['field']['#parents'][$field_name][$lang][$last_index]);
        }

and i get notice:

Notice: Undefined index: #max_delta in field_collection_field_attach_form() (line 1202 in /sites/all/modules/field_collection/field_collection.module).
Notice: Undefined index: #max_delta in field_collection_field_attach_form() (line 1204 in /sites/all/modules/field_collection/field_collection.module).

Perhaps in this case to check for this property? Or is it my particular case? What do you think?

if(!empty($form[$field_name][$lang]['#max_delta'])){
//...
}
John Pitcairn’s picture

Nice patch jox, thanks. I think this will keep my users happy.

I'm getting the same notice as #21 when editing the parent node - user has view/edit/create permission, but field collection is using the "hidden" widget. I think adding a #max_delta check would be a good thing.

jox’s picture

Thanks for the reports.

Ok, there is two situations where the notices appear:

  1. The user does not have access to the field.
  2. The field collection has widget type 'hidden' (as opposed to 'embedded').

To fix both cases I added two checks.

if ($field_info['type'] == 'field_collection' && $field_info['settings']['hide_blank_items']
  // 1. Added check for edit access:
  && field_access('edit', $field_info, $entity_type)
  // 2. Added check for hidden widget:
  && $form_state['field'][$field_name][$lang]['instance']['widget']['type'] != 'field_collection_hidden'
  && $form[$field_name][$lang]['#max_delta'] >= 0) {

I hope this fixes it.

A new patch is attached.

John Pitcairn’s picture

Patch in #23 is good for me thanks, using either the embedded or hidden widget. I have not checked against field-level access permissions (which I am not currently using).

pegah_m’s picture

But yet it does not work for me!!!
the select list has not default value but it shows the first item and save it as a value in database
what is wrong with it?
regards

jox’s picture

@pegah_m: Did you enable the "Hide blank items" option in the field settings for the field collection (in the content type that contains it)?

pegah_m’s picture

Wow
thanks alot, i did not enable it. really thanks

charlie-s’s picture

I applied #23 just fine and checked the "Hide empty fields" box, but I'm still seeing an empty field on node edit and then 2 empty fields on save, etc. The field collection that I'm using doesn't have any default values or required fields, just 3 text fields and 1 checkbox -- does the checkbox equate to a default value if it's off?

jox’s picture

@csdco: Did you flush the caches after patching?

charlie-s’s picture

Indeed. Will any other info help?

jox’s picture

Ok, strange then... What confuses me is that you got this behavior with no default values and no required fields. It should not behave like this even without this patch...

The checkbox should not be the problem. But who knows. You might want to try once without the checkbox and see what happens. (Temporarily remove it).
I tested a field collection as you described (checkbox and text fields), but can not reproduce it. Even if I give the checkbox distinct values for on and off (like "On" and "Off").

I realized that (with this patch) if I create a new node, then there is indeed a single blank item set (with "Hide blank items" enabled).
But, if I...
a) ...save the content, there will still be only one item when editing again.
b) ...remove the blank item and save it, there will be no items when editing again.

Here is some questions:

  1. When you remove all (!) the field collection items from a node and save it. Will the blank item be back, when opening it for editing again?
  2. Did you try to once create a new field collection (similar to the one you're using) and test it's behavior? Just to check if there maybe is something wrong with your previously existing field collection.
  3. Could you try that also in another content type?
  4. What Drupal core version are you using?

I will take a look at the blank item when creating new nodes. This shouln'd happen.

jox’s picture

I got something odd happening here now. It's regarding the number of default blank items when creating new nodes.

With "Hide blank items" off, I have the case where there is two blank items for a field collection. When I enable "Hide blank items" there is just one.
After saving a new node for the first time, everything works ok. I can remove them all and they stay removed.

I've tried several configurations. Sometimes it works like it should (and like it did in the beginning): when creating new items there is no blank item in the form. But I have not yet found out what causes two blank items to appear.

So far it seems kind of arbitrary. It will need some more debugging to narrow that down.

There might be something wrong in the field api. I have also other issues with fields that are used in more than one content type. For example the widget for a field that is configured in one content type will be used for the other content type as well. The widget configuration in the other content type has no effect at all.

I'm working with Drupal 7.9 here, so an update to 7.10 is also still due.

jox’s picture

All right, I found it. The reason for the two items in a new node form is that the "default item" is added plus the usual blank item. The "default item" is the one defined in the content type of the containing element.

If the default item has some default value in it, then it will be added additionally to the blank item. At least as far as I can tell right now.

Some bonus nuisance: If the field collection gets edited and some default value is added, then the above effect will not take place until the containing content type is saved once (with or without any change). This gave it some special mystical touch.

The only (kind of) proper way to fix it was to add a 'default_value_function' hook to field_collection instances and have it return an empty array() (zero items).

Unfortunately core is ignoring the 'default_value_function' hook, unless you patch it. See my patch at #1342984: Field API: Default value function not called. It will work ok without this core patch, there might only still be one (or two) blank items when creating new nodes.

After all I'm getting a bit tired, since the root of all this evil (with growing tendecy) actually lies in the limited core functionality/support. Some small core addition could solve all this in a much cleaner way.

RedJane’s picture

Yes... Much better now !! Thank you.
I also solved it using a form_id_alter (checking if there were empty fields).
Patching the "Default value function" (Core) and using your latest patch => all OK !!
Me is happy :-)

jox’s picture

Glad to hear that! :-)

charlie-s’s picture

jox, thanks for the additional patch. My problems appear to be solved! This has plagued me for quite some time, I really appreciate the work you've put into this.

jox’s picture

@csdco: Great! You're welcome and thanks for reporting back.

tim.plunkett’s picture

Status: Needs review » Needs work

Thanks for the patch. Code review time!

+++ b/field_collection.moduleundefined
@@ -559,11 +559,16 @@ function field_collection_field_info() {
+        'hide_blank_items' => FALSE

Missing trailing comma.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+ * Implements callback 'default_value_function'. Returns zero items to prevent two blank items
+ * to appear in forms when creating new nodes.

Adapt this to http://drupal.org/node/1354#callbacks and move the second part into a summary section after a blank line.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+  $defaults = field_info_field_settings($field['type']);
+  $settings = array_merge($defaults, $field['settings']);

Why do this? I haven't seen this done. If there is a reason for it, document it.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+  ¶

Be mindful of trailing whitespace. There are 4 or 5 more.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+    '#description' => t('A blank item is always added to any multivalued field\'s form.'

Wrap with double quotes to avoid escaping apostrophes.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+      .' If a field collection has default values or required fields, then the blank items will always undesirably be added when the node is saved.'
+      .' To avoid this, the blank items can be hidden with this option.'),

Don't break t() strings over multiple lines.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+ * Function to alter the fields overview and display overview screen.

See the section within http://drupal.org/node/1354#hookimpl about hook_form_alter callbacks

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+  // Add extra information for the webmaster about the 'Hide blank items' option.

I don't know what term to use, but I don't think webmaster is a common one.

+++ b/field_collection.moduleundefined
@@ -572,7 +577,45 @@ function field_collection_field_info() {
+  $form['#prefix'] = (isset($form['#prefix']) ? $form['#prefix'] : '') . t('<p class="field-collection-help">Note that if you have field collections with required fields or fields that have default values, you might consider checking the \'Hide blank items\' option in the field settings of the field collection field.</p>');

Move HTML outside of t(). Use double quotes so you don't need to escape apostrophes. Consider removing the ternary in favor of if (!isset($form['#prefix'])) { $form['#prefix'] = ''; } $form['#prefix'] .= t('String');.

jox’s picture

Status: Needs work » Needs review
FileSize
5.44 KB

Thanks for the detailed list. Fixed everything. New patch is attached.

cjoy’s picture

is the core patch required for this field_collection patch to work?
I've patched the module with the latest patch (Dec-29), but not core, and still find additional array items after each edit/save of the entity. If there is no other way than to hack core, so be it ... but if it is not absolutely needed I'd rather not.

tim.plunkett’s picture

@cjoy think of it as *patching* core, not hacking.

If you closely track that issue, and continue to update your version of core with any new patches, it's not really hacking.

jox’s picture

@cjoy: Without the core patch there should only (possibly) be a blank item when creating new nodes. If that is removed manually right away, everything should be working as expected.

Just in case: Did you flush caches after patching?

Could you patch core once to see if that fixes your issue?

If not, could you describe your situation / content type / field collection a bit more in detail to get an idea of what might be the cause?

Btw. the core patch is only temporary and will be comitted soon hopefully. It is more a fix than a hack.

cjoy’s picture

@tim.plunkett, @jox:
thanks to both of you for the clarification.
I must admit the problem was sitting on this side of the screen - I did not configure the field-collection settings properly ('hide blanks').

The patch works just as expected, in my case without *patching* core. I've checked both the form content in the UI and the actual entity object for unwanted (duplicate default) instances of FCs: all clean.

Thank you ever so much for resolving this issue!

jox’s picture

@cjoy: Sometimes it's good news when somebody made a mistake. ;-) I'm glad you got it working. Have a good start in the next year!

will_in_wi’s picture

Status: Needs review » Reviewed & tested by the community

I tested #39 as well and it works.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs work

This doesn't work for me. Tried it with and without the core patch, and I made sure I checked the "Hide blank items" checkbox.

My field collection contains a date field with a default value, and a text field with no default.

jox’s picture

Could you give more details? What is not working? What happens if you create a new content.? What happens if you edit the content afterwards?

Some instructions to reproduce would be helpful.

tim.plunkett’s picture

The steps are exactly the same as they are in the OP. Nothing has changed. If I have more time I will try to debug it more myself.

I tried both adding new nodes, as well as editing existing ones. Just hitting edit, then save, new values were added each time.

charlie-s’s picture

You cleared your cache, correct?

tim.plunkett’s picture

Hmm, after creating a new instance it seems to be working, but not like I expected. I would think it would only affect edit, not add. Going to add a new node and having to click "Add another item" is strange.

jox’s picture

Hmm, ok. You know what?

It's not working as you would expect. And you're probably right.

It is workin as I expect. And I'm probably right too.

You don't want blank items, but you want a default item. I don't want blank items nor a default item.

I want zero or more (0..*) you want one ore more (1..*).

So who is right? We're both right. It's different requirements, both of which are legitimate.

So my conclusion is, we need yet another field level option like "hide default item" (next to "hide blank items"). This would make it most flexible and everybody happy. With this, every single field collection can be configured as the requirements demand.

What do you think?

Btw. it's the part that needs the core patch which hides the default item.

Fidelix’s picture

"hide default item"

Jox, this is one of the things missing in core fields which would be nice to have ^^

Thanks for the good work!

Pepe Roni’s picture

I applied the patch in #39, too, and it worked as desired (even with required fields within the field collection). The field collection itself is not required.

I have an image and a computed field in the field collection. I use the field collection mechanisms to upload images to a node without editing the node, but keeping track of the "author" of this image (the computed field).

Great work. Thanks!

bradjones1’s picture

Re: #50, I concur this probably isn't where it needs to be, at least for existing field_collections. I applied the patch at #39 (without touching core) and created a new node of an existing content type using field_collections, and I continue to get a "new" instance on every node save. Haven't yet tried from scratch, creating a new field collection and placing it on a "new" node type.

In summary, this doesn't appear to work on top of an existing deployment.

Edit: I had not yet checked the "hide blank items" - however, on creation of a new node, you still get a single "default" item in the form, and it's saved even if not edited. But at least now the problem doesn't compound on successive page edits.

jox’s picture

@bradjones1: the single "default" item on creation af a new node is probably because you did not touch core. This is already documented in this thread. See #42 for example.

bradjones1’s picture

@jox - you are correct - sorry I missed that. In that vein, I think this patch makes the module work a heckuva lot better than without it... What's required here to get to RTBC?

tim.plunkett’s picture

AFAIK my problem in #50 hasn't been addressed. Please correct me if I'm wrong (and set it back to needs review).

bradjones1’s picture

@tim.plunkett - Hi Tim - per #54 and #55, I think the way this is working (without the patch to Core) is that you DO get a single default instance of the field collection on the node add form. It's still slightly buggy in that if you save the node without explicitly removing it, the instance gets created anyway. But what this patch does successfully address is the annoying auto-population of instances on node edits. So at least in my testing locally it looks like you don't have to click "add another" when creating new.

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Needs work » Needs review

Hm, maybe I tried it with the core patch. I'll have to look again.

Pepe Roni’s picture

Assigned: tim.plunkett » Unassigned
Status: Needs review » Needs work

@#50, #57

I do not want to have a default item added on node add! I want the user to explicitly add another item even if it was the first item. My use case is described in #53. If there is currently no image loaded, I want to display "No Image" and not an empty image (or even worse: a number of empty images). So I set the image in the field collection required but the field collection field itself not. If there would be a default item on node add, I would require the user to add an image immediately which is not what I want.

I did not patch the core!

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Needs work » Needs review

#60, you would need to patch core for that to happen.

jox’s picture

Please read #51. It is two different use cases, two different valid requirements. I think it would not be wise to just decide for only one of them. The option to choose between them is necessary in my eyes.

tim.plunkett’s picture

@jox, so then this is needs work?

jox’s picture

Assigned: Unassigned » tim.plunkett
Status: Needs work » Needs review

@tim.plunkett: If you agree, I'll work on it. If not, I won't waste my time. ;-)

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Status: Needs review » Needs work

@jox Oh I completely agree! I thought you said it was impossible to cover both cases without patching core, but if you can make it a setting for the module I will commit that no problem!

jox’s picture

@tim.plunkett: Well, I not sure if I said that, but it's true. The core patch (better: fix) would be necessary. But this doesn't make the option useless. And are we not assuming that the core thing gets committed one day?

Pepe Roni’s picture

Assigned: tim.plunkett » Unassigned
Status: Needs review » Needs work

@#61: Why patch the core? It works like I expected.

And I agree with jox: We need a configuration option for default items. But I think this option should only be related to the form display: how many empty items should be displayed on the node edit/add form? And an option how to handle empty items on node save (not "hide empty items", but "save empty items,too"). And a third option "how many empty items should be added to the node form when pressing "add another item" (but that's another issue).

cjoy’s picture

is it just me, or is the patch no longer applicable to the latest beta3 build?

cjoy:field_collection cjoy$ git apply --check fix-blank-item-problems-1239946-39.patch
 0 files changed, 0 insertions(+), 0 deletions(-)
cjoy’s picture

Status: Needs work » Needs review
deepchandra84’s picture

klonos’s picture

@jox, #51:

I want zero or more (0..*) you want one ore more (1..*).

...wouldn't having it "zero or more" + make the whole field required make it a "one ore more"? So, we don't actually need an extra checkbox/setting - we have the per-field "Required field" checkbox. I don't know, just a thought.

The "Add another" button when a field is set to have multiple values is a core feature AFAIK, but what if the "Add another" button was in fact a "Add one" button initially and changed to "Add another" only after one value was added? I'm not talking about the button's label here but rather about the logic behind it and the way it works/behaves...

That way:

- Having a field (or a field_collection field for that matter) multi-value-enabled would simply present a "Add one" button in the edit form (no field/widget shown on the form by default). It's like telling the user: "This field is not required, so you can omit it, but the add button is here if you want to enter a value." Even if the field has default values defined, these are not pre-filled (there's no instance of any field/widget) until only after the user hits the "Add one" button.

- Now, only if you also mark the field as required would the form contain a field/widget with a "Add another" button. It's like telling the user: "This field is required! That's why by default there is an instance of it in the form for you to fill in. You cannot opt-out!" If the field has default values, then these are pre-filled when the form (and the field/widget) is shown to the user.

klonos’s picture

...perhaps my previous brainstorming fits more in #1374138: Provide option to use "hidden" widget UI from parent form, but rather than having it be a field_collection hack we should make it a core feature. A very similar issue exists in #968112: Allow addressfield to be optional for the Address Field module.

george.plescan’s picture

I have to ask which patch to apply and to what version it is intended for. The number of posts is quite big and it seems that there are two opinions on how the right behavior should be for this module on default values.

Also I want to ask if any patch might affect things like field collection permissions, especially the Field Permissions module or the Field Group module.

Thank you for the patch(es).

rasmusth’s picture

Any news on this? Can't get any of this to work. If I unset the last field manually in my module the remove button seizes to function.

Fidelix’s picture

@george.plescan, you must apply this patch to Drupal core:

http://drupal.org/files/fix-default-value-function-callback-1342984-4.patch

And then this patch, by jox, to field_collection:
http://drupal.org/files/fix-blank-item-problems-1239946-39.patch

After that, go to 'Manage fields' (either at content type or field collection editing) and edit the field.
Down at the bottom of the page you'll find the option 'Hide blank items'.

BTW, Thank you again jox. I hope the core bug is fixed soon, and this gets commited on field_collection.

charlie-s’s picture

@rasmuth follow the instructions precisely in #75 and then clear your cache @ admin/config/development/performance

muschpusch’s picture

When cardinality of the field is set to one and both patches are applied. The field collection doesn't show up anymore

super_romeo’s picture

Patch #75 works. Thank you.
I like node edit page now.
But
1) I think, that on node add page field should have one empty value by default (now I must click "Add").
2) My field is required, but i CAN save node with empty value. It is not suit for me.

GaëlG’s picture

GaëlG’s picture

The "1 or more" option is resolved by #17, isn't it?

super_romeo’s picture

From #17 "- initial create of a node still adds 1 blank collection" - it is not so after applying patch #75. And it is not good.

GaëlG’s picture

#75 is for the "0 or more" case. Note that it this case, set the field collection to hidden might fit the needs.

droath’s picture

Patch #75 works, but I agree with @super_romeo on that we should allow for one empty field to be created by default. Since there are different situations where having a default collection present, may or may not be needed. So I think the only way to resolve this for all use cases is to have some configurations that react on these exceptions.

As for being able to save a node, even if a field collection field is required, feels like a separate issue. So i didn't include anything in the below patch to help resolve this problem.

I have attached a re-rolled patch that exposes some exceptions within the field settings, it only contains one exception at the moment. :) If enabled and it a is valid, it will dismiss hiding the first blank field collection item. Let me know if this is a desirable approach, or have any suggestions on how we might go about this a different way.

neddstark’s picture

I believe patch #83 intends to prevend hiding the field collection if it has no values. It seems to work, but it has an issue: if you click "Add another item" first time, no empty widget is shown.

neddstark’s picture

I modified the patch to solve the problem:

neddstark’s picture

Sorry, I previously cut your exception setting, here's the right one:

damontgomery’s picture

Can we update the OP with some info for people? I use patch in #39 without any other patches and it works great for this problem. That seems to be the current accepted best solution. I appreciate the other work being done recently to expand this for other use cases.

Something really needs to become part of the module since it's broken as it is.

neddstark’s picture

#86 is patch #39 with a new setting, to have the first empty widget shown

bobslee’s picture

Thanks for the patch (in #86).
I've still some questions about implementation:

1. The 2nd option in the field_collection's settings form First item in collection, doesn't make sense to me yet.
When I enable it, the (multiple field) form_widget doesn't hide the field collection.
What could I expect from it?

2. I tested validation on node save. Only when the field collection and at least 1 field inside the field collection are both required, then validation occurs.
If only the field is required, no validation occurs.
Is this expected behaviour / meant to be?

lukus’s picture

#86 works well for me.

Thanks very much

wjaspers’s picture

Status: Needs review » Reviewed & tested by the community

Works well for me.
I dont know if an update would be required to handle the new settings made available so prior versions are properly handled.
Otherwise it works!

thoughtcat’s picture

I'm afraid I'm a non-coder and don't use a VCS. I'm also not sure which of the patches posted here now apply as there seem to be several, but I really favour fago's option (b) in comment 5 "Never display a new empty/default item when editing a node". If any of these patches do this, can anyone supply me with a complete replacement (patched) file in some form that I can upload to my site via FTP? Thanks in advance :)

rooby’s picture

Status: Reviewed & tested by the community » Needs work

I hate to hold this up longer but I would suggest adding an update function to set default values for existing fields.

For example, apply the patch and then edit an existing field collection field and you get:

Notice: Undefined index: hide_blank_items in field_collection_field_settings_form() (line 612 of /var/www/mysite/sites/all/modules/contrib/field_collection/field_collection.module).
Notice: Undefined index: hide_blank_items_exceptions in field_collection_field_settings_form() (line 623 of /var/www/mysite/sites/all/modules/contrib/field_collection/field_collection.module).

rooby’s picture

A little more review:

* I agree with #89. Either the 'First item in collection.' option is buggy or it is badly described. And I don't think the main option performs exactly as advertised either.

What I would expect to happen when the 'Hide blank items' option is checked and the 'First item in collection.' option is:
- Unchecked: When creating a new node no items are displayed by default. You have to click the 'Add another item' button to get one. When editing content, only the items with data are shown, with no empty items until you click the button.
- Checked: When creating new content the first item is displayed by default, and when you are editing an existing node with data in the field you would get one additional 'Empty' item.

What actually happens:
- Unchecked: When creating a new node the first item is displayed by default, if you save the content the default value automatically saves (what I am trying to avoid with this patch). When you edit the content again no additional 'empty' item is displayed (as is expected).
- Checked: You get two empty items displaying, if I enter data into the first one it gets saved along with the default values of the second one. When editing the content again I only see items with existing data, and no empty items.

So its all a bit confusing and still doesn't solve the original problem as there is no way to save a new node without saving the default values of the first item with any combinations of settings.

I'm also not sure about the terminology 'Hide blank items' seeing as they are not necessarily blank in the case of default values.
Although until I think of something better I don't really have a proper argument against it.

rooby’s picture

Also, the title 'Hide blank item(s) exceptions' should probably be 'Hide blank items exception(s)'.

Coding standards look good aside from a few places where comments go past 80 characters on a line.

I haven't had a chance for a proper functional review of the code yet.

Fidelix’s picture

Actually, it should be "Hide blank item(s) exception(s)"

rooby’s picture

But the setting is called 'Hide blank items' so the exception(s) to that should be 'Hide blank items exception(s)' because they are the exception(s) to the hide blank items setting.

John Pitcairn’s picture

I think just the simple plural is OK - "Hide blank items exceptions". One-or-many is implied, and the result is more readable.

If you put the checkbox and exceptions in their own fieldset then perhaps just "Exceptions" is sufficient.

Akshita’s picture

FileSize
46.25 KB

Is there a way to remove the required condition on default values of type 'field collection' ?

wjaspers’s picture

@akshita:
Your issue should be filed separately.

In response, though, field requirements don't step on each other.
The collection as a whole can be required/not required on the content that references it.
Additionally, each field inside the collection can be individually required/not required.
If you start completing a collection on the parent form, the collection's field requirements are enforced.
If you don't fill in a collection on the parent form, and it is required, it will force you to create one.
If you don't fill in a collection on the parent form, and it is NOT required, it will be left alone.

Akshita’s picture

@wjaspers-thanks for the info

Additionally, each field inside the collection can be individually required/not required. -How can we do this?

Any help is appreciated.

wjaspers’s picture

This is rather tough to explain, but here goes:
The requirements aren't inherited where used (ie. you can't define which fields in the collection are required when its attached to a "Page" node, for instance), they're set explicitly on the field collection's bundle definition -- all instances of the collection's fields carry the same requirements.

field_U, field_M ---> collection X <--field_collection_reference_x-->content type A
field_U, field_M ---> collection X <--field_collection_reference_x-->content type B

Both content types have access to the same collection/field. But, each content type is given the opportunity to define if the reference field attached to them is required. BOTH content types will be forced to respect the collection definition's internal field requirements.

I don't remember the exact path to the field collection bundle administration page (/admin/structure/field-collections/) ??, but that's where you would alter these settings.

Akshita’s picture

I figured out how to make each field inside the collection to required or not required based on my requirement.

Anyways thanks and appreciate for the prompt responses.

Anonymous’s picture

It seems to work perfectly !
Very good job.
Thank you very much

damontgomery’s picture

Ok, this thread is such a jumbled mess. The forum really needs an option to float the currently accepted solution (#39 or #86 for this thread) to the top so the thread is actually usable without reading 100 comments.

What do we need to do to get some type of fix into field_collection? I would consider field collection to be considered almost completely broken without the fixes in this issue.

Can we get a fix v1.0 approved and then work on the bonus advanced configurations (and nitpicking pluralization) after that? I appreciate this work, but it shouldn't delay the basic fix.

#39 basically works and was submitted and tested 4 months ago.

rooby’s picture

The plural issues were just a side note.

Please see #93 & #94 for the reason why this was put back to 'needs work'.
- I based that on the patch in #86, which I don't believe works as described.

If the patch in #39 is a legit differenet approach then I agree this is confusing.
I will try out #39 soon to see the differences.

I also agree that this module is not very useful without a solution to this.

damontgomery’s picture

Here is my proposal.

#39 works or needs some clarification of what the option means. From what I remember, #39 allows you to check a box which will prevent any field collections from loading by default for each item. The user must click "Add another item" to add the first or any subsequent fields. Blank items are not saved.

#86 introduces usability options to extend this behavior, but also introduces issues (as far as I know) because of this. This allows users to select from:
A: Default to no items in create / edit
B: Default to 1 blank item in create / edit
C: Default to add a new blank item with each create / edit and save blank items (CURRENT DEFAULT BEHAVIOR)

I'm not really sure what all the discussion about #86 was about.

The last issue is the issue of inherited "required" fields. #86 specifically states it was not intended to solve this issue since it is a different one. I think this is true.

I think we should commit #39 in its current state or at least change the wording to be clearer and create a new issue / thread for extensions described in #86 and for the "required" field issue. #39 fixes an issue, #86 adds usability tweaks for niche cases, the "required" fields issue is completely separate.

rooby’s picture

I agree with the logic in #107.

I'll test the #39 patch tonight.

jide’s picture

Another possibility would be to add a "Empty value" setting in field settings, the same way "Default value" exists. That would allow to choose what combination of values would be considered empty. If a field collection has these values it would not be saved.

I'm not sure this is a better solution, just had the idea, what do you think ?

jide’s picture

One advantage I already see with my proposal is for existing field collections that have been added despite they should be considered empty...

dufferin’s picture

patch 39-2 works for me.
though I still have field displayed not under field collection even without value... but that's another topic.

rooby’s picture

@jide:

I would imagine that generally the only values considered empty would be actually empty or the default value in some cases.
I can't see much of a use case for anything else, although I could be wrong.

Either way, such a setting would be a separate issue to this one, and would likely not be a field collection specific thing.

jide’s picture

@robby: The way I see it, the problem described here happens when field collections include a field with a default value. This can only happen with a field collection.

The proposed fix kind of sounds like a workaround rather than a real fix to me : We change the regular core behavior for multiple fields having additional empty fields at the end of the form to prevent them to be saved. So we won't be able to have these additional fields with the proposed fix.

To quote fago in #5 :

But we cannot know whether the user intends to save the defaults or wants to ignore them.

So we cannot consider a field collection with empty values + default values = empty field collection. That's why I suggest we propose the user to optionally choose what values should be considered an empty field collection.

rooby’s picture

Yeah, I see your point now.

I also think there is room for a solution for hiding items until the add another item button is clicked to live along side something like you propose.

So the existing approach could go in and then your idea added after.

mgoh1’s picture

I'm also using this hack to prevent unwanted entity items of being created but I've a problem with the delete button now. I'm getting this ajax error "Recoverable fatal error : Argument 2 passed to drupal_array_set_nested_value() must be an array, null given, called in /var/www/intranet/includes/form.inc on line 2438 and defined dans drupal_array_set_nested_value() (ligne 6384 dans /var/www/intranet/includes/common.inc)."
Any ideas?
PS: I'm also using field_collection_table and field_collection_view modules with D7.12

edit: I'm using field_collection beta4 and tried with dev version but same result

rooby’s picture

@mgoh1:
The patch in #86 or #39?

mgoh1’s picture

I followed instruction on #75 so I think it's #39. Should I test with #86? Thanks

edit: I checked my code and in fact I'm using #86 but it's an additional functionality to keep the first item that I don't need. I also don't really like the core hack of field.module.

rooby’s picture

I have found #86 to not really do what it is supposed to.

I have not had a chance to check #39 yet but pandaeskimo says it is good for an initial fix.

With regards to patching core, in my opinion it is no different to patching any other module.
Don't do it unless you have to, but if you have to, try to use patches that might be committed at some point where possible.
And document your modifications somewhere so you can easily tell what is patched when it comes to upgrading.

mgoh1’s picture

The fix works. Only the delete button have an ajax error and it becomes impossible to remove an item. I really have to get rid of this problem. Field collection is a major fonctionality of my backoffice...

mgoh1’s picture

After investigations, the problem seems to come from these lines (field_collection.module line 1124):

drupal_process_form($form['#form_id'], $form, $form_state);

// Get the information on what we're removing.
$button = $form_state['triggering_element'];
// Go two levels up in the form, to the whole widget.
$element = drupal_array_get_nested_value($form, array_slice($button['#array_parents'], 0, -3));

Unfortunatly I don't see the trick... any ideas?

mgoh1’s picture

rooby’s picture

To be clear, the fix mentioned in #121 is not a fix to this issue but to the delete issue.

mgoh1’s picture

@rooby: the 2 issues seems related to me, sorry if I'm wrong

rooby’s picture

I was just saying so that people dont think that that patch fixes this issue because people may not realise you were referring to your previous posts (a lot people tend to often not read whole issues but only the latest post).

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
6.44 KB

I've rerolled the patch from #89.

The help text still needs some work.

BWPanda’s picture

I'd love to see this committed!

Only issue I noticed after I applied #125 patch was:
Notice: Undefined index: field_action_item in field_collection_field_attach_form() (line 1295 of /var/www/[snip]/modules/field_collection/field_collection.module).

I get 4 of those notices when editing an existing node (created before I applied patch), and 1 when creating a brand new node.

Note: field_action_item is my nested field_collection field (i.e. it resides in another field_collection field).

rooby’s picture

Doesn't look like anything has changed in the #125 patch from the #86 patch and the #86 patch was set to needs work because of #93 & #94, which have not been addressed as far as I know.

I'm happy to be proven wrong about #94 but from my usage back then it didn't do what was supposed to do.

I'll try again with the new patch soon.

BWPanda’s picture

Based on my usage of patch #125 (with both checkboxes checked), I'm not experiencing the issues described in #94 - everything works as expected...

petrovnn’s picture

#125 not working for me

My field collection item have two fields:

1. Image field (no default image)
2. single checkbox (not checked by default)

Checkbox "Hide blank items" are checked (settings of Field Collections Field)

Maybe i need replace checkbox with other control?

petrovnn’s picture

I replaced single CHECKBOX by SELECT, now all working fine

lukus’s picture

#125 isn't working for me.

I have a field in my field collection which is a term reference displayed as a select field widget. Option '-None-' is registering a new entry.

lukus’s picture

I've just tried #33 with the latest 7.1.x-dev release and it still applies (with whitespace errors). It works better than #125 / #86 for me.

rooby’s picture

@lukus:
The patch in #39 should be a better version of the patch in #33.

It has been said by a few people that that one is better than the #86 / #125 solution.

lukus’s picture

@rooby - thanks, I'll try that as well then.

I definitely think that the #86 / #125 solution has some problems.

fago’s picture

Status: Needs review » Needs work

* Only support one way of setting defaults, option 2) as it works regardless of the widget.
* So, we need to remove option 1)

I've implemented that + committed it. See http://drupalcode.org/project/field_collection.git/commit/15c64a9. That solves the issues as long as you don't have any non-empty default values in your fields that are part of the collection.

This might need a re-save of your field-collection field using an embedded widget to take affect.

Leaving the issue open, as it's still unaddressed for the non-empty default value case. I see two ways to so:

1.) Avoid showing the empty form, i.e. the approach from #89 / #125.
2) Make an option for ignoring some non-empty fields when check for an empty collection item. Similar to the proposed solution to the addressfield issue: #1263316: Configurable non-empty value conditions

To me, both options make sense. Even together as 2) would be nice even when showing only the form of the first field-collection, so it can be ignored if left empty. 2) could also be used to do required validation only for non-empty field-collections, thus could be useful also for single, optional field-collections having required fields. Also 2) is probably helpful for the single case anyway as well, as it prevents saving and thus displaying an empty field-collection.

Thus, I tend to think that it's probably a good idea to implement both options.

Ad 1):
I think the reasonable default behaviour of field-collection should be to always show the initial, empty form of the first item, but to don't show any additional empty form for adding a non-first item. Having other behaviour configurable is probably nice, but not a must-have for me.

fago’s picture

fago’s picture

Reviewed #128 for option 1):

+++ b/field_collection.module
@@ -586,7 +592,60 @@ function field_collection_field_info() {
+function field_collection_default_value_function($entity_type, $entity, $field, $instance, $langcode) {

That's now unnecessary, as I've fixed that already by disabling the field-api default value behaviour for the widget.

+++ b/field_collection.module
@@ -586,7 +592,60 @@ function field_collection_field_info() {
+    '#title' => ('Hide blank item(s) exceptions'),

That option sounds unnecessary complex to me. Why not just add an additional single checkbox that says something in the lines of "Also hide the first item" ?

+++ b/field_collection.module
@@ -586,7 +592,60 @@ function field_collection_field_info() {
+  $form['#prefix'] .= '<p class="field-collection-help">' . t("Note that if you have field collections with required fields or fields that have default values, you might consider checking the 'Hide blank items' option in the field settings of the field collection field.") . '</p>';

Let's just add this somewhere nearby its setting, i.e. to its description.
The default just be sensible though, so an additional help isn't required.

+++ b/field_collection.module
@@ -1179,7 +1240,47 @@ function field_collection_remove_submit($form, &$form_state) {
+function field_collection_field_attach_form($entity_type, $entity, &$form, &$form_state, $langcode) {
+  if (!isset($form['#entity_type']) || !isset($form['#bundle'])) {
+    return;
+  }

That unnecessarily goes around the corner. Let's use hook_field_widget_WIDGET_TYPE_form_alter instead. We could also go with our own multiple-handling logic but that would probably result in some unnecessary code duplication.

RobW’s picture

Revised description and marked #1615204: Additional input for collection showed on node edit as a dupe.

Nitebreed’s picture

#39 works for me, without any core patch

Nitebreed’s picture

Issue summary: View changes

The summary is pretty complex with that specific example, lots to mentally sort. Made language more general and easier to parse while keeping specific example.

Mat77’s picture

Idem, #39 worked for me as expected.
Thanks.

fago’s picture

Status: Needs work » Fixed

ok, I took a stab on that. I've reworked the patch to
* only have the "hide blank item" option and enabled it by default, but always show at least 1. (if it turns out having 0 as well is a use-case let's add this in a separate issue). Made that option only appear if cardinality is != 1.
* I've changed the approach and changed the embedded widget to not generate a form element for the blank item, but just return FALSE. That does the job in a very simple way. Also fixed the widget to always show empty items if a collection does not load.
* But with that deleting items had troubles as deleted items re-appeared when deleted + added new items. I reworked the deleting code a bit to make that more robust.
* Turned out that this adds an additional empty row to the widget if "add more" is used via ajax. Problem is core's ajax code which adds a div around the last item which we have removed. Fixed that by adjusting #max_delta with an approach similar to the patch of #125.

Given all that, it works good for me. -> Committed. Please test.
Let's also implement #1662998: Create configuration option to ignore chosen collected fields when checking for an empty field collection so themers don't have to deal with not-theming an empty-first-item.

charlie-s’s picture

fago, using latest dev release looks to solve your points and the issues I had as well. The only odd behavior I see is that if a field_collection contains a required field that is left empty, clicking the "add another item" button adds an empty div (with handlebar) below the default field_collection form. Is that related to your reference to #1662998?

kaizerking’s picture

save with default values,
I have setup a field collection field,the values of fields will always be set by rules action 'set data value',
it is single value field, with three fields I have set the default values to be set on creation of new field collection.
this is not happening, to get the values saved i need to click on 'add' and then save.
How to achieve my requirement , if this issue is not related to the current issue please feel free to push it to proper issue que

fago’s picture

ad #142 : Thanks for testing, I've committed a follow-up fix for that.

yannickoo’s picture

When using the latest dev version I cannot create a field collection anymore. I create a node with a field collection inside and when saving the node the field collection is empty. When going to edit page I can see that all field collection were emtpied.

EDIT: I just applied the patch and it works now.

charlie-s’s picture

@144 Just tested and working *perfectly*. Thanks, Fago :)

damontgomery’s picture

I know it's bad form to bump a topic without adding anything to the discussion, but,

HURRAY!

klonos’s picture

Status: Fixed » Needs work

Sorry for setting this back to NW. Following the steps to reproduce this bug from the issue summary, I see a similar behavior as the one described by Charlie in #142...

So, using the latest dev and hitting the "Add another item" button the first time simply does an AJAX refresh (in order to save the first item I guess - I don't know). After this "blink", only by hitting the "Add another item" button for a second time it actually adds a new item. Do you want me to open a new, follow-up issue for that and set this back to "fixed"?

Editing a "Playlist" node throws this:

Notice: Undefined index: el in field_collection_field_attach_form() (line 1142 of /var/www/my-site/sites/all/modules/field_collection/field_collection.module).

Does this belong here or is it related to #1193004: Editing a node should not automatically add a new field_collection item perhaps? Do you want me to file a separate issue for it too?

Also, I do have a use case where 0+ instead of 1+ fields are required. Is there a separate issue filed for that? Because my post in #1193004-10: Editing a node should not automatically add a new field_collection item feels out of place and would really need a home so it has a hope to actually be addressed someday.

klonos’s picture

...and just to make sure: this no longer requires/depends on #1342984: Field API: Default value function not called does it?

Anonymous’s picture

Hello everyone,

For the sake of getting us back on track I'm going to outline my understanding of the situation so far and the solution I have worked out for the site I'm working on.

First off - the dev version works, so if you're not wanting to patch, you can skip all the patching by using the dev version (woohoo) although there are a few caveats:

Caveat #1 - the "blank" entry still appears when you edit (more on this below)

Caveat #2 - required fields must be set to not-required (sad face?)

Caveat #3 - default values should be left blank. Date fields put default values - unset that.

So, if you respect caveats 1, 2, and 3 the dev version will automatically REMOVE the new/blank collection when you save. So... live nodes will NOT show extra fields. Also, if you keep revising a node, you will not end up with a new blank field each time - you will only have ONE blank field when you edit the node, which will be deleted when you save, and re-added when you come back. Get it?

Why is this so crazy? It is because Drupal wants to be "helpful" and offer you a blank field to input data for any "unlimited" fields. It is meant to help out when you're adding images and stuff so you don't have to keep clicking "add new" and then "upload". To put it into other words, it really needs a core patch to do the super-slick settings we want here, because this is a core behavior for fields. Here's hoping we can get a setting in D8 to disable this automatic "add new" for certain fields!

rooby’s picture

To put it into other words, it really needs a core patch to do the super-slick settings we want here, because this is a core behavior for fields. Here's hoping we can get a setting in D8 to disable this automatic "add new" for certain fields!

So is there an issue for it for drupal core?

tim.plunkett’s picture

This is also causing notices.

+++ b/field_collection.moduleundefined
@@ -1179,7 +1240,47 @@ function field_collection_remove_submit($form, &$form_state) {
+      && $field_settings['hide_blank_items']

This line (thought it was moved around) should probably be wrapped with !empty()

tim.plunkett’s picture

I'm rolling this back. Between #150 and #152, I'm not sure this was fully baked.

Attached is the patch from this issue as well fago's tweaks.

http://drupalcode.org/project/field_collection.git/commit/1b68602

Anonymous’s picture

@rooby (re: #151) I am not aware of any core issue for this as it was the first time I finally understood the issue. If you post one, please cross reference it here and I'll add what I can to the discussion. Thanks

rooby’s picture

I am currently using the patch in #153 with the fix from #152 and it works well enough for me.
In response to the caveats:

Caveat #1 - the "blank" entry still appears when you edit (more on this below)
- I think this is how it was intended. You get a blank row, and if no one fills it out it doesn't get saved.

Caveat #2 - required fields must be set to not-required (sad face?)
- This doesn't happen for me. I have a collection with an addressfield, select field & check box and the select field is required and it works. (or do you mean the field collection itself required?)

Caveat #3 - default values should be left blank. Date fields put default values - unset that.
- This is expected whenever there is an empty row (as per caveat #1) because the system can't know if the default value is selected because it was the default or because the user wanted it. THat is the nature of default values.

rooby’s picture

+++ b/field_collection.module
@@ -1095,6 +1124,29 @@ function field_collection_field_widget_form(&$form, &$form_state, $field, $insta
+        && $form[$field_name][$langcode]['#max_delta'] > 0) {

$langcode can be NULL so there should be something along the lines of this before hand:

if (is_null($langcode)) {
  $langcode = LANGUAGE_NONE;
}
thoughtcat’s picture

Caveat #2 - required fields must be set to not-required (sad face?)
- This doesn't happen for me. I have a collection with an addressfield, select field & check box and the select field is required and it works. (or do you mean the field collection itself required?)

I've updated to 7.x-1.0-beta4+10-dev and am still finding the problem with a field collection containing required fields.

Weirdly I ran the update script after uploading the new dev version and Drupal said "No pending updates." Does that mean it didn't work?

rooby’s picture

No pending updates just means there are no new update hooks to run. Module updates don't necessarily have these.

If you are using the latest dev without patching, make note that in comment #153 Tim rolled back the committed patch, which means it is no longer in the module.
So to test with the latest dev you need to apply the patch in #153.

You may encounter a couple of error messages at times though as it is still a work in progress.

thoughtcat’s picture

Thanks rooby, I've applied the patch and tried again but I'm still getting the "fields are required" error when trying to submit changes :(

fago’s picture

I'm rolling this back. Between #150 and #152, I'm not sure this was fully baked.

!? Why that? Yep, it's not totally complete as e.g. date fields still have default values. For that another issue is open, so this should overally improve things.. Or where there any regressions? E.g. with required fields?

retiredpro’s picture

had issues with field collections table module. in some cases the page would load with a bunch of errors which say "The field collection item form has not been embedded to avoid recursive loops." and will not allow me to add another item. Other times the an extra row does get added but an extra incomplete row gets pre-appended to the table and will not save.

Renee S’s picture

I've applied #153 and #152, and have "required" fields set, and yet... it works... ?!? :)

rooby’s picture

I've applied #153 and #152, and have "required" fields set, and yet... it works... ?!? :)

Me too.

retiredpro’s picture

it does work for me too but only when displaying field collections as embedded. has issues with add another item when displaying the field collection as table.

rooby’s picture

Oh. Mine are all embedded at the moment. With fancy themeing to emulate field collection tables.

gmclelland’s picture

Patch in #153 fixed the problem for me.

I tested my embedded field collections that contain required fields. I also tested the add link using the field collection table module on node view and everything worked fine for me.

Hope that helps

radiobuzzer’s picture

Caveat #1 - the "blank" entry still appears when you edit (more on this below)
- I think this is how it was intended. You get a blank row, and if no one fills it out it doesn't get saved.

This is not necessarily intuitive. I would rather prefer that no blank row appears, just the button "add new item": The field collection may contain many fields with "default value", so even if you do not specify anything for these fields, submitting will detect field values in the form and store the superficially "empty" row as a new item.

In my case, there are so many minor options contained, that I would prefer to keep the "default values" and then delete the "blank entry" manually.

Is it sure that this is a problem in the core? If this is the case, then this should be submitted there as an issue.

rooby’s picture

@radiobuzzer:
I think that is outside the scope of this patch.
At least for now. Maybe a follow up issue could be created after.

The reason I say that is because other multiple value fields in core work this way. So the default behaviour should be the same here, which is to give an empty row.

radiobuzzer’s picture

Advising the users to not have default values as a solution contradicts with the actual initial issue report, which is entitled "Embedded field collection items with a default value result in new items on save". If users didn't need default values in their embedded field collection this issue wouldn't have been posted in the first place.

If I am missing something and the point was otherwise, then we should at least proceed with submitting another issue

gmclelland’s picture

@radiobuzzer - I have default values set on my field collection fields and everything seems to be working fine when I used the patch in #153.

radiobuzzer’s picture

I'm now trying to disable all default values from field collections. There is a problem with on/off checkbox widgets. It is not possible to avoid the default value. It has to be either 0 or 1.

thoughtcat’s picture

I'm using the patch in #153 and it's not working when I have required fields, and my form won't make sense without required fields.

Renee S’s picture

Ok, I was wrong, it doesn't work quite -- for the second field (Unlimited), you have to press "Add more items" twice to make it appear. Weird.

klonos’s picture

Yes, I've reported this back in #148.

fago’s picture

ok, I took another look on this and figured out there were some troubles with it left:
- In forms with an assigned langcode the wrong langcode was used for fixing the max_delta, what result in max_delta not being fixed and the empty-item bug. Fixed that.
- When the first field collection item is *empty* and add more got pressed, nothing happened. Fixed that.
- The code did not correctly handle the recursion counter what can result in wrong recursion errors. Fixed that.

Altogether, this works now pretty well for me in a rather complex involving 4 different field collection forms. Thus - committed (again) :-)

See #1662998: Create configuration option to ignore chosen collected fields when checking for an empty field collection for the remaining issue.

fago’s picture

Status: Needs work » Fixed
FileSize
7.68 KB

Note: Make sure to clear caches after updating.

retiredpro’s picture

patched and cleared cache. works in that it doesnt create additional rows upon save. but when i try to add another item i get an incomplete row (without enough td cells or any fields) added along with my emtpy row. removing any row (new or already there) will also remove this incomplete row.

retiredpro’s picture

on a not so related note to the thread, patch #176 broke one of my custom modules I had for setting fields to readonly. perhaps I can get a quick pointer if maybe my code is incorrect or is it incompatible with the patch? thanks in advance

<?php
function calc_readonly_form_alter(&$form, &$form_state, $form_id){
  $form['#after_build'][] = 'fa_expenses_after_build';	
}

function fa_expenses_after_build($form, &$form_state) {
  //this part targets an individual field. works fine.
  calc_readonly_field_readonly($form['field_fa_current']);

  //this part targets fields within field collections. it works without the patch
  for($i = 0;$i < $form['field_faex_other_taxes']['und']['#max_delta']+1;$i++) {
    calc_readonly_field_readonly($form['field_faex_other_taxes']['und'][$i]['field_faex_other_taxes_c2']);
  }
  
  return $form;
}

function calc_readonly_field_readonly(&$elements) {	
  foreach (element_children($elements) as $key) {
    if (isset($elements[$key]) && $elements[$key]) {

      // Recurse through all children elements.
      calc_readonly_field_readonly($elements[$key]);
    }
  }

  if (!isset($elements['#attributes'])) {
    $elements['#attributes'] = array();
  }
  $elements['#attributes']['readonly'] = 'readonly';  
  $elements['#attributes']['class'][] = 'readonly';
  
}
?>
fago’s picture

['#max_delta'] gets adapted by the patch (decreased by 1).

ad #177: Please do some more testing, i.e. find out whether it works with a basic field-collection (it really should) and then what makes the difference.

fearlsgroove’s picture

Status: Fixed » Active

If you have a boolean field attached to the field collection, #176 successfully prevents multiple new field collection items from being added each time the host entity is saved, but it still creates an initial field collection item even if no data on the field collection item has been entered.

Maybe a core bug tho?

EDIT: Set back to active on the assumption that it's not actually fixed yet.

ANOTHER EDIT: Nevermind. #1339868: Boolean value in field collection causes field collection item to be added when user has left it blank

fago’s picture

Status: Active » Fixed

Thus, setting status back.

gmclelland’s picture

I just tested the latest dev. I have an embedded field collection that contains fields that are required and have non-empty default values.

Here is what happens:
1. When I create a new node it automatically creates a field collection item. (Is this supposed to happen?, I wasn't wanting this to happen. Sad Face) What happens when nodes get imported using the Feeds module? Will a field collection item automatically be created for each node?

2. When I edit and resave the node, it doesn't automatically add another field collection. (Happy Face)

3. When editing the node I click "Add another item" , everything works and saves correctly. (Happy Face)

So going back to #1. What is this intended behavior for nodes that get created/saved that have embedded field collections which contain fields with non-empty default values?

I like the idea of only displaying the "Add Name-of-field-collection" button instead initially showing the entire form. What is everyone else's thoughts? Maybe this needs to be a separate issue? Maybe this could be a checkbox option when configuring the field collection?

dwalker51’s picture

I applied #176 and it worked in a field collection, instead of creating a new field on each edit, it now saves one field collection, even if leave it empty. Which is fine for my case since, I need at least one.

Thanks very much

rooby’s picture

@gmclelland:

1. When I create a new node it automatically creates a field collection item. (Is this supposed to happen?, I wasn't wanting this to happen. Sad Face) What happens when nodes get imported using the Feeds module? Will a field collection item automatically be created for each node?

All multiple value fields on drupal core have blank fields to begin with so this module should work the same way by default.

What you want would be a separate feature request IMO.

We probably need to stop using this ticket and start opening new tickets for further issues seeing as this one has been committed.

Easier to keep track of everything that way.

thoughtcat’s picture

The latest dev (30th August) works for me! I edited an existing node and no new field collection was added. Thank you! :)

gmclelland’s picture

RobW’s picture

nitinsa’s picture

Status: Fixed » Needs review
fago’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

maxplus’s picture

Hi,
current DEV version of 25th of september also solved my problem of adding unwanted empty field collection items on save

Thanks!

EDIT on 1st of december:

I was a little bit to happy to see the first results but now I also get an internal server error.
The only way to go back to a working website was to use the beta release of field collection and use a patch posted in another thread so solve the issue of the empty field collections.

priapurnama’s picture

The current dev version gives me internal server error (500) when trying to edit a node with field collection, remove one, and try to save again.

ian-moran’s picture

golubovicm’s picture

I could not apply those patches because my code was different. But good news is that after I updated the module to version 7.x-1.0-beta5 (December 25, 2012) it started working well - bug fix is implemented I guess. BTW, I had to update entity module too because after update of field collection module site started shooting some errors. So my advice here is - use latest versions of those 2 modules.

marcoka’s picture

"drush rr" helps with these errors like "can not redeclare ...."

shubhangibb’s picture

Hi,
This patch is not getting applied for field_collection.module in 7.x-1.x dev version.
When I tried to apply it gives me an error that "This patch is not getting applied to selected context" .

truyenle’s picture

#86 works for me. Thanks

John Pitcairn’s picture

I have added a separate feature request to allow setting per-instance default values, which I think is an important consideration, even if it does present problems: #2008542: Restore per-instance (parent entity) default values.

John Pitcairn’s picture

Issue summary: View changes

made language flow better, more precise.

vistree’s picture

Sorry for reopen this topic. But I am not sure if here I can find the solution for my problem ...
In edit mode I never want to show empty fields of my field collection. I always want first to hit the "Add item". Is this possible?
Background: I have a lot of fields in my content type and all the empty fieldset fields makes it very difficult to browse through the edit form ...
Any idea?

rooby’s picture

@vistree:

You didn't actually reopen it :)

klonos’s picture

klonos’s picture

...and if you do find a solution, please be kind enough to report back telling us about it. Personally, I find it interesting and could use it in future projects. Thanx in advance.

vistree’s picture

Hi klonos,
thank you so much! Your links were absolutly helpful. I found a snippet one could put in template.php to hide empty fields:

/**
 * Implements hook_form_NODE_FORM_alter().
 */
function MYTHEME_form_node_form_alter(&$form, &$form_state, $form_id) {
  if (isset($form['#node']->nid) && $form['#node']->nid > 0) {
    $form_fields = array_keys($form);

    foreach ($form_fields as $index => $field_name) {
      if (substr($field_name, 0, 6) == "field_") { // Check if the field is a custom field
        $field_language = $form[$field_name]['#language'];
        if (isset($form[$field_name][$field_language]['#cardinality']) && 
            $form[$field_name][$form[$field_name]['#language']]['#cardinality'] == -1) {

          if (empty($form[$field_name])) {
            continue;
          }

          $max_delta = $form[$field_name][$field_language]['#max_delta'];
          unset($form[$field_name][$field_language][$max_delta]);

        }
      }
    }
  }
}

After adding this, I got an error after pushing the "Add item" button. I were able to solve this to by modifying core field module with the patch from comment #6 on an other forum topic:
https://drupal.org/comment/5725004#comment-5725004

klonos’s picture

@vistree: Always glad to help. Thanx for reporting back with the solution that worked ;)

f0ns’s picture

My solution for this issue:

I check the amount of records for the field collection, when there is only one I check the fields that should be filled in (these are the fields without a default value). When these fields are all empty I hide the field.

<?php
/*
 * Implements hook_node_view
 */
function CUSTOM_MODULE_NAME_node_view($node, $view_mode, $langcode) {

  if($node->type == 'YOUR_CONTENT_TYPE' && $view_mode == 'full') {

    //get amount of records
    $aantal_voertuigen = count($node->field_voertuigen[LANGUAGE_NONE]);

    //when there is only one voertuig record we check if bouwjaar, sterren, plaatsen and merk is empty. When it's empty we hide field_voertuigen
    if($aantal_voertuigen == 1) {
      $field_id = $node->field_voertuigen[LANGUAGE_NONE]['0']['value'];
      $field = field_collection_item_load($field_id);

      $bouwjaar_empty = empty($field->field_bouwjaar);
      $sterren_empty = empty($field->field_sterren);
      $plaatsen_empty = empty($field->field_plaatsen);
      $merk_empty = empty($field->field_merk);

      if($bouwjaar_empty && $sterren_empty && $plaatsen_empty && $merk_empty) {
        unset($node->content['field_voertuigen']);
      }
    }
  }
}
?>
hondaman900’s picture

Did anyone find a solution to the case with this issue where the field collection keeps adding blank field collection records when multiple entries are allowed (as opposed to hiding them)? Surely there should be a check for no input before committing a field collection set to the database?

Nevermind ! After days of searching, right after posting this I found a reference to the "Hide blank items" checkbox in the Field Collection's settings screen, and that fixed my issue. Don't know how I missed that prior to this (:P)