-
Notifications
You must be signed in to change notification settings - Fork 665
FilterBuilder: fix invalid date formatting (T1319193, T1311486) #32286
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
FilterBuilder: fix invalid date formatting (T1319193, T1311486) #32286
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.
Pull request overview
Fixes FilterBuilder’s handling of invalid date values by preventing attempts to format invalid Date instances and expanding/relocating related test coverage.
Changes:
- Treat invalid
Dateobjects as invalid input informatHelper.format()(returningvalue.toString()instead of passing them to date localization formatting). - Adjust FilterBuilder value formatting to avoid coercing non-string/non-numeric values into
Dateobjects. - Update/add tests: expand
formatHelper.format()coverage and move FilterBuilder formatting tests from QUnit to Jest (plus add a regression case).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/testing/tests/DevExpress.utils/utils.formatHelper.tests.js | Switches to ESM import and adds QUnit coverage for formatHelper.format() edge cases (including invalid dates). |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/filterBuilderParts/utilsTests.js | Removes the old QUnit “Formatting” module tests (migrated to Jest). |
| packages/devextreme/js/format_helper.js | Prevents formatting invalid Date values by validating getTime() before delegating to date localization. |
| packages/devextreme/js/__internal/filter_builder/m_utils.ts | Updates FilterBuilder value-to-text formatting (incl. typed refactor) to avoid unintended Date coercions. |
| packages/devextreme/js/__internal/filter_builder/tests/utils.test.ts | Adds Jest coverage for FilterBuilder value formatting, including an invalid date string regression. |
4e80475 to
c311758
Compare
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.
Pull request overview
Copilot reviewed 9 out of 12 changed files in this pull request and generated 4 comments.
cde9e83 to
003b1d0
Compare
003b1d0 to
779e531
Compare
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.
Pull request overview
Copilot reviewed 9 out of 12 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/filter_builder/m_utils.ts:604
- New
// @ts-expect-errorsuppressions were added for thecustomizeTextpayload. Rather than relying on suppression comments, it would be more robust to define an explicit internal payload type (or cast the payload toanyin one place) so the expected shape is clear and future type changes don't get silently masked.
valueText = field.customizeText.call(field, {
// @ts-expect-error
value,
valueText,
target,
});
| const GRID_CONTAINER = '#container'; | ||
|
|
||
| // T1319193, T1311486 | ||
| test('Proper handle custom filter operations for dates with non-date values', async (t) => { |
Copilot
AI
Jan 26, 2026
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.
Test name grammar: "Proper handle ..." → "Properly handle ...".
| test('Proper handle custom filter operations for dates with non-date values', async (t) => { | |
| test('Properly handle custom filter operations for dates with non-date values', async (t) => { |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
| return this.getHeadersContainer().child(`.${this.addWidgetPrefix(CLASS.filterRangeOverlay)}`); | ||
| } | ||
|
|
||
| getFilterRangeStartEditor(): DateBox { |
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.
The filter range start/end editor can also be NumberBox
…26_1__T1319193-filter-builder-date-format
No description provided.