Problem/Motivation
#3384140 introduced a regression of sorts; filtering in the NameWidget uses array_search() only returns the first key:
If needle is found in haystack more than once, the first matching key is returned. To return the keys for all matching values, use array_keys() with the optional filter_value parameter instead.
https://www.php.net/manual/en/function.array-search.php
So, if we have both title and generational in the form and the user leaves both blank, only one of these is going to get massaged to remove the "_none".
Steps to reproduce
Use a name field with multiple selects (e.g. title, generational) and leave both unassigned. When saving, only one of the values will be massaged and the other will have the literal "_none" persisted to the database.
Proposed resolution
Proposed patch attached
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | name_field_issue_after.png | 121.38 KB | sourabhsisodia_ |
| #7 | name_field_issue_before.png | 120.41 KB | sourabhsisodia_ |
| name_none_placeholder-3384140-10.patch | 682 bytes | tkiehne |
Issue fork name-3491471
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:
Comments
Comment #2
tkiehne commentedComment #3
tkiehne commentedComment #6
sourabhsisodia_ commentedComment #7
sourabhsisodia_ commentedAttaching screenshot for reference.
Comment #8
sourabhsisodia_ commentedComment #9
heddnFeedback posted on the MR.
Comment #10
sourabhsisodia_ commentedComment #11
alan d. commentedI 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, then this minor fix may need a tweak to only target option based components.
Comment #12
heddnFeedback posted on the MR.
Comment #13
sourabhsisodia_ commented@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.
Comment #14
sourabhsisodia_ commentedComment #15
heddnI think this could still add some more edge cases to the tests to address #11.
Comment #17
megachrizTo 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.".
Comment #18
megachrizI've added a possible fix for the case from #11.
Comment #19
sayan_k_dutta commentedReviewed 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.
Comment #20
penyaskitoHad 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:
Comment #21
brad.bulger commentedI 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.
Comment #22
penyaskito@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.
Comment #23
brad.bulger commentedThis 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.
Comment #24
penyaskitoI'd say that's expected from $form_state. You'd need to access the widget and call extractFormValues.
Comment #25
liam morlandComment #26
texas-bronius commentedThank you! Did not extensively test but confirmed patch from https://git.drupalcode.org/project/name/-/merge_requests/24 working in my use case.
Comment #27
bkosborneBumping to critical as this can lead to data issues. The MR works well for me.
Comment #28
clairero commentedThanks, this MR resolves the issue for me too.
Comment #31
jlancaster commentedConfirming MR24 works fine in multiple production environments. Would be nice to get this merged into an official release. In my case, the literal _none value translates to auto_entitylabels generating label values with an unintended "_none" inserted.
Comment #32
heddnThanks everyone for their contributions on this issue.