Problem/Motivation

Validation errors from an untriggered dependent should be removed so that fields excluded by a dependency do not prevent the form from saving.

However, ConditionalFieldsFormHelper::dependentValidate() returns as soon as it finds an error:

if (!empty($field_errors)) {
  $form_state_addition['errors'] = $field_errors;
  return;
}

The dependent is normally registered in conditional_fields_untriggered_dependents immediately after this block. Because of the early return, a dependent that has an error is never registered.

formValidate() therefore has no record of the dependent and cannot remove its validation errors. The fields that most need this handling are precisely the ones excluded from it.

This is the behaviour reported in #3344587: Support for field hidden by condition losing its required status. It reproduces on a normal entity form and is not specific to Inline Entity Form.

Steps to reproduce

  1. On Article, add a plain text field field_trigger.
  2. Make Body required.
  3. Make Body depend on field_trigger: state Unchanged, condition Value, value trigger.
  4. Add an Article. Give it a title, enter something other than trigger in field_trigger, leave Body empty, and save.

Actual: Drupal reports Body field is required. and the node is not saved.

Expected: The dependency is untriggered, so the Body validation error is removed and the node saves.

Proposed resolution

Register errored untriggered dependents in conditional_fields_untriggered_dependents so that formValidate() can remove their errors.

Remaining tasks

  • Determine whether to register $form_state_addition before retaining the early return, or remove the early return entirely.
  • Add functional coverage for an untriggered dependent with a Drupal validation error.

ConditionalFieldNumberZeroRequiredTest does not cover this path. It tests the conditional required state, where the validation error is never created in the first place.

Issue fork conditional_fields-3619935

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Related issues: +#3344587: Support for field hidden by condition losing its required status

Also likely related #3344587: Support for field hidden by condition losing its required status

orkutmuratyilmaz’s picture

orkutmuratyilmaz
Primary language Turkish
Location İzmir
commented

Confirming this from a different angle, with an inline entity form dependent, and adding one result that may matter for how the fix is shaped.

Environment:

  • Drupal 11.4.7, PHP 8.4, PostgreSQL 18
  • conditional_fields 4.x-dev at 22d97e6, so the snapshot already contains "Reset the target to its default values when untriggered" from #3573010
  • inline_entity_form 3.0.0

Setup

A content type record with:

  • field_party_type: taxonomy term reference, options_select widget, required
  • field_individual: entity reference to content type individual, widget inline_entity_form_simple, not required
  • field_organization: entity reference to content type organization, widget inline_entity_form_simple, not required

individual has two required fields, organization has three. Exactly one of the two references is meant to be filled in per record.

Conditional Fields rules on the record form display:

  • field_individual visible when field_party_type equals the term "Individual"
  • field_organization visible when field_party_type equals the term "Organization"

What we measured

Logging around the block this issue points at, on a real form POST (session from a one time login link, form_build_id and form_token taken from the rendered markup) where "Organization" is selected and only that subform is filled in:

  • field_organization evaluates as triggered and returns early, as expected.
  • field_individual evaluates as untriggered, and at that moment $form_state already carries the errors of its embedded subform:
    field_individual][0][inline_entity_form][field_first_name][0][value
field_individual][0][inline_entity_form][field_last_name][0][value
  • Those errors are collected into $field_errors, the early return fires, and the dependent is never added to conditional_fields_untriggered_dependents.
  • formValidate() then returns at its first guard, nothing is removed, and the form cannot be submitted in either direction.

So in this case the errors already exist when dependentValidate() runs, which is exactly what trips the early return. The subform is validated inside the widget's own element validation, so its violations are raised earlier than the entity level violations of an ordinary field.

Applying the proposed resolution

We applied the resolution described in the issue summary locally (keep the collected errors in the form state addition and drop the return):

  • Both directions submit successfully, HTTP 303, no validation errors. Error removal works.
  • However, the hidden side still created a child entity. The saved record that references a real organization also referenced a newly created, empty individual node with no title and no field values. The mirror case created an empty organization node.
  • Enabling the rule option "reset to default value when hidden" did not change this, even though the tested snapshot includes the reset work from #3573010. The empty child entity was still created and referenced.

That second part is inline_entity_form's own behaviour, tracked in #3099844 (still "needs work"): the widget builds and saves a child entity for an untouched, empty, optional IEF field. We are not suggesting this issue should solve that.

It is worth flagging though: with an IEF dependent, removing the errors alone turns a blocked form into silently created empty entities. Anyone testing a patch here against an IEF setup should check what ends up in the database, not only whether the form saves. A test covering an untriggered dependent whose values are discarded, not just whose errors are removed, would catch that.

I'll try to test a patch or MR on the setup above and report both the form result and what gets written to the database.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Re #3 Can you post the code in an MR please from your work? It’s easier to see how the prose fits with what you tested against (instead of me reading between the lines…)

It’s hard to tell if that should be its own issue or be truly part of this one.

I don’t mind the AI output but could you trim the verbosity a bit please 🙏

orkutmuratyilmaz opened merge request !122

orkutmuratyilmaz’s picture

orkutmuratyilmaz
Primary language Turkish
Location İzmir
commented
Status: Active » Needs review

@joelpittet, sorry for the crowded paragraphs, I'm trying to trim them now:)

I just created an MR for the issue.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Thank you for the cleanup.

I’m leery of this change so might spend more time pondering 🤔 it.

I feel like I ran into this, it was annoying BUT if it’s untriggered shouldn’t it have a valid default?

There is a longstanding issue where Drupal has relied on form validation (rant coming) but as we move to jsonapi or even programmatic saves the validation is more important on the entity save for data integrity.

With that in mind, this bug sounds like a feature in disguise 🥸 as it prevents saves where the data is invalid…

A workaround I’ve used is setting a sane and valid default value. I know that doesn’t work in all cases.

I’m typing from my phone so not looking at the solution. But data integrity is really important with this type of fix.

So first look at the problem different ways and maybe even different results (valid but meaningful default) not saying this shouldn’t be fixed like this, just the issue worries me for the side effects it poses.

orkutmuratyilmaz’s picture

orkutmuratyilmaz
Primary language Turkish
Location İzmir
commented

Re #7: you are right, this is a fair worry. My own measurement shows part of it. If we only remove the errors, Drupal saves an entity that its own field definition calls invalid. The required field stays empty. With an inline entity form we also get an empty child node. Drupal does not validate again on save, so nothing catches this later.

Still, three things make me doubt that it is a feature:

  • dependentValidate() promises the removal itself. Its docblock says the errors of an untriggered dependent "must be removed, together with its submitted values", so that the field looks like it is not in the form at all.
  • Today the module is not protective, it is inconsistent. formValidate() reads the form errors again for every registered dependent. An error raised after dependentValidate(), for example an entity constraint, is already removed. Only an error raised before it stays. That is the required error core sets on a child element. The early return keeps the dependent out of the register, so nothing removes that error. The data is invalid in both cases, but one is blocked and the other is saved. It only depends on the moment the error appears.
  • The early return also breaks the valid default you mention. The "reset to default when untriggered" flag travels in the same form state data that the early return skips. So a dependent with errors is never reset. Your workaround works because the default fills the widget in the form, not because the module resets anything. If the user clears the field, the form is blocked again. I measured this with a required dependent that has a valid default and the reset option on: without the change the form is blocked and no reset happens, with the change the node is saved and the field holds its default value again.

If data integrity is the line, I am happy to narrow the MR. We can remove the errors only when the result stays valid:

  • reset to default is on, or
  • the dependent is not required in its own field definition. This is the inline entity form and paragraphs case, where the errors come from a subform inside an optional field.

A required dependent with no reset would still block the form, so your protection stays. And for a field that should be required only while it is visible, the conditional "required" state is still the right tool.

Which direction do you prefer? I will reshape the MR and its test.

Also, the pipeline is green again, the cspell failure was a word in a comment I had written.