Problem/Motivation

It's a common task to take source values and change them based on a configuration mapping.

Proposed resolution

Add this plugin. Documented in the handbook already.

Remaining tasks

Find a better name. Map? IdMap makes that confusing. lookup? MigrateIdMapInterface has lookupSourceId and lookupDestinationId and this isn't like them. I don't know a better name.

CommentFileSizeAuthor
#10 2147817_6.patch3.76 KBchx
#10 interdiff.txt1.95 KBchx
#3 2147817_3.patch3.68 KBchx
#2 2147817_2.patch3.63 KBchx
migrate_map.patch2.94 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan commented
Status: Needs review » Needs work

dreditor went a little funky on me, but the transform() method doesn't look right at all - it's returning the last translated value from the map, and if $value was empty it's going to return the whole configured map.

We need more tests - at least empty values, and values with no match in the map.

Is there any way to set options on the process plugin? Thinking case-sensitivity might be a useful switch to flip now and then...

As for naming... Yes, map is kind of overloaded, and my first instinct, translation table, would also cause confusion. Wikipedia seems to consider lookup table the canonical name for this sort of thing...

chx’s picture

chx CreditAttribution: chx commented
Title: Migrate: add a map(?) process plugin » Migrate: add a static map process plugin
Status: Needs work » Needs review
FileSize
2147817_2.patch3.63 KB

I think static map will do. The plugin got exceptions and the test has been expanded to cover them. While at it, I have refactored the test to be slightly smaller.

chx’s picture

chx CreditAttribution: chx commented
FileSize
2147817_3.patch3.68 KB

Just doxygen fixes for the test.

Status: Needs review » Needs work

The last submitted patch, 3: 2147817_3.patch, failed testing.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Needs review

3: 2147817_3.patch queued for re-testing.

Drupal\system\Tests\Form\FormTest failed. I have nothing to do with that.

Status: Needs review » Needs work

The last submitted patch, 3: 2147817_3.patch, failed testing.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Needs review

3: 2147817_3.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community 
  1. +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/process/StaticMapTest.php
@@ -0,0 +1,76 @@
+/**
+ * @group migrate
+ */
+class StaticMapTest extends MigrateProcessTestCase {

    As always pointing to the tested code + @group Drupal would be cool 

  2. +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/process/StaticMapTest.php
@@ -0,0 +1,76 @@
+  function setUp() {

    ... just a side-note: setUP used to be protected, so we change the visibility technically here.

webchick’s picture

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

chx and I were talking this through trying to come up with suitable docs for transform() (since @inheritdoc is useless on these) and he came to the realization that we could probably just use NestedArray here and move those exceptions there instead.

Also needs a @see to the extended docs in the handbook, and Daniel's @group thing. chx wasn't as sure about the visibility of setUp().

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.95 KB
2147817_6.patch3.76 KB

I am not moving the exception but inlineing NestedArray::getValue is not helpful.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC. Changes look great.

chx’s picture

chx CreditAttribution: chx commented

I must add that this new code raises a new idea (obviously new issue): an extract plugin where the heart of the plugin is the same, just swap the $value and the $configuration:

$new_value = NestedArray::getValue($value, $this->configuration['indexes'], $key_exists);. What's that good for? Set indexes to ['und', 0, 'value'] and it's quite visible.

webchick’s picture

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

Much better. :)

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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