Problem/Motivation

Migrations will create content types from source sites that may conflict with existing, uninstalled modules on a site. This can then cause an issue when the uninstalled module is installed at a later time (post migration).

Example:

  1. A user has a source site with a "forum" content type from the Forum module.
  2. The user performs a full site migration and a new "forum" content type is created (as well as all of its content) on the destination.
  3. The user then attempts to enable the Forum module on the destination - this will fail because there is already a "forum" content type.

Proposed resolution

alexpott suggests that migrate should check uninstalled modules on the destination site for configuration that may conflict with incoming content types.

When migrating from the UI, if a conflict is found, do not allow the migration to continue until the proper module is installed on the destination.

When migrating from the command line, warn the user, but allow them to force the migration to continue. This would allow for the case where a source site coincidentaly has a content type that has the same machine name as a content type provided by a present, but uninstalled D8 module, but where the source site's content type is not related to the uninstalled D8 module.

Remaining tasks

Make is all happen.

User interface changes

API changes

Data model changes

Original Description and Proposed resolution

Migrate will create the forum node type regardless of whether forum is installed (also in Drupal 6 & 7 the forum node type has no dependency on the forum module). So if the migration creates a forum node type and forum is not installed on the Drupal 8 site - you won't be able to install it.

@catch suggested that migrate should detect that forum was enabled on the source site and enable it.

CommentFileSizeAuthor
#15 check_existing-2692247-15.patch7.83 KBmikeryan
#2 2692247-2.patch497 bytesalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Active » Needs review
FileSize
2692247-2.patch497 bytes

Status: Needs review » Needs work

The last submitted patch, 2: 2692247-2.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

So something like the following:

- migrate looks in the 6.x database to see which modules are enabled
- equivalent modules on the 8.x site get enabled
- potentially where an 8.x module supplies a migration for a different 6.x moudle, we could allow modules to provide a mapping. i.e. 8.x entity references vs. 6.x CCK node reference.

Although even if we do that, there's still going to be a potential conflict between the configuration that the 8.x module provides as defaults vs. the configuration on the 6.x site that might get migrated.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

@catch and also what happens if you don't want to migrate the forum? I guess this more advanced use case should be coped with by using drush and not the UI.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Yes I think you just have to migrate everything if you use the UI.

Or later on we add a more advanced UI where you can pick and choose from a list of available migrations.

Also if you really wanted to, you could migrate forum, then go through and delete all the content and uninstall it.

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented

Wasn't the idea that the UI would show which migrations were missing/available and then you'd have to enable them before running the migration?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

@benjy it does show a list not sure how easy it is to work out what is going on - also it doesn't force you to have forum installed if you had it installed in the d6 or 7 site. And that causes a problem because if you do and you migrate with it off then you can't enable forum - without deleting all the migrated forum content...

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented

If the forum module is not enabled in D8 then the forum migrations should not be running at all, so you wouldn't expect any forum content. So this problem is specific to modules that use nodes to stores their content content. That's an interesting problem.

What catch suggests of trying to enforce parity between the modules enable in the source and destination makes sense, but that would be problematic for users trying to upgrade right now when certain contrib modules may not exist? Or, what happens if the module no longer exists at all?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

@benjy but it is trickier than that with forum. Forum just uses taxonomies and node types and fields - none of these things in D6 and D7 have any dependencies on the forum module. In fact it is entirely possible to uninstall the forum module and rebuild the UI in views or use another module.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

What catch suggests of trying to enforce parity between the modules enable in the source and destination makes sense, but that would be problematic for users trying to upgrade right now when certain contrib modules may not exist? Or, what happens if the module no longer exists at all?

I think we'd only do it one-way. So if you have modules in the 8.x site that match a 6.x module, we enable the 6.x module for you. If you don't, we might list modules that don't have an equivalent but wouldn't stop you migrating without. Some modules only have settings or even no settings at all, and we can't know that just looking at a 6.x database whether they might want migrating or not.

If the forum module is not enabled in D8 then the forum migrations should not be running at all, so you wouldn't expect any forum content. So this problem is specific to modules that use nodes to stores their content content. That's an interesting problem.

It's really any module that provides entity bundles or default fields/instances in config/install combined with the existence of those in a 6.x or 7.x database. Book module likely has the same issue. Commerce is likely to be the toughest example.

In irc, I suggested moving forum's default configuration to config/optional - since both node type and vocabulary are configurable, forum has no hard-dependency on the defaults, and config/optional doesn't blow up on a name clash. However that's not a small change to make - neither is this but it'd be mostly contained to migrate rather than a new best practice for configuration.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented

Note the book module has the same issue: #2714497: Pre-existing 'book' content type prevents book module installation.

It was an explicit decision early on in migration system development not to automatically enable modules, my memory is fuzzy on the reasoning - I'm sure there were implementation concerns, as well as the idea that leaving a module "off" would be a poorman's customization of the process (trimming stuff you didn't want to keep).

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike at DrupalEasy commented
Title: Migrations and enabling modules later for example forum. » Check existing (uninstalled) modules on a site for conflicting content type machine names prior to migrating
Issue summary: View changes

This was discussed at DrupalCon New Orleans at the Migrate Criticals meeting with xjm, alexpott, mikeryan, and the other migrate contributors.

It was decided that this is definitely a "migrate critical" issue.

-mike

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented

Considering this more carefully, I don't believe this is a problem for the migration system to solve. Consider:

  1. Install D8 with the standard profile.
  2. Manually create a "forum" content type.
  3. Attempt to enable the forum module.
  4. "Unable to install Forum, core.entity_form_display.node.forum.default, core.entity_view_display.node.forum.default, core.entity_view_display.node.forum.teaser, field.field.node.forum.body, node.type.forum already exist in active configuration."

catch said:

In irc, I suggested moving forum's default configuration to config/optional - since both node type and vocabulary are configurable, forum has no hard-dependency on the defaults, and config/optional doesn't blow up on a name clash. However that's not a small change to make - neither is this but it'd be mostly contained to migrate rather than a new best practice for configuration.

The thing is, the root problem is not contained to migrate...

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented
Version: 8.1.x-dev » 8.2.x-dev
Status: Needs work » Needs review
FileSize
check_existing-2692247-15.patch7.83 KB

POC moving the config from install to optional.

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented
Status: Needs review » Reviewed & tested by the community

This makes sense to me.

EDIT: Note, this does only solve it for forum, not sure if we need solve this for Book or others?

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented
Title: Check existing (uninstalled) modules on a site for conflicting content type machine names prior to migrating » Pre-existing 'forum' content type prevents forum module installation
Component: migration system » forum.module
Issue tags: -Migrate critical

Yes, this only addresses forum, we should clone this for book.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Nice that #15 passes without any other changes.

I think the patch is fine, but just want to get my head around a couple of things:

- if the forum node type already exists, then forum is fine - it won't have a forum node type configured but you can sort this out.
- when forum is uninstalled, if we installed from config/optional then because of the forced dependency the node type will be uninstalled
- however if we already had the node type, it won't have the forced dependency so it stays in the system

Also because the patch moves all the fields etc. over too, we're not going to add body fields to forum node types that didn't have them or anything strange like that.

That all sounds exactly how things should work.

Is it really that simple?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

This should work. And it makes it clear that forum is not really dependent on these things.

One interesting thing to ponder is should we remove the enforced dependency that most of these configs have. I'm inclined to say no - because I still think the expected behaviour is that if these configs are created during install of the module then they should be deleted when it is uninstalled.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Also because the patch moves all the fields etc. over too, we're not going to add body fields to forum node types that didn't have them or anything strange like that.

I don't think that is true. I think if an existing forum type didn't have a body field then this will create one.

  • catch committed 0e0c6bc on 8.2.x 
    Issue #2692247 by alexpott, mikeryan: Pre-existing 'forum' content type...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Yes #20 is right. So forum will add a body field to the existing node type, and because there's no enforced dependency, that won't get removed on uninstall.

So this probably isn't perfect, but it's a lot better than fatal errors either from clicking around or migrating.

Committed/pushed to 8.2.x. If there's genuine unexpected fallout that's worse than status quo, I don't think we'll find out another way.

Don't think we can do this in 8.1.x so straight to fixed.

Status: Fixed » Needs work

The last submitted patch, 15: check_existing-2692247-15.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Needs work » Fixed
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented

Corresponding book issue is #2714497: Pre-existing 'book' content type prevents book module installation.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.