Steps to reproduce:

- Add some taxonomy terms to tags vocab
- Create new content view
- add 'Has taxonomy term' filter
- select tags vocab
- Try to autocomplete your terms - will not do return anything

This will fix it, but we also need some tests I guess. I will work on these.

CommentFileSizeAuthor
#17 2024111-17.patch2.73 KBdamiankloip
#17 interdiff-2024111-17.txt662 bytesdamiankloip
#15 2024111-15.patch2.74 KBBerdir
#15 2024111-15-interdiff.txt1.79 KBBerdir
#14 2024111-14.patch3.11 KBdamiankloip
#14 interdiff-2024111-14.txt687 bytesdamiankloip
#12 2024111-12.patch3.09 KBdamiankloip
#12 interdiff-2024111-12.txt1.04 KBdamiankloip
#10 2024111-10.patch3.05 KBdamiankloip
#6 2024111-6.patch5.81 KBdamiankloip
#6 interdiff-2024111-6.txt1014 bytesdamiankloip
#4 2024111-4.patch5.76 KBdamiankloip
vdc.taxIndexTid-filter.patch2.64 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

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

The last submitted patch, vdc.taxIndexTid-filter.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

OH I even thought we could have tests for that.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

I plan to add tests for it.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
2024111-4.patch5.76 KB

I was thinking with an actual web test like this..

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/views/lib/Drupal/views/Tests/ViewsTaxonomyAutocompleteTest.phpundefined
@@ -0,0 +1,54 @@
+    $expected = drupal_map_assoc((array) $label);

I know you worked on replacing drupal_map_assoc() with a static method.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
interdiff-2024111-6.txt1014 bytes
2024111-6.patch5.81 KB

ok, you got me...

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

On the longrun it should be certainly possible to have a proper unit test but for now this is good.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Fixed » Active
Issue tags: +Random test failure

I just got

Value array ( 'xrZjt3aq' => 'xrZjt3aq', ) is identical to value array ( 'iFSCXR4g' => 'iFSCXR4g', 'xrZjt3aq' => 'xrZjt3aq', ).

from this test in an unrelated issue.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Active » Needs review
FileSize
2024111-10.patch3.05 KB

Ouch, yeah. I see. I guess using a random string for terms, then autocompleting them isn't the best idea :)

So maybe we should just create our own vocab and terms for this test instead of using taxonomy term base? Then we can hardcode the term names, and the searching is reliable.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/views/lib/Drupal/views/Tests/ViewsTaxonomyAutocompleteTest.phpundefined
@@ -7,22 +7,44 @@
+   * @var \Drupal\taxonomy\Plugin\Core\Entity\Vocabulary
...
+   * @var Drupal\taxonomy\Term
...
+   * @var Drupal\taxonomy\Term

I guess we should use interfaces with proper starting "\"

+++ b/core/modules/views/lib/Drupal/views/Tests/ViewsTaxonomyAutocompleteTest.phpundefined
@@ -32,6 +54,20 @@ public static function getInfo() {
+    parent::SetUp();

setUp instead of SetUp

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
interdiff-2024111-12.txt1.04 KB
2024111-12.patch3.09 KB

Thanks! I realised not just the leading '\' was missing on those namespaces but they were just plain wrong.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

This looks great now.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
interdiff-2024111-14.txt687 bytes
2024111-14.patch3.11 KB

Oops, just missed one doc namespace change. still rtbc as far as I'm concerned..

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Reviewed & tested by the community » Needs review
FileSize
2024111-15-interdiff.txt1.79 KB
2024111-15.patch2.74 KB

Changed the type hints to interface and change the second term name so that it doesn't overlap with the first, so that we don't have to change the behavior of the test.

Status: Needs review » Needs work

The last submitted patch, 2024111-15.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
interdiff-2024111-17.txt662 bytes
2024111-17.patch2.73 KB

That won't work with the current tests, the terms would have to be like this instead.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Oh, sorry, that was stupid, I had just "another" in there initially, and just added the _term to prove that it is in fact the CONTAINS that causes so many failures here, discussed it with @dawehner and then forget to revert that before creating the patch.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Ha :) yeah, I thought it may be something like that.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

This looks perfect now.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed ab22b87 and pushed to 8.x. Thanks!

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Updated issue summary.