-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
feat(core): Print the name of the migration that cannot be reverted when using n8n db:revert
#9473
Conversation
…hen using `n8n db:revert`
expect(logger.error).toHaveBeenCalledWith( | ||
'The last migration "Test Migration" was irreversible and cannot be reverted.', | ||
); |
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.
Just as with error messages, should we avoid asserting on specific strings?
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.
If you ask me I'd say it depends.
If we just want to make sure that some error is thrown then we can apply DRY here similar to how it already is done with UM_FIX_INSTRUCTION
.
On the other hand, matching against a literal makes sense if the error became part of the external contract and we want to have an extra assertion making sure it's not changed without acknowledging that the contract changed. E.g. if I change a constant and all tests pass I don't expect the FE to break. In that case asserting against a literal creates value because every time I change a test I have to assume that the FE could break.
In this case I can create a constant for the constant error message, for the dynamic one I will have to continue to construct it in the tests.
1 flaky test on run #5115 ↗︎
Details:
cypress/e2e/5-ndv.cy.ts • 1 flaky test
Review all test suite changes for PR #9473 ↗︎ |
✅ All Cypress E2E specs passed |
✅ All Cypress E2E specs passed |
* master: refactor(core): Use consistent CSRF state validation across oAuth controllers (#9104) feat(core): Print the name of the migration that cannot be reverted when using `n8n db:revert` (#9473) fix(editor): Hard load after logout to reset stores (no-changelog) (#9500) refactor(core): Stop reporting `EAUTH` error codes to Sentry (no-changelog) (#9496) fix(core): Upgrade sheetjs to address CVE-2024-22363 (#9498) refactor: Remove skipped tests (no-changelog) (#9497) feat(editor): Add initial code for NodeView and Canvas rewrite (no-changelog) (#9135) fix(editor): Show input panel with not connected message (#9495) fix(editor): Prevent XSS in node-issues tooltip (#9490) # Conflicts: # pnpm-lock.yaml
Got released with |
Summary
Print the name of the migration that cannot be reverted. This should make it easier to figure out which migration was found to be the last one that was applied.
Also now completely refactor the revert command and the tests.
Related tickets and issues
https://n8nio.slack.com/archives/C062YRE7EG4/p1716199306266429?thread_ts=1716198135.426559&cid=C062YRE7EG4
Review / Merge checklist
(no-changelog)
otherwise. (conventions)