Documented on https://drupal.org/node/2152731 . Agreed with catch we will link to the role migration besides the handbook page when that lands for an example. The most common use case for this will be moving the values of customizable (field API) fields to top properties.

CommentFileSizeAuthor
#16 interdiff.txt1.96 KBchx
#16 2152735_13.patch3.31 KBchx
#13 2152735_11.patch3.26 KBchx
#10 interdiff.txt10.25 KBchx
#10 2152735_7.patch14.16 KBchx
#4 interdiff.txt1.33 KBchx
#4 2152735_4.patch3.31 KBchx
#3 2152735_3.patch3.26 KBchx
#3 interdiff.txt2.26 KBchx
#1 m_extract.patch3.21 KBchx
m_extract.patch3.21 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented
FileSize
m_extract.patch3.21 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
  1. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/process/Extract.php
@@ -0,0 +1,41 @@
+    if (!is_array($value)) {
+      throw new MigrateException('Invalid input.');
+    }
...
+    if (!$key_exists) {
+      throw new MigrateException('Extraction failed.');
+    }

    I wonder whether we can give a more helpful explanation of what is exactly the invalid input and how it should look like? 

  2. +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/process/ExtractTest.php
@@ -0,0 +1,67 @@
+  function setUp() {
...
+  function testExtract() {
...
+  function testExtractFromString() {
...
+  function testExtractFail() {

    It would be cool to have proper visibility.

chx’s picture

chx CreditAttribution: chx commented
FileSize
interdiff.txt2.26 KB
2152735_3.patch3.26 KB
chx’s picture

chx CreditAttribution: chx commented
FileSize
2152735_4.patch3.31 KB
interdiff.txt1.33 KB

Tweaked the second exception.

The last submitted patch, 4: 2152735_4.patch, failed testing.

chx’s picture

chx CreditAttribution: chx commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +PHPUnit

Thank you!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented 
  1. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/process/Extract.php
@@ -0,0 +1,41 @@
+ * This plugin extract a value from an array.

    extracts 

  2. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/process/Extract.php
@@ -0,0 +1,41 @@
+ * @see https://drupal.org/node/2152731

    I'd rather see the basic example in @code?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs work
chx’s picture

chx CreditAttribution: chx commented
Title: Migrate process: extract » Migrate process: extract and plugin refactor
Status: Needs work » Needs review
FileSize
2152735_7.patch14.16 KB
interdiff.txt10.25 KB

Extracts, yes, I caught that myself but I am pushing back on replicating the handbook in core. We can (and do) work on the handbook constantly and I'd rather not submit a document patch all the time.

Now, this patch includes the next step in the plugin evolution: a process plugin base and a specific annotation class. This is step one in multiple handling that will be needed for the role migration. I have tried very hard to keep the patch as small as possible and so the multiple handling in MigrateExecutable is not in here, it'll be in the role migration.

chx’s picture

chx CreditAttribution: chx commented
Issue summary: View changes
chx’s picture

chx CreditAttribution: chx commented
Title: Migrate process: extract and plugin refactor » Migrate process plugins redux, add extract and flatten
Issue summary: View changes
Status: Needs review » Postponed
chx’s picture

chx CreditAttribution: chx commented
Title: Migrate process plugins redux, add extract and flatten » Migrate process plugin: extract
Issue summary: View changes
FileSize
2152735_11.patch3.26 KB
chx’s picture

chx CreditAttribution: chx commented
Status: Postponed » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
  1. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/process/Extract.php
@@ -0,0 +1,42 @@
+    $new_value = NestedArray::getValue($value, $this->configuration['indexes'], $key_exists);

    I wonder whether we should support also 'index' as many times you think more of a singular value. 

  2. +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/process/ExtractTest.php
@@ -0,0 +1,67 @@
+ */
+class ExtractTest extends MigrateProcessTestCase {

    (optional) I kind of love @see to the actual class so you can jump to the class really fast in case you use an IDE. Nice test coverage!

chx’s picture

chx CreditAttribution: chx commented
Issue summary: View changes
FileSize
2152735_13.patch3.31 KB
interdiff.txt1.96 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Thank you

webchick’s picture

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

Nothing to complain about here, this is the same as other process plugins, and thanks for linking to the full docs!

Only thing is that handbook page makes me nervous. It shows we're replacing a line of extremely basic PHP 101 with 8 lines of Drupal-specific YAML that has to be formatted "just so." I'm concerned about the DX impact of that. :\ But I guess this ship has already sailed on using YAML for these inputs, so not much can be done about that. Maybe worth a once-over once things are closer to ready to see if we can improve some of the verboseness, not sure.

In the meantime, however...

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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