Problem/Motivation

ContentEntityStorageBase::createTranslation fires two hooks when a new translation is created: hook_field_values_init and hook_translation_create. hook_field_values_init is invoked with the new entity translation object, but the hook_translation_create is invoked with the previous entity object. Both hooks have been introduced in #2382675: hook_entity_create() affects the data of new translations of existing entities in unexpected and undocumented ways.

The entity API states that the new entity translation is passed, but it is not:

/**
 * Acts when creating a new entity translation.
 *
 * This hook runs after a new entity translation object has just been
 * instantiated.
 *
 * @param \Drupal\Core\Entity\EntityInterface $translation
 *   The entity object.
 *
 * @ingroup entity_crud
 * @see hook_ENTITY_TYPE_translation_create()
 */
function hook_entity_translation_create(\Drupal\Core\Entity\EntityInterface $translation) {
  \Drupal::logger('example')->info('Entity translation created: @label', ['@label' => $translation->label()]);
}

Proposed resolution

Call the hook_translation_create with the new entity translation object.

Remaining tasks

none

User interface changes

none

API changes

none

Data model changes

none

Comments

hchonov created an issue. See original summary.

hchonov’s picture

Status: Active » Needs review
StatusFileSize
new7.21 KB
hchonov’s picture

StatusFileSize
new7.92 KB

The failing test with the fix.

hchonov’s picture

Issue summary: View changes
hchonov’s picture

StatusFileSize
new588 bytes

Oh, I've forgot the interdiff....

The last submitted patch, 2: 2649602-2-failing_test.patch, failed testing.

gábor hojtsy’s picture

Issue tags: +sprint, +language-content

Question is if this would be a "backwards compatible change" in terms of people expecting the new functionality anyway, or not. It would be good to figure this out.

hchonov’s picture

Issue summary: View changes
hchonov’s picture

Issue summary: View changes
chx’s picture

The BC way is

-    $this->invokeHook('translation_create', $entity);
+    $this->invokeHook('translation_create', $entity, $translation);
chx’s picture

But of course if previous code was unusable then there's no point in keeping BC.

xjm’s picture

The D8 core committers all agreed that this sounds like a major bug. Thanks @hchonov for reporting this.

hchonov’s picture

Issue tags: +SprintWeekend2016

Adding the tag SprintWeekend2016 in hope someone will review it this sprint weekend.

gábor hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks like there is agreement in fixing it this way, the patch looks fine, so let's get this in!

  • catch committed b53d68d on 8.1.x
    Issue #2649602 by hchonov: hook_translation_create is not invoked with...
catch’s picture

This looks great to me.

It's 8.0.3 release day today, so it can't go in there today, also just in case it did break a contrib module, I think it's worth doing early in a patch release cycle than late.

So committed/pushed to 8.1.x, leaving RTBC for 8.0.x for when the branch is open again.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2649602-3.patch, failed testing.

gábor hojtsy’s picture

Testbot error with composer.

gábor hojtsy’s picture

Status: Needs work » Reviewed & tested by the community

  • catch committed 8b2fa59 on
    Issue #2649602 by hchonov: hook_translation_create is not invoked with...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Cherry-picked to 8.0.x, thanks!

xjm’s picture

Issue tags: +Triaged core major

Status: Fixed » Closed (fixed)

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

gábor hojtsy’s picture

Issue tags: -sprint

Yay!