Problem/Motivation

Migrated field instances are not removed when the migration is rolled back.

Proposed resolution

The rollback() function of the Entity class cannot handle the multi-field $destination_identifier provided by the field instance migration destination plugin.

Remaining tasks

Add a rollback() function to the EntityFieldInstance class.

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 field_instance_rollback_fails-2706951-4-complete.patch3.58 KBjofitz
#4 field_instance_rollback_fails-2706951-4-tests.patch2.93 KBjofitz
#2 field_instance_rollback_fails-2706951-2.patch662 bytesjofitz

Comments

Jo Fitzgerald created an issue. See original summary.

jofitz’s picture

jofitz
he/him
commented
Status: Active » Needs review
StatusFileSize
new field_instance_rollback_fails-2706951-2.patch662 bytes

Add a rollback() function to the EntityFieldInstance class.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Let's have a test-only patch demonstrating the failure.

Looking more carefully, I'm not sure why we're using a multi-column destination key here in the first place (array('node', 'article', 'field_foo')) rather than using the ID as it's understood by the entity system ('node.article.field_foo'). I.e., couldn't we implement getEntityId() like EntityBaseFieldOverride does, instead of getIds()? EntityConfigBase::import() would need some adjustment, but ultimately I think the whole thing will be simpler (including not having overridden rollback() implementations).

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new field_instance_rollback_fails-2706951-4-tests.patch2.93 KB
new field_instance_rollback_fails-2706951-4-complete.patch3.58 KB

Add a test to demonstrate the failure.

The last submitted patch, 4: field_instance_rollback_fails-2706951-4-tests.patch, failed testing.

The last submitted patch, 4: field_instance_rollback_fails-2706951-4-tests.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Reviewed & tested by the community

Tested locally on 8.2.x, works fine, patch looks good!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 7f5305b to 8.3.x and f73729a to 8.2.x and 08aceee to 8.1.x. Thanks!

  • alexpott committed 7f5305b on 8.3.x 
    Issue #2706951 by Jo Fitzgerald: Field instance migration rollback fails

  • alexpott committed f73729a on 8.2.x 
    Issue #2706951 by Jo Fitzgerald: Field instance migration rollback fails...

  • alexpott committed 08aceee on 8.1.x 
    Issue #2706951 by Jo Fitzgerald: Field instance migration rollback fails...

Status: Fixed » Closed (fixed)

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