When a node translation set contains 2 items, and one of those has been deleted, the tnid of the remaining node is does not updated to zero, because the node_delete_multiple() call the hooks before delete the records from the table {node} and {node_revision}, etc.
Therefore record counting in the function translation_remove_from_set() is not accurate.

if (db_query('SELECT COUNT(*) FROM {node} WHERE tnid = :tnid', array(':tnid' => $node->tnid))->fetchField() == 1) {
  // …
}

When $node->tnid is not empty, the result will be equal to 2 or higher.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sweetchuck’s picture

Status: Active » Needs review
FileSize
752 bytes

Hasty fix

Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/modules/translation/translation.moduleundefined
@@ -420,7 +420,7 @@ function translation_remove_from_set($node) {
-    if (db_query('SELECT COUNT(*) FROM {node} WHERE tnid = :tnid', array(':tnid' => $node->tnid))->fetchField() == 1) {
+    if (db_query('SELECT COUNT(*) FROM {node} WHERE tnid = :tnid AND nid <> :nid', array(':tnid' => $node->tnid, ':nid' => $node->nid))->fetchField() == 1) {
       // There is only one node left in the set: remove the set altogether.

The comment under the query indicates the intention of the query was to include the node itself.

Gábor Hojtsy’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +D8MI, +language-content

Also most probably applies to Drupal 8, should be fixed there first, then backported to Drupal 7 (and possibly to Drupal 6 if applies there).

Sweetchuck’s picture

FileSize
772 bytes

Same patch for D8. Without tests :-(

Gábor Hojtsy’s picture

Status: Needs work » Needs review

Sending for testing anyway. How come existing tests do not cover this?

Sweetchuck’s picture

FileSize
4.36 KB

Still needs work on this patch. To translate the interface strings and maybe more.
And maybe the whole test could be part of the TranslationTest::testContentTranslation().

Gábor Hojtsy’s picture

Status: Needs review » Needs work

Looks generally like a good direction. I think its fine to keep it as its own test.

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.phpundefined
@@ -242,6 +242,58 @@ class TranslationTest extends WebTestBase {
+   *
+   * @todo Improve and enclose in t() the messages.

Test assertion messages should not be t()-ed anymore in Drupal. You should skip doing this.

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.phpundefined
@@ -242,6 +242,58 @@ class TranslationTest extends WebTestBase {
+    // The translation source is not the english node,
+    // this require a proper tnid handling in createTranslation().

I don't understand this comment at all. Also, it does not wrap properly.

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.phpundefined
@@ -242,6 +242,58 @@ class TranslationTest extends WebTestBase {
+    // In this case maybe random which node was selected to translation source.
+    // @todo Write asserts to cover.

What is random?

Also, complete that todo :)

Gábor Hojtsy’s picture

Still interested in working on this?

Gábor Hojtsy’s picture

Related: #1645156: URL generation only works on port 80 when using domain based language negotation (that is about generation not detection). (Posted on the wrong issue).

lazysoundsystem’s picture

Fixed the comments as per #7. Appears that the @todo was already done.

lazysoundsystem’s picture

Removed an added t() function.

ygerasimov’s picture

Status: Needs work » Needs review

I have reviewed this patch. Lets wait for the bots tests.

Status: Needs review » Needs work

The last submitted patch, tnid_update-1688144-11.patch, failed testing.

lazysoundsystem’s picture

Status: Needs work » Needs review
FileSize
4.43 KB

okay - that last minor change was a little too eager. Here goes, with quotes.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

The latest patch looks good. Fix makes sense and it is thoroughly tested.

Gábor Hojtsy’s picture

Issue tags: +sprint

Adding sprint tag.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)

Did not have the backport tag, but need to be backported (http://drupal.org/node/1688144#comment-62426529).

penyaskito’s picture

Version: 7.x-dev » 8.x-dev
Priority: Normal » Major
Status: Patch (to be ported) » Needs work
Issue tags: +Needs backport to D7
penyaskito’s picture

FileSize
250 bytes

A proof.

penyaskito’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8.x-is-broken.patch, failed testing.

webchick’s picture

Priority: Major » Normal

Ok, confirmed this was breaking testbot, so I've rolled this patch back.

The problem (at least on localhost) was that it was still failing the final "Translation set is terminated." test introduced by this patch. Not sure if one of the other ones I committed today conflicted or what.

chx’s picture

Status: Needs work » Needs review
FileSize
4.41 KB

Try this.

Status: Needs review » Needs work

The last submitted patch, tnid_update-1688144-14.patch, failed testing.

Gábor Hojtsy’s picture

In other words (after discussion with @chx), this did not work anymore due to node_load() changes in http://drupal.org/node/1744046 (depreciated $conditions argument removed), which was committed before this one.

Brace yourself for more of such conflits as APIs change on an ever higher pace and developers find it harder and harder to keep up :/

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
4.4 KB

Did not apply, rerolled.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC then. Thanks @chx for your help!

catch’s picture

What about the existing data suffering from this bug? It should be able to provide an upgrade path to find the nodes that are in a translation set all of their own and mark those back to 0 no?

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs upgrade path

Yes, that makes sense.

Gábor Hojtsy’s picture

Issue tags: -D8MI, -sprint, -language-content

I don't think this makes sense to keep on the sprint, given that we target the removal of this making this patch be stripped out of Drupal 8 anyway. It was targeted at Drupal 8 purely as an application of process. If we need someone to spend more time on this (and people don't seem to be flocking here to do it), then it should not take away resources from the people who are trying to focus on meaningful D8MI contributions, which is what the D8MI and sprint tag combinations are for. Honestly at this point it is not easy to tell if this node-copy translation functionality will be removed for sure (still 87 days to go until feature freeze), so not turning this back to Drupal 7, but if that happens, this should be turned back there as being not applicable to Drupal 8. If that does not happen, we need to keep it on Drupal 8 and fix the bug there.

Removing D8MI tag as well in anticipation of the underlying functionality being removed from core.

maxtorete’s picture

Assigned: Unassigned » maxtorete
Issue tags: +D8MI, +sprint, +language-content

I'm working on the upgrade path.

Gábor Hojtsy’s picture

Issue tags: -D8MI, -sprint, -language-content

Cross-post.

maxtorete’s picture

Assigned: maxtorete » Unassigned
thekatic’s picture

D7 patch worked for me, but I was wondering why it's still not included in Drupal 7?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 7d7aa97 on 8.3.x
    Temporary rollback of #1688144 as it seems to be failing.
    
  • webchick committed d5702e7 on 8.3.x
    Issue #1688144 by Sweetchuck, lazysoundsystem: Fixed tnid does not...

  • webchick committed 7d7aa97 on 8.3.x
    Temporary rollback of #1688144 as it seems to be failing.
    
  • webchick committed d5702e7 on 8.3.x
    Issue #1688144 by Sweetchuck, lazysoundsystem: Fixed tnid does not...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 7d7aa97 on 8.4.x
    Temporary rollback of #1688144 as it seems to be failing.
    
  • webchick committed d5702e7 on 8.4.x
    Issue #1688144 by Sweetchuck, lazysoundsystem: Fixed tnid does not...

  • webchick committed 7d7aa97 on 8.4.x
    Temporary rollback of #1688144 as it seems to be failing.
    
  • webchick committed d5702e7 on 8.4.x
    Issue #1688144 by Sweetchuck, lazysoundsystem: Fixed tnid does not...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

  • webchick committed 7d7aa97 on 9.1.x
    Temporary rollback of #1688144 as it seems to be failing.
    
  • webchick committed d5702e7 on 9.1.x
    Issue #1688144 by Sweetchuck, lazysoundsystem: Fixed tnid does not...

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 7.x-dev
Component: translation.module » ajax system
Issue summary: View changes
Issue tags: +Bug Smash Initiative

Did some checking and this is no longer relevant for Drupal 9 but still is for Drupal 7. Moving back to the D7 issue queue.