-
Notifications
You must be signed in to change notification settings - Fork 9k
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(core): Fix update workflow cli command being unable to activate all workflows #8412
Conversation
6d5b495
to
6173a03
Compare
await Container.get(WorkflowRepository).deactivateAll(); | ||
this.logger.info(`${action} all workflows`); | ||
if (newState) { | ||
// TODO: Maybe there should be an `updateActiveStateForAll(state)`? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if that makes sense? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the existing deactivateAll
is only used here, it's fine if we rename it and make it more generic so we can just use a single flag, but either work. The current naming is also very explicit and clear in what it does.
6173a03
to
86e9b97
Compare
can be changed later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
2 flaky tests on run #3872 ↗︎
Details:
5-ndv.cy.ts • 1 flaky test
24-ndv-paired-item.cy.ts • 1 flaky test
Review all test suite changes for PR #8412 ↗︎ |
✅ All Cypress E2E specs passed |
Got released with |
Summary
This work is based on #6759, thank you @schroedan for tackling this ❤️
Before this change activating all workflows led to all workflows being deactivated instead:
Now it actually activates them:
This also adds
WorkflowRepository#activateAll
and a test for it, as well as a test forWorkflowRepository#deactivateAll
Related tickets and issues
https://linear.app/n8n/issue/PAY-1206/cli-activating-workflows-fails
https://community.n8n.io/t/setting-all-workflows-to-active-not-working/34901
#4753
#6759
Review / Merge checklist
(no-changelog)
otherwise. (conventions)