Problem/Motivation

From #2940204-24: Translatable fields with synchronization enabled should behave as untranslatable fields with respect to pending revisions:

[...] it's the logic in EntityUntranslatableFieldsConstraintValidator that's buggy, I didn't realize that earlier because field widgets are hidden in that case, but here's an example of what might happen if widgets weren't hidden:

  1. Create an English article and publish it
  2. Create an Italian translation and publish it
  3. Save an English draft with untranslatable fields/synchronized property changes
  4. Save a new Italian default revision with untranslatable fields/synchronized property changes
  5. Publish the English draft -> you just overrode the Italian changes

Depending on the selected mode, you can only perform untranslatable field/sync property changes to the default revision XOR the default translation.

Proposed resolution

  • Make sure changes in non-default translations are not allowed, when only the default translation is affected by untranslatable field changes, regardless of the revision being pending or default.
  • Use different validation error messages for the two modes, as suggested by @Gábor Hojtsy.

Remaining tasks

  • Validate the proposed solution
  • Write a patch
  • Reviews

User interface changes

None

API changes

None, the affected validator was not released in any stable release yet.

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach created an issue. See original summary.

plach’s picture

The last submitted patch, 2: et-untranslatable_fields_fix-2941062-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 2: et-untranslatable_fields_fix-2941062-2.test.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

plach’s picture

Wrong @covers annotation, the failing patch is failing legitimately though.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

effulgentsia’s picture

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

Looks good. Pushed to 8.6.x. Leaving at RTBC for 8.5.x to discuss with other committers whether to allow this into either the RC phase or in a patch release.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: et-untranslatable_fields_fix-2941062-5.patch, failed testing. View results

plach’s picture

Status: Needs work » Reviewed & tested by the community
webchick’s picture

Talked about this on the core committer call, and there was agreement among Alex + Alex + plach to backport this to 8.5.0 prior to release, since it's fixing a bug that hasn't yet hit a tagged release.

plach’s picture

Assigned: plach » Unassigned

effulgentsia’s picture

Status: Reviewed & tested by the community » Fixed

since it's fixing a bug that hasn't yet hit a tagged release

Correction: hasn't yet hit a stable release. But the rest of #11 still holds, so cherry picked this to 8.5.x.

Status: Fixed » Closed (fixed)

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