Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update last value when triggering nested flows with array values #16180

Merged
merged 8 commits into from Nov 9, 2022

Conversation

licitdev
Copy link
Member

@licitdev licitdev commented Oct 26, 2022

Description

Fixes #15653.

  • $last updated to the correct payload for array values when triggering another flow
  • keyedData in the child flow has a separate context from the parent flow

Type of Change

  • Bugfix
  • Improvement
  • New Feature
  • Refactor / codestyle updates
  • Other, please describe:

Requirements Checklist

  • New / updated tests are included
  • All tests are passing locally
  • Performed a self-review of the submitted code

If adding a new feature:

  • Documentation was added/updated. PR:

@licitdev licitdev marked this pull request as draft October 26, 2022 12:16
@licitdev licitdev marked this pull request as ready for review October 26, 2022 12:45
@licitdev licitdev requested a review from nickrum October 26, 2022 12:46
Copy link
Member

@nickrum nickrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if it would be a good idea to fully isolate the sub-flow's data chain from the parent one except for the payload that is passed to $trigger (and that should be available in $last in the first operation after the trigger) by omitting it from context.
Flows triggered by other flows are kind of like sub-routines and $trigger is like a list of parameters. In that context, I think it makes sanse to isolate the scope of the parent from the child.

Regarding adding global shortcuts to $* values in the data chain to the exec operation, I'm not a huge fan of global variables, but I can see how this might be useful 👍

@licitdev
Copy link
Member Author

isolate the scope of the parent from the child

@nickrum Agreed, users can define the payload, nesting values they require within.

However, I wonder if this scope limitation is considered a breaking change as the past behaviour of the $trigger variable is the $trigger from the parent context. If I understand correctly, the value of the $trigger and $last after scoping should be the payload triggering the "child" flow. Thoughts?

adding global shortcuts to $* values in the data chain to the exec operation

Just as a background, this was a suggestion in https://github.com/directus/docs/pull/185#issuecomment-1291147554 which I thought would be nifty and consistent within the flows data chain. 😄

@nickrum
Copy link
Member

nickrum commented Oct 27, 2022

However, I wonder if this scope limitation is considered a breaking change as the past behaviour of the $trigger variable is the $trigger from the parent context. If I understand correctly, the value of the $trigger and $last after scoping should be the payload triggering the "child" flow.

Very good question! Yes, that's exactly how it was supposed to work. That's also how it is documented and how some users understood the feature (#14069). So, I think we should consider it a bug fix and add a notice in case some users relied on the broken behavior.

Just as a background, this was a suggestion in directus/docs#185 (comment) which I thought would be nifty and consistent within the flows data chain. 😄

Ohh, interesting! I thought @rijkvanzanten was suggesting to add the env vars as $env to the data chain, so all of the operations can access them instead of just the exec operation 🤔

@licitdev
Copy link
Member Author

Yes, that's exactly how it was supposed to work.

Updated! Users have to define the payload to pass when triggering another flow.

all of the operations can access them instead of just the exec operation

Wow, this is a different interpretation! Was too focused on the run script operation, whereas this would make it truly consistent across the entire data chain. 😄 Thanks!

@licitdev
Copy link
Member Author

@nickrum Just removed the global variables as they're hard to document and might get confusing. 😄

@licitdev licitdev requested a review from nickrum October 28, 2022 16:13
api/src/operations/trigger/index.ts Outdated Show resolved Hide resolved
@licitdev licitdev requested a review from nickrum October 29, 2022 03:22
freekrai pushed a commit to freekrai/directus that referenced this pull request Nov 9, 2022
…ectus#16180)

* Update last payload when triggering array values

* Reuse keyedData when triggering other flows

* Expose flows data chain variables in run script operation

* Fix unit test

* Separate context for child flows

* Remove global variables from run script operation

* Simplify by using omit
@rijkvanzanten rijkvanzanten merged commit 4b1789a into main Nov 9, 2022
@rijkvanzanten rijkvanzanten deleted the fix/trigger-flow-last branch November 9, 2022 15:51
@rijkvanzanten rijkvanzanten added this to the Next Release milestone Nov 17, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flows: Run script doesn't have access to last node's data
3 participants