Hello everyone.

Hierarchical select is a great module, but I personallly lack a way to filter a list of terms selected by hierarchical_select. I think that hook_query_alter would do in my case, so I rewrote the queries of hs_taxonomy module using DBTNG layer in order to properly support it.

Please take a look at the patch.

CommentFileSizeAuthor
#14 1838422-14-alter_term_list_in _hierarchy.patch2.9 KBsoul88
#8 1838422-8-alter_term_list_in _hierarchy.patch2.25 KBsoul88
#6 1838422-6-alter_term_list_in _hierarchy.patch2.42 KBsoul88
#5 1838422-5-alter_term_list_in _hierarchy.patch2.42 KBsoul88
hs_taxonomy_01.patch2.24 KBsoul88

Comments

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Needs review » Reviewed & tested by the community

looks good for me

andypost’s picture

andypost
he/him
Primary language Russian
commented

makes sense at all. This queries are not often used so no problem to make then dynamic

ioskevich’s picture

ioskevich commented

Wouldn't it be useful to add "term_access" tag to the query so this query could be also affected by HOOK_query_term_access_alter()? So all taxonomy queries can be altered in the single place.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

Patch looks sensible.

But needs work as per #3.

soul88’s picture

soul88
Primary language Ukrainian
Location Ukraine
commented
Status: Needs work » Needs review
StatusFileSize
new 1838422-5-alter_term_list_in _hierarchy.patch2.42 KB

Rerolled the patch. Don't have a working install around, so hope it works. If not - will check/fix it in a few days.

soul88’s picture

soul88
Primary language Ukrainian
Location Ukraine
commented
StatusFileSize
new 1838422-6-alter_term_list_in _hierarchy.patch2.42 KB

//deleted a tab for better code formatting.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

Lots of whitespace issues have been introduced.

  1. +++ b/modules/hs_taxonomy.module
@@ -799,11 +799,21 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -
+    $result = $query->execute();
+
+     foreach ($result as $term) {
+       $children[$vid][$term->parent][] = $term->tid;

    Indentation is off. 

  2. +++ b/modules/hs_taxonomy.module
@@ -799,11 +799,21 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -
+       $terms[$vid][$term->tid] = $term;	   ¶

    Trailing whitespace. 

  3. +++ b/modules/hs_taxonomy.module
@@ -843,12 +853,22 @@ function hs_taxonomy_term_count_nodes($tid, $type = 0) {
+    if (!is_numeric($type)) {
+      $query->condition('n.type', $type);
+     }
+

    Indentation is off. 

  4. +++ b/modules/hs_taxonomy.module
@@ -843,12 +853,22 @@ function hs_taxonomy_term_count_nodes($tid, $type = 0) {
+

    Extraneous newline.

soul88’s picture

soul88
Primary language Ukrainian
Location Ukraine
commented
StatusFileSize
new 1838422-8-alter_term_list_in _hierarchy.patch2.25 KB

fixed everything except for №4. Didn't find any empty line there.

soul88’s picture

soul88
Primary language Ukrainian
Location Ukraine
commented
Status: Needs work » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks! Now we need others to test it and confirm that it does not break things.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Tested this locally, works fine! Only small nitpick:

+++ b/modules/hs_taxonomy.module
@@ -843,12 +853,21 @@ function hs_taxonomy_term_count_nodes($tid, $type = 0) {
+    $query->addTag('hs_taxonomy_term_count_nodes');

Any reason for this tag?

soul88’s picture

soul88
Primary language Ukrainian
Location Ukraine
commented

Yes, actually the whole topic is about this tag as for me :). It allows to alter HS queries easily (to distinct them from other queries).

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work

So this should be documented somewhere in README.txt or .api.php

soul88’s picture

soul88
Primary language Ukrainian
Location Ukraine
commented
Status: Needs work » Needs review
StatusFileSize
new 1838422-14-alter_term_list_in _hierarchy.patch2.9 KB

Added a note into the README.TXT

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Great! Wim I think this good to go now

stefan.r’s picture

stefan.r commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Re-rolled and committed, thanks!

  • stefan.r committed ff3151f on 7.x-3.x 
    Issue #1838422 by Soul88: Added Need a way to alter terms list in the...
stefan.r’s picture

stefan.r commented
Status: Fixed » Closed (fixed)
stefan.r’s picture

stefan.r commented
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

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