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(API): Fix workflow project transfer #10651

Merged
merged 5 commits into from
Sep 27, 2024

Conversation

benrobot
Copy link
Contributor

@benrobot benrobot commented Sep 3, 2024

Summary

Transferring a workflow to a new project via the Public API was failing.

The unit test responsible for detecting this type of issue was only passing because there were no other workflows and projects in the database. After updating the test to match a more real-world scenario the test started to fail as shown in the screenshot below:
image

Related Linear tickets, Github issues, and Community forum posts

none

Review / Merge checklist

  • [✅] PR title and summary are descriptive. (conventions)
  • [ n/a ] Docs updated or follow-up ticket created.
  • [✅] Tests included.
  • [n/a] PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

This commit updates the unit test to reproduce an issue that was detected.
A separate commit will be used to fix the problem.

The output below is from this test failure:
n8n:test:   ● PUT /workflows/:id/transfer › should transfer workflow to project
n8n:test:
n8n:test:     expect(received).toBe(expected) // Object.is equality
n8n:test:
n8n:test:     Expected: "htfVcAUznnWmLzQE"
n8n:test:     Received: "gLQLBvrhUglvs1DC"
n8n:test:
n8n:test:       1537 |          const workflowsInProjectResponse = await authMemberAgent.get(`/workflows?projectId=${secondProject.id}`).send();
n8n:test:       1538 |          expect(workflowsInProjectResponse.statusCode).toBe(200);
n8n:test:     > 1539 |          expect(workflowsInProjectResponse.body.data[0].id).toBe(workflow.id);
n8n:test:            |                                                             ^
n8n:test:       1540 |  });
n8n:test:       1541 |
n8n:test:       1542 |  test('if no destination project, should reject', async () => {
n8n:test:
n8n:test:       at Object.<anonymous> (test/integration/publicApi/workflows.test.ts:1539:54)
n8n:test:
…er workflow to project via public api

Transfering a workflow to another project using the public API was failing because the code was
dereferencing that wrong parameter name and getting undefined.
@CLAassistant
Copy link

CLAassistant commented Sep 3, 2024

CLA assistant check
All committers have signed the CLA.

@n8n-assistant n8n-assistant bot added community Authored by a community member core Enhancement outside /nodes-base and /editor-ui in linear Issue or PR has been created in Linear for internal review labels Sep 3, 2024
@Joffcom
Copy link
Member

Joffcom commented Sep 3, 2024

Hey @benrobot,

Thanks for the PR, We have created "GHC-199" as the internal reference to get this reviewed.

One of us will be in touch if there are any changes needed, in most cases this is normally within a couple of weeks but it depends on the current workload of the team.

ivov
ivov previously approved these changes Sep 3, 2024
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

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

Good catch! Thank you for your contribution.

@benrobot benrobot changed the title fix(API): fix workflow project transfer fix(API): Fix workflow project transfer Sep 3, 2024
@ivov
Copy link
Contributor

ivov commented Sep 13, 2024

@benrobot Thanks again for your contribution. Once the lint issues are fixed we can bring this in. Let us know if we can help or if you'd like for us to take over.

@ivov ivov merged commit 5f89e3a into n8n-io:master Sep 27, 2024
15 checks passed
MiloradFilipovic added a commit that referenced this pull request Sep 30, 2024
* master:
  feat(Iterable Node): Add support for EDC and USDC selection (#10908)
  test(Schedule Trigger Node): Add tests and extract trigger test helper (no-changelog) (#10625)
  fix(Todoist Node): Fix listSearch filter bug in Todoist Node (#10989)
  fix(AwsS3 Node): Fix search only using first input parameters (#10998)
  fix(editor): Fix bug causing node issues to not be assigned before first interaction (no-changelog) (#10980)
  fix(editor): Allow resources to move between personal and team projects (#10683)
  fix(Respond to Webhook Node): Node does not work with Wait node (#10992)
  fix(core): Upgrade @n8n/typeorm to address a rare mutex release issue (#10993)
  refactor(core): Separate execution `startedAt` from `createdAt` (#10810)
  refactor(core): Make all pubsub messages type-safe (#10990)
  feat(Question and Answer Chain Node): Customize question and answer system prompt (#10385)
  fix(editor): Fix performance issue in credentials list (#10988)
  fix(RSS Feed Trigger Node): Fix regression on missing timestamps (#10991)
  fix(editor): Fix workflow executions list page redirection (#10981)
  fix(editor): Fix filter execution by "Queued" (#10987)
  fix(API): Fix workflow project transfer (#10651)
This was referenced Oct 2, 2024
@janober
Copy link
Member

janober commented Oct 2, 2024

Got released with [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member core Enhancement outside /nodes-base and /editor-ui in linear Issue or PR has been created in Linear for internal review Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants