Drupal 8.6 compatibility - ConfigSync::finishBatch() deprecated and not called

Thanks for the bug report and the patch.

It would be nice if we could avoid forking the entire ConfigSync::submitForm() method. One alternative would be:

In a new class, extend ConfigImporterBatch, overriding ::finish() as follows:

  /**
   * {@inheritdoc}
   */
  public static function finish($success, $results, $operations) {
    parent::finish($success, $results, $operations);
    if ($success) {
      // Dispatch an event to notify modules about the successful import.
      \Drupal::service('event_dispatcher')->dispatch(ConfigDistroEvents::IMPORT);
    }
  }

Then implement hook_batch_alter() to swap in our class for the batch 'finished' value.

Here's a patch implementing the approach from #3, and also leaving the pre-8.6 code intact. On my testing this works in 8.6 and in prior versions.

Filling in details on how to reproduce this bug.

I agree! We need tests quite urgently.

I am also not a fan of having it split in several locations but the class is an internal class of drupal core that we extend. So it is explicitly not an API and our own fault for extending it. Since 8.5 is now supported until next may we should also support it still until then.
Tests are crucial for that too.

That said, core will need an api for this since the patch in #2991683: Move configuration transformation API in \Drupal\Core\Config namespace does essentially the same and has to do some more trickery too. So we can definitely improve this. I committed the patch now so that we can move forward and things are not left broken.

Thanks for reporting and providing patches!

To answer on how to test. I would create a module and set the previous state of the config in the database directly. So the test module in the file system is after the update. So we install the test module, go in and change the snapshot manually as part of the test and then test the expected difference when importing. But that would be a test for config_sync. For config_distro we won't need to test the installed config, just that the filters for config_distro are taken into account and that the import works properly.

@bircher

I'm getting continued reports of breakage in Configuration Synchronizer due to this bug.

Ahead of test coverage, okay to post a new release with this fix?