When using a text field as an exposed grouped filter with the checkbox to allow multiple selections, I get the following error when trying to use the pager to go to the next page of results:
An illegal choice has been detected. Please contact the site administrator.
This shows up in the watchdog as:
Illegal choice 0 in field_presenter_last_name_value element.
Using Better Exposed Filters with the exposed form in a block.
I've attached screenshots and the display export.
Comment | File | Size | Author |
---|---|---|---|
#43 | views-illegal-choice.png | 39.76 KB | capysara |
#36 | illegal_choice_0_in-1986306-36.patch | 1.09 KB | nerdcore |
#32 | illegal_choice_0_in-1986306-32.patch | 1.17 KB | epringi |
#30 | illegal_choice_0_in-1986306-30.patch | 1.09 KB | fietserwin |
| |||
#1 | test-search-exported-view.txt | 5.64 KB | daniel.rolls@flightcentre.com.au |
Comments
Comment #1
daniel.rolls@flightcentre.com.au CreditAttribution: daniel.rolls@flightcentre.com.au commentedConfirming this bug. It still exists in 7.x-3.5, and on 7.x-3.x-dev branch.
This not only affects pagination links, but anything that uses the
views->raw_exposed_filters
property to generate links. Table click-sort links are also affected by this.To reproduce this error one can do the following on a test site:
You should see something like the following:
Comment #2
daniel.rolls@flightcentre.com.au CreditAttribution: daniel.rolls@flightcentre.com.au commentedThe problem is that pagination and quicksort links are being created from a set of inputs that does not understand that checkboxes set to zero should not be sent as part of the request.
Have found a fix that uses array_filter to remove values that have a value of zero, if it is a multiple exposed checkbox.
Comment #3
daniel.rolls@flightcentre.com.au CreditAttribution: daniel.rolls@flightcentre.com.au commentedComment #4
daniel.rolls@flightcentre.com.au CreditAttribution: daniel.rolls@flightcentre.com.au commentedNote: better exposed filters introduces a complication here as you have the option to move fields into a field-set named secondary (ie by making them a secondary filter). The above fix doesn't work in this case as it would need to check
$form['secondary'][$key]
for the type.Comment #5
daniel.rolls@flightcentre.com.au CreditAttribution: daniel.rolls@flightcentre.com.au commentedHave modified this patch so it checks any children on the form (such as fieldsets) if it cannot find the $key directly on
$form
Comment #6
Darren OhThe patch in comment #5 works.
Comment #7
markocall CreditAttribution: markocall commentedThe patch in #5 worked for me as well, thanks a lot!
Comment #8
dawehnerTo be honest, this amount of change is to awkward to be just committed without some serious test coverage, I am sorry.
Comment #9
rreiss CreditAttribution: rreiss commentedThe patch in #5 worked for me. Thanks @rollsd !
Comment #10
enjaygee CreditAttribution: enjaygee commentedThanks @rollsd -- the patch in #5 worked for me as well.
Comment #11
rmathew CreditAttribution: rmathew commentedThe patch in #5 resolves the "illegal choice" issue for me, however the pager URL is not quite right. eg:
http://[...]=All&search_api_views_fulltext=digital&&&page=1
Note the "&&&" - due to two checkbox values being ignored by the patch.
Comment #12
Crell CreditAttribution: Crell commentedI just ran into this issue as well. I am seeing the &&& in the URL even without this patch, so that appears to be a separate issue.
Comment #13
FerCamp CreditAttribution: FerCamp commented#5 worked for me too. only thing I wasn't aware need to logout and login again sessions to see changes.
Comment #14
mr. chips CreditAttribution: mr. chips commentedI had to change one line in #5 patch:
from this
if (isset($field) && $field['#type'] === 'checkboxes') {
to this
if (isset($field['#type']) && $field['#type'] === 'checkboxes') {
or I would get this:
Notice: Undefined index: #type in views_exposed_form_submit() (line 2079 of ..... httpdocs/sites/all/modules/views/views.module)
I have several exposed filters with some as checkboxes, filtering entities from a custom table. I get the notice whether or not I use hook_form_alter. I can't duplicate that notice by filtering on Body of content type Article.
Other than that it worked for me. Paging was my problem before - if a checkbox group was left un-checked it would throw an error and check ALL the boxes in that group when going to the next page of results.
Comment #15
fietserwinSame as #14, patch works but is giving undefined index notices. So that case should be handled as in #14.
BTW: BEF does not have this problem, as it leaves the #type to select, but changes the theme, and only in the theme function it renders checkboxes instead of a select.
Comment #16
rcodina CreditAttribution: rcodina commentedPatch on #5 works for me too.
Comment #17
Darren OhI can understand holding up a feature until it has tests, but continuing to release this module without a bug fix that passes existing tests makes no sense.
Comment #18
fietserwinReroll of #5 (line number offset) + #14
Comment #19
patrick.thurmond@gmail.comI am going to agree with #17, this is getting to be irresponsible at this point. You have had a valid fix for this, on that is passing unit tests and peer review, and refuse to add the patch. Despite what was said in #8 this isn't that big a change. If you call that a big change then I don't want to know how you manage to code in a team environment. That is piddly at best.
So the question remains, when will this patch (or a better one if you have it) going to be applied?
Comment #20
mbopp CreditAttribution: mbopp as a volunteer commentedWhile it certainly isn't ideal, if you want to 'patch' the views module without actually modifying the code you can use hook_form_alter to do so. This would at least make upgrading the views module a bit easier, as this is something you will most likely want to do as time goes on.
An example of this in a custom module would be something like...
Then the submit handler... which is the modified views exposed form submit handler.
Comment #21
deggertsen CreditAttribution: deggertsen as a volunteer commented@pthurmond. Don't be so surprised. Views is a big project that goes through a lot of quality control. There have been some tests here, but status is still Needs review and only a few comments ago (#14) a bug in the patch was discovered which was just fixed in #18. There have been very few test since #18, which a maintainer would probably look at and say there needs to be more tests. That said, I think the change in #18 is minor enough that all the other tests are still valuable and I agree that it's at least time for a maintainer to take a good look at the patch. Thus I'm marking to RTBC to get their attention. Obviously that doesn't necessarily mean it will pass quality control, but at least we can get somebody looking at this who can make a decision.
The patch in #18 looks good to me. Perhaps a maintainer can weigh in?
Comment #22
freexia CreditAttribution: freexia as a volunteer commentedI've applied the patch in #18, and this is working using the basic filter settings. However, when I apply Better Exposed Filters, the issue remains, and "An illegal choice has been detected." is visible. The URL looks something like "view?search=&&sc&etc&hr&res&page=2" where sc, etc, hr are multiple select filters. Interestingly I have a filter st that has > 4 options and it doesn't show up in the URL and the rest of the filters have < 4 options are in the URL causing the error.
Comment #23
colanWe've recently switched our testing from the old qa.drupal.org to DrupalCI. Because of a bug in the new system, #2623840: Views (D7) patches not being tested, older patches must be re-uploaded. On re-uploading the patch, please set the status to "Needs Review" so that the test bot will add it to its queue.
If all tests pass, change the Status back to "Reviewed & tested by the community". We'll most likely commit the patch immediately without having to go through another round of peer review.
We apologize for the trouble, and appreciate your patience.
Comment #24
MLZRApplied the patch, thing's worked in de preview (under the views working area), bud not in the fronend. But after the suggestion in #13 (logout- and login again) things get working on the frontend too!!
Top!
Comment #25
fietserwinAs per #23: re-upload of #18 to trigger the testbot.
Comment #27
fietserwinWindows format :(. Again.
Comment #28
mstrelan CreditAttribution: mstrelan commentedI'm guessing
continue;
should bebreak;
Also "burried" should be "buried", or maybe "nested" is more appropriate.Ideally, if the filtered array is empty it should not be added to the exposed input, otherwise the pager link URLs have a bunch of unnecessary ampersands. But I think that is a separate issue because it is not just these checkboxes that are causing it.
Comment #29
marameodesign#27 works with grouped filters and BEF
Comment #30
fietserwinAs per #28. I also combined the ternary operator with the check on null directly afterwards into 1 if/else
Comment #31
Darren OhThe latest patch improves on the previous versions and does not introduce any new issues.
Comment #32
epringi CreditAttribution: epringi as a volunteer and at OpenConcept Consulting Inc. commentedUpdated patch for latest version.
Comment #34
pbirk CreditAttribution: pbirk commentedConfirming patch in #30 worked against Views 3.14.
Comment #35
gerson.analista CreditAttribution: gerson.analista as a volunteer commentedConfirming patch #30 worked in Views 3.14.
Comment #36
nerdcore CreditAttribution: nerdcore at OpenConcept Consulting Inc. commentedRe-roll patch #32 to use correct path
Comment #37
jannis CreditAttribution: jannis commentedpatch #36 applies cleanly to v7.x-3.17
Comment #38
jannis CreditAttribution: jannis commentedComment #39
marucci CreditAttribution: marucci commentedThis patch is not planned for a next release?
Comment #40
capysara CreditAttribution: capysara commentedI was having a similar issue, but with sorting. I have an exposed group filter and when I tried to sort, all the grouped filters would be checked, I would get no results, and I would get the 'illegal choice' error.
This patch resolves that.
Comment #41
capysara CreditAttribution: capysara commentedPatch still applies cleanly to 3.21
Comment #42
DamienMcKennaCould folks here please test the current -dev snapshot to see if the problem still exists? I tried to reproduce it with Darren Oh's help but was unable to do so. Thanks.
Comment #43
capysara CreditAttribution: capysara commentedSteps to reproduce
dl & en views 7.x-3.x-dev and dependencies
dl & en devel and devel_generate
to Basic page, add two list (integer) fields, select list with allowed values: 1, 2
drush genc 20
add new view
—Content: Basic page
—display format: table
—add 2 new List fields
—Table settings: make all fields sortable
—Add filter for both fields
——Expose, Grouped, allow multiple selections, Widget type: Radios
——Label: One, Operator: Is not empty (NOT NULL), Value: select all
When you go to the view, sort one of the tables, and both Exposed filters get selected, and you get the 'illegal choice.'
Comment #44
DamienMcKennaThanks for the steps, @capysara.
Let's turn this into a test so we can make sure the bug doesn't come back again.
Comment #45
DamienMcKennaComment #46
Katy Jockelson CreditAttribution: Katy Jockelson commentedThe patch in #5 is working great for me with a very complicated view with multiple exposed filters, all of which are grouped, and multiple sort columns. But I can't really have a patched version of Views on the live site. Is there any progress with getting this rolled in the module? Can I help at all?