Issues like #1266466: Features should not are caused by the fact that exporting HS settings through features results in a feature that requires the taxonomy vocabularies to have the same serial ID as they did on the original site. Consequently, most of these settings are unusable on the destination site because these vocabularies end up with different identifiers.

The export should leverage something like UUID, or use the machine name of the vocabulary instead.

CommentFileSizeAuthor
#13 hierarchical_select-machine-name-features-1477292-13.patch8.57 KBguillaumev
#10 hierarchical_select-machine-name-features-1477292-10.patch10.11 KBscottrigby
#10 interdiff.txt6.21 KBscottrigby
#5 hierarchical_select-machine_name_for_features_export_with_fix_for_install-1477292-5.patch9.87 KBhani.movahed
#2 hierarchical_select-machine_name_for_features_export-1477292-2.patch8.6 KBguypaddock
#1 hierarchical_select-machine_name_for_features_export-1477292-1.patch4.43 KBguypaddock

Comments

guypaddock’s picture

guypaddock commented
Status: Active » Needs review
StatusFileSize
new hierarchical_select-machine_name_for_features_export-1477292-1.patch4.43 KB

The attached patch converts HS taxonomy over to using the machine name of vocabularies in config names, instead of the serial vocabulary ID. Since Features just rolls with whatever HS calls the configs, there was no change necessary to the features export code.

This might need additional work to add logic to update the profiles when vocabularies are renamed, but I'll leave that as an open exercise for now.

After applying this patch, you will need to run update.php to rename old configs to follow the new convention.

guypaddock’s picture

guypaddock commented
StatusFileSize
new hierarchical_select-machine_name_for_features_export-1477292-2.patch8.6 KB

I found a few more places where the VID was being used instead of the machine name. In addition, I wanted to make sure that HS taxonomy reacts to changes in the vocabulary machine name or deletion of vocabularies.

The attached patch should address all of these concerns, as well as including all of the changes from #1. Use this one instead.

rwilson0429’s picture

rwilson0429 commented

I received errors when trying to apply the patch.

hierarchical_select-machine_name_for_features_export-1477292-2.patch:34: trailing whitespace.
  require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'hierarchical_select') .                                         '/includes/common.inc';
hierarchical_select-machine_name_for_features_export-1477292-2.patch:35: trailing whitespace.

hierarchical_select-machine_name_for_features_export-1477292-2.patch:99: trailing whitespace.
 *
hierarchical_select-machine_name_for_features_export-1477292-2.patch:102: trailing whitespace.
 *
hierarchical_select-machine_name_for_features_export-1477292-2.patch:108: trailing whitespace.

Checking patch includes/common.inc...
Checking patch modules/hs_taxonomy.install...
Checking patch modules/hs_taxonomy.module...
error: while searching for:
// Private functions.

/**
 * Drupal core's taxonomy_term_count_nodes() is buggy. See
 * http://drupal.org/node/144969#comment-843000.
 */

error: patch failed: modules/hs_taxonomy.module:892
error: modules/hs_taxonomy.module: patch does not apply
takki’s picture

takki commented

I could apply the patch without errors. I have only tested a content type feature and it works fine. As config id the machine name of the taxonomy is now used. But this needs more testing to be sure there are no issues.

hani.movahed’s picture

hani.movahed commented
StatusFileSize
new hierarchical_select-machine_name_for_features_export_with_fix_for_install-1477292-5.patch9.87 KB

Well the patch almost works. The only thing missing was the actual include of the common.inc in the hierarchical_select.features.inc around line 88. I made a small update to that patch and attached it for review.

ruloweb’s picture

ruloweb commented

#5 works for me :). Thanks!!!

mrsimonelliott’s picture

mrsimonelliott commented

Patch in #5 works for me. Thanks

ryan_courtnage’s picture

ryan_courtnage commented
Status: Needs review » Reviewed & tested by the community

Patch in #5 is working for me as well.

core44’s picture

core44 commented

Thanks for the patch... this is a very useful addition, will this be committed?

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
StatusFileSize
new interdiff.txt6.21 KB
new hierarchical_select-machine-name-features-1477292-10.patch10.11 KB

The patch in #5 allows exportability and features integration, but it's full of spacing and no newline errors - also odd HTML markup inside Doxygen comments. I re-rolled a patch (and attached interdiff to #5) to make this more commit-worthy.

Other places in the module have wonky coding standards issues, but at least this patch ensures no more of those are added by this issue!

mpgeek’s picture

mpgeek commented

#10 Worked for me. Thanks.

wim leers’s picture

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

Wow, thanks for cleaning up the mess in #5, scottrigby!

Patch no longer applies, and I have some feedback:

  1. +++ b/modules/hs_taxonomy.module
@@ -192,6 +193,43 @@ function hs_taxonomy_form_taxonomy_form_term_alter(&$form, &$form_state) {
+ * Implements hook_taxonomy_vocabulary_save().

    Docs say "_save()", hook is "_update()" -> mismatch! 

  2. +++ b/modules/hs_taxonomy.module
@@ -1040,6 +1079,59 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -
+function hs_taxonomy_get_config(stdClass $vocabulary) {
...
+function hs_taxonomy_apply_config(array &$element, stdClass $vocabulary) {
...
+function hs_taxonomy_set_config(stdClass $vocabulary, array $config) {

    These wrappers don't make sense to me. 

  3. +++ b/modules/hs_taxonomy.module
@@ -1040,6 +1079,59 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -
+function hs_taxonomy_get_config_id(stdClass $vocabulary) {

    This wrapper makes sense — thanks!

guillaumev’s picture

guillaumev commented
Status: Needs work » Needs review
StatusFileSize
new hierarchical_select-machine-name-features-1477292-13.patch8.57 KB

New patch which applies on the latest dev and takes into account the comments made by Wim Leers.

guillaumev’s picture

guillaumev commented
Issue summary: View changes

Added link.

NaX’s picture

NaX commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

#13 applied cleanly and worked great for me.
Thanks.

one_orange_cat’s picture

one_orange_cat commented

In case it helps anyone - I have created a new patch building on the patch in #13 but incorporating enhancements to support the request in https://www.drupal.org/node/1280994 to allow separate configurations per field (i.e. not just per vocabulary). The patch is currently available at: https://www.drupal.org/node/1280994#comment-9331047

  • stefan.r committed ffa2a7b on 7.x-3.x authored by guillaumev 
    Issue #1477292 by GuyPaddock, scottrigby, guillaumev, hani.movahed:...
stefan.r’s picture

stefan.r commented
Status: Reviewed & tested by the community » Closed (fixed)

Committed #13 for now -- the per field patch still needs some further review.

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.