Allow different HS configs for different Taxonomy Term fields (instead of 1 HS config per vocabulary)

Are you using alpha 4 or a dev snapshot that's newer than alpha 4?

I don't know *how* you're adding multiple instances. How are you configuring just one of them to get the dropbox?

P.S.: Hehe. Yes. It's very complex. It's the simple UI that matters! But it of course needs to *work* first…

The same field should get the same widget. However, you seem to be using different fields. Correct?

This is simply not supported yet. Currently, only one configuration is supported per vocabulary. This is fairly easy to add though. Patch is welcome.

I wrote a quick patch for this issue -- I am on a windows eclipse system so please someone clean up patch as needed. All I am doing on the patch is changing the id to a unique id per field (from taxonomy - vid to taxonomy - vid - fieldid). I did also find an issue with how unlimited is handled, shouldn't the drop box be enabled by default when someone chooses unlimited, or am I missing something? Can't wait for hs4. Willing to help.

patch worked like a charm axiom3279 Thanks!

I am trying to do the same (have several HS configs for the same vocab). I will try to "hack" this with a feature and see what hapens, but is this ported to dev version or going to be at some point?
Now that we have a patch!

Same problem.

I'm have two different fields on two different node types, but are using a common Taxonomy.

axiom3279's method of appending the file id worked for me too. I can now set and save two separate widget configurations. Thanks!

Hi

this is a multiply requested issues and quiet an annoyance using HS.

The patch has some coding style issues that should be fixed first. Please rework it for the latest dev.

And in the long run imho we shouldn't use variables for the settings but put it in field API.

I have created a new patch which hopefully resolves this problem against the latest dev build using the same principle detailed in #5 but using machine_names instead of ids. This is due to features related issues and follows on from, and builds on, the patch in https://www.drupal.org/node/1477292#comment-7893157.

I agree that these settings could probably move out of variables altogether but given the work already carried out in the features machine_name patch I have continued on that line for now. Hopefully this patch will help someone else out.

Patch is fine, it works well. Though it still need to be reworked against latest dev.

Btw shoudln't we rework update function?

I guess we should force this issue - it is VERY important.
I've rerolled patch against the latest alpha12, it successfully work on my install for a while. Please review.

Looks good but doesn't this needs an update hook for backwards compatibility?

Also comments should start with an upper-case letter and end with a full stop (as well as sticking to 80 characters per line).

Yes, will improve comments and make update function.

Improved comments and introduced update function. Update function was tested as a separated code, not as an update function, so needs community's review.

1. +++ b/includes/common.inc
   @@ -12,7 +12,7 @@
   - *   "taxonomy-vocab_machine_name".
   

   "taxonomy-vocab_machine_name-field_name" is not a very good example of "module-someid" as someid has a dash in it. Maybe use an example from another submodule?

2. +++ b/modules/hs_taxonomy.module
   @@ -170,7 +168,7 @@ function hs_taxonomy_form_taxonomy_form_term_alter(&$form, &$form_state) {
   -    'level_labels'    => $vocabulary_config['level_labels'],
   +    'level_labels'    => array('status' => FALSE, 'labels' => array()),
   

   Just to be clear, why was this replaced?

3. +++ b/modules/hs_taxonomy.module
   @@ -1097,16 +1105,67 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -
   + */
   +function hs_taxonomy_get_config_id(stdClass $vocabulary, $field_name) {
   +  return "taxonomy-{$vocabulary->machine_name}-{$field_name}";
   +}
   +
   

   If it is possible for the machine name/field names to have dashes, the separator here may have to be a double dash in order to prevent duplicate config IDs?

1. Agree. I just use only hs_taxonomy, but will look for an another example.
2. Earlier we had one config per vocab. Thus when we added HS functionality to "Parent" field in taxonomy term edit form, we could use vocab config for it. Actually only level labels were used. New concept is one config per field = multiple configs per vocab. Thus we just use default empty settings, that's why I've replaced this.
3. Only underscore is allowed for fieldnames. Same for vocabulary name.

Cool I'll be happy to commit this once this has had some further review.

What makes you think the update hook would be problematic as an update hook as opposed to as a separate function? :)

I don't think it WOULD be problematic, I just think it CAN be problematic because I just don't have old-style configs on my install. Although code is quite simple and clear, I would be happy if smbd can test it on machine where there are old-style configs and report about results.

Patch on #17 works for me. Many thanks!

I updated from alpha9 to 7.x-3.0-alpha12+1-dev with patch #17 of this issue applied (I also applied patch 6 on issue 1169486). I executed update.php and all went ok. I just lost one setting from my old HS configuration: The "Enable the dropbox" checkbox was checked before update and now is unchecked.

@stefan.r Just took a look into the code and found only 2 types of config id: taxonomy-vocab_id-fieldname and taxonomy-views-viewname-displayid-filterid (which is even worse for our case). Thus I suggest just to simplify that comment guessing that developers are smart enough and can find examples of config ids when implementing their sub-modules. Patch is attached.

@rcodina Thanks for testing and reviewing, but strange! I do nothing with configs, just getting it and saving under new id.

I guess the patch needs some more testing after #22, so problem described there should be either confirmed or disproved.

Yes, losing settings would be problematic!

@rcodina can you debug the update hook and see how that setting got lost?

@rcodina did you have multiple allowed vocabularies on any fields? It seems the module will get the settings from the "first defined" vocabulary in the allowed vocabularies (with index 0), whereas the update hooks will loop through all the allowed vocabularies for a specific term reference field and mistakenly attempt to get the settings from every of the vocabulary (instead of only from the first one).

@Georgique so in the update hook, instead of doing $vocabulary_name = $allowed_value['vocabulary'];
it should likely do $vocabulary_name = $allowed_values[0]['vocabulary'];

Further, not to complicate things to much, we should probably should stick to defining settings on just a per-field level (instead of a combination of field and term). That way the config ID would become just taxonomy-{$fieldname} instead of taxonomy-{$vocabulary_name}-{$fieldname}. This would also solve the issue in #2393341: Hierarchical Select Taxonomy can break node/field/widget edit pages for term references with nonexistent/undefined allowed vocabularies (which occurs when there is no existing allowed vocabulary defined for a field). That issue is critical, and as this issue is now a dependency, I am upgrading the priority of the current issue.

I believe we should delete configs with old ID. Since the code is committed and new version was released, I suggest to create one more update hook cleaning up old configs.

Yes, I agree. I took out the config deletion so we could still recover the old configs in a next release in case there were still any problems with the update hook.

If in a couple of weeks no-one reports any issues, we'll add an update hooks that removes the old configs.

I see. Well, for me update hook works well, if there are no new issues about update hook, we can apply this patch.

We'd want to delete "taxonomy-{$vocabulary_name}", not "taxonomy-{$vocabulary_name}-{$field_name}" right?

The latter would only apply to those who have any of the old patches (before #27) in this issue, not to any dev versions or releases.

Oops, sure. Here is right patch.

Fixed comment.

OK let's leave this for a few weeks until we confirm there are no problems with people losing their config in the previous update hook (from alpha13).

Lowering priority as this is just about an update hook anymore.

Added update hook.