Config import validation needs to provide error messages

@fgm I had the same problem with the Google PHP API. There was more error information being returned but the exception hid them.

+++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
@@ -755,7 +756,7 @@ protected function validate() {
-      if ($collection == StorageInterface::DEFAULT_COLLECTION) {
+      if ($this->configManager->supportsConfigurationEntities($collection)) {

Is this debug code not intended for the patch?

+++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
@@ -727,8 +727,10 @@ protected function validate() {
+      $errors = $errors = $this->getErrors();

That needs fixing.

Also, this will result in real test failures. The actual exception output is checked so those will all have to be updated.

Tried unsuccessfully to get the error message, "There were errors validating the config synchronization", to pop up. With "Full Import", I changed various config to invalid values, I imported improperly compressed files, and finally imported an empty file.

'drush config-import' would display errors: "The import failed due for the following reasons: This import is empty and if applied would delete all of your configuration, so has been rejected." The UI gave me nothing though.

I'd say this ticket needs steps to reproduce. Also, rerolling.

Added to config management usability meta #2642404: [meta] Usability improvements to configuration management post 8.0

Postponing on steps to reproduce, thanks!

I'm not sure how to produce this via the UI. However, the search api module is currently having this issue in one of it's tests (#2982693: Test failing on main branch) since #2788777: Allow a site-specific profile to be installed from existing config went in and required system.site, so developers can reproduce the steps this way.

This is a re-working of #8 since the test itself has moved (and thus no easy interdiff either). I made the test more explicit in terms of checking for the expected error messages.

Also @steven.winchers should get credit for his work in the duplicate #2852296: ConfigImporter has an obtuse error message.

This fixes the content moderation config importer test.

This is really helpful when those errors occur, and would be a good improvement.

Adding this to the exception message is a great idea.

Committed c7afa35 and pushed to 8.6.x. Thanks!

The UI lists the errors already but this makes things easier for developers if the exception is thrown but not caught and handled correctly. Here's the code that does that from \Drupal\config\Form\ConfigSync

      catch (ConfigImporterException $e) {
        // There are validation errors.
        $this->messenger()->addError($this->t('The configuration cannot be imported because it failed validation for the following reasons:'));
        foreach ($config_importer->getErrors() as $message) {
          $this->messenger()->addError($message);
        }
      }

This is such an important improvement for the site builder (and developer) experience!

@Wim Leers I'm going to tentatively remove those tags. The site builder already has this information when they import configuration. This is mostly useful for a developer when writing a test which does a config import and there's a problem.