Properly override the node/%node/translate menu router path

There should not be, if there is we must fix it. The intended behavior is that the translation overview page for any entity is provided only if the /translate path is not already "taken" by i18n.

Can you describe your setup and how to reproduce the issue? Is enabling ET + i18n enough or do you performed some particular configuration?

Module versions? ;)

I can confirm that enabling Translation Overview 7.x-2.0-beta1 is causing this problem, in my case.

Disabling this module does the translation possible again.
Nothing better when installing the 7.x-2.x-dev version !

I have this problem too. On the next page load I get the following error messages which I believe are related:

Warning: Missing argument 2 for entity_translation_overview() in entity_translation_overview() (line 55 of /home/mvc/Work/sprint/entity_translation/entity_translation.admin.inc).
Notice: Undefined variable: entity in entity_translation_overview() (line 57 of /home/mvc/Work/sprint/entity_translation/entity_translation.admin.inc).
Notice: Undefined variable: entity in entity_translation_overview() (line 68 of /home/mvc/Work/sprint/entity_translation/entity_translation.admin.inc).
Warning: Illegal offset type in isset or empty in entity_get_info() (line 7351 of /home/mvc/Work/sprint/drupal/includes/common.inc).
Warning: Illegal offset type in isset or empty in entity_get_info() (line 7351 of /home/mvc/Work/sprint/drupal/includes/common.inc).

Reproduced with:

Drupal core 7.8
Entity translation 7.x-1.0-alpha1
Internationalization 7.x-1.0
Multilingual content (must be enabled to reproduce this error)
Translation Overview 7.x-2.0-beta1
Variable 7.x-1.1

The error is a result of conflicts between i18n_node, entity_translation and translation_overview. All 3 modules are implementing hook_form_alter and modifying the entry for 'node/%node/translate'.

I believe this issue should be fixed by #1172530: No way to assign existing nodes as translations with i18n_node, thus closing this issue as a duplicate.

We may be able to fix this in entity_translation. Re-opening for further investigation.

Here's a first attempt at a patch which calls the appropriate callback after hook_menu and hook_menu_alter hooks run.

We may want to improve the filename handling. I'm not sure if the extension will always be .inc. Perhaps someone can comment.

Thanks!

+++ b/entity_translation.admin.inc
@@ -167,6 +160,17 @@ function entity_translation_overview($entity_type, $entity) {
+    $file_basename = basename($callback['file'], '.inc');
+    module_load_include('inc', $callback['module'], $file_basename);

I think these lines can be replaced with something like the following:

<?php
if (isset($callback['file'])) {
  $path = isset($callback['file path']) ? $callback['file path'] : drupal_get_path('module', $callback['module']);
  include_once DRUPAL_ROOT . '/' . $path . '/' . $callback['file'];
}
?>

See the related docs.

Thanks plach.

I've attached a second attempt.

+++ b/entity_translation.admin.inc
@@ -167,6 +160,22 @@ function entity_translation_overview($entity_type, $entity) {
+      $file = DRUPAL_ROOT . '/' . $path . '/' . $callback['file'];
+      if (is_file($file)) {
+        require_once $file;
+      }

I know this is how module_load_include(), does but I'd say this code triggers two I/O operations instead of one. What about the following?

<?php
@include_once DRUPAL_ROOT . '/' . $path . '/' . $callback['file'];
?>

After all avoiding to issue an error here is pretty useless since the execution is likely to terminate immediately after due to a fatal, since the page callback cannot be found.

Sounds good! I've attached another patch.

@djac:

Sorry for the delayed feedback, but this revealed trickier than what I originally thought to address: I tested your patch with core content translation and it appeared not to work. The main reason is that by implementing hook_menu() we are not able to backup an existing node/%node/translate router path, since there is no way to tell if it's there, and we simply override it with the ET router path. To address this problem I moved everything to hook_menu_alter(). Moreover access callbacks were not proxied.

I tested the attached patch with core content translation and it seems to behave well. Would you please test it with i18n_node and translation_overview and report?

Hello!
After applying the path against -dev version of Entity Translation, I received this error message:

Warning: require_once(/var/www/sky37.pp.ua/public_html//entity_translation.admin.inc) [function.require-once]: failed to open stream: No such file or directory in menu_execute_active_handler() (line 501 of /var/www/sky37.pp.ua/public_html/includes/menu.inc).

Obviously there should be public_html/SITES/ALL/MODULES/entity_translation.admin.inc, not public_html//entity_translation.admin.inc

@djac:

Aside from #22, do you have anything else to report?

Translation Overview 7.x-2.0-beta1 also caused the problem in my case

The patch missed one thing which is the 'module' key, otherwise drupal will assume the file is in drupal root dir. After applying the patch, no error messages anymore. Now let's see how testbot will react.

One little effect, after changing the way you do menus in this module, other modules depending on it, like Translation Overview, needs an update, otherwise it won't work after applying this patch.

Patch from #26 works fine for me. But I did not experienced any problems without it, so my result is not significant.

#26 seems to work for me, too.
Thanks

@good-man:

Thanks for working on this. The goal of the patch is to avoid the need for other modules to worry about ET, hence, unless I'm missing something, there should be no need to warn their maintainers.

I missed one 'module' key for the config page of ET (admin/config/regional/entity_translation) otherwise it'll display that fatal require_once error.

Re. Translation Overview, it's menu_alter() is executed before ET, therefore, ET will overwrite the previous TO path. Quoted from OT code:

function translation_overview_menu_alter(&$items) {
  // Replace the translation module's node tab with our own.
  // @todo this conflicts i18n_node and entity_translation

So I think that it's now up to them to solve this @todo.

The patch looks good, tests pass and I manually tested core node translation, i18n_node and translation overview successfully.

Thanks to everyone working on this! Committed to HEAD.

#31: et-1174242-31.patch queued for re-testing.

The patch has been committed. No point in retesting it.

Fatal error: Class name must be a valid object or a string in /home/prouser/drupal/sites/all/modules/entity_translation/entity_translation.module on line 495

with latest dev:
Warning: call_user_func_array() expects parameter 1 to be a valid callback, no array or string given in entity_translation_node_tab_access() (line 72 of \modules\entity_translation\entity_translation.node.inc).

with alpha-1:

Warning: Missing argument 2 for entity_translation_overview() in entity_translation_overview() (line 55 of /drupal/sites/all/modules/entity_translation/entity_translation.admin.inc).
Notice: Undefined variable: entity in entity_translation_overview() (line 57 of /drupal/sites/all/modules/entity_translation/entity_translation.admin.inc).
Notice: Undefined variable: entity in entity_translation_overview() (line 68 of /drupal/sites/all/modules/entity_translation/entity_translation.admin.inc).
Warning: Illegal offset type in isset or empty in entity_get_info() (line 7463 of /drupal/includes/common.inc).
Warning: Illegal offset type in isset or empty in entity_get_info() (line 7463 of /drupal/includes/common.inc).

Please open a new issue.

Was it ever opened?

Update: Component was changed automatically due to "Code" not being available anymore.

I've opened this: http://drupal.org/node/1541388

In my case, I got it working by enabling node entity in the config page: /admin/config/regional/entity_translation. For some reason taxonomy terms was the only entity type enabled. Version 7.x-1.0-alpha1
Thanks