Disclaimer: this is definitely an obscure bug, I hope this report makes sense. ;)
As you probably know, I'm working on a project (with Earl) that's making heavy use of field_collection (yay!). I've been trying to come to grips with all the field_collection-specific menu items available on the site, since not all users should be able to access all of these menu items directly. I've been studying field_collection_item_access() and I think I see the best way to solve those problems.
However, in testing this stuff and poking around, I noticed something fairly weird about these menu items. For example, on my dev site, field_collection entity #101 is a "field_activity_log" field_collection item attached to uid #28. This user can access this entity directly at:
field-collection/field-activity-log/101
However, it also works at any URL where the 2nd item in the path happens to be a valid field_collection field name on the site. For example, all of these also work:
field-collection/field-messages/101
field-collection/field-employment-history/101
field-collection/.../101
But, if you use a 2nd path element that's not a field_collection field name, you get a 404:
field-collection/field-foo/101
The reason is that you're registering every possible field_collection field name as a separate menu item in field_collection_menu():
foreach (field_info_fields() as $field) {
if ($field['type'] == 'field_collection') {
$path = field_collection_field_get_path($field);
$count = count(explode('/', $path));
$items[$path . '/%field_collection_item'] = array(
'page callback' => 'field_collection_item_page_view',
'page arguments' => array($count),
'access callback' => 'field_collection_item_access',
'access arguments' => array('view', $count),
'file' => 'field_collection.pages.inc',
);
...
However, the access callback doesn't know anything about the field name. You only pass the operation and the entity ID. Furthermore, the menu item loader function doesn't care about the field name, either:
function field_collection_item_load($item_id, $reset = FALSE) {
$result = field_collection_item_load_multiple(array($item_id), array(), $reset);
return $result ? reset($result) : FALSE;
}
Given all of the above, I have no idea why you even bother with the complication of trying to embed the field name in these menu paths. Nothing actually enforces that the field collection entity matches the field names in the paths. It'd probably be simpler in a lot of places if you just went with paths like these:
field-collection/101
field-collection/101/edit
field-collection/101/delete
...
I don't really see what embedding the host entity field name buys you, and how it works now gives you a bunch of additional menu items with duplicate content. Makes it harder to lock this stuff down, adds needless work for the menu router, etc.
Thoughts?
Thanks!
-Derek
p.s. Now that I'm done writing this issue, a better title might be "Embedding host entity field names in field_collection entity menu paths is a waste of effort" or something. ;)
Comment | File | Size | Author |
---|---|---|---|
#2 | field-collection_alter_menu_items-1388156-2.patch | 3.54 KB | 30equals |
Comments
Comment #1
30equals CreditAttribution: 30equals commentedI was having some issues today as wel because of this. For example, the registration module adds some menu items based on a pattern 'entitytype/entityid' - i don't know for certain to what extend other modules rely on this pattern, but it seems to me that i would be a logical path to apply. eg. 'field-collection-item/fieldcollectionid'.
Also, the basepath for the field collections are constructed with the following function
But there's no way to set the path yourself right ? (Or maybe i looked over something), so i'm not sure what the use of the function is...?
Anyway, i'm with dww here ;)
Comment #2
30equals CreditAttribution: 30equals commentedSoooo, i haven't looked into it in detail, but what i'm suggesting is something like what's changed in the attached patch.
Comment #4
jmuzz CreditAttribution: jmuzz commentedI like this idea but it wouldn't be good to change paths that people may be relying on now for their site functionality.
Comment #5
jmuzz CreditAttribution: jmuzz commentedI have added this issue to #2444471: [Meta] Field collection 7.x-2.x branch.