Editing a node should not automatically add a new field_collection item

Yes to #1. Validation needs to notice that an extra bundle was added when node was prepared for editing, and if nothing has been entered in any of the fields then assume this bundle was not intended to be added.

I can think of another problem that would be caused by the automatic addition of an extra bundle. If the new field collection consisted entirely of 'required' select values which were all pre-populated with a default value, then there would be no way to know whether this was an intentionally added new bundle or just one that the system put there helpfully/suggestively. It would probably be better if no new bundles were added automatically upon node editing.

In fact I am increasingly sure of this as a potentially serious problem, so I'm going to rename this issue - if I am wrong I expect someone will rename it back and I will accept that.

I am seeing similar results. We have two fields in our field collection (file_description and file). Essentially, anytime I edit a content item with the field collection it adds a new bundle to the content node and doesn't remove it on save even though it's empty.

Entity API (entity) Module Enabled 7.x-1.0-rc1
Field collection (field_collection) Module Enabled 7.x-1.x-dev

Additional information on this issue, for me it only seems to happen with field collections when fields have a "default value" or when a boolean field is used. Not sure if this is helpful in finding the issue. Also I haven't tested all fields.

I'm also seeing this issue. I have tried this out with a variety of fields. The example I'm looking at right now is a field collection that has two text fields, one text selection, and one long text field. The settings are set to allow unlimited - so user can add more than one of the field collection sets to the record. Every time the Edit button is clicked for a record, it creates a new (empty fields) field collection entry. When the Save button is pressed, it adds that new fields collection to the record.

It looks like it was adding the additional field collection bundle, because it wasn't checking against the field default value before determining if the field is empty. I have a patch that implements this fix, it works for me a the moment. Let me know if anyone else has a different outcome, or if you think we need to implement additional checks?

#6: field_collection_fixed_automatically_adding_a_new_field_collection_bundleon_edit_1193004_6.patch queued for re-testing.

I tried to apply the patch but there was some problem in applying it... the terminal wasn't responding and the .patch file was blank. I had to quit terminal because it was simply not working. Please keep in mind that I'm a newbie, but I was able to successfully apply patches (and on the same module!).

Thanks for your efforts

The patch #6 applied for me. But it creates an undefined index 'module' error. I'm wondering why $field = $item->{$field_name}[$langcode]; is added by the patch. It looks like $field is an already used variable name, which causes the error.

More generally, considering a default value field collection as empty doesn't seems so good to me: sometimes we WANT to add a field collection with the default values...

Actually, it looks like there are two kinds of problems but a single cause: an empty field collection is added to the form, even if it already has one.
If your field collection contains required fields, you won't be able to save the form because one of the field collections has an empty required field. You'll have to click 'Remove' to only keep the first field collection(s) (the not empty one(s), added previously, typically on parent node creation).
If your field collection does not contain any required field, you'll pass validation but it may create a field collection if there's a default value making it not empty.

martin_q is right in #2:

It would probably be better if no new bundles were added automatically upon node editing.

But if I'm right, it needs to override usual core behavior for multivalued fields. All this is discussed in #1239946: Embedded field collection items with a default value result in new items on save. See especially comment #17 (and #51 for a global view of the problem).

Validation needs to notice that an extra bundle was added when node was prepared for editing, and if nothing has been entered in any of the fields then assume this bundle was not intended to be added.

There wouldn't be any need to validate this if instead of a new empty bundle we had a "Add one"/"Add another" button. Instead of assuming that a new empty bundle needs to be there on node edit, we should provide the option for the editor to add it on demand.

I am no coder, but I believe that the logic behind this would solve the issue at hand no matter if the field collection had default values defined for any of its fields or not. I also think it is "safer" (no wondering if our validation checks are correct and whether we've thought of everything or not). So, I propose this issue to change to a feature request and to re-purpose it to add this "Add bundle" functionality.

One thing we need to take under account here is required fields: Having a field collection that includes required field(s) should not necessarily make that field collection required too.. There should be a separate setting that would mark the whole field collection as required. Well, kinda...

The way I see it, we have two cases when it comes to required fields and/or required field collections:

1. The node doesn't set the field collection itself as required at all: we don't care if any fields included in the field collection are required or have default values set. We have the "Add one" button!
2. The node requires at least one bundle of the field collection to be added (we currently have no way to do this - we need to implement it somehow*): This can be addressed by having only one bundle on the node form (since we require at least one) and a "Add another" button in order to allow users to add more bundles.

* We need to have a way to mark field collections that are allowed to have multiple, unlimited bundles as either "0 or more" or "at least 1 required - more allowed".

So to sum things up, we have these use cases when it comes to allowing multiple field collection bundles or not:

• only one (I personally see no reason to use field collections in this case)
• 0 or more allowed - a limit can be defined (this can be considered a "not required" field collection)
• at least 1 required - more allowed - a limit can be defined (this can be considered a "required" field collection)

PS: I have posted this idea previously in #1239946-71: Embedded field collection items with a default value result in new items on save

PPS (personal note for future reference): I want to know what others think of this idea of mine(?). If you think I'm on the right track here, then I'd like to bring to the table the idea that instead of implementing a way to define required vs optional field collections perhaps we should have it so that a min and a max value can be defined. If these two values are set to be the same, we'd also cover the "only one" use case mentioned above as well as the "exactly 4" use case I purposely didn't mention. The UI could look something like this:

Number of values allowed/required:

Require at least: [  ] bundles
Allow up to: [  ] bundles

use case => "Require at least" field value / "Allow up to" field value
Up to one (optional) => 0 / 1
Exactly one (required) => 1 / 1
Exactly 3 (all required) => 3 / 3
Up to 5 => 0 / 5
At least one required, up to 5 max => 1 / 5

See the picture? ;)

I agree with the first part. An empty field collection should appear in the form only in one case: it's a creation form and the field collection is required (with Included widget). * In all other cases, no empty field collection should appear.
Some comments though:

• A field collection is a field containing fields, so it behave as a usual field: it's already possible to set this field as required, and it's also possible to set the max cardinality (1, 2,... 10 or unlimited).
• What is not possible is to set a min cardinality different from 0 (not required) and 1 (required). But this is not really useful as we can work around this by using two fields or a custom form validation function. This is a more global issue which concerns all kinds of fields. So I don't really agree with your PPS.
• A required field inside a field collection is actually required only if the field collection form appears in the global form. So this won't be the case if we show no empty field collection in the form (*).

I think what you describe (*) is already existing in patches. In the 1 or more case, #1239946-17: Embedded field collection items with a default value result in new items on save should work. In the 0 or more case, you can set the field collection to hidden to add the field collections in a separate form, or use #1239946-75: Embedded field collection items with a default value result in new items on save which asks you to patch the core.

I mark this as a duplicate from #1239946: Embedded field collection items with a default value result in new items on save, because it seems to be more than related. Undo that if I'm wrong.

I don't exactly feel that strong about either keeping this issue open or not. What I feel though is that we should provide a single, generic solution for all these related issues instead of "hacking" our way out in separate issues. This issue's title seemed like the most proper among the other related ones, so I posted my comment here.

While I agree that field collections behave (mostly) like fields, they surely need "special" treatment. We wouldn't be having this conversation otherwise. Perhaps (ideally) this functionality would even be in core in the first place (field API??? - I couldn't possibly know) and there would be no reason for this module to exist either.

I guess what I am trying to say is that I don't agree on the way we treat these issues independently from each other. The solution I proposed (the logic behind it - no code provided or possible implications thought of) seems pretty solid to me.

I'm reopening this issue based on all the above + the fact that I'd like to hear what this module's maintainers actually think about my proposal. If it seems a duplicate to them too, then perhaps I'll file a separate feature for D8 so that the "minimum-cardinality-other-than-0" feature gets in core. Well, unless any of you reading through this issue are already aware on any previous issues filed for that of course.

[stupid moment]

...it's great that we now have fieldable entities in our site-building arsenal. Well, this module here provides us with fieldable fields too :P

[/stupid moment]

After the latest commits in #1239946: Embedded field collection items with a default value result in new items on save there is no new field_collection bundle added when editing a node, but there is this error thrown:

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

Don't know if this is related to this issue here or the other one mentioned or if it is a whole new thing altogether caused by something else (couldn't find any related issues in the queue).

el looks like a language variable, and is probably better posted in it's own issue with steps to reproduce, etc. It might be connected to one of the many content translation bugs in the queue (maybe one of these: #1344672-9: Field Collection: Field translation (entity_translation) support. #1540416-2: +add Link in node view doesn't add the collection item, #1366220: Field collection translatable Field language for setHostEntity).

I think people aren't clear on the meanings of "bundle" here. It's a term on the generic level of entity in Drupal: bundles are to entities as content types are to nodes. So I think the issue here is adding a field collection item (aka a new field collection item entity), not a new field collection bundle.

As it stands, this issue is a duplicate of the first half of #1239946: Embedded field collection items with a default value result in new items on save, and the new branched issue #1788222: Allow "add another" button instead of empty field collection item form when using the embedded widget.