At the moment entity_translation expects menu arguments to be a entity load token. Ex. "%node" vs "%".

In most cases this works great, but when attempting to integrate another module as ECK that does not declare a load callback for it's variable entity types then various entity_translate callbacks will attempt to make do with the id causing unexpected results.

I propose that when an id is passed to the handler that we go ahead and load the entity via entity_load and use a new method for the translation handler $handler->getEntity(); so that we don't have this issue when '%' is used over '%LOAD_CALLBACK'.

CommentFileSizeAuthor
#7 entity_translation-add_support_no_load_tokens-1866076-7.patch2.96 KBgrndlvl
#6 entity_translation-add_support_no_load_tokens-1866076-6.patch2.97 KBgrndlvl
#1 entity_translation-add_support_no_load_tokens-1866076-1.patch3.04 KBgrndlvl

Comments

grndlvl’s picture

grndlvl commented
Status: Active » Needs review
StatusFileSize
new entity_translation-add_support_no_load_tokens-1866076-1.patch3.04 KB
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Category: support » feature

Looks sensible to me, thanks.

+++ b/entity_translation.admin.inc
@@ -204,6 +204,10 @@ function entity_translation_overview($entity_type, $entity, $callback = NULL) {
+  // In some cases the entity passed as an argument is not an entity, however,
+  // the handler is smart enough to figure this out so let's just ask the handler
+  // for the usable entity.

Can we have a shorter comment, but everywhere we do this? Something like: "Ensure $entity holds an entity object and not an id."

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

#1: entity_translation-add_support_no_load_tokens-1866076-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, entity_translation-add_support_no_load_tokens-1866076-1.patch, failed testing.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

...coming from #1798646: Make ECK entities translatable (entity translation integration).

grndlvl’s picture

grndlvl commented
Status: Needs work » Needs review
StatusFileSize
new entity_translation-add_support_no_load_tokens-1866076-6.patch2.97 KB

Re-roll against 7.x-1.x

grndlvl’s picture

grndlvl commented
StatusFileSize
new entity_translation-add_support_no_load_tokens-1866076-7.patch2.96 KB

Adding changes mentioned in #2.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

Looks good to go to me. Can anyone confirm this is working with ECK?

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Needs review » Reviewed & tested by the community

Yes, we use it in combination with eck

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed, thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit a4475e5 on 7.x-1.x, factory, et-fc, revisions authored by grndlvl, committed by plach: 
    Issue #1866076 by grndlvl: Added support for non-entity load tokens when...

  • Commit a4475e5 on 7.x-1.x, factory, et-fc, revisions, workbench authored by grndlvl, committed by plach: 
    Issue #1866076 by grndlvl: Added support for non-entity load tokens when...