TaxonomyIndexTid doesn't check whether there are any values before trying to loop over it

Seems legit!

This happens when you use the autocomplete option targeting an empty vocabulary.

Step to reproduce:
- Add a vocabulary, but don't add terms
- Make a content View
- add Content: Has taxonomy term filter
- Expose this filter
- In Settings target your empty vocabulary
- Chose Selection type: autocomplete

results in Warning: Invalid argument supplied for foreach() in Drupal\taxonomy\Plugin\views\filter\TaxonomyIndexTid->validateExposed()

Needs tests, so back to 'needs work'.

Tests for #1.

An attempt to fix errors from #5.

We got tests, nice!

Couple of things:

1. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -191,4 +200,19 @@ public function testExposedFilter() {
   +  public function testEmptyTaxonomyIndexTid() {
   

   method is missing a docblock, but maybe the tests can just be added to testExposedFilter(), no real need to bootstap Drupal all over again for this.

2. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -28,14 +29,14 @@ class TaxonomyIndexTidUiTest extends UITestBase {
   +  public static $modules = ['node', 'taxonomy', 'views', 'views_ui', 'taxonomy_test_views', 'dblog'];
   
   @@ -191,4 +200,19 @@ public function testExposedFilter() {
   +    $this->drupalGet('/admin/reports/dblog');
   +    $result = $this->xpath('//a[contains(@title, :title)]', [':title' => 'Warning: Invalid argument supplied for foreach() in Drupal\taxonomy\Plugin\views\filter\TaxonomyIndexTid->validateExposed()']);
   +    $this->assertNotEqual(count($result), 1, "Warning message is not displayed");
   

   I don't think there is any need to go into the dblog to check the error. The test will fail just fine if a PHP error is thrown.

   The test does need some more assertions I think. If you add it to testExposedFilter() maybe just make sure the View result is empty, since no nodes will have tags from the empty vocabulary.

3. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -12,6 +12,7 @@
   +use Drupal\views\Views;
   
   @@ -191,4 +200,19 @@ public function testExposedFilter() {
   +    $view = Views::getView('test_taxonomy_term_name');
   

   This is not needed when you change the Views config via PostForm.

@Lendude thank you for reviewing my patch and the feedback.

I made changes as per #10. Let me know if they are acceptable.

@aerozeppelin looking much better, couple of little things:

1. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -12,6 +12,7 @@
   +use Drupal\views\Views;
   

   not needed.

2. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -189,6 +198,24 @@ public function testExposedFilter() {
   +
   +    $edit_0['name[taxonomy_term_field_data.tid]'] = TRUE;
   +    $this->drupalPostForm('admin/structure/views/nojs/add-handler/test_taxonomy_term_name/default/field', $edit_0, 'Add and configure fields');
   +    $edit['name[taxonomy_term_field_data.tid]'] = TRUE;
   

   some documentation/comment as to what is going to be tested would be nice.

3. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -189,6 +198,24 @@ public function testExposedFilter() {
   +    //$this->drupalPostForm('admin/structure/views/nojs/handler-extra/test_taxonomy_term_name/default/filter/vid', [], 'Apply');
   

   Commented out line should be removed.
   

4. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -189,6 +198,24 @@ public function testExposedFilter() {
   +    $this->drupalPostForm(NULL, [], t('Apply'));
   

   Why the 'Apply'? Don't think that is needed here right?

@Lendude thanks again for your feedback. :D I've made some changes as per #12.

Why the 'Apply'? Don't think that is needed here right?

I agree, that line appears to be redundant.

@aerozeppelin nice clean up, couple of little things left, then this is good to go for me:

1. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -189,6 +197,28 @@ public function testExposedFilter() {
   +    // Select 'Term ID' as the field to be displayed
   ...
   +    // Expose the filter
   ...
   +    // Filter 'Taxonomy terms' belonging to 'Empty Vocabulary'
   

   all comments need full stop/period/. at the end.

2. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyIndexTidUiTest.php
   @@ -189,6 +197,28 @@ public function testExposedFilter() {
   +    $edit = ['name[taxonomy_term_field_data.tid]' => TRUE,
   +             'name[taxonomy_term_field_data.vid]' => TRUE
   +      ];
   

   remove the extra white space/indents and add a comma at the end of the last item in the array. Something along these lines:

     $edit = [
       'name[taxonomy_term_field_data.tid]' => TRUE,
       'name[taxonomy_term_field_data.vid]' => TRUE,
     ];
   

@Lendude here you go. Changes as per #14.

@aerozeppelin nice! We got tests and a fix.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

There is still a small issue with this patch: if it is a grouped filter with 'All' as default value, it will try to loop around a string... which is bad.
Please review the attached patch

Please open a new issue and upload your patch there.

This one has been closed for two years.

Thanks!