If you attempt to view revisions on a content type that doesn't have WM enabled you get this error.

[12-Mar-2011 07:17:36] PHP Fatal error: Call to undefined function node_revision_overview() in /ld/www/htdocs/sys/sops/sites/all/modules/workbench_moderation/workbench_moderation.module on line 164

Rather odd, as it's in core.

CommentFileSizeAuthor
#1 issue1089834-r1.patch440 bytesmechler
#1 issue1089834-r2.patch686 bytesmechler

Comments

mechler’s picture

StatusFileSize
new686 bytes
new440 bytes

I have two imperfect ways to fix this, and the real solution is unclear to me. While I keep digging, here are the two methods that at least get you back to working without diff.

Note: The only use cases that I'm exploring here involve workbench_moderation NOT being used to manage the content type. With workbench_moderation enabled for the content type, we never get this error.

issue1089834-r1.patch

diff --git a/workbench_moderation.module b/workbench_moderation.module
index ed6e290..670ca6b 100644
--- a/workbench_moderation.module
+++ b/workbench_moderation.module
@@ -161,6 +161,7 @@ function workbench_moderation_node_revisions_redirect($node) {
   }
   // Return the normal node revisions page for unmoderated types.
   else {
+    module_load_include('inc', 'node', 'node.pages');
     return node_revision_overview($node);
   }
 }

Here we include the /modules/node.pages.inc file because node's hook_menu() implementation doesn't specify a 'file_path' because the path for where it's going to find it's 'file' is implicit. Changing the 'page callback' in workbench_moderation's hook_menu_alter() implementation means that we lose track of where the files are that were previously going to be included.

This has the benefit of never providing an error. The problem -- aside from inelegance -- is that while it doesn't cause an error, it also breaks diff so that it never loads the diff_diffs_overview() but instead just keeps using core's revisioning system.

issue1089834-r2.patch

diff --git a/workbench_moderation.module b/workbench_moderation.module
index ed6e290..4e3cc51 100644
--- a/workbench_moderation.module
+++ b/workbench_moderation.module
@@ -127,6 +127,7 @@ function workbench_moderation_menu_alter(&$items) {
   // Redirect node/%node/revisions
   $items['node/%node/revisions']['page callback'] = 'workbench_moderation_node_revisions_redirect';
   $items['node/%node/revisions']['page arguments'] = array(1);
+  $items['node/%node/revisions']['file path'] = drupal_get_path('module', 'node');
 
   // For revert operations, use our own access check.
   $items['node/%node/revisions/%/revert']['access callback'] = '_workbench_moderation_revert_access';

The alternative method is to explicitly tell the menu_router_item where to be looking for node.pages.inc (/modules/node). The problem here is that the diff module breaks and spits out an error when you try to view the revisions with the diff module enabled. The reason for this is similar. We tell the router item to include 'node.pages.inc' and (with this patch) where to find it (drupal_get_path('module', 'node')). The diff module's hook_menu() and hook_menu_alter() implementation includes a different file and page callback that their file creates but doesn't explicitly specify the file path. For the same reason that the node module loses track of where the include will be, diff loses track as well. This could be solved by similarly patching diff to explicitly define 'file path' in its hook_menu and hook_menu_alter().

The Upshot

Ultimately I'm not sure what is supposed to be done about this. The implicit directory for including files when doing a hook_menu or hook_menu_alter implementation is handy and all, but I don't understand the way around it here without submitting patches for every module that explicitly state the 'file path' in each menu item.

There might be a better way to solve this that involves not altering the 'page callback' for 'node/%node/revisions'. Doing that would require the menu to be rebuilt all the time, though, so that seems ridiculous, too. Maybe the 'node/%node/moderation' is just the wrong route to go and we should be hooking the forms that 'node/%node/revisions' generate instead. I'm unsure and don't fully understand how the modules are supposed to stay out of each others' way.

dave reid’s picture

Status: Active » Postponed (maintainer needs more info)

I can't duplicate this. Drupal by default saves drupal_get_path('module', $item['module']) into the 'include_path' property which is saved into the menu router.

mechler’s picture

To duplicate:

  1. Make sure that each of the following are enabled:
    • workbench
    • workbench_moderate
  2. Enable revisions for content type "page" via admin/structure/types/manage/page, but don't enable moderation:
    1. Under the "Publishing options" section, check the box next to "Create new revision."
    2. Make sure the the box next to "Enable moderation of revisions" is not checked.
    3. Click the "Save content type" button at the bottom of the form.
  3. Visit any content of type "page" and click on the "Revisions" tab.
    • You need to make sure that the node has multiple revisions in order to see this tab. You can do this by editing the node after saving it, and saving it with changes.
    • If you know the nid of the node of type "page" that you are using to test a revision, you can visit node/%nid/revisions instead.

To further clarify: the function node_revision_overview() from the node module is not able to be found because the menu item's "file path" isn't included when we change the menu item with hook_menu_alter(). Because the .module file making the call is working from a different directory, it can't find the file implicitly. One of the suggested fixes above includes this file as required, but then when the diff module implements hook_menu_alter(), now our "file path" parameter is in place instead of their and their diff_diffs_overview() function isn't found.

stevector’s picture

Status: Postponed (maintainer needs more info) » Active

I also cannot replicate this with your instructions mechler.

In #3 you say

To further clarify: the function node_revision_overview() from the node module is not able to be found because the menu item's "file path" isn't included when we change the menu item with hook_menu_alter().

However I am not finding this to be true. In this implementation of hook_menu_alter, the 'file' attribute of the 'node/%node/revisions' array is not touched. If you print out this array you should see


Array
(
[title] => Revisions
[page callback] => workbench_moderation_node_revisions_redirect
[page arguments] => Array
(
[0] => 1
)
[access callback] => _node_revision_access
[access arguments] => Array
(
[0] => 1
)
[weight] => 2
[type] => 132
[file] => node.pages.inc
[module] => node
)

That being said you and binford2k did find this error. Perhaps the safest option is to actually unset the the 'file' in hook_menu_alter and then use the include technique you layout in the first patch in #1. Thanks for reporting this and for your thorough explanation.

stevector’s picture

Status: Active » Closed (duplicate)

OK, so I'm able to reproduce this error, but only when I have Diff enabled. Since we have another issue specifically about Diff I am going to close this ticket and we can move forward with an integration plan over there:

#1083720: Incorporate Diff to Compare Revisions

technikh’s picture

Version: 7.x-1.0-beta5 » 7.x-1.3
Issue summary: View changes
Status: Closed (duplicate) » Active

the other issue https://drupal.org/node/1083720 is marked fixed but I still get the error
PHP Fatal error: Call to undefined function diff_diffs_overview() in /*********/modules/workbench_moderation/workbench_moderation.module on line 230,

Workbench Moderation 7.x-1.3
Diff 7.x-3.2