Per #2267251-10: Do not rely on "content language" in hook_entity_presave(), the Title module patch in that issue requires that code doing a programmatic entity save and changing the entity title needs to call the Entity Translation handler's setFormLanguage() method before saving.

Since Drafty does a programmatic entity save, it definitely needs to call this method. (In theory it could check if the save is changing the title, and only call the method if it is, but it seems cleaner and less prone to future bugs to just set it always.)

We are running that Title module patch on a site and have seen major bugs as a result (if you edit and save an entity translation twice in a CPS changeset, then after the second save, the translated title gets saved to the English version of the entity, on the live site!). This patch fixes the problem.

This issue is related to #2395793: Remove title module special-casing but not the same thing since it's not clear the old code can be removed yet, even once the above-referenced Title module patch is committed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Status: Active » Needs review
FileSize
1.49 KB

Here is the patch.

catch’s picture

Issue tags: +Needs tests

If #2395793: Remove title module special-casing passes tests with #2267251: Do not rely on "content language" in hook_entity_presave() applied to entity_translation, but we still need to patch for this case, that suggests a hole in the test coverage.

DamienMcKenna’s picture

Status: Needs review » Needs work
Pol’s picture

Hello,

The Title module will get rid of title_active_language() function used in Drafty and replace that using the Entity Translation Handlers properly. (See #2882888: Use new Entity Translation API).

I see that the return value of that function is not even used in Drafty.

Should we get rid of the line calling the function ? Or something else ?

Pol’s picture

Status: Needs work » Needs review
FileSize
1.68 KB

Here's an updated patch, working with Title + patch from #2267251: Do not rely on "content language" in hook_entity_presave().

Status: Needs review » Needs work

The last submitted patch, 5: 2487013.patch, failed testing.

joseph.olstad’s picture

DamienMcKenna’s picture

Thanks for working on this, @joseph.olstad.

joseph.olstad’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2487013.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

DamienMcKenna’s picture

So the tests won't work because #2267251 hasn't been committed. *sigh*

DamienMcKenna’s picture

richardcanoe’s picture

Status: Needs review » Needs work

The last submitted patch, 13: title-module-fix-2487013-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.