fix(editor): Prevent creation of input connections for nodes without input slot #5425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
n8n-assistant
bot
added
n8n team
OlegIvaniv
commented
Feb 9, 2023
| @@ -239,6 +239,14 @@ export class Start extends Command { | |||
| const { flags } = this.parse(Start); | |||
|
|
|||
| try { | |||
| // Load all node and credential types | |||
There was a problem hiding this comment.
We had to move the loading of nodes and credentials up to make sure they are available in the migrations. @netroy Do you see any issues with this?
There was a problem hiding this comment.
This should be fine. I don't foresee any issues.
krynble
suggested changes
Feb 9, 2023
packages/cli/src/databases/migrations/mysqldb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/databases/migrations/mysqldb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
...ges/cli/src/databases/migrations/postgresdb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
...ges/cli/src/databases/migrations/postgresdb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/databases/migrations/sqlite/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/databases/migrations/sqlite/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
|
Another thing we could do is remove the duplicate code off the migration files to a helper function, but this is not urgent. It won't be reused so it's fine. |
krynble
suggested changes
Feb 9, 2023
packages/cli/src/databases/migrations/mysqldb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
...ges/cli/src/databases/migrations/postgresdb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/databases/migrations/mysqldb/1675940580449-PurgeInvalidWorkflowConnections.ts
Outdated
Show resolved
Hide resolved
krynble
approved these changes
Feb 9, 2023
janober
pushed a commit
that referenced
this pull request
Feb 9, 2023
…input slot (#5425) * fix(editor): Prevent creation of input connections for nodes without input * WIP: Workflow checks service and controller * fix: Created SQLite migration to remove broken connections * Cleanup & add mysql/posgres migrations * Linter fixes * Unify the migration scripts * Escape migration workflow_entity * Wrap the migration in try/catch and do not parse nodes and connection if mysql/postgres * Do migration changes also fro mysql * refactor: Wrap only the necessary call in try catch block --------- Co-authored-by: Omar Ajoue <krynble@gmail.com>
janober
pushed a commit
that referenced
this pull request
Feb 9, 2023
…input slot (#5425) * fix(editor): Prevent creation of input connections for nodes without input * WIP: Workflow checks service and controller * fix: Created SQLite migration to remove broken connections * Cleanup & add mysql/posgres migrations * Linter fixes * Unify the migration scripts * Escape migration workflow_entity * Wrap the migration in try/catch and do not parse nodes and connection if mysql/postgres * Do migration changes also fro mysql * refactor: Wrap only the necessary call in try catch block --------- Co-authored-by: Omar Ajoue <krynble@gmail.com>
|
Got released with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the recent changes to
__addConnectionwe've changed the previous if/else condition to just if. Doing this, thethis.workflowsStore.addConnection({ connection });call would always be executed no matter if the connection could be made or not. So when a user would have selected a regular node and added a trigger node, these would be connected even though the trigger node has no inputs.This PR fixes it by removing this call completely since
addVisualConnectionparameter is always sent withtrueanyway. Without forcingthis.workflowsStore.addConnectioncall, the connection will be made only if the target node has a valid input.For already broken workflows, there is migration that will detect corrupted connections and purge them.
Github issue / Community forum post (link here to close automatically): https://community.n8n.io/t/schedule-trigger-node-is-getting-triggered-multiple-times-during-a-workflow/22822