Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(editor): Remove invalid connections after node handles change #12247

Conversation

alexgrozav
Copy link
Member

Summary

Screen.Recording.2024-12-16.at.15.24.46.mov

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/CAT-403/delete-connection-when-switching-to-node-that-doesnt-have-connection

Review / Merge checklist

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

@alexgrozav alexgrozav self-assigned this Dec 16, 2024
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Dec 16, 2024
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 81.25000% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ackages/editor-ui/src/components/canvas/Canvas.vue 25.00% 6 Missing ⚠️
...rc/components/canvas/elements/nodes/CanvasNode.vue 45.45% 6 Missing ⚠️
...s/editor-ui/src/composables/useCanvasOperations.ts 93.47% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

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

Works as expected just 2 comments that are up for discussion

packages/editor-ui/src/utils/objectUtils.ts Outdated Show resolved Hide resolved
packages/editor-ui/src/composables/useCanvasOperations.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Copy link

cypress bot commented Dec 17, 2024

n8n    Run #8384

Run Properties:  status check passed Passed #8384  •  git commit c6d6a9c819: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 alexgrozav 🗃️ e2e/*
Project n8n
Branch Review cat-403-delete-connection-when-switching-to-node-that-doesnt-have
Run status status check passed Passed #8384
Run duration 04m 51s
Commit git commit c6d6a9c819: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 alexgrozav 🗃️ e2e/*
Committer Alex Grozav
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 481
View all changes introduced in this branch ↗︎

Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Copy link
Contributor

✅ All Cypress E2E specs passed

…ection-when-switching-to-node-that-doesnt-have
Copy link
Contributor

✅ All Cypress E2E specs passed

@alexgrozav alexgrozav merged commit 6330bec into master Dec 17, 2024
37 checks passed
@alexgrozav alexgrozav deleted the cat-403-delete-connection-when-switching-to-node-that-doesnt-have branch December 17, 2024 14:57
@github-actions github-actions bot mentioned this pull request Dec 19, 2024
@janober
Copy link
Member

janober commented Dec 19, 2024

Got released with n8n@1.73.0

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
n8n team Authored by the n8n team Released ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants