Problem/Motivation
When we have something like:
process:
'body/format':
plugin: explode
delimiter: "|"
source: format
'body/value':
plugin: explode
delimiter: "|"
source: value
'body/summary':
plugin: explode
delimiter: "|"
source: teaser
in the migration yaml this will give a structure like this in the row destination:
[
body => [
'value' =>
[0] => [
'Body text 1',
]
[1] => [
'Body text 2',
]
]
...
]
]
When it should be:
[
body => [
[0] => [
'value' =>
'Body text 1',
]
...
]
[1] => [
'value' =>
'Body text 2',
]
...
]
]
]
All the destinations are configured as multiple because of this bug #2639556: InvalidArgumentException: Placeholders must have a trailing [] if they are to be expanded with an array of values.. The explode process plugin is just an example.
When the destination array is wrong it throws an error
InvalidArgumentException: Placeholders must have a trailing [] if they are to be expanded with an array of values.
and the migration fails for that row.
Proposed resolution
Make Drupal\migrate\Row::setDestinationProperty handle this special case by setting the parents correctly for
NestedArray::setValue
https://api.drupal.org/api/drupal/core!modules!migrate!src!Row.php/funct...
Remaining tasks
- Write tests.
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#19 | 2789125-19.patch | 3.77 KB | jofitz |
#19 | interdiff-16-19.txt | 630 bytes | jofitz |
#16 | 2789125-16-complete.patch | 3.79 KB | jofitz |
#16 | 2789125-16-test_only.patch | 1.81 KB | jofitz |
#16 | interdiff-5-16.txt | 2.59 KB | jofitz |
Comments
Comment #2
rodrigoaguileraComment #4
rodrigoaguileraComment #5
rodrigoaguileraI use composer and I made the patch for drupal/core instead of drupal/drupal
Comment #7
rodrigoaguileraOk Writing the test first it becomes clear that Row::getDestinationProperty() needs to altered too.
In the meantime here is the patch with the test only.
Comment #9
youfei.sun CreditAttribution: youfei.sun commentedI think I'm caught by the exact same error, but is this patch working correctly now?
Comment #11
mikeryanWouldn't the example in https://www.drupal.org/node/2639556#comment-11981287 address the real-world use case behind the constructed example in the issue summary?
Comment #12
gmaxwelled CreditAttribution: gmaxwelled commentedHow would the solution in #11 work? Are you suggesting something like the following?
If so, what goes in place of [value]? I can reference a specific item in an array using the extract plugin, but is it possible to access the value of an iteration?
edit: Also, how would we pass the array to the iterator plugin? In the above example, it attempts to run the iterator plugin on each element. I'm confused as to how we get the array to feed to the iterator plugin in the first place. I'm reading data from a CSV.
Comment #13
StryKaizerPatch from #5 fixes my issue (I was working with multivalue daterange fields, trying to set both value and end_value).
Thanks!
PS: suggestion from #11 did not work for me. It looks like this is a bug to me
Comment #14
mikeryanComment #15
mikeryanThe patch in #5 fails tests, that needs to be fixed. The patch in #7 is a test only - what we need in the next iteration here is a complete patch (fix and test) that passes the tests, and a test-only patch that fails the tests.
Comment #16
jofitz CreditAttribution: jofitz at ComputerMinds commentedI have re-uploaded the test-only patch from #7 and extended the fix from #5.
Comment #19
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #23
dmurphy1 CreditAttribution: dmurphy1 at Savas Labs commentedIn response to the suggestion in comment #11 and the follow-up question in comment #12, I was able to work around this issue by following this example: https://gist.github.com/rocketeerbkw/239219a6493c2adee34fb7ceb5af2072
To summarize here for posterity, that gist suggests using prepare row to process your links into the format:
And then use the iterator plugin (now deprecated in favor of the SubProcess plugin that works in the same way) as follows:
Comment #25
safetypin@dmurphy1 How does that format translate into the body field? Using a configuration like this doesn't seem to migrate anything:
Comment #26
safetypinTurns out I was looking at a node that had an empty body field, so it turns out that the pattern in #23 works for me. For the body field, this works:
Comment #28
adevms CreditAttribution: adevms at REEA commentedThe patch works nicely with migrating multivalue link fields but only for uri and title values. Not for attributes for ex. Any ideas as how to make this work?
In the process plugin is have:
This works nicely for multiple entries.
The following does not work:
Comment #34
danflanagan8Here's a way to do this that I came up with using Migrate Sandbox. Let's say this is your source.
The process pipeline below uses the
transpose
plugin from migrate_plus.Which results in
I personally think it's not quite right to consider this a bug since the source data is extremely different from any source data that one would expect when migrating versions of Drupal. It's a challenge, not a bug!
Comment #35
benjifisherI agree with #34. The
transpose
process plugin is designed for this problem. Now that it is available, @MikeRyan's initial response (#11 on this issue) is right. We should use thesub_process
plugin (previously namediterator
) here.Untested, but I think it can be simplified: something like this:
Comment #36
benjifisherThe issue summary quotes the error message:
That is not very helpful. Maybe we can detect this situation and provide a better message before sending it t the database layer.
If we can do that, I am not sure whether we should do it in a new issue or repurpose this one.