Config import validation needs to be able to list multiple issues and the messages should be translatable

1: 2236553.1.patch queued for re-testing.

1. +++ b/core/lib/Drupal/Core/Config/ConfigImportValidateEventSubscriberBase.php
   @@ -0,0 +1,75 @@
   +   * Checks that the configuration synchronisation is valid.
   
   +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -127,6 +127,17 @@ class ConfigImporter extends DependencySerialization {
   +   * synchronisation will not occur. If errors occur during an import then best
   +   * efforts are made to complete the synchronisation.
   
   @@ -433,19 +444,42 @@ public function import() {
   +        throw new ConfigImporterException('There were errors validating the config synchronisation.');
   
   +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php
   @@ -112,7 +112,10 @@ function testSiteUuidValidate() {
   +      $this->assertEqual($e->getMessage(), 'There were errors validating the config synchronisation.');
   
   +++ b/core/modules/system/lib/Drupal/system/SystemConfigSubscriber.php
   @@ -7,37 +7,32 @@
   +   * Checks that the configuration synchronisation is valid.
   

   American English: synchronisation → synchronization

2. +++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.php
   @@ -256,21 +257,30 @@ public function submitForm(array &$form, array &$form_state) {
   +        drupal_set_message($this->t('The configuration synchronsaiotn failed validaton for the reason(s) listed below.'));
   
   +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
   @@ -305,6 +305,31 @@ function testImportDiff() {
   +    $this->assertText('The configuration synchronsaiotn failed validaton for the reason(s) listed below.');
   

   Typo: synchronsaiotn → synchronization

3. +++ b/core/modules/system/lib/Drupal/system/SystemConfigSubscriber.php
   @@ -7,37 +7,32 @@
   +      $event->getConfigImporter()->logError($this->t('Site UUID in source storage does not match the target storage.'));
   

   Usability-wise, I would recommend something more like "Hey, that's not the same site!" instead of something about UUIDs.

Rerolling.

Rerolled.

This applies on HEAD (pulled about 5m ago).

1. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -473,19 +477,42 @@ public function import() {
   +        throw new ConfigImporterException('There were errors validating the config synchronisation.');
   

   So there is no translation here. Generally we don't translate exceptions because they are not front user facing. So now we are exposing these for users. Is using exceptions the right thing then? If we only translate the wrapping then this is not really resolved...

2. +++ b/core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php
   @@ -29,20 +28,15 @@ class ConfigImportSubscriber implements EventSubscriberInterface {
   +          $message = $this->t('The config name @config_name is invalid due to: @reason.', array('@config_name' => $name, '@reason' => $e->getMessage()));
   

   So we only translate the wrapping here.

See #2055851: Remove translation of exception messages where xjm is explaining why we don't translate exceptions :)

A better approach might be to throw a different typed exception? Edit: @msonnabaum also suggested moving the translatability to a followup.

Have bigger problems, so will NW again after testbot passes.

Looks better. Mixing the untranslated messages in would be confusing...

1. +++ b/core/lib/Drupal/Core/Config/ConfigImportValidateEventSubscriberBase.php
   @@ -0,0 +1,75 @@
   + * ConfigImportValidateEventSubscriberBase makes creating config sync validators
   + * easy.
   

   Minor: Can this somehow fit on one line instead?

2. +++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.php
   @@ -257,21 +258,30 @@ public function submitForm(array &$form, array &$form_state) {
   +        drupal_set_message($this->t('The configuration synchronsaiotn failed validaton for the reason(s) listed below.'));
   
   +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
   @@ -306,6 +306,31 @@ function testImportDiff() {
   +    $this->assertText('The configuration synchronsaiotn failed validaton for the reason(s) listed below.');
   

   Typo in user facing message: "synchronsaiotn". Also it is simpler to not say "for the reason(s) below" and just list the reasons. People will understand those are the reasons, the whole "reason(s)" form where we admit we are lazy coders to handle the singular/plural case is not a good idea, its hard to translate also... :)

3. +++ b/core/modules/system/lib/Drupal/system/SystemConfigSubscriber.php
   @@ -7,37 +7,32 @@
   +   * This event listener implements two checks:
   +   * - prevents deleting all configuration.
   +   * - checks that the system.site:uuid's in the source and target match.
   

   Minor: The list items need to be indented, no?

Addresses issues in #19. It's hard creating an 80 character sentence when the class name takes up 39 itself ;)

+++ b/core/lib/Drupal/Core/Config/ConfigImportValidateEventSubscriberBase.php
@@ -0,0 +1,74 @@
+   * Checks that the configuration synchronisation is valid.

+++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
@@ -135,7 +135,11 @@ class ConfigImporter extends DependencySerialization {
+   * synchronisation will not occur. If errors occur during an import then best
+   * efforts are made to complete the synchronisation.

@@ -658,14 +662,19 @@ protected function getNextConfigurationOperation() {
+        throw new ConfigImporterException('There were errors validating the config synchronisation.');

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.php
@@ -257,21 +258,30 @@ public function submitForm(array &$form, array &$form_state) {
+        drupal_set_message($this->t('The configuration synchronisation failed validation.'));

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
@@ -306,6 +306,31 @@ function testImportDiff() {
+    $this->assertText('The configuration synchronisation failed validation.');

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImporterTest.php
@@ -114,7 +114,10 @@ function testSiteUuidValidate() {
+      $this->assertEqual($e->getMessage(), 'There were errors validating the config synchronisation.');

+++ b/core/modules/system/lib/Drupal/system/SystemConfigSubscriber.php
@@ -7,37 +7,32 @@
+   * Checks that the configuration synchronisation is valid.

Still all British and stuff. Fixed in attached.

All the changes look good. Thanks for resolving the patch so it fixes my concerns :)

+++ b/core/core.services.yml
@@ -557,6 +557,7 @@ services:
+    arguments: ['@string_translation']

+++ b/core/modules/system/system.services.yml
@@ -26,6 +26,7 @@ services:
+    arguments: ['@string_translation']

Just remove these, and I think its good to go!
Please reRTBC once they're gone.

Nice find!

Committed and pushed to 8.x. Thanks!

This appears not to be complete: validation errors actually ignore the messages. Followup in #2503263: Config import validation needs to provide error messages.