-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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(editor): Add isEmpty on DateTime, add is empty to all types in filter component #9645
feat(editor): Add isEmpty on DateTime, add is empty to all types in filter component #9645
Conversation
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.
Looks good. Added a comment about the tests but good to go otherwise
{ left: 15, expected: false }, | ||
{ left: -15.4, expected: false }, | ||
{ left: NaN, expected: true }, | ||
{ left: null, expected: true }, |
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.
Are these tests needed if the option is hidded for numbers? Also should we add tests for other data types?
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.
It's hidden in the expressions because of the bug, this test is about the filter component. But will add tests for the types that are missing (DateTime,boolean)
|
2 flaky tests on run #5358 ↗︎
Details:
cypress/e2e/5-ndv.cy.ts • 2 flaky tests
Review all test suite changes for PR #9645 ↗︎ |
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.
Looking good. Thanks for adding tests!
✅ All Cypress E2E specs passed |
…ressions-and-filter
✅ All Cypress E2E specs passed |
Got released with |
Summary
isEmpty
to Date/DateTime autocompleteis empty
/is not empty
to all types in filter componentRelated tickets and issues
https://linear.app/n8n/issue/NODE-1375/make-isempty-more-visible-in-expressions-and-filter-component
Review / Merge checklist
(no-changelog)
otherwise. (conventions)