Problem/Motivation

As pointed out by @Gábor Hojtsy in #2837022-71: Concurrently editing two translations of a node may result in data loss for non-translatable fields the entity changed constraint validator doesn't take into account removed translations, which means that we might end up adding back a deleted translation if the entity object we hold is an obsolete one.

Proposed resolution

Iterate over all the translations of the current entity and ensure that either the translation is present on the unchanged entity or it is flagged as new on the current entity.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 2920889-11-on-top-of-2837022-69.patch12.17 KBhchonov
#11 interdiff-7-11.txt6.23 KBhchonov
#7 interdiff-2837022-69-2920889-7.txt3.21 KBhchonov
#7 2920889-7-on-top-of-2837022-69.patch8.12 KBhchonov
#7 2920889-7-on-top-of-2837022-69-test-only.patch7.89 KBhchonov
#5 2920889-5_with_2837022-69.patch7.36 KBhchonov
#3 2920889-3_with_2837022-69.patch1.94 KBhchonov
#3 2920889-3-standalone-do-not-test.patch1.94 KBhchonov

Comments

hchonov created an issue. See original summary.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented

This issue is a follow-up of #2837022: Concurrently editing two translations of a node may result in data loss for non-translatable fields, which is critical. I think that this is one is not less important, but I'll let other decide if it is critical as well.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
StatusFileSize
new 2920889-3-standalone-do-not-test.patch1.94 KB
new 2920889-3_with_2837022-69.patch1.94 KB

Here a standalone patch based on #2837022-69: Concurrently editing two translations of a node may result in data loss for non-translatable fields and a full patch containing the one from the other issue.

Status: Needs review » Needs work

The last submitted patch, 3: 2920889-3_with_2837022-69.patch, failed testing. View results

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Status: Needs work » Needs review
StatusFileSize
new 2920889-5_with_2837022-69.patch7.36 KB

Oups.. wrong diff ...

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Issue tags: +Needs tests
hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Issue tags: -Needs tests
StatusFileSize
new 2920889-7-on-top-of-2837022-69-test-only.patch7.89 KB
new 2920889-7-on-top-of-2837022-69.patch8.12 KB
new interdiff-2837022-69-2920889-7.txt3.21 KB

I've added test coverage.

The last submitted patch, 5: 2920889-5_with_2837022-69.patch, failed testing. View results

The last submitted patch, 7: 2920889-7-on-top-of-2837022-69-test-only.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 7: 2920889-7-on-top-of-2837022-69.patch, failed testing. View results

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-7-11.txt6.23 KB
new 2920889-11-on-top-of-2837022-69.patch12.17 KB

The reason for at least some of the test fails is that the default language code of the entity might be altered and the condition to detect a removed translation !$saved_entity->hasTranslation($saved_entity_langcode) && !$translation->isNewTranslation() is simply not sufficient. Also a bug that we already have is that we iterate over the common translation languages, but if the default language code has changed then we'll skip checking the default translation at all. To cover this case I've added the new method ContentEntityInterface::getOriginalDefaultLangcode().

By working on this I've noticed something that we've somehow missed in #2863336: Default revision flag doesn't propagate to all entity translation objects where we've introduced a general test for the cloning of each entity property - the defaultLangcode property is not shared between the translation objects. I've created a new issue for this as it is a problem on its own - #2922626: Default langcode property changes don't propagate to all entity translation objects

Status: Needs review » Needs work

The last submitted patch, 11: 2920889-11-on-top-of-2837022-69.patch, failed testing. View results

matsbla’s picture

matsbla commented

I found 1 issue which I think is maybe similar/related to this. When you add a new translation concurrently as another translation is edited, the EntityChangedConstraintValidator doesn't lock, so this, just as when you removed translations, will revert those changes. I'm not sure if that should be part of this issue or if I should make another issue for it?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

I'd say let's fix translations once and for all here, unless the fix ends up being completely different, which I doubt :)

matsbla’s picture

matsbla commented

The same issue goes for when you revert revisions and use "Revert content shared among translations", then there is also no lock and you will revert changes that were made concurrently on those field. I guess in that case there should maybe also be a lock?

matsbla’s picture

matsbla commented
Title: EntityChangedConstraintValidator doesn't take removed translations into account » EntityChangedConstraintValidator doesn't take adding, removing and reverting translations into account
Priority: Major » Critical

I agree with #2 that this also should be considered as a critical for same reasons as #2837022: Concurrently editing two translations of a node may result in data loss for non-translatable fields.

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

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.

matsbla’s picture

matsbla commented
Issue tags: +data integrity

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Closed (duplicate)
Related issues: +#3087086: [DATA LOSS] Concurrent translation editing lead to data loss

Marking #3087086: [DATA LOSS] Concurrent translation editing lead to data loss as duplicate.

catch’s picture

catch
he/him
Primary language English
commented
Status: Closed (duplicate) » Needs work

Whoops, didn't mean to change status.

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

part of the above patch was committed in #2837022: Concurrently editing two translations of a node may result in data loss for non-translatable fields

I started trying to reroll it until I found that part of this patch was already committed with #2837022: Concurrently editing two translations of a node may result in data loss for non-translatable fields

Then I realized that this issue isn't the issue that I'm trying to resolve

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#2892132: Entity validation does not always prevent concurrent editing of pending revisions

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

stephencamilo’s picture

stephencamilo commented
Status: Needs work » Closed (won't fix)
matsbla’s picture

matsbla commented
Status: Closed (won't fix) » Active

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.