translation_node_insert() updates the node table directly without also flushing the entity load cache

Linked to example of persistent entity load cache

Are you happy to make this RTBC, or do you want additional eyes on it? (noting that it took more than a year to get your own response).

If you want more review from others, perhaps adding a note to that effect to your Related Issues would be of benefit?

FWIW I wrote the patch, and have been using it in a production environment ever since.

For testing in SimplyTest.me, you only need to add entitycache. You don't need the i18n or variable modules.

Steps to reproduce/demonstrate (see also the original description for this issue).

* Enable core Content Translation (and Locale dependency), and contrib Entity Cache modules.
* Add a language: admin/config/regional/language/add
* Edit a content type: admin/structure/types/manage/page and under Publishing settings choose "Enabled, with translation".
* Add a page of that type: node/add/page, selecting a language.
* Go to the translate tab, and add a translation for the other language.
* Visit the translate tab for the new translation. You should see both nodes.
* Visit the original node, and then go to its translate tab.

Without the patch: The translation node is not shown; and if you now edit and save the original node (thus updating the database with the incorrect state) the translation relationship is permanently lost -- neither node knows about the other one.

With the patch: The translation node is shown, and the nodes can safely be edited and saved.

I should really have mentioned how to observe the other fixes as well. It can be beneficial to add & enable the devel module as well, so that you can take a look at the tnid properties of the nodes.

In each case start with a translated node, clear the caches (admin/config/development/performance) and then view all nodes to ensure they are correctly loaded in entitycache.

1. Edit the source node and attempt to "Flag translations as outdated" (under the Translation settings fieldset).

Without the patch, this will have no effect on the translations, until the cache is cleared.

2. Delete the source node, and check the translations tab for one of the remaining nodes in the set.

Without the patch, there will now be no "Source" node in the translation set (as all the nodes will still have a tnid set to the nid of the deleted source node).

(As before, editing and saving one of the inconsistent nodes afterwards will make the incorrect state permanent.)

(The following has now been logged as #2367939: Translation sets should be 'removed' when they contain only one node).

As it happens, I've spotted another core bug while writing this. translation_remove_from_set() runs during hook_node_delete() which is before the node is removed from the database. That means that the "there is only one node left in the set" case will never occur (unless we are deleting the only node left in the set) because the COUNT query result will include the node being deleted. I'm 99% certain that the intention here was that if you reduce the set to a single node, it will revert to a tnid of zero (which is the same state as when you initially create a new un-translated node).

n.b. The code for choosing a new translation node in the other branch is still okay, because in that case the db_update() query has set the tnid of the node being deleted to zero before it queries for other nodes with the original tnid, so that query doesn't include the node being deleted.

I don't know whether to add a fix for that into this patch. I think I'd rather make that a separate issue. Perhaps if this one gets committed, we could then quickly roll a patch for this other issue and get it reviewed as well.

That seemed random. Is the testbot now automatically re-testing RTBC patches after core commits?

I'm assuming a false-positive, but I may have requested the re-test too soon to get a different result.

What the heck is going on with the test bot? This is really annoying. It's hard enough to get Drupal 7 bugs looked at, without RTBC patches being arbitrarily marked as failures like this.

Fixing the status.

Anyone know offhand if there's an existing d.o issue for this testbot problem?

Much appreciated, dcam.

Thanks David.

I've logged the related issue as #2367939: Translation sets should be 'removed' when they contain only one node.