Required fields in collection not validated when empty

Same problem for me, D7x
Field Collection 7.x-1.x-dev

I have the same problem. It seems like field collection is not validating select fieldtypes.

I've looked into this and it seems function field_collection_field_widget_embed_validate() validates '_none' as a normal value instead of empty, i've added a patch for 7.x-1.0-beta3.

I get this issue but with ALL field types, not only select. I have a field collection with an image, and 3 text fields. All of which I can leave empty if they are required and submitting the form simply doesn't save the field to the node (instead of producing errors). Same goes for another field collection with a term select and a link field.

Tested again, and confirmed the bug reported in #1, #2, #4.

Workarounds:

• Setting a default value on the form element. The means it will not be rendered with the empty option ("None", "Select a value", &c.), so validation should occur normally.
• If I made the outer Field Collection required as well, then the interior select list began validating normally (ie was now also required). Not sure how well this works with lots of fields in the collection.

Patch in #3 did not resolve the issue for me.

I think the real problem is that field_collection_item_is_empty() is returning TRUE, so we skip validation. That's definitely not the behavior we want, and likely explains the issue reported in #4. Not sure how Form API does it, but if an element is required, then we should always validate. And if the field collection item is empty, that should fail validation.

Yup I'm using latest dev of field_collection. I've also tried making both the field collection on the content type and the field collection fields required but still get the same result.

better title.

It seems to me that this issue persists in the latest version.

I also have this problem

+1

for me issue reproduces too:

if field collection item has at least one filled field & other fields are required - they are pass validate.

After a few hours debugging i solve issue for myself, but I don't know if it help others:

First - requred fields are validating by field collection, I was wrong, simply fields like text field works fine, but more complex like datepicker - isn't.

As I see field collection works like this:
first - it see what fields are required, then afterbuild callback

function _field_collection_collect_required_elements(&$element, &$required_elements) {
  // Recurse through all children.
  foreach (element_children($element) as $key) {
    if (isset($element[$key]) && $element[$key]) {
      _field_collection_collect_required_elements($element[$key], $required_elements);
    }
  }
  if (!empty($element['#required'])) {
    $element['#required'] = FALSE;
    $required_elements[] = &$element;
    $element += array('#pre_render' => array());
    array_unshift($element['#pre_render'], 'field_collection_field_widget_render_required');
  }
}

unset default #required key & collect within itself to further validation.

When validate callback is trigged:

function field_collection_field_widget_embed_validate($element, &$form_state, $complete_form)

There are issue related to multi depth array date field value structure, so validate don't see any errors:

 if (isset($elements['#needs_validation'])) {
        $is_empty_multiple = (!count($elements['#value']));
        $is_empty_string = (is_string($elements['#value']) && drupal_strlen(trim($elements['#value'])) == 0);
        $is_empty_value = ($elements['#value'] === 0);
        if ($is_empty_multiple || $is_empty_string || $is_empty_value) {
          if (isset($elements['#title'])) {
            form_error($elements, t('!name field is required.', array('!name' => $elements['#title'])));
          }
          else {
            form_error($elements);
          }
        }
      }

as array has sturcture like this: $element['#value']['date']['value'] = ''; Validate think it's filled & don't cause any errors.

I solve it for myself but I didn't care about some complex module usecases, so please review it:
7.x-1.x version

change task status*

Thanks Pavel! I will review the patch as soon as I have a chance.

We had the same issue for a select field (dropdown, not multiple) reported in #3 so we integrate the previous patch with the "_none" case.

My scenario:

- ECK-created entity type
- bundle contains a single Field Collection field, which allows unlimited values
- The Field Collection entity contains two fields, a textarea and radio buttons, both of which are required

The problem:

- Leaving the required fields empty for the second & subsequent Field Collection items does not trigger validation errors

The solution:

The above patches did not help, so I dug into the code and came across these lines in field_collection_field_widget_form():

  if (empty($element['#required'])) {
    $element['#after_build'][] = 'field_collection_field_widget_embed_delay_required_validation';
  }

... this ends up removing core validation on the fields in favor of FieldCollection's own validation, which doesn't do the trick.

From the documentation for hook_field_widget_form(), we read:

#required: A Boolean indicating whether the element value is required; for required multiple value fields, only the first widget's values are required.

... this explains why only the first item gets validate: all others match the above condition and lose their validation.

The actual solution is pretty simple:

  /**
   * Implements hook_field_attach_form().
   */
  function hook_field_attach_form($entity_type, $entity, &$form, &$form_state, $langcode) {
    if ($entity_type == 'field_collection_item') {
      $info = entity_get_info($entity_type);
      $bundle_key = $info['entity keys']['bundle'];

      if ($entity->{$bundle_key} == 'field_quality_set') { // CHANGE this field name to match your use-case
        $form['#required'] = TRUE;
      }
    }
  }

The patch in #18 worked for me.

As i see problem, that we have complex error with process of validation fields inside embed field collection item.
Simple case:
i have not-required field collection field, and some required inside it. And it will works with simple fields - textfields, etc.
But it we have some complex field with custom validation, i.e. taxonomy reference tree - we'll have a problem.

Reason is next. Field collection have proper logic for field level validation, but widget level validation will run without #required attribute, because field collection unset them for inner fields. It means that if some widget have validation and rely on #require attribute it will not be proceeded correct
In field_collection_field_widget_embed_validate() we have simple check for empty/non-empty values but in general it's not enough, widget originally could provide anything during validation.

That's why i propose to find a way to:
- avoid widget validation with replaced #required property, i.e. via unsetting of #element_validate options from inner fields
- call widget validation with original #required option in field_collection_field_widget_embed_validate()

The patch from #18 works for me , but I found another bug in this functionality, FILES in field collection does not validate correctly(they don't validate at all), so no matter if field is chosen required , you can submit form. So I add small fixes in patch #22 with files case, please review it:

The patch from #22 FIXED !
please review it:

I have tested approach in #19, but it didn't help. When I even fill all my field -- I still get validation error. Probably my field collection is more complicated than one selectbox and radios (I have several textfields, media field and color_field).

Regarding patch #26. It is not per coding standards (trailing spaces and formatting). Also @ndrosev please do not add credentials to Propeople in the public patch (even I work for the same company).

For the solution of this problem I propose more general approach. The thing is that different fields might have different value structure (for my particular case "media" field). Lets have a hook that other modules can use for proper handling of cases when we need to check whether require field is empty or not.

See attached patch.

thank you @ygerasimov, the #27 patch works well for me but with one small change. I have 'select list' as widgets of my field collections' fields. Drupal as a value of 'empty option' element of the widgets set "_none" not "0"
<option value="_none">- Select a value -</option>
so I changed one line of the code
$is_empty_value = ($elements['#value'] === 0);
into
$is_empty_value = ($elements['#value'] === '_none');

I agree with #21. There won't be a complete solution unless it makes use of the widget validation logic that is already available. It shouldn't be necessary to replace the validation logic that is already in place everywhere with a new custom hook for everybody to implement.

I think it is best to call _form_validate on the elements directly, rather than copying parts of it. It does lead these elements being validated twice, but the first time does not give a legitimate result since the validation hooks would have been called with #required = FALSE.

For this to work with date_popup this patch is also required: #2199587: Second validation of date popup causes error

Last patch looks to work, but doesn't apply against my current version so just uploading a do-not-test version, please ignore :)

Hi,

Im still having problems despite using the patch in 30. Most field collections are working but I have 2 in a form that don't validate, these just contain file, number and entity reference fields and all fields are required. Seems that field_collection_item_is_empty($field_collection_item) returns TRUE for the ones that don't validate. I've been trying to debug but I cant work out why some fields are working and some arnt

Hey guys,

I am having the same issue with a media field.

All fields are optional except the media field, and if you leave it blank, it does not trigger validation.

Any proposed solutions for now?

Thanks,

Hi guys,

If we already do a field_is_empty to check if field collection is empty by checking all fields, why not doing this here?
I was able to resolve FIle/Image field issues with validation by invoking field_is_empty, which most of fields should implement anyway.

This approach was tested and proved to work with nested field collections also, which we try to avoid, but there are cases anyway when people use those.

Please take a look at the attached patch.

Adding patch for 7.x-1.0-beta5 to use in our builds.

Baaaaam,

#34 breaks some stuff (Select lists would not correctly validate because list_field_is_empty does something completely different than it was expected.)

After some investigation I've found that hook_field_is_empty is never user to validate required fields in Drupal, it's just used to filter field values list to get rid of empty fields. All fields are actually implementing their #required validation in their own validation handlers. Which is not bad, but it's coupled with other validation checks, not only checking for required fields.
What field collection wants to do, is validate only the required property, because all other properties were already validated before.

Here is the part that we're missing in our code from _form_validate that might fix the issue:

foreach ($elements['#element_validate'] as $function) {
  $function($elements, $form_state, $form_state['complete form']);
}

This might fix the problem, but it will result in double validation of fields, which proved to be a bad thing (#2199587: Second validation of date popup causes error)
Some of us tend to change stuff in validate callback, which should not happen, for example flatten some array, or change values. Double-validation will definitely break something else.

One thing that comes to my mind is adding all specific cases we might find to that patch until we find a way to decouple #required validation from other validation stuff.

Here is what we might do to handle files validation.

I just ran into this issue (with select-lists) myself and tried to get the above patches to work but couldn't get them to work properly.

After walking away for a little while, I came up with a rather simple 'fix' in the function field_collection_field_widget_embed_validate:

$is_empty_value = ($elements['#value'] === 0 || $elements['#value'] === '_none');

My select-lists had a #value of '_none' when nothing was selected.

@SpadXIII: please attach a patch!

Came across this issue, which already fixes the validation issue with options-widgets with an empty (_none) value: https://www.drupal.org/node/1676206#comment-6211088
And it's already merged!

Is this still happening at all? I just tested it and it seems to show the correct validation errors, even for selects and date popups.

jmuzz: the issue I linked to is already merged. Afaik, this issues described here have been solved by that commit.

The issue of required select lists validating is solved but not the issue of required file/image fields. I've created a simple patch for my needs to validate files, but it looks like previous patches in this thread had logic to deal with files / images / media. Should this be turned into a new issue Required validation for files is ignored.

Is there a reason that Field Collection will only validated required fields if the entity is not empty? I believe that this is one of the primary reasons to have required field, so that empty forms cannot be submitted. I came to this question after creating a field collection with only textfields with some of them required, and others not. As long as some of them were filled, it would validate. However, when all of them were empty, nothing would validate. When modifying the module code, and getting rid of the field_collection_item_is_empty() check, it seemed to work as expected.

Please let me know if I'm missing something here.

mfsamar if you want it to validate even a blank field collection you need to set the field in the host to required. It is a common use case to have an unlimited field containing field collection items that each must be validated but with the list itself allowed to be empty of any field collection items, so this behavior isn't likely to change.

Here's a reroll of #35 for beta7, this fixed my issue where I could save a field collection that contained a date and text field where both were required by only entering a value into the text field, the date would then default to 1 Jan 1970.

This is what me and my friend have done to fix the error

@asgorobets or @acbramley: This patch looks like a reasonable approach. Is there some reason it was never submitted for review?

@jmuzz good question, here's a patch against the latest 7.x-1.x for review

#49 works for me for required text fields, not a for a required entity reference to another content type. It said the field was empty even when a value was selected.

The problem is in $elements['#array_parents'], which contains only five keys, instead of the six (count($elements['#field_parents']) + 3) needed to extract the proper field value for 'field_is_empty' testing.

I was able to get around the problem by using a reset to get down to the sixth level like this:

          $field_values = drupal_array_get_nested_value($form_state['values'], array_slice($elements['#array_parents'], 0, $field_depth));
          // Begin edit
          if (count($elements['#array_parents']) < $field_depth && is_array($field_values)) {
                $field_values = reset($field_values);
          }
          //End edit
          $is_empty_field = module_invoke($field['module'], 'field_is_empty', $field_values, $field);

But that seems too kludgy for an official patch.

The right solution would be to fix the problem of the missing '#array_parents' key. Perhaps someone with more knowledge and experience with this could look into that.

Sorry, I accidentally hid files with my last comment. This should bring them back.

This is also not working for file field.

This patch incorporates a workaround for lists somewhat how #50 suggests. The only change is I'm special casing lists. Also I put back the check for is_empty_option as that was, what I believe, accidentally removed.

Added to the special casing from the previous patch to also include 'taxonomy'.

#49 working for me but it have notice:
Notice: Undefined index: value trong list_field_is_empty() (dòng 408 của D:\xampp\htdocs\vec\modules\field\modules\list\list.module).

#53 worked for me but it not support for condition field module drupal 7.

We have lots of field collections, and a user could get lost trying to find where the missing field is located. I modified the form_error message to include the label.

1. I can't apply patch #57;
2. Patch #54 not fix problem #50. I've add code from comment #50 - and it works.

Also had problems to apply #57

Attached a new patch, fixed to apply against current --dev, and looks to work well.

Please, review!

I tried #59 and it applies cleanly. However, it doesn't validate properly for me. I tried a Dev version as well as Beta 11 and in both instances, my select list, which is required inside of a field collection (which itself is not required) will submit without anything selected.

It seems to me that the issue (for me) is that this if statement:

if (!field_collection_item_is_empty($field_collection_item) && !empty($element['#field_collection_required_elements'])) {

is only firing if the field collection item is NOT empty (which seems backwards, if we are trying to validate an empty field).

as soon as I remove the !, suddenly the validation works for me. I have re-rolled #57 to remove the ! (otherwise it's the same) and now this works for me.

@lalweil I think your issue is that the field itself is not required. That means it won't validate if it is empty.
If you make a field-collection-field required with required and optional fields, then apply your patch, you will not get errors about the required field in the field-collection being empty; validation only runs when it is empty...

If you make the field required, you should get an error about the field-collection being empty, just like default fields.
In that sense, I think the patch in #59 is still valid and requires more extensive review :)

@SpadXIII when you say the field itself, do you mean the field collection field, or the field within the field collection? My field collection isn't required, but the field within it is. I can certainly experiment with that as an option and see how it behaves.

@lalweil if the field-collection-field itself isn't required, you will not get an error if you leave all fields empty, even though there are fields inside the field-collection-field that are required. Those requirements will only be checked when there is any kind of data (the !empty-check). Making the field-collection-field itself required, gives you an error when there is not data in there.

Note that this doesn't really work when it's a multiple-field and there already is data in there. In this case, you could write your own (element-)validation-handler of course.

+++ b/field_collection.module
@@ -1250,7 +1250,25 @@ function field_collection_field_widget_embed_validate($element, &$form_state, $c
+          // Special case lists since we don't get the correct array_parents.
+          if (in_array($field['module'], array('list', 'taxonomy'))) {
+            $field_values = $field_values[0];
+          }

Listing modules that have a different model for field_values won't scale very well. I have updated these lines to the generic approach suggested in #50 so it also works for entityreference. The interdiff-59-67.txt reflects my changes to the patch in #59.

#67 works well for me.

Thanks all.

A lot of different ideas here. I like the final solution. If there's anything it doesn't cover please open a new issue.

I'm sorry to reopen this issue, but there's a small issue in the patch: the variable $instance does not exists. ;)

form_error($elements, t('@name field is required in the @collection collection.', array(
              '@name' => $elements['#title'],
              '@collection' => $instance['label']
            )));

This solution does not work when the field collection field is required.

My solution is the following.

Looking at the code, @matthiasm11 appears to be correct in #71 that it's using an $instance variable that doesn't exist. However, strangely, the code work for me and the message includes the label from $instance['label']... I looked for extract() (the usual culprit) but it's not used, so I'm not sure how that's possible?

Hacked a fix to #71. No familiarity with this code, but noticed the $field_state array has an instance subarray with the label.

But why the "in the X collection." wording? An end user will have no idea what a "collection" is. To me, the error styling (e.g. red outline) would seem sufficient to help locate the field that failed validation. So I'd propose removing this wording.

Re #73: I don't see this behavior. For me, with 7.x-1.x HEAD, the label is missing and I get a PHP notice.

Versus #74, added comma after last element of multi-line array per coding standards.

I could open a new issue for #71 if preferred, per #69.

AohRveTPV, I agree that the end user might not know what a collection is.

I think the text is helpful if you have various collections where a field may be located with similar labeling. Trying to click through the various collections to locate which Image field is missing can be mitigated by calling out the collection name.

Thoughts?

Thanks @AohRveTPV.

Please open a new issue for any further problems related to this.

Possibly one for the wording issue mentioned in #74. The wording may need work, but there usually is a message at the top of a form which indicates what field has the error. For a field within a field both levels deserve a mention to avoid ambiguity.