Literal "_none" value saved for seelct fields

Attaching screenshot for reference.

Feedback posted on the MR.

I cranked up my email account linked to drupal.org and saw this thread...

I can't remember the sequence of the field widget submission processing, but can someone manually check that the validation in a required textfield component still triggers if you enter _blank for that component? If it does, the this minor fix may need a tweak to only target option based components.

Feedback posted on the MR.

@heddn In required textfield components i have tested that when we write _none then validation error is not triggering and _none is getting replaced by "" (empty strings) . When we write _blank then also there is no validation error and the field is having _blank value.

I think this could still add some more edge cases to the tests to address #11.

To address #11, I've added test coverage for using "_none" as value for text fields. And the tests are failing. If you for example enter "_none" for the field "Family" an empty value gets saved for that field. If the name field is required you get the more cryptic error message "This value should not be null.".

I've added a possible fix for the case from #11.

Reviewed the MR. Checked all the changes made.
1. Checked that on submitting more than one empty components for the name field, all the "_none" values are replaced by empty strings.
2. Checked that the validation does not trigger if we give "_blank" or "_none" as input.
3. Checked all the tests written and all of them passed.

Hence moving it to RTBC.

Had this same issue when leaving title and generational components empty (only if BOTH were empty).
This MR fixed the issue.

As I'm using the name output as the entity title, had to write an upgrade path in my project. If it helps, customize your entity type id and field name:

function mymodule_update_8001(&$sandbox): void {
  $nids = Database::getConnection()->select('node_revision__FIELD_NAME', 'name')
    ->fields('name', ['entity_id'])
    ->condition('FIELD_NAME_generational', '_none')
    ->execute()->fetchAll(\PDO::FETCH_COLUMN, 0);
  Database::getConnection()->update('node__FIELD_NAME')
    ->fields(['FIELD_NAME_generational' => ''])
    ->condition('FIELD_NAME_generational', '_none')
    ->execute();
  Database::getConnection()->update('node_revision__FIELD_NAME')
    ->fields(['FIELD_NAME_generational' => ''])
    ->condition('FIELD_NAME_generational', '_none')
    ->execute();
  \Drupal::entityTypeManager()->getStorage('node')->resetCache($nids);
}

I am testing this using the MR diff as a patch on Name 1.0. We have code that runs in an ajax callback function that builds a fullname string to store in another field. $form_state->getValue('field_name') is returning "_none" in the array of components. So the name.formatter service's format() function treats those like strings - we get "_none John Doe _none".

Is this the intended behavior? Anyone else who was counting on empty components returning empty strings is going to have a similar incompatibility problem.

@brad.bulger Check my last comment where I posted a workaround for a similar usecase. Check if _none already hit the database before adding this patch.

This is an AJAX callback in the form that is getting the name field value at runtime. Nothing is being written to the db. The relevant part is that $form_state->getValue('field_mynamefield') returns "_none" as the value for select list components.

What I'm not clear on is if that's intended. Does the change here only acts on submit or something like that. Should the value be "_none" or "" for field validation? For an entity constraint on the field?

I guess the change from blank to "_none" already went out in 1.0, I don't see any notes about the incompatibility change.

I'd say that's expected from $form_state. You'd need to access the widget and call extractFormValues.

Thank you! Did not extensively test but confirmed patch from https://git.drupalcode.org/project/name/-/merge_requests/24 working in my use case.

Bumping to critical as this can lead to data issues. The MR works well for me.

Thanks, this MR resolves the issue for me too.

bluegeek9 → changed the visibility of the branch 8.x-1.x to hidden.