Problem/Motivation

Currently CEB::validate will validate only the current translation on which the ::validate() method is being called. Considering that it is called only in the FAPI and there we could edit only a single translation leads to the conclusion that it is fine to validate only a single translation instead of the whole entity and each of its translations ... but actually this is not an entirely correct statement, because while editing a single translation over the API we might edit field values of different translations as well by some form element which if changed will trigger changes in other translations.

An example from the core where multiple translations will be edited on entity form submit -
\Drupal\content_translation\ContentTranslationHandler::entityFormAlter adds the 'retranslate' form element and \Drupal\content_translation\ContentTranslationHandler::entityFormEntityBuild will set the field "content_translation_outdated" of all other translations but the current one to TRUE. Currently we will save all the translations without validating them!

This possibility to validate all entity translations is also required by #2927547: Add an option to show all validation failures even for fields not included in the form display.

Proposed resolution

Introduce a new method FieldableEntityInterface::validateAllTranslations(). By default let the content entity form validate only the current translation.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#16 2890405-16.patch6.36 KBhchonov
#12 interdiff-10-12.txt1.33 KBhchonov
#12 2890405-12.patch3.46 KBhchonov
#10 interdiff-7.10.txt1.78 KBhchonov
#10 2890405-10.patch3.47 KBhchonov
#7 interdiff-6-7.txt909 byteshchonov
#7 2890405-7.patch3.49 KBhchonov
#6 2890405-6.patch2.38 KBhchonov
#4 2890405-4-translation-errors.png42.79 KBhchonov
#4 2890405-4.patch2.28 KBhchonov
#2 2890405-2.patch948 byteshchonov
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hchonov created an issue. See original summary.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
FileSize
2890405-2.patch948 bytes

Status: Needs review » Needs work

The last submitted patch, 2: 2890405-2.patch, failed testing. View results

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review
FileSize
2890405-4.patch2.28 KB
2890405-4-translation-errors.png42.79 KB

We already have a case in core where the validation of an other translation is failing because of the constraint ModerationState. Making the errors shown for other translations prettier.

Status: Needs review » Needs work

The last submitted patch, 4: 2890405-4.patch, failed testing. View results

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review
FileSize
2890405-6.patch2.38 KB

I've accedently prepared 2890405-4.patch for 8.3.x and it doesn't apply to 8.4.x ... Here a reroll for 8.4.x.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
FileSize
2890405-7.patch3.49 KB
interdiff-6-7.txt909 bytes

And here is the fix for the failing constraint .. basically if the state hasn't changed then we don't have to check if the transition is allowed.

The last submitted patch, 6: 2890405-6.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 7: 2890405-7.patch, failed testing. View results

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review
FileSize
2890405-10.patch3.47 KB
interdiff-7.10.txt1.78 KB

Status: Needs review » Needs work

The last submitted patch, 10: 2890405-10.patch, failed testing. View results

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review
FileSize
2890405-12.patch3.46 KB
interdiff-10-12.txt1.33 KB

Grrr...

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
Issue tags: +Needs tests

Looks good, but I guess we need specific test for this?

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented

Sure! I'll write a test.

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

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

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov as a volunteer and at bio.logis Genetic Information Management GmbH commented
Title: CEB::validate is validating only the current translation instead of all translations » New method to validate all entity translations
Category: Bug report » Task
Issue summary: View changes
FileSize
2890405-16.patch6.36 KB

After rethinking this I guess that it is not right to change FieldableEntityInterface::validate() to validate all entity translations. Instead it would be better to provide a new method FieldableEntityInterface::validateAllTranslations(), which will take of this and when a validation of all entity translations is required then this method should be used.

I've update the documentation on the validation methods and properties to indicate that they are bound to the curren entittranslation.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler as a volunteer commented

Just a cursory review, but I like the idea of the new patch!

+++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php
@@ -447,6 +450,50 @@ public function validate() {
+    $translation_languages = $this->getTranslationLanguages();
+    unset($translation_languages[$this->language()->getId()]);

I think this can be shortened to $this->getTranslationLanguages(FALSE)

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov as a volunteer and at bio.logis Genetic Information Management GmbH commented

I think this can be shortened to $this->getTranslationLanguages(FALSE)

That would be nice, but unfortunately the parameter of ::getTranslationLanguages() determines whether the default entity translation language should be included or not, but currently we might be on a non-default entity translation and in this case setting the parameter to FALSE would not be sufficient as we still need the default entity translation. We only want to remove the current translation from the iteration as it is being validated as first.

Status: Needs review » Needs work

The last submitted patch, 16: 2890405-16.patch, failed testing. View results

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented

Oops, sorry. So much for drive by reviews. You are, as always, correct of course.

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.

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.

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.

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.

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.

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.