Problem/Motivation

If a reference field has a limited cardinality and uses the autocomplete widget in the entity form, then the user can enter more values than the field allows, and the surplus values are discarded silently when the form is submitted.

Steps to reproduce

To reproduce:

- create a reference field, say on article nodes
- set its widget to autocomplete
- create a new article node
- enter one node title in the widget and let it autocomplete
- enter a comma and start typing another node title
- the widget lets you do this: it should not.
- save the node
- the node saves: it should not. The form should fail validation
- edit the node: only the last entered node title is shown
- check the DB: only the last entered node was saved in the field value

Proposed resolution

The form should fail validation when too many values have been entered in the autocomplete widget.

CommentFileSizeAuthor
#32 2951134-after_patch-MR_2362.mov2.29 MBabhijith s
#32 2951134-before_patch-MR_2362.mov4.29 MBabhijith s
#20 drupal-autocomplete_cardinality_validation-2951134-20-TEST-ONLY-FAIL.patch1.94 KBguypaddock
#9 2951134-9.patch1.08 KBbibliophileaxe
#7 test_only_patch_2951134.patch1.46 KBsathish.redcrackle
#5 patch_2951134.patch742 bytessathish.redcrackle
#4 patch_2951134.patch746 bytessathish.redcrackle

Issue fork drupal-2951134

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

joachim created an issue. See original summary.

cilefen’s picture

cilefen commented
Priority: Critical » Major

A broken or incorrect form submission is major. I feel like I’ve seen this issue reported already but I have not searched as of writing this.

joachim’s picture

joachim commented

I had a search but didn't find anything matching.

I assumed critical because data the user enters is silently lost.

sathish.redcrackle’s picture

sathish.redcrackle commented
StatusFileSize
new patch_2951134.patch746 bytes

I have attached the patch file. So if use enters multiple values with comma seperated, it will return the first id.

sathish.redcrackle’s picture

sathish.redcrackle commented
StatusFileSize
new patch_2951134.patch742 bytes

Reattached the patch file.

joachim’s picture

joachim commented
Status: Active » Needs work 
+++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
@@ -356,7 +356,7 @@ public static function extractEntityIdFromAutocompleteInput($input) {
+    if (preg_match('/\s\((.*?)\)/', $input, $matches)) {

That's changing the autocomplete behaviour for ALL fields. We need to change it only for fields with a cardinality limit.

And there should be a validation error when the user has tried to enter more items than the field's cardinality.

sathish.redcrackle’s picture

sathish.redcrackle commented
StatusFileSize
new test_only_patch_2951134.patch1.46 KB

Added test only patch to prove by adding comma separated entity values.

joachim’s picture

joachim commented

I don’t really understand your patch. It seems to be testing that the *currrent* behaviour happens.

There *should* be a form error!

bibliophileaxe’s picture

bibliophileaxe
she/her
commented
Status: Needs work » Needs review
StatusFileSize
new 2951134-9.patch1.08 KB

Patch to set error if user enters multiple values.

Status: Needs review » Needs work

The last submitted patch, 9: 2951134-9.patch, failed testing. View results

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

guypaddock’s picture

guypaddock commented
Related issues: +#1959806: Provide a generic 'entity_autocomplete' Form API element

Code for this bug was introduced in #1959806: Provide a generic 'entity_autocomplete' Form API element and #2434697: Remove UserAutocompleteController.

guypaddock’s picture

guypaddock commented
Related issues: +#2434697: Remove UserAutocompleteController
guypaddock’s picture

guypaddock commented
Assigned: Unassigned » guypaddock

Working on this right now.

guypaddock’s picture

guypaddock commented
StatusFileSize
new drupal-autocomplete_cardinality_validation-2951134-20-TEST-ONLY-FAIL.patch1.94 KB

Here is a test to illustrate the problem. Unlike the earlier test-only patch, this one relies on the existing structure of other error-case tests and validates the error message text.

guypaddock’s picture

guypaddock commented

GuyPaddock opened merge request !2362

guypaddock’s picture

guypaddock commented
Assigned: guypaddock » Unassigned
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 20: drupal-autocomplete_cardinality_validation-2951134-20-TEST-ONLY-FAIL.patch, failed testing. View results

guypaddock’s picture

guypaddock commented
Status: Needs work » Needs review
guypaddock’s picture

guypaddock commented
Assigned: Unassigned » guypaddock
Status: Needs review » Needs work

Back to me for rework. It appears that a referenced node that has commas in its title (e.g., "Nodes, amirite?") loads without quotes surrounding its title, leading to a cardinality validation failure because the first part of the title is recognized as a separate node from the second part of the title.

guypaddock’s picture

guypaddock commented
Assigned: guypaddock » Unassigned
Status: Needs work » Needs review

As it turns out, this was a red herring -- in our install, we also have a patch for #2796341: Entity reference field View output is not used for selected entity display applied, and that is what is causing the fields not to be quoted. Either way, this resulted in more test coverage for this ticket, so it's not a total bust.

guypaddock’s picture

guypaddock commented
Title: autocomplete widget doesn't warn about field cardinality, loses data silently » Single-value autocomplete widgets don't warn about field cardinality, lose data silently
Issue summary: View changes
guypaddock’s picture

guypaddock commented
Issue summary: View changes

Updated title.

guypaddock’s picture

guypaddock commented
Assigned: Unassigned » guypaddock
Status: Needs review » Needs work

Setting back to me to address test failures.

guypaddock’s picture

guypaddock commented
Assigned: guypaddock » Unassigned
Status: Needs work » Needs review

Wrong ticket. Back to NR.

abhijith s’s picture

abhijith s commented
StatusFileSize
new 2951134-before_patch-MR_2362.mov4.29 MB
new 2951134-after_patch-MR_2362.mov2.29 MB

Applied MR 2362 on 9.4.x.The error is displaying for single value autocomplete widget after applying this patch.
Attaching screen recordings for reference.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

This could use an issue summary update. Mainly what is the proposed solution to solve the issue? I see steps to reproduce but what was the solution decided upon? WIll help committers later?

joachim’s picture

joachim commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

Added detail to IS.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Can someone verify if this is still an issue?

Just tested in Drupal 10.1 with a standard install
Changed article tags field to cardinality of 1
Created an article by adding TestA then TestB waiting for the autocomplete icon to go away each time
When I clicked save I get

Tags: this field cannot hold more than 1 values.

codechefmarc’s picture

codechefmarc
he/him/his
Primary language English
commented

I also just tested on 10.0.3 with a standard install, similar details as smustgrave, and I also get the "Tags: this field cannot hold more than 1 values." message when trying to save the article. So, perhaps this is fixed in Drupal 10 which is great to hear.

guypaddock’s picture

guypaddock commented

Can someone confirm that there are tests for this behavior in 10.x? If not, I can at least reroll for the added test coverage.

smustgrave’s picture

smustgrave commented

From what I can tell searching for "hold more than" there are instances of test coverage. But this constraint in fieldItemList has been in for 5 years? That doesn't right.

Want to make sure we can move credit for everyone that helped work on this.

smustgrave’s picture

smustgrave commented

Brought this up in the #bugsmash channel and others reported they were able to replicate

From @rkoller

i’ve added an entity reference field to the article content type and picked the reference type basic page. and field settings were limited to 1. and under manage form display i’ve used the default autocomplete option.

and

changed the option from autocompleteto autocomplete (tag style) in manage form display. with that option active i get the limited to 1 warning and i am unable to proceed to save the form. if i switch back to autocomplete i am again able to save more than one tag and just the last is saved as described in the issue summaries steps to reproduce section

pameeela’s picture

pameeela commented

Yeah, confirmed it occurs with the regular 'Autocomplete' widget and not the tags style one.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

GuyPaddock changed the visibility of the branch 2796341-entity-reference-field to hidden.

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.