Problem/Motivation

API page: https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Plugi...

Isn't the second source: key redundant in the example? When chaining process plugins the plugin get as their source the processed value of the plugin before, right? As per https://www.drupal.org/docs/8/api/migrate-api/migrate-process-plugins/mi... :
"The second plugin and so on does not need a source as their input is the output of the previous plugin. This is why it's called a pipeline."
Is this just a confusing (i.e. 'bad') example or is there something else going on here?

plugin: substr
source: some_text_field <-- this seems redundan

Steps to reproduce

N/A

Proposed resolution

Remove the line * source: some_text_field from the substr process in this example, not from skip_on_empty.

 * process:
 *   new_text_field:
 *    -
 *      plugin: skip_on_empty
 *      method: process
 *      source: some_text_field
 *    -
 *      plugin: substr
 *      source: some_text_field
 *      start: 6
 *      length: 10

Remaining tasks

Patch
review
commit

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#12 interdiff_2989893_9-12.txt427 bytesankithashetty
#12 2989893-12.patch467 bytesankithashetty
#9 2989893-9.patch533 bytessrilakshmier
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

firfin created an issue. See original summary.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Title: Redundant (confusing) source: key in example. » Redundant (confusing) source: key in Substr example
Category: Bug report » Task
Priority: Normal » Minor
Issue summary: View changes
Issue tags: +Novice, +migrate

Yes, this could be improved. The documentation is not incorrect though, the given example does work. It just doesn't make use of the abilities of the pipeline process.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue tags: +Bug Smash Initiative

Forgot to tag.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Title: Redundant (confusing) source: key in Substr example » Remove redundant source: key in Substr example
srilakshmier’s picture

srilakshmier CreditAttribution: srilakshmier at Valuebound for Valuebound commented
Assigned: Unassigned » srilakshmier
srilakshmier’s picture

srilakshmier CreditAttribution: srilakshmier at Valuebound for Valuebound commented
Assigned: srilakshmier » Unassigned
Status: Active » Needs review
FileSize
2989893-9.patch533 bytes

Attaching the patch.

Thanks

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate/src/Plugin/migrate/process/Substr.php
@@ -50,10 +50,8 @@
- *      source: some_text_field

We shouldn't be removing the first instance of this line - the first plugin in the chain needs a source field.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue summary: View changes

Clarified the resolution.

ankithashetty’s picture

ankithashetty
Location Karnataka, India
CreditAttribution: ankithashetty as a volunteer and at Specbee commented
Status: Needs work » Needs review
FileSize
2989893-12.patch467 bytes
interdiff_2989893_9-12.txt427 bytes

Updated patch in #9 addressing #10, thanks!

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Thanks!

  • catch committed 4b4895c on 9.3.x 
    Issue #2989893 by ankithashetty, srilakshmier, quietone, firfin,...

  • catch committed e7e8416 on 9.2.x 
    Issue #2989893 by ankithashetty, srilakshmier, quietone, firfin,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.9.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

Status: Fixed » Closed (fixed)

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