Problem/Motivation

Here's the error.

array_flip(): Can only flip STRING and INTEGER values!                 [warning]
EntityStorageBase.php:232

Fatal error: Call to a member function save() on a non-object in ....

I _think_ the error is something to do with the original node not existing. We have some logic to catch missing revisions, but not missing nodes entirely.

Proposed resolution

TBD

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 2603010-12.interdiff.txt1.56 KBneclimdul
#12 entityrevision-2603010-12.patch4.54 KBneclimdul
#9 entityrevision-2603010-9.patch4.34 KBwebflo
#5 2603010-5.interdiff.txt2 KBneclimdul
#5 entityrevision-2603010-5.patch4.66 KBneclimdul
#3 entityrevision-2603010-3.patch4.64 KBneclimdul

Comments

neclimdul created an issue. See original summary.

neclimdul’s picture

neclimdul
He/Him
commented

getEntity() is returning false when it can't load an entity but ContentEntityBase::import() isn't setup to handle that.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Active » Needs review
StatusFileSize
new entityrevision-2603010-3.patch4.64 KB

In the process of ensuring it fixes the actual bug we're seeing in the migration but here's a patch with tests.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php
@@ -74,6 +75,9 @@ public static function create(ContainerInterface $container, array $configuratio
+      throw new MigrateSkipRowException('Unable to load entity.');

"Unable to load entity" assumes there was an old entity to be loaded (the problem you're addressing here), but getEntity() also creates new entities, so the message could be more general.

MigrateSkipRowException will set the IGNORED status for the row in the map table (meaning the item is being skipped intentionally), I think MigrateException (which sets an error status) would be more appropriate here.

Otherwise, looks good to me.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review
StatusFileSize
new entityrevision-2603010-5.patch4.66 KB
new 2603010-5.interdiff.txt2 KB

Sure.

mikeryan’s picture

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

Looks good, let's see if we can get this in.

effulgentsia’s picture

effulgentsia commented
Issue tags: -rc target triage +rc eligible

Only changes Migrate module code, so "rc eligible" per https://www.drupal.org/core/d8-allowed-changes#rc.

webchick’s picture

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

No longer applies.

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new entityrevision-2603010-9.patch4.34 KB
mikeryan’s picture

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

Preliminary RTBC - no reason this should fail, just removing a use statement made redundant by the stub patch commit.

neclimdul’s picture

neclimdul
He/Him
commented

Confirmed looks the same, back to RTBC.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new entityrevision-2603010-12.patch4.54 KB
new 2603010-12.interdiff.txt1.56 KB

EntityContentBase got a new constructor argument. Updated patch.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

I'd reset to RTBC, but it's still there:P. Anyway, I didn't look closely enough to see that the test is constructing a content entity, and since the stub patch added a new argument (field type manager) it broke the test here. Interdiff looks good and phpunit passes locally, lgtm (again).

The last submitted patch, 5: entityrevision-2603010-5.patch, failed testing.

The last submitted patch, 9: entityrevision-2603010-9.patch, failed testing.

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 0718153 on 8.1.x 
    Issue #2603010 by neclimdul, webflo, mikeryan: EntityRevision...
  • webchick committed 1f0466c on 8.1.x
    Issue #2603010 by neclimdul, webflo, mikeryan: EntityRevision...
  • webchick committed 3c1413a on 8.1.x
    Revert "Issue #2603010 by neclimdul, webflo, mikeryan: EntityRevision...

Status: Fixed » Closed (fixed)

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