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(editor): Canvas showing error toast when clicking outside of "import workflow by url" modal #9001

Conversation

RicardoE105
Copy link
Contributor

@RicardoE105 RicardoE105 commented Mar 28, 2024

Summary

FE attempted to import workflow even when the modal was closed - by clicking outside the modal.

Before:

https://www.loom.com/share/67ab4c920c334ecfa7ac1341603c882a

Now:

https://www.loom.com/share/8ce132a9edd2423a9995c05a07982c29

Related tickets and issues

https://linear.app/n8n/issue/ADO-960/closing-import-form-url-modal-by-clicking-outside-causes-error

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)

@RicardoE105 RicardoE105 changed the title fix issue when clicking outside of import workflow by url modal fix(editor): Issue when clicking outside of import workflow by url modal Mar 28, 2024
@RicardoE105 RicardoE105 changed the title fix(editor): Issue when clicking outside of import workflow by url modal fix(editor): Canvas showing error toast when clicking outside of import workflow by url modal Mar 28, 2024
@RicardoE105 RicardoE105 changed the title fix(editor): Canvas showing error toast when clicking outside of import workflow by url modal fix(editor): Canvas showing error toast when clicking outside of "import workflow by url" modal Mar 28, 2024
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Mar 28, 2024
Copy link
Contributor

@MiloradFilipovic MiloradFilipovic 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. I would love to see a quick test for this added also 🙏

Copy link
Contributor

github-actions bot commented Apr 2, 2024

✅ All Cypress E2E specs passed

Copy link

cypress bot commented Apr 2, 2024

2 flaky tests on run #4554 ↗︎

0 345 12 0 Flakiness 2

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 RicardoE105 🗃️ e2e/*
Project: n8n Commit: 0a399bdff9
Status: Passed Duration: 03:51 💡
Started: Apr 4, 2024 9:05 AM Ended: Apr 4, 2024 9:09 AM
Flakiness  5-ndv.cy.ts • 1 flaky test

View Output Video

Test Artifacts
NDV > should not retrieve remote options when required params throw errors Test Replay Screenshots Video
Flakiness  17-sharing.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Sharing > should work for admin role on credentials created by others (also can share it with themselves) Test Replay Screenshots Video

Review all test suite changes for PR #9001 ↗︎

Copy link
Contributor

@MiloradFilipovic MiloradFilipovic left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for addressing. One nitpick though: Does it make sense to move other tests that are testing workflow import (like this one) to this file also? Now we are having them in a few places which can be confusing

Copy link
Contributor

github-actions bot commented Apr 4, 2024

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

@RicardoE105
Copy link
Contributor Author

Good catch, @MiloradFilipovic. That is most likely the same test. I will probably delete the one you mentioned and keep this separate file.

Copy link
Contributor

@MiloradFilipovic MiloradFilipovic left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for addressing.

Copy link
Contributor

github-actions bot commented Apr 4, 2024

✅ All Cypress E2E specs passed

@RicardoE105 RicardoE105 merged commit f6ce81e into master Apr 4, 2024
28 checks passed
@RicardoE105 RicardoE105 deleted the ado-960-closing-import-form-url-modal-by-clicking-outside-causes branch April 4, 2024 09:15
MiloradFilipovic added a commit that referenced this pull request Apr 4, 2024
* master:
  fix(core): Ensure only leader handles waiting executions (#9014)
  fix(editor): Fix execution with wait node (#9051)
  fix(editor): Issue showing Auth2 callback section when all properties are overriden (#8999)
  fix(editor): Rerun failed nodes in manual executions (#9050)
  fix(editor): Canvas showing error toast when clicking outside of "import workflow by url" modal (#9001)
  fix: Workflows executed from other workflows not stopping (#9010)
  fix: Fix missing input panel in node details view (#9043)
  fix(editor): Prevent saving workflow while another save is in progress (#9048)
despairblue pushed a commit that referenced this pull request Apr 4, 2024
@github-actions github-actions bot mentioned this pull request Apr 4, 2024
despairblue pushed a commit that referenced this pull request Apr 4, 2024
@janober
Copy link
Member

janober commented Apr 5, 2024

Got released with n8n@1.36.1

@github-actions github-actions bot mentioned this pull request Apr 10, 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 Released ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants