{taxonomy_index} and {taxonomy_term_hierarchy} doesn't join taxonomy data table in views

Thanks! That looks sane. We should probably have some automated test coverage of this.

This is at least major. Maybe we can move this fix to #2456713: Custom taxonomy field views handler needs to be replaced with generic Field API handler.

Tested patch #1 and fixes error.

Still needs a test. @John Cook see http://drupal.org/simpletest for a start on using SimpleTest. | Automated testing in Drupal: http://bit.ly/mwds-testing-video | Writing tests for core: https://drupal.org/contributor-tasks/write-tests

Yeah but we need tests to make sure it that it won't happen again.

Yeah but we need tests to make sure it that it won't happen again.

Working on the test and fix is incomplete

diff --git a/core/modules/taxonomy/src/TermViewsData.php b/core/modules/taxonomy/src/TermViewsData.php
index d101f84..61a3f65 100644
--- a/core/modules/taxonomy/src/TermViewsData.php
+++ b/core/modules/taxonomy/src/TermViewsData.php
@@ -255,7 +255,7 @@ public function getViewsData() {
         'left_field' => 'tid',
         'field' => 'parent',
       ),
-      'taxonomy_term_data' => array(
+      'taxonomy_term_field_data' => array(
         // Link directly to taxonomy_term_data via tid.
         'left_field' => 'tid',
         'field' => 'tid',
@@ -266,7 +266,7 @@ public function getViewsData() {
       'title' => t('Parent term'),
       'help' => t('The parent term of the term. This can produce duplicate entries if you are using a vocabulary that allows multiple parents.'),
       'relationship' => array(
-        'base' => 'taxonomy_term_data',
+        'base' => 'taxonomy_term_field_data',
         'field' => 'parent',
         'label' => t('Parent'),
         'id' => 'standard',

Also #2456713: Custom taxonomy field views handler needs to be replaced with generic Field API handler is related I thought we can move this fix in that issue but that is separate problem.

Previous patch only fixes this at one spot, there are actually two lines that need to be changed to fix the parent term.

To reproduce this bug, create a new Taxonomy term view, try to add a filter (or a contextual filter, or a relationship) on the parent term and you'll see that's not one of your choices anymore. Try in D7, and it's there. Initial patch didn't fix the relationship.

This is a rather small fix for a mistake in a previous commit, what do you suggest as far as tests? The original issue #2429447: Use data table as views base table, if available. that broke this wasn't required to have any tests - it seems to me that it should be easier to fix things and less easy to break things...

Adding a test to check just the parent term seems a little weak - what about the other thousand terms defined in core?

Found one more issue while scanning TermViewsData. Here is fix only patch comments/suggestions welcome.

This is a rather small fix for a mistake in a previous commit

If we'd have test back then we wouldn't have introduced this bug on the first place.

Right, and when we find bugs and fix them in issues, our general requirement is also to add a test so they do not come back again.

Added test for views data generation and checked both relationships for a view. Test only patch is an interdiff.

+++ b/core/modules/taxonomy/src/TermViewsData.php
@@ -248,6 +248,7 @@ public function getViewsData() {
+    $data['taxonomy_term_hierarchy']['table']['provider']  = 'taxonomy';

It is views by default which is not true. Is it worth it to add?

/me likes Drupal\Core\Database\DatabaseExceptionWrapper: Exception in test_taxonomy_term_relationship[test_taxonomy_term_relationship]: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ') subquery' at line 2: SELECT COUNT(*) AS expression FROM (SELECT 1 AS expression FROM {taxonomy_term_field_data} taxonomy_term_field_data LEFT JOIN {node} node_taxonomy_index ON nid = node_taxonomy_index.nid LEFT JOIN {taxonomy_term_data} taxonomy_term_data_taxonomy_term_hierarchy ON parent = taxonomy_term_data_taxonomy_term_hierarchy.) subquery; Array ( ) in Drupal\views\Plugin\views\query\Sql->execute() (line 1456 of /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/src/Plugin/views/query/Sql.php). Drupal\views\Plugin\views\query\Sql->execute(Object) Drupal\views\ViewExecutable->execute() Drupal\views\Tests\ViewTestBase->executeView(Object) Drupal\taxonomy\Tests\Views\TaxonomyRelationshipTest->testTaxonomyRelationships() Drupal\simpletest\TestBase->run(Array) simpletest_script_run_one_test('1363', 'Drupal\taxonomy\Tests\Views\TaxonomyRelationshipTest')

Just hit this bug. Patch looks great and test does as well.

Nitpick can either be ignored or hopefully fixed on commit, so marking RTBC.

+++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyRelationshipTest.php
@@ -0,0 +1,112 @@
+    $this->terms[] =$this->term1;
+    $this->terms[] =$this->term2;

Nitpick: There should be a space after the "=".

Reroll and fixed #17

There is nothing to review here so back to RTBC.

Yes, thanks, RTBC++

Nice find! Thanks for the fix and test. This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed and pushed to 8.0.x.

+++ b/core/modules/taxonomy/src/TermViewsData.php
@@ -169,8 +169,8 @@ public function getViewsData() {
-    // @todo This stuff needs to move to a node field since really it's all about
-    //   nodes.
+    // @todo This stuff needs to move to a node field since really it's all
+    //   about nodes.

Note: In the future, please keep coding standards cleanups like rewrapping comments separate from bugfixes, since it makes patches harder to review, adds unnecessary merge conflicts, etc. Reference: http://webchick.net/please-stop-eating-baby-kittens