Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

Component: node.module » taxonomy.module
marcingy’s picture

Assigned: Unassigned » marcingy
marcingy’s picture

Status: Active » Needs review
FileSize
11.25 KB

First stab, I can't get tests to run locally on my windows box without timing out.

Also a lot of the queries against taxonomy_term_data are not efqable so this patch also extends interfaces. Also doesn't touch anything in views yet.

Status: Needs review » Needs work

The last submitted patch, remove-taxonomy-data-query.patch, failed testing.

marcingy’s picture

Status: Needs work » Needs review

#3: remove-taxonomy-data-query.patch queued for re-testing.

twistor’s picture

  1. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/TermStorageControllerInterface.php
    @@ -31,4 +31,58 @@ public function deleteTermHierarchy($tids);
    +   * @param Integer $tid
    +   *   Term id to retrieve parents for.
    ...
    +   * @param Integer $tid
    +   *   Term id to retrieve parents for.
    

    *int

  2. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/TermStorageControllerInterface.php
    @@ -31,4 +31,58 @@ public function deleteTermHierarchy($tids);
    +   * @param Integer $vid
    +   *   An optional vocabulary ID to restrict the child search.
    ...
    +   * @param Integer $vid
    +   *   Vocabulary ID to retrieve terms for.
    ...
    +   * @param Integer $vid
    +   *   Vocabulary ID to retrieve terms for.
    ...
    +   * @param Integer $vid
    +   *   Vocabulary ID to retrieve terms for.
    

    $vid is a string.

  3. +++ b/core/modules/taxonomy/taxonomy.module
    @@ -522,14 +522,7 @@ function taxonomy_term_load_parents($tid) {
    +    $tids = \Drupal::entityManager()->getStorageController('taxonomy_term')->loadParents($tid);
    ...
     
    
    @@ -577,17 +570,7 @@ function taxonomy_term_load_children($tid, $vid = NULL) {
    +    $tids = \Drupal::entityManager()->getStorageController('taxonomy_term')->loadChildren($tid);
    
    @@ -628,16 +611,7 @@ function taxonomy_get_tree($vid, $parent = 0, $max_depth = NULL, $load_entities
    +    $result = \Drupal::entityManager()->getStorageController('taxonomy_term')->loadTree($vid);
    
    +++ b/core/modules/taxonomy/taxonomy.tokens.inc
    @@ -167,21 +167,15 @@ function taxonomy_tokens($type, $tokens, array $data = array(), array $options =
    +          $replacements[$original] = \Drupal::entityManager()->getStorageController('taxonomy_term')->nodeCount($vocabulary->id());
    

    Can we wrap these?

marcingy’s picture

1 & 2 yes I'll do point 3 to be honest is more readable as is in my opinion.

twistor’s picture

Works for me.

marcingy’s picture

Ok docs fixed.

pfrenssen’s picture

Looks good to me, only found some tiny nitpicks with namespace ordering (my favourite pet peeve :), and capitalization of the word "ID" in the documentation. AFAIAC this is RTBC.

slashrsm’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

Xano’s picture

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

plach’s picture

Status: Fixed » Closed (fixed)

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