Problem/Motivation

The migration process plugin never saves anything to the id map when a stub is created. Because of this, stubs are created that are never updated. :(

Proposed resolution

Add the source and destination to the id map so the stub can be updated at a later time.

Remaining tasks

  1. Write Patch

User interface changes

N/A

API changes

N/A

Data model changes

Stubs created with the migration process plugin will be added to the id map.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

davidwbarratt created an issue. See original summary.

davidwbarratt’s picture

Status: Active » Needs review
FileSize
1.21 KB
davidwbarratt’s picture

davidwbarratt’s picture

Issue summary: View changes

The last submitted patch, 2: migration-idmap-2682705-2.patch, failed testing.

benjy’s picture

This makes sense, I'm surprised it hasn't worked all this time, I did wonder if we should have an integration test but I guess the unit test is enough.

benjy’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

I think this is worth an integration test as well - we should at least open a follow-up if not doing that here.

catch’s picture

Status: Reviewed & tested by the community » Needs review
benjy’s picture

I've closed #2639144: Stub row isn't saved in map as a duplicate because the patch there was red. But I asked the two contributors to comment here so they could be credited.

xjm’s picture

Title: Migrate process plugin does not save stubs to the idmap » Migrate process plugin does not save stubs to the idmap, which interferes with incremental migrations
Version: 8.0.x-dev » 8.1.x-dev
Related issues: +#2683421: Remove incremental and rollback options from the UI (and add them back when they are more stable), +#2687307: Fix Migrate UI form when incremental or rollback selected

Retitling to get at what (I think) is the implication of this.

Given how different Migrate is in 8.1.x vs. 8.0.x at this point let's make sure the patch works with 8.1.x.

xjm’s picture

Okay I did not mean to queue three separate test runs but at least one of them is for 8.1.x. :)

Status: Needs review » Needs work

The last submitted patch, 3: migration-idmap-2682705-4.patch, failed testing.

xjm’s picture

Issue tags: +beta target
vasi’s picture

@xjm, this is a problem for non-incremental migrations as well. It seriously affects any migrations related via eg: entity references. Eg, if nodes of type A point to taxonomy terms of type B, after migrating both I'll end up with:

* a bunch of garbage sample-data terms (created as stubs)
* nodes pointing to these garbage terms
* real terms, but nothing pointing to them

benjy’s picture

Issue tags: +Needs reroll

This needs a re-roll against 8.1

vprocessor’s picture

Assigned: Unassigned » vprocessor
vprocessor’s picture

Hi guys,
reroll is ready

vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Needs work » Needs review
andypost’s picture

Issue tags: -Needs reroll
benjy’s picture

As per #8, does anyone want to write an integration test?

catch’s picture

Title: Migrate process plugin does not save stubs to the idmap, which interferes with incremental migrations » Migrate process plugin does not save stubs to the idmap, leads to duplicates and broken references

Tried an issue title update to reflect #15, which sounds like a good candidate for test coverage.

catch’s picture

alexpott’s picture

Here's an addition to an integration test that proves it works... not in the migrate module though :(

The last submitted patch, 24: 2682705.24.test-only.patch, failed testing.

benjy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

  • catch committed 5650763 on 8.2.x
    Issue #2682705 by davidwbarratt, alexpott, vprocessor, miiimooo, milesw...

  • catch committed 6de7a90 on 8.1.x
    Issue #2682705 by davidwbarratt, alexpott, vprocessor, miiimooo, milesw...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and 8.1.x, thanks!

This doesn't cherry-pick to 8.0.x, re-open if you port it.

miiimooo and milesw didn't comment here after 9 days, but added to the commit message.

vasi’s picture

Version: 8.1.x-dev » 8.0.x-dev
Status: Fixed » Needs review
FileSize
2.78 KB

Porting to 8.0.x.

alexpott’s picture

Issue tags: -beta target

This is in 8.1.x and 8.2.x therefore no longer a beta target.

xjm’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs review » Fixed

Thanks @vasi.

I actually think we should not backport this, since 8.0.x is not a good development target for Migrate at the moment and is EOL in three weeks. So marking back to fixed for 8.1.x.

Status: Fixed » Closed (fixed)

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