There's only 2 usages of the function
TermFormController and TermDelete (confirm form after #1946456: Convert taxonomy_term_confirm_delete() and taxonomy_vocabulary_confirm_delete() to the new form interface)

So better move the function to TermStorageController

CommentFileSizeAuthor
#13 1988712-13.patch6.44 KBjibran
#10 1988712-tax-hierarchy-10.patch5.92 KBh3rj4n
#3 interdiff.txt822 bytesandypost
#3 1988712-tax-hierarchy-3.patch6.38 KBandypost
#1 1988712-tax-hierarchy-1.patch6.37 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Active » Needs review
FileSize
1988712-tax-hierarchy-1.patch6.37 KB

And here's a patch

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented 
+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/VocabularyStorageController.phpundefined
@@ -104,4 +105,49 @@ public function resetCache(array $ids = NULL) {
+  public function checkHierarchy(VocabularyInterface $vocabulary, $changed_term) {

sorry for pointing that array missing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
FileSize
1988712-tax-hierarchy-3.patch6.38 KB
interdiff.txt822 bytes

fixed, nice catch

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

It is RTBC IMO but lets see what other thinks about it.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

I think it is good to go.

alexpott’s picture

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

Are we sure about moving this to the storageController to me checkHierarchy should be a method on the vocabulary entity and also it should be called checkAndFixHierarchy as that is what it does.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

So proposed was implementation of invalidateHierarchy() methid in term interface and class because of #1893772-30: Move entity-type specific storage logic into entity classes

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

#3: 1988712-tax-hierarchy-3.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1988712-tax-hierarchy-3.patch, failed testing.

h3rj4n’s picture

h3rj4n CreditAttribution: h3rj4n commented
Status: Needs work » Needs review
FileSize
1988712-tax-hierarchy-10.patch5.92 KB

I haven't done a reroll. Just copied an paste the code in the right place.

The function is now in the entity as suggested by alexpott. I also used the function name as alexpott said. All the taxonomy tests success locally. Let's see what the testbot says.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

10: 1988712-tax-hierarchy-10.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 10: 1988712-tax-hierarchy-10.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
1988712-13.patch6.44 KB

RTBC?

Status: Needs review » Needs work

The last submitted patch, 13: 1988712-13.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented 
  1. +++ b/core/modules/taxonomy/src/Entity/Vocabulary.php
@@ -183,4 +183,33 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+  function checkHierarchy($changed_term) {
+    $tree = taxonomy_get_tree($this->id());

    would be nice to move that to storage controller as well, this breaks unit-testing of the entity 

  2. +++ b/core/modules/taxonomy/src/Entity/Vocabulary.php
@@ -183,4 +183,33 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+    $hierarchy = TAXONOMY_HIERARCHY_DISABLED;
...
+        $hierarchy = TAXONOMY_HIERARCHY_MULTIPLE;
...
+        $hierarchy = TAXONOMY_HIERARCHY_SINGLE;

    Suppose needs separate issue to move to vocabulary interface

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

I still not sure that entity class is a right place for the function,
better to place it in storage controller

+++ b/core/modules/taxonomy/src/Entity/Vocabulary.php
@@ -183,4 +183,33 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+  function checkHierarchy($changed_term) {
+    $tree = taxonomy_get_tree($this->id());

+++ b/core/modules/taxonomy/taxonomy.module
@@ -192,51 +189,6 @@ function taxonomy_theme() {
-function taxonomy_check_vocabulary_hierarchy(VocabularyInterface $vocabulary, $changed_term) {
-  $tree = taxonomy_get_tree($vocabulary->id());

taxonomy_get_tree() should be replaced with proper injection of term storage controller

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Version: 8.0.x-dev » 9.x-dev
Status: Needs work » Postponed

I don't think we can move it around anymore at this point of release. This is an API break so we have to wait for D9.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Version: 9.x-dev » 8.0.x-dev
Status: Postponed » Needs work

I disagree, we can deprecate it in favour of proper method that would api clean-up

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

Ok then let's finalize the location of the function entity class or storage controller

Status: Needs work » Needs review

andyceo queued 13: 1988712-13.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 13: 1988712-13.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs work » Closed (outdated)

We're planing to deprecate taxonomy_check_vocabulary_hierarchy() in #2957381: Data model problems with Vocabulary hierarchy, so we won't need to move it anywhere :)