Moderation-aware formatters for EntityReference fields

I like the idea of merging, but let's get the code in patch format first.

This has my vote.

@fgm, did you ever get a patch released?

hi
Please has this code been merged to 7.x-3.dev version so that patch could be applied easily ?

Thanks

I happen to be interested - my client has a sizeable D7 site/platform which could really do with this. I'll probably just use the patch for now anyway; though I found a tiny issue with the formatter settings form which I've fixed in the attached patch.

The previous patches here included an unnecessary whitespace change which makes it less compatible with various other patches / revisions, so here's a version that avoids that.

The patches provided so far have some serious performance issues, specifically because they use the workbench_moderation_node_live_load function which skips any sort of caching when loading the node. This function is essentially called for every single source node and (probably) most referenced nodes too. Expensive.

I've remove most of the usages of that function from this attached patch, essentially by checking what it was going to check directly, rather than using it as a 'helper', which it isn't really imho. For example, if the source node we're given is already the published revision, we can determine that and then use that exact object in our function, we don't need to needlessly load up the node again to get the exact same data.
Well, that's my thinking anyway, maybe someone could review and point out the flaw :)

There's also a couple of minor issues in the patch in #7 in that the way the assert is written is deprecated, and we can iterate over nodes in a more efficient way using node_load_multiple rather than node_load in a loop.

Good catch! This seems to work fine for my use case. This leaves us with two node_load_multiple() calls in workbench_moderation_field_formatter_prepare_view() - I would think that could be brought down to just one? We also have six (!) foreach loops - which isn't necessarily a problem - but I do wonder if the code could generally be simplified further.

Reducing the number of DB queries is going to be far more important than the number of loop iterations, but it's not an easy piece of code to grok. I do realise the current code is an evolution of the original entityreference_field_formatter_prepare_view(), but the complexity that has been added on top means it might be worth refactoring rather than trying to remain close to the originally-copied code.

That said, I'd still be delighted to see this merged in its current state anyway! Already-written, working code is much better than imaginary 'beautiful' code ;-)

Found a minor warning in the code for #8 fixed in the attached patch.

@james.williams I do agree that the code could be easier to grok/read/work with, but yes, don't let perfect be the enemy of good :)

I've found a minor performance optimization, in:

+++ b/workbench_moderation.module
@@ -142,6 +142,154 @@ function workbench_moderation_menu() {
+          $has_view_access = (entity_access('view', $target_type, $target_id) !== FALSE);
+          $has_update_access = (entity_access('update', $target_type, $target_id) !== FALSE);
+          $items[$nid][$delta]['access'] = ($has_view_access || $has_update_access);

We always compute the $has_update_access value, but actually, if $has_view_access == TRUE, the we don't need to.
If the update access is expensive to compute, then we can skip all of that by doing this:

$items[$nid][$delta]['access'] = (entity_access('view', $target_type, $target_id) !== FALSE) || (entity_access('update', $target_type, $target_id) !== FALSE);

Simple, and no functional change, except I suppose if the entity_access call has side-effects, but urgh, let's not go down that rabbit hole.