restws_page_callback() breaks workbench_moderation

The 1.x branch is frozen and receives security updates only, please test against the 2.x branch.

The problem is that workbench moderation does not invoke the menu callback correctly, the page arguments are missing.

Not understanding the method that page arguments are passed between callbacks, I came up with the following code to solve _my_ problem between workbench_moderation and restws. in workbench_moderation.node.inc

function workbench_moderation_router_item_page_callback($node) {
  $router_item = menu_get_item('node/' . $node->nid);
  if ($router_item['include_file']) {
    require_once DRUPAL_ROOT . '/' . $router_item['include_file'];
  }

  // Call whatever function is assigned to the main node path but pass the
  // current node as an argument. This approach allows for the reuse of of Panel
  // definition acting on node/%node.
  return $router_item['page_callback']($node, 'node_page_view', $node);

}

Any idea if the suggested fix is likely to be confirmed as a potential valid solution? I'm currently working on a site where this is an issue but an awful lot of debugging is going to have to be done to ascertain whether we think this won't break or affect other things.

I am working with

• Drupal 7.28
• Workbench Moderation 7.x-1.3 (2013-Jan-18)
• Restws 7.x-2.1 (2013-Aug-07)

I tried:

1. updating to workbench_moderation 7.x-2.x-dev (2013-Oct-26), installing state_machine, state_flow with version 2.99+ as required, it didn't work, giving on a draft page call Fatal error: Call to undefined function state_flow_entity_load_state_machine() in state_flow/state_flow.module on line 78
2. 7.x-1.x-dev (2014-Jun-24): idem as above
3. updated restws to 7.x-2.x-dev (2014-Jun-15): no wanted results
4. applied the two restws patch posted above in #1, #6: no results

Then I built a patch according to suggestions in comment #8.
I performed some non exhaustive test on nodes and menu callbacks invoking, it works. Placing in review queue.

Works for us too - but we haven't performed exhaustive testing either.

Fixes it for me. Didn't perform any heavy testing.

menu.inc's menu_execute_active_handler does it this way: call_user_func_array($router_item['page_callback'], $router_item['page_arguments'])

I guess we should be as close as possible to that with the code in workbench_moderation_router_item_page_callback().

Does it still apply against v7.x-1.x-dev?

Hello Friends,

While working with restws module, I came across a issue of "Node view draft" page is showing blank.

And at the same time if I try to access some other page I got warning of "Illegal offset type in restws_get_resource_info()" as follow:

For fixing such problem I have modify patch "workbench_moderation-fix_callback_argument-1838640-10.patch" a bit.

And after applying my modified patch, my issue get resolved.

Below is my patch file:
workbench_moderation-node_view_draft_page_fix-for-restws-mp-1356.patch

Regards,
Rohit S

i just want to add that #1 also works for the 7.x-2.2 branch of the RESTful web services except the change needs to happen near line 372.

#16 worked for me on 7.x-2.3

I rerolled the patch in #16 to apply, the fix looks correct, and fixes the issues. This is the same of #10 (without an extra newline), and this should normally apply.

Not sure why #20 is failing, here it is re-rolled today.

aschmoe, my patch uses the string 'node' as the first argument of the callback. You're patch creates warnings on the draft pages from rest-ws not able to define the resources.
Let's test attached patch which uses the callback approach from menu.inc as suggested in #13. It actually passes the same params as in #20 ('string node', string 'node_page_view', and node object).

And this does indeed pass tests. Yes!

#23 worked nicely. Thank you. How do we go about putting this in a dev branch?

@znabavi, if this works you could set it to RTBC, and then one of the maintainers could commit this.

+1 tested and working here, please commit

Double confirming this fixes the issue.

Thanks @mallezie. This is fixing the issue for us.

I tried the patch in #23 and found that it broke the View links under the Moderate tab: they all show only the currently published revision rather than the expected previous revisions. Is anyone else seeing this behavior, or is it just me?

Everything works as expected after reverting to the current stable release (with Restful web services disabled).

#23 patch works with estws-7x-2.4 and workbench_moderation-7.x-1.4.

@daemonchrist

We are using the above versions, and the "View"-links under the Moderate tab are working fine.

Edit:
#23 does indeed break the "View"-links under Moderate tab. No matter which "View"-link is clicked, the same content is shown.

#10 do not have this behavior.

I've found that replacing the $router['page_arguments'][2] with the $node argument in the workbench_moderation_router_item_page_callback() function, solves the problem described by #32.

The node return by menu_get_item() are the published node. Hence the use of $router_item['page_arguments'] in the fixed return call, always yield the same node content.

Anyone care to comment on the legitimacy of this hack?

/**
 * Get the menu router item for nodes.
 *
 * @param $node
 *   The node being acted upon.
 * @return
 *   A fully themed node page.
 */
function workbench_moderation_router_item_page_callback($node) {
  $router_item = menu_get_item('node/' . $node->nid);
  if ($router_item['include_file']) {
    require_once DRUPAL_ROOT . '/' . $router_item['include_file'];
  }

  //replace published revision with the passed node
  $router_item['page_arguments'][2] = $node;

  // Call whatever function is assigned to the main node path but pass the
  // current node as an argument. This approach allows for the reuse of of Panel
  // definition acting on node/%node.
  return call_user_func_array($router_item['page_callback'], $router_item['page_arguments']);

}

This definitely breaks the 'View draft' functionality for nodes, you see the published version and not the actual draft because we're no longer actually passing $node object (which contains the draft version) into the page callback.

It's a shame that restws has to *prepend* it's page arguments instead of adding them on to the end, because it would be a lot easier to just have $page_arguments[0] = $node but we cannot rely on it being argument 0 with restws.

Since the previous patches broke functionality but passed tests, we need tests to cover this condition.

Dave - got a summary of the functionality it broke?

Committed the fix for now.

Ah, can you slow down a bit please, you are committing things that are in 'need work' and not updating their status after

No, I committed it from Needs Review after I've been personally using it on a couple sites in production, so I felt confident in it. I'm working on the tests for this, which is why I marked it to Need work after commit.

This should add test coverage for the fix, and with a partial revert of my fix shows that the new test fails.

Tests only this time. This should pass.

Committed the tests in #49 to 7.x-1.x.

Nice work! thanks!

Fix needs to be applied to the 7.x-2.x branch if we're going to keep it open.

It looks like both the fix and the test have been added to Workbench Moderation 7.x-3.x.

http://cgit.drupalcode.org/workbench_moderation/commit/?h=7.x-3.x&id=635...
http://cgit.drupalcode.org/workbench_moderation/commit/?h=7.x-3.x&id=ef4...