field_views_data() merges in all field data, but not recursively.
Image module's views implementation uses hook_field_views_data_views_data_alter() to add reverse relationships, which is fine because it is operating on a table it doesn't define.
Field Collection's views implementation uses image as a guideline, but the alter is operating on our own table, so this could theoretically be done in the original hook itself, not the alter, which would let other modules more reliably alter it, without worrying about weights.
The current field_collection_field_views_data() adds information about each field to $data, while the alter adds info to $data['field_collection_item']. If the two functions are combined, the value of $data['field_collection_item'] is overwritten by each call to the alter.
Because of the nature of the data being provided, I don't believe the change to array_merge_recursive will cause any problems. Patch forthcoming.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | views-1291088-9.patch | 745 bytes | tim.plunkett |
| #6 | views-1291088-6.patch | 746 bytes | tim.plunkett |
| #1 | views-1291088-1.patch | 744 bytes | tim.plunkett |
Comments
Comment #1
tim.plunkettSee attached.
Comment #2
fagosub
yep, imho views should follow module_invoke_all() here.
Comment #3
merlinofchaos commentedarray_merge_recursive() can cause arrays to nest too deep if the keys are the same, however, and that can break the data. I don't trust that.
Comment #4
fagoyep, that's a problem module_invoke_all() faces too, see #791860: array_merge_recursive() is never what we want in Drupal: add a drupal_array_merge_recursive() function instead.
While it's not ideal, I think it makes sense to follow core + it would allow views to avoid implementing its own alter hook.
Comment #5
tim.plunkettI didn't realize http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal... actually existed.
It still carries the problem of later hooks overwriting earlier ones, but not in a broken way. Not sure if this is more acceptable.
Comment #6
tim.plunkettReroll with drupal_array_merge_deep().
Comment #7
tim.plunkettShould the order of $data and $result be switched? So that $result can only add to $data, not overwrite it?
Comment #8
fagoI think so. It would be more natural if the first one wins in case of "conflicts".
Comment #9
tim.plunkettI thought I'd rerolled this, sorry for the delay.
Comment #10
dawehnerFrom my perspective this looks fine.
Comment #11
dawehnerThanks commited to 7.x-3.x