Problem/Motivation

In Issue #3236774, it was discovered that the ability to reference the current value of a process pipeline already existed but was undocumented. Specifically, migration process plugins that accept multiple source properties (like concat, callback, download, etc.) can reference the current pipeline value by using null as a source parameter.

This feature works through the get plugin's hidden behavior: when $property is null, an empty string, or an empty array, the plugin returns the current pipeline value. However, this capability is not documented in the official API documentation.

Example usage:

process:
  processed_value:
    - plugin: callable
      source: field_product
      callable: ucfirst
    - plugin: concat
      source:
        - 'constants/i'
        - null  # References current pipeline value

As noted in comment #9, any falsey value except the string '0' would work to get the pipeline value, not just null. This suggests the documentation may be incomplete.

Steps to reproduce

N/A - This is a documentation issue.

Proposed resolution

Add documentation to the code itself to explain how to reference the current pipeline value. This documentation will feed into the API documentation page that is linked from the list of migration plugins.

The documentation should clarify:

  • That null can be used to reference the current pipeline value
  • The behavior of other falsey values (empty string, empty array, etc.) and whether they should also be documented or if only null is the recommended approach
  • The implications for other plugins that accept source properties

This documentation enhancement requires subsystem maintainer review to ensure accuracy and completeness, particularly regarding the behavior of falsey values beyond null.

Remaining tasks

  • Subsystem maintainer review of the proposed documentation to confirm whether only null should be documented or if other falsey values are also valid/recommended
  • Clarify implications for other plugins accepting source properties
  • Review and finalize MR !13995
  • Update documentation in code
  • Commit the changes

User interface changes

None.

API changes

None. This is a documentation-only change.

Data model changes

None.

CommentFileSizeAuthor
#16 3560542-nr-bot_uq6xiv8r.txt549 bytesneeds-review-queue-bot

Issue fork drupal-3560542

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jan kellermann created an issue. See original summary.

jan kellermann opened merge request !13991

jan kellermann’s picture

jan kellermann commented
Status: Active » Needs review

Added documentation in code. Please review and feedback.

jan kellermann’s picture

jan kellermann commented
smustgrave’s picture

smustgrave commented
Version: 11.3.x-dev » 11.x-dev
Status: Needs review » Needs work
Related issues: +#3236774: Provide ability to reference current value of process pipeline as a source property

Have not reviewed but MRs need to be against 11.x as the current development branch

If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

jan kellermann opened merge request !13995

jan kellermann closed merge request !13991

jan kellermann’s picture

jan kellermann commented
Status: Needs work » Needs review

Sorry, created MR against 11.x.

dcam’s picture

dcam commented
Status: Needs review » Needs work
Issue tags: +Needs subsystem maintainer review, +Needs issue summary update

Thank you for your suggestion to improve Drupal!

I looked over the Get plugin code. It seems like any falsey value except the string '0'* would work to get the pipeline value, not just NULL. This is just an educated guess; I haven't tested it. So these new docs may be incomplete. Also, this could have implications for other plugins that accept lists of source properties. I'm tagging it for subsystem maintainer review to get an opinion.

It is preferred for issues to have the entire summary template with all headings, even if some sections are left empty. So I'm tagging this for needing a summary update and setting the status to Needs Work.

*There's a condition to allow the string '0' as a property name.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Issue summary: View changes

With a bit of AI agent help, I updated the summary

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Needs work » Needs review

I addressed the feedback from #9

I also simplified the example and made it more similar to the other @code example in this plugin.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

Good find by @dcam that false has the same effect as null.

And the new example looks correct.

But I think before this "weird trick" is officially documented, I think someone needs to confirm that there is test coverage somewhere. I don't recall ever looking into that.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

So looking around I do see any test coverage around this plugin, so guess we need to add some

quietone’s picture

quietone commented
Status: Needs work » Needs review

All the migrate process plugins in the Migrate module have tests. They are at core/modules/migrate/tests/src/Unit/process.

In effect, this is documenting how the Get plugin works by using Concat as the example. In that regard I am not convinced this extra documentation is needed here. And I don't want to set a precedence such that similar examples are added to all the other process plugin.

Instead, I would rather improve the Get process documentation. It already has something for this behavior but it is buried in the examples. So. at least move this text to before the examples.

 * If the list of source properties contains an empty element then the current
 * value will be used. This makes it impossible to reach a source property with
 * an empty string as its name.

I am leaving the "Needs subsystem maintainer review" tag to allow the other maintainers to express a view.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3560542-nr-bot_uq6xiv8r.txt549 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review

Fairly positive this is a false positive

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Since it's been 2+ weeks think we should follow what @quietone suggested in #15

Thanks!

drupal.ninja03’s picture

drupal.ninja03 commented

I checked core/modules/migrate/src/Plugin/migrate/process/Get.php on the main branch as per comment#15 & #18.

The behavior mentioned in the issue is already documented there, but it is currently placed after the list-of-source-properties example:

"If the list of source properties contains an empty element then the current value will be used. This makes it impossible to reach a source property with an empty string as its name."

Based on comment #15, I agree that the current MR does not fully incorporate the feedback yet because it documents the behavior in Concat.php, while the behavior belongs to the Get process plugin.

A better next step may be to move/expand the existing Get.php explanation before the examples, so the current pipeline value behavior is easier to discover, without adding concat-specific examples that could set a precedent for documenting this in every process plugin.

Leaving as Needs work.

drupal.ninja03’s picture

drupal.ninja03 commented
Status: Needs work » Needs review

Updated the MR to address the feedback in #15 and #18.

Changes made:
- Moved/expanded the documentation for using the current process pipeline value to Get.php, where this behavior belongs.
- Placed the explanation before the examples so it is easier to discover.
- Removed the concat-specific pipeline value example from Concat.php to avoid documenting Get plugin behavior in individual process plugins.
- Fixed the existing Concat.php delimiter example typo where the 'bar' value should be "Parks".

Setting back to Needs review.

smustgrave’s picture

smustgrave commented

@quietone as a migration maintainer thoughts?

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

This looks like all feedback has been addressed and is a good update to the docs.

@quietone left the subsystem maintainer tag, but it's been 7 months, I think it's safe to move to the next step.

  • quietone committed 10cc9b3c on 11.4.x 
    task: #3560542 Improve documentation for migrate-plugin concat

By: jan...

  • quietone committed ec521941 on 11.5.x 
    task: #3560542 Improve documentation for migrate-plugin concat

By: jan...

  • quietone committed 2a641b74 on 11.x 
    task: #3560542 Improve documentation for migrate-plugin concat

By: jan...

  • quietone committed c3030784 on 12.0.x 
    task: #3560542 Improve documentation for migrate-plugin concat

By: jan...

  • quietone committed f9f36243 on main 
    task: #3560542 Improve documentation for migrate-plugin concat

By: jan...
quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs subsystem maintainer review

Thanks everyone! As subsystem maintainer I have removed the tag. This does make it clearer how the get plugin behaves.

Committed and pushed to all branches as a documentation improvements are allowed.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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