Problem/Motivation

Hi!

This is a variation of #3416819 (where it was reported for list(string)); but this one is on list(integer)
Tested in latest 11.2 but also happens in 10.5 (so maybe can be backported once is solved)

The issue is that, if you have a content type with a field list(integer), and one item contains 0 as value, and then you have data already stored in that field, when trying to edit the field (eg: to update its label) or to re-use that field in a different content type, you get the following error: Value field is required. on that 0 Value (that now is disabled as the option is in use)

Steps to reproduce

(Similar to original issue)

1. Vanilla D11.2
2. Choose the basic page or article content type and add a new field: Selection / List (integer)
3. Add one item with any name and 0 as value
4. Add a second item with any name and any value (eg: 1)
5. Save settings
6. Add new content (node/add) to the content type with the new field and choose the option with value 0 in the form. Save the content
7. Go back to the content type -> manage fields and edit the list (integer) one
8. Field with 0 as value now is disabled (cause "Cannot be removed: option in use."). Try to update the label field and then save the changes (or add a new item)
->Error: Value field is required. and field cannot be updated

Second error noticed (partial error)
1. After you have done 1 to 6 on the previous list, go to 'manage fields' on the other content type (to article if the field was added in basic page)
2. Click on 're-use an existing field' and choose the list(integer) from the other content type
3. You see in the page that opens that item with value 0 has the field disabled (as there is content). Then try to save
-> same error: Error: Value field is required. BUT the field is added to the content type
-> then trying to do save the field returns the same error and the field in the new content type cannot be saved

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3534125

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:

Comments

gorkagr created an issue. See original summary.

quietone’s picture

Version: 11.2.x-dev » 11.x-dev
Issue summary: View changes

In Drupal core changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies. Also mentioned on the version section of the list of issue fields documentation. Thanks.

annmarysruthy made their first commit to this issue’s fork.

gorkagr’s picture

I think changing to 'readonly' could do the trick, but users will be confused why the field cannot be edited (is does not have the 'disabled' style in the end)

I was taking a look at what causes the field to be validated, is within the 'FormValidator.php' class in \Drupal\Core\Form\ namespace
if we see line 261

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

and when the field is disabled, #value contains 0 (int) while is not disabled, #value contains '0' (string)
So maybe we need to check also within that function that the field is not disabled (aka, there is no '#attributes') when the element is required within that same function...
Maybe

$is_empty_value = ($elements['#value'] === 0 && !isset($elements['#attributes']['disabled']));

But i am not core expert...

libbna made their first commit to this issue’s fork.

libbna’s picture

Status: Active » Needs review
StatusFileSize
new36.01 KB
new35.74 KB

I agree with @gorkagr that using readonly does not visually indicate that the value field is non-editable, which can be confusing to users. Therefore, I updated the code as suggested in comment #5.

With this change, the value field is correctly disabled, and I am able to update the label or add a new item without encountering validation errors. I have attached screenshots demonstrating the behavior with both the readonly and disabled attributes for comparison.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

abhijith s made their first commit to this issue’s fork.

abhijith s’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

joelpittet made their first commit to this issue’s fork.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

I ran into this problem too, and while the solution did resolve the problem I don't believe it was the correct solution.

Moved the the regression test onto OptionsFieldUITest which has a 0 value test already as per @smustgrave's comment about the new functional test vs extend existing. And looked for a solution that matches the prior-art of the issue summary mentioned/related issue.

AI disclosure: I used AI to evaluate the previous solution and address the MR comments. I guided it to a different solution because as mentioned in the Issue summary as 'prior-art' #3416819: Not able to add option with key and label '0' to List string field solved and committed a similar issue.

joelpittet’s picture

Test-only fail as expected to show the regression and pass otherwise. 🔴🟢 — keep your stick on the ice!

joelpittet’s picture

StatusFileSize
new390.72 KB

A quick manual test where it was previously failing showed that I could save this page. It is an OG access field that has 0 values in it in the db.
Saving existing with 0 values

smustgrave’s picture

Status: Needs review » Needs work

Sorry to be that guy can we fill in the rest of the summary

Also should this be attached to the options module?