Problem/Motivation

ConfigReverter::import() provides importing of configuration objects provided by extensions but not already present on the site.

New configuration entities may have unmet dependencies. Example: a content type has been added to a module at the same time as various fields that are attached to that content type. Before one of the new fields can be added, the new content type must first be present (imported).

Is dependency testing and handling needed?

Proposed resolution

Remaining tasks

User interface changes

API changes

Comments

nedjo’s picture

I thought I'd seen a place where this was handled in core but when I looked in ConfigImporter I couldn't find it.

nedjo’s picture

I was thinking of e.g. the documentation in StorageComparer::addChangelistCreate():

The list of creates is sorted so that dependencies are created before configuration entities that depend on them. For example, field storages should be created before fields.

jhodgdon’s picture

Yeah, but that is for a bulk operation, which this module doesn't do. It's only importing a single config item.

The relevant thing to compare (which I haven't looked at yet but will) is what happens when you go to the core Config manager page that imports a single config from a paste, vs. what happens here if you do a revert or import.

jhodgdon’s picture

Status: Active » Closed (works as designed)

OK. If you take a look at what happens in
\Drupal\config\Form\ConfigSingleImportForm
(in Drupal Core) in the submitForm() method

vs. what is happening in
\Drupal\config_update\ConfigReverter
(in this module) in the import() and revert() methods, you will see that they are pretty much identical (this is not a coincidence).

My feeling is that if Core doesn't care about dependency handling when importing config, I don't need to either.

nedjo’s picture

Using the single import form, I tested importing a field before its field storage and got the error " Attempt to create a field field_example that does not exist on entity type node."

ConfigSingleImportForm::submitForm() uses try/catch for the entity save, which will catch an exception if dependencies are missing (or the save fails for any other reason).

So it's probably worth considering using try/catch for the entity saves in the import and update.

jhodgdon’s picture

Well, the only thing I'm doing different is not catching the exception. If tries to import config that won't import, then they'll get an exception rather than a nicer message. It will not screw up their system, however.

Since it should not screw up their system, I'm not too inclined to worry about this, but if you would like you can file a UI bug.

nedjo’s picture

I opened #2426189: More user-friendly exception/error handling on revert and import.

Beyond error handling, there are workflow challenges. Taking the example I tried above, after trying to import a field and hitting an error, the workflow might include:

  • Navigate to a report that includes the missing item (since it may or may not be on the report I was viewing). In this case, which report to consult might not be clear. The error report mentions a "field" that doesn't exist. If I bring up the "Field" config type report, though, it won't be there; what I need instead is the "Field storage" report. Import the item.
  • Navigate to a report that includes the original item and import it.

Of course, this is the simplest case. There might be a more extensive dependency chain to follow.

Options to make the workflow easier might be:

  • Test dependencies and offer a message like: "Before you can import the [item name] [config type], you must import the [link]Example[/link] field storage."
  • Add a confirm step like that on module enabling, where we test dependencies and offer to import them: "You must import the [item name] [config type] module to import [item name] [config type]. Would you like to continue with the above?"

None of this is done in core's import of a single config item. But there we don't have access to the required updates.

jhodgdon’s picture

Let's discuss that on the other (open) issue rather than on this (closed) issue.