Problem/Motivation

The 'Has taxonomy terms (with depth)' filter does not modify the query at all.

To reproduce:

  1. Add 3 terms to the 'tags' vocab, and nest them under each other
  2. Create a view of article content
  3. Add the 'Has taxonomy terms (with depth)', and set depth to 3
  4. Update the view preview and note the SQL does not change at all (eg, the filter is not applied)

Proposed resolution

Fix the filter and add tests.

Remaining tasks

None

User interface changes

None

API changes

None

Suggested commit message

lgalanter discovered this bug through diligent manual testing.

git commit -m 'Issue #2467895 by jhedstrom, lgalanter: taxonomy filter with depth completely broken'

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because the filter doesn't work at all
Issue priority Major because this is a significant regression
Prioritized changes The main goal of this issue is usability since this filter is available through the UI but does nothing when used
CommentFileSizeAuthor
#14 taxonomy-filter-with-depth-2467895-14.patch12.3 KBjhedstrom
#14 interdiff.txt1.85 KBjhedstrom
#6 taxonomy-filter-with-depth-2467895-06.patch12.24 KBjhedstrom
#6 taxonomy-filter-with-depth-2467895-06-TEST-ONLY.patch11.31 KBjhedstrom
#6 interdiff.txt581 bytesjhedstrom
#4 taxonomy-filter-with-depth-2467895-04.patch11.67 KBjhedstrom
#4 taxonomy-filter-with-depth-2467895-04-TEST-ONLY.patch10.74 KBjhedstrom
#4 interdiff.txt10.74 KBjhedstrom
#2 taxonomy-filter-with-depth-2467895-02.patch953 bytesjhedstrom

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
Issue tags: +Needs tests
StatusFileSize
new taxonomy-filter-with-depth-2467895-02.patch953 bytes

The filter was referencing incorrect class variables (table_queue instead of the now-protected tableQueue).

lgalanter’s picture

lgalanter commented
Issue summary: View changes
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue tags: -Needs tests
StatusFileSize
new interdiff.txt10.74 KB
new taxonomy-filter-with-depth-2467895-04-TEST-ONLY.patch10.74 KB
new taxonomy-filter-with-depth-2467895-04.patch11.67 KB

This adds some tests for the term with depth filter.

The last submitted patch, 4: taxonomy-filter-with-depth-2467895-04-TEST-ONLY.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
StatusFileSize
new interdiff.txt581 bytes
new taxonomy-filter-with-depth-2467895-06-TEST-ONLY.patch11.31 KB
new taxonomy-filter-with-depth-2467895-06.patch12.24 KB

There was an instance of createTerm() that was passing in the vocabulary object even though it wasn't used within the method.

The last submitted patch, 6: taxonomy-filter-with-depth-2467895-06-TEST-ONLY.patch, failed testing.

The last submitted patch, 4: taxonomy-filter-with-depth-2467895-04.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes

Adding a beta phase evaluation, and adding an explanation for the suggested commit message.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes
lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Reviewed & tested by the community

Fix and tests look good to me.

Doesn't help with the problems for adding the filter via the UI as outlined in #2017829: "Show hierarchy in dropdown" doesn't work for taxonomy field views filter., but at least with this fix in place it does something when you do manage to add it.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTermFilterDepthTest.php
@@ -0,0 +1,140 @@
+use Drupal\views\Tests\ViewTestData;

    Not used 

  2. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTermFilterDepthTest.php
@@ -0,0 +1,140 @@
+    // top-level terms w/o childrend.

    childrend. 

  3. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTermFilterDepthTest.php
@@ -0,0 +1,140 @@
+  /**
+   * Changes the tid filter to given term and depth.
+   *
+   * @param integer $tid
+   *   The term ID to filter on.
+   * @param integer $depth
+   *   The depth to search.
+   */
+  protected function assertTermWithDepthResult($tid, $depth, array $expected) {

    Missing $expected documentation. 

  4. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTestBase.php
@@ -121,20 +121,24 @@ protected function mockStandardInstall() {
    * @return \Drupal\taxonomy\Term

    Should be \Drupal\taxonomy\Entity\Term - lets fix that here.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.85 KB
new taxonomy-filter-with-depth-2467895-14.patch12.3 KB

Good catches!

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

Since those were cosmetic changes, bumping this back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Looks like Alex's feedback was addressed.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed ca3422f on 8.0.x 
    Issue #2467895 by jhedstrom, lgalanter: taxonomy filter with depth...

Status: Fixed » Closed (fixed)

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