Steps to reproduce:

  • Edit an entity with an already filled viewsreference field
  • Change the selected views and wait for the displays to reload
  • Choose something other than master as display (and a display with another machine-readable name as the one before)
  • Try to save

The FormValidator throws an "An illegal choice has been detected." error for the display field. While debugging I saw that the options array which the systems validates the new display value against is still the list from the previous view.

Thats the reason why it works, as long as the views displays use the same machine-readable names (or master, which both have).

CommentFileSizeAuthor
#22 viewsreference-8.x-1.x-illegal-choice-2877306-22.patch2.88 KBjasonawant
#15 viewsreference-ajax-revamp-8.x-2.x-2877306-14.patch13.11 KBrecrit
#14 interdiff-2877306-10-14.txt1.89 KBrecrit
#14 viewsreferece-ajax-revamp-8.x-2.x-2877306-14.patch13.11 KBrecrit
#10 viewsreferece-ajax-revamp-8.x-2.x-2877306-10.patch13.03 KBrecrit
#6 viewsreference_illegalchoice_fix.patch2.74 KBPatrick Bauer
#5 viewsreference_illegalchoice.patch2.69 KBPatrick Bauer

Comments

Patrick Bauer created an issue. See original summary.

Patrick Bauer’s picture

Patrick Bauer commented
Issue summary: View changes
Patrick Bauer’s picture

Patrick Bauer commented

Inside ViewsReferenceTrait.php in line 53 where the system retrieves the ViewDisplayIDs, its not using the newly chosen view to generate the options value but still the old original value after changing the view.

Patrick Bauer’s picture

Patrick Bauer commented

I tried re-setting the options inside the getDisplayIds() function (which is called via Ajax to replace the DisplayID Form Element), but it didn't do anything.

After line 137:

//get form from form_state (returns a reference)
$completeForm = $form_state->getCompleteForm();
    
//reset options in form and form_state
$form[$field_name]['widget'][$delta]['display_id']['#options'] = $options;
$completeForm[$field_name]['widget'][$delta]['display_id']['#options'] = $options;
Patrick Bauer’s picture

Patrick Bauer commented
StatusFileSize
new viewsreference_illegalchoice.patch2.69 KB

I've got a dirty quick fix which fixes it in our case. But I don't think this will work everywhere.

Patrick Bauer’s picture

Patrick Bauer commented
StatusFileSize
new viewsreference_illegalchoice_fix.patch2.74 KB

Updated version with a fix for the fix, still quick & dirty ;)

NewZeal’s picture

NewZeal commented
Status: Active » Needs review

Thanks for this patch. At this stage it appears not many people have tested it, nor it appears have many people recorded it as a problem. Given the amount of code involved in the patch I'm not sure if we can introduce it just yet.

The last submitted patch, 5: viewsreference_illegalchoice.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 6: viewsreference_illegalchoice_fix.patch, failed testing. View results

recrit’s picture

recrit commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
StatusFileSize
new viewsreferece-ajax-revamp-8.x-2.x-2877306-10.patch13.03 KB

@New Zeal This still is an issue in 2.x. The ViewsReferenceTrait does not account for form state changes during the AJAX rebuild of the widget. If there are 2 views each with different display ID's the AJAX does not reset the form state input causing an "An illegal choice has been detected".

Example:
view01: displays = ["block"]

view02: displays = ["not_block", "grid"]

Save a node with the field set to view01.
Edit the node and do the following:
- Select "view02".
- Allow AJAX to populate displays list.
- Select "view01".
- RESULT: ERROR: "An illegal choice has been detected".
- REASON: The AJAX only updates display_id element's #options which leaves the form state values in an illegal combination of view id and display id. In this example: "view01", "not_block".

FIX:
The attached patch revamps the AJAX implementation.
This was loosely based on the address module's country AJAX refresh.
- Dynamic form changes while preserving the view_id to display_id relationship.
- Follows the form API so the hook_widget_form_alters() can be applied to the entire widget during the AJAX refresh.
- Fixes the above bug "An illegal choice has been detected".

recrit’s picture

recrit commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: viewsreferece-ajax-revamp-8.x-2.x-2877306-10.patch, failed testing. View results

recrit’s picture

recrit commented

Patch #10 should be tested on viewsreference 8.x-2.x but it is not available in the test suite's custom options.

recrit’s picture

recrit commented
StatusFileSize
new viewsreferece-ajax-revamp-8.x-2.x-2877306-14.patch13.11 KB
new interdiff-2877306-10-14.txt1.89 KB

Switched #parents to the correct #field_parents.

recrit’s picture

recrit commented
StatusFileSize
new viewsreference-ajax-revamp-8.x-2.x-2877306-14.patch13.11 KB

same patch, just fixed misspelling in the file name

recrit’s picture

recrit commented
Status: Needs work » Needs review

  • seanB committed 4011c44 on 8.x-2.x authored by recrit 
    Issue #2877306 by recrit, Patrick Bauer, seanB: "An illegal choice has...
seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Needs review » Fixed

Was able to reproduce the issue and confirm the latest patch worked. Fixed some minor nits on commit (unused variables etc).
Nice work! Thanks.

  • seanB committed 156d62d on 8.x-2.x 
    Issue #2877306 by recrit, Patrick Bauer, seanB: Added back some lines...
rajab natshah’s picture

rajab natshah
he/him
commented

Thank you

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented
StatusFileSize
new viewsreference-8.x-1.x-illegal-choice-2877306-22.patch2.88 KB

Hi,

Here's a reroll of patch in #6 for 8.x-1.x version.

jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented

Previous patch is garbage. I'd really like to just go with the 2.x version if we can get a beta, or better yet a full release. See #3019753: Plan for Views Reference Field 2.x-1.0 release