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

fix(Execute Workflow Node): Passing the workflow that is supposed to be executed as a paramter #8614

Conversation

despairblue
Copy link
Contributor

Summary

This fixes a regression introduced with 216ec07, e.g. in version 1.23.0.

I'd love to write a test for this too, but the node is not testable right now with the unit test framework. First this function here has to be implemented:

executeWorkflow: async (workflowInfo: IExecuteWorkflowInfo): Promise<any> => {},

I've tried, but I stopped after a while, not to loose too much time on it. Maybe the nodes team can take a look at adding a test for this.

Related tickets and issues

https://linear.app/n8n/issue/PAY-1362/execute-workflow-node-with-source-parameter-does-not-work-anymore

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.

    A bug is not considered fixed, unless a test is added to prevent it from happening again.
    A feature is not complete without tests.

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Feb 12, 2024
Copy link
Contributor

@krynble krynble left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

cypress bot commented Feb 13, 2024

1 flaky test on run #4085 ↗︎

0 343 5 0 Flakiness 1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Project: n8n Commit: 0e7db8eaab
Status: Passed Duration: 03:25 💡
Started: Feb 13, 2024 11:35 AM Ended: Feb 13, 2024 11:38 AM
Flakiness  cypress/e2e/38-custom-template-repository.cy.ts • 1 flaky test

View Output Video

Test Artifacts
In-app templates repository > should save template id with the workflow Test Replay Screenshots Video

Review all test suite changes for PR #8614 ↗︎

Copy link
Contributor

✅ All Cypress E2E specs passed

@despairblue despairblue merged commit b54488a into master Feb 13, 2024
28 checks passed
@despairblue despairblue deleted the pay-1362-execute-workflow-node-with-source-parameter-does-not-work branch February 13, 2024 11:49
@@ -29,7 +29,8 @@ export async function getWorkflowInfo(this: IExecuteFunctions, source: string, i
workflowInfo.code = jsonParse(workflowJson);
} else if (source === 'parameter') {
// Read workflow from parameter
workflowInfo.code = this.getNodeParameter('workflowJson', itemIndex) as IWorkflowBase;
const workflowJson = this.getNodeParameter('workflowJson', itemIndex) as string;
Copy link
Member

@netroy netroy Feb 14, 2024

Choose a reason for hiding this comment

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

A better fix for this would be to make sure that getNodeParameter does jsonParse when the parameter type is json. either that, or we get rid of type: 'json', and convert such parameters to be strings.

Created a backlog ticket for this: ENG-101

netroy pushed a commit that referenced this pull request Feb 14, 2024
@github-actions github-actions bot mentioned this pull request Feb 14, 2024
@janober
Copy link
Member

janober commented Feb 15, 2024

Got released with [email protected]

@github-actions github-actions bot mentioned this pull request Feb 15, 2024
despairblue added a commit that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team node/improvement New feature or request Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants