admin/config/content/hierarchical_select/configs generates incorrect links (should use field_info_bundles())

Hurray! Thanks to Field API, at least this compatibility nightmare is over :)

Additional info about this was posted at http://drupal.org/node/1068070#comment-4838856.

Here's what I said in the other issue:

As for the issue of the missing paths ("export" and "edit" on the config screen), it looks like it is because we have an Activity entity type which has a different admin path than what is assumed by the HS module: /admin/structure/types/manage/activity/fields/field_pubtype/widget-type is wrong, /admin/structure/event/activity/manage/fields/field_pubtype/widget-type is right. HS should look at the hook_entity_info() implementations when building the paths in hook_menu(). But that is a separate issue, so I will post it as such.

I was doing it almost correctly. Just a teeny tiny mistake!

From
'edit link' => "admin/structure/types/manage/$bundle/fields/$field_name/widget-type",
to
'edit link' => $bundles_info[$bundle]['admin']['real path'] . "/fields/$field_name/widget-type",
because the types part in the first URL is actually dependent on the entity type, and I didn't anticipate other entity types than "node" yet. Apparently HS works just fine with Profile 2 etc. This was a hell in D5/D6 due to content_taxonomy and poor AHAH/AJAX support in FAPI. This is now a thing of the past. HURRAY!

http://drupalcode.org/project/hierarchical_select.git/commit/7a98093

Discovered that this can still be an issue (sorry). We have a custom entity type that has no $bundles_info[$bundle]['admin']['real path'], but only has a $bundles_info[$bundle]['admin']['path'], since there is only one bundle type for the entity type.

It seems to me like the solution would be to check if there is a $bundles_info[$bundle]['admin']['real path'] set, and if not, to use the $bundles_info[$bundle]['admin']['path'].

Admittedly, this may be an edge case, but I'm not sure how other people have implemented the entity API - might be more common than just on our site.

I was under the impression that 'real path' would be generated by the Entity API… But apparently that's not the case.

In any case, it's good practice to mirror core's behavior, so I'm tempted to consider this a bug in your code. Look at node_entity_info() in modules/node.module. Mirror that.

Or am I off here?

Any news?

We don't use the Entity API module for this entity; we just defined the entity in a hook_entity_info() function and didn't set a 'real path'.

I think that you are probably right about mirroring core's behavior. I will consult with Brian about this.

Just taking a quick look at the API, in the hook_entity_info() it says this about the path property:

'path: the path of the bundle's main administration page, as defined in hook_menu(). If the path includes a placeholder for the bundle, the 'bundle argument' and 'real path' keys below are required.'

I assumed the converse - since I am not using a bundle placeholder, the 'bundle argument' and 'real path' keys are not necessary.

While I can certainly add the 'real path' property to my hook_entity_info(), the docs don't seem to require it, and therefore there could very well other implementations that don't use it. Perhaps HS should account for that possibility in general usage?

Okay, so you don't have something like
'path' => 'admin/structure/types/manage/%node_type',
but you do have something like
'path' => 'admin/structure/types/manage/this-is-not-a-placeholder',
— correct?

If this is the case, then I could just check if the path contains a percentage sign, and if it doesn't, I can just use path instead of real path.

P.S.: this all smells like overengineering. Why doesn't core just make it simple? Now I have to check for the optional edge case where path does not contain a bundle placeholder…

Your statement about our setup is correct - we have a path that doesn't have a placeholder, since the placeholder would be for a bundle.

My suggestion had been something like

$path = isset($bundles_info[$bundle]['admin']['real path']) ? $bundles_info[$bundle]['admin']['real path'] . "/fields/$field_name/widget-type" : $bundles_info[$bundle]['admin']['path'] . "/fields/$field_name/widget-type"

However, you're right that this might not cover all edge cases, so you might have to actually check for a percent sign in path.

I, too, think this is awkward. I'm not a big fan of the way entities were designed in D7.

Your approach seems subtly better to me, actually :) I committed it: http://drupalcode.org/project/hierarchical_select.git/commit/77f0adf.

Hopefully this marks the final fix for this tiny yet surprisingly resilient issue :)