Value should be null when is produced skip process.
In this example the migration plugin throw a MigrateSkipProcessException keeping the value empty so set to destination property in:
// No plugins or no value means do not set.
if ($plugins && !is_null($value)) {
$row->setDestinationProperty($destination, $value);
}
This produce: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'field_project_name_target_id' because is a EntityReference.
Given the process configuration in the migration is:
process:
field_project_name:
-
plugin: migration
migration: project_name
no_stub: true
source: project_name
and project_name = ''
set a value = NULL in processRow when is produce skip process.
This avoid additional code:
plugin: static_map
bypass: true
map:
'': nullWrite a patch.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | interdiff-2692373-14-16.txt | 2.51 KB | rakesh.gectcr |
| #16 | 2692373-16.patch | 2.96 KB | rakesh.gectcr |
| #14 | 2692373-14.patch | 2.17 KB | rakesh.gectcr |
| #11 | drupal-MigrateSkipProcessException_null-2692373-8.patch | 2.17 KB | heddn |
| #8 | interdiff_5-8.txt | 627 bytes | heddn |
Comments
Comment #2
edysmpComment #3
edysmpComment #5
heddnI've reviewed the failing testing. In this case, it seems to return a '0' back from the migration process plugin because the pid for the parent comment is 0 in D6 and is therefore empty. I agree that returning NULL is a more acceptable approach. There aren't entity_ids in D6/D7 that allow the usage of '0' as a valid entity_id. To add to that argument, we already set the precedent to return NULL in the case of 'no_stub'. So I've fixed the tests.
Comment #6
benjy commentedThis one doesn't make sense because I see this, right afterwards.
$value = $new_value;We could set the value to NULL in the Migration process plugin if we're sure that's the right behaviour in all cases? Otherwise, I've seen this handled from the migration side of things in the yml with a default value, which is a little more granular.
Comment #7
heddnRe #6: I see that now too. That should probably be a
$new_value[] = NULL;A default value or a static_map both fix this. But also changing the result to NULL does too. And it is one less thing to need to troubleshoot when your migration fails. How many times does the referenced migration (it's gotta be through an ER field) ever return a valid value of 0 for a remote entity_id? Never. Which is why 'no_stub' already sets NULL.
Comment #8
heddnComment #10
heddnComment #11
heddnComment #12
mikeryanComment #13
cmanalansan commentedWorking with @willwh on this
Comment #14
rakesh.gectcrI have rerolled the patch.
Comment #16
rakesh.gectcrComment #17
mikeryanLooks good to me!
Comment #18
catchPatch looks fine but doesn't this need a change record?
Comment #19
mikeryanChange record added (not sure I should set to RTBC if I wrote it, but let's see...)
Comment #21
alexpottCommitted 17b2166 and pushed to 8.1.x and 8.2.x. Thanks!
Added to both branches becuase migrate is experimental and keeping the behaviour the same makes things simpler.
Comment #25
boromino commentedReopening it because the taxonomy term hierarchy in 8.1.2 and 8.2.x-dev doesn't get migrated correctly. Hence the taxonomy reference fields are displayed empty on node edit forms, because the corresponding database query makes an inner join on the taxonomy_term_hierarchy table. And the vocabulary term lists are empty, too.
Comment #26
heddnre #25: Please open a follow-up issue to discuss. And link it back to here. It is best to leave closed issues in a closed status.