Problem/Motivation

The migration ID map tables have a rollback_action column - setting MigrateIdMapInterface::ROLLBACK_PRESERVE in this column should mean that on rollback, the destination item should not be deleted. This is not being respected - entities get deleted regardless of the setting. Blocker for #2598696: Rollback should not delete uid 1

Proposed resolution

Respect the flag!

Remaining tasks

  1. Submit failing test.
  2. Fix it.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#10 rollback_preserve_is-2599152-6.patch3.06 KBmikeryan
#6 interdiff.txt1.11 KBmikeryan
#6 migrateexception_should-2559571-7.patch4.34 KBmikeryan
#3 rollback_preserve_is-2599152-3.patch3.06 KBmikeryan
#3 rollback_preserve_is-2599152-3-FAIL.patch1.64 KBmikeryan

Comments

mikeryan created an issue. See original summary.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Parent issue: #2598696: Rollback should not delete uid 1 »
Related issues: +#2598696: Rollback should not delete uid 1
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Active » Needs review
StatusFileSize
new rollback_preserve_is-2599152-3-FAIL.patch1.64 KB
new rollback_preserve_is-2599152-3.patch3.06 KB

Actually pretty simple.

The last submitted patch, 3: rollback_preserve_is-2599152-3-FAIL.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

This test and fix make sense to me. One small nit:

+++ b/core/modules/migrate/src/MigrateExecutable.php
@@ -330,11 +330,14 @@ public function rollback() {
+        if ($map_row['rollback_action'] == MigrateIdMapInterface::ROLLBACK_DELETE) {
+          $this->getEventDispatcher()
+               ->dispatch(MigrateEvents::PRE_ROW_DELETE, new MigrateRowDeleteEvent($this->migration, $destination_key));
+          $destination->rollback($destination_key);
+          $this->getEventDispatcher()
+               ->dispatch(MigrateEvents::POST_ROW_DELETE, new MigrateRowDeleteEvent($this->migration, $destination_key));
+        }

Indentation needs to be fixed here.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
StatusFileSize
new migrateexception_should-2559571-7.patch4.34 KB
new interdiff.txt1.11 KB

Indentation fixed, gotta figure out how to get PhpStorm to not do that...

Status: Needs review » Needs work

The last submitted patch, 6: migrateexception_should-2559571-7.patch, failed testing.

mikeryan’s picture

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

Found the setting, turned off "Align if multiline"...

Status: Needs review » Needs work

The last submitted patch, 6: migrateexception_should-2559571-7.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Needs review
StatusFileSize
new rollback_preserve_is-2599152-6.patch3.06 KB

No idea how I managed to upload that old patch...

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +rc target triage

I think this is good to go.

The last submitted patch, 3: rollback_preserve_is-2599152-3-FAIL.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Issue tags: -rc target triage +rc eligible

Since these changes are purely against an experimental module, it's RC eligible; no need to triage it.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 0bccb26 on 8.0.x 
    Issue #2599152 by mikeryan, jhedstrom: ROLLBACK_PRESERVE is not...

The last submitted patch, 3: rollback_preserve_is-2599152-3-FAIL.patch, failed testing.

Status: Fixed » Closed (fixed)

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