raphaelsales opened a new pull request, #43839:
URL: https://github.com/apache/superset/pull/43839
### SUMMARY
A column holding duration strings — `00:01:54`, `0 days 00:01:54` — is not a
parseable timestamp. It reaches a d3 time formatter as an `Invalid Date` and is
rendered as `NaN:NaN:NaN`, discarding the value the database returned.
Two paths lead there:
- the column is typed **temporal**, so `transformProps` wraps the cell in
`DateWithFormatter` and `formatValue` hands that object to the formatter;
- the column is typed **string** but has a d3 time format set in the column
config, so `transformProps` assigns a time formatter anyway (`isTime ||
config.d3TimeFormat`).
They converge in `stringifyTimeInput`, which formats whatever `Date` it ends
up with, valid or not. The guard therefore lives there rather than in the
plugin: fixing only the table plugin would leave the string-column path broken,
and every other consumer of a time formatter exposed to the same thing.
`stringifyTimeInput` now falls back to the input's own representation when
it does not resolve to a valid date, exactly as it already does for `null` and
`undefined`. `DateWithFormatter.toString()` carries the same guard and returns
the input it already retains — that is also what makes the fallback terminate
for a wrapped value, since coercing the object would otherwise re-enter the
formatter. Both table plugins carry the identical class, so both are covered.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Rendered in Storybook, not the full app. `call_period` is typed temporal
(path A); `call_period_str` is a string column with `%H:%M:%S` set in the
column config (path B). Same data in both columns.
| value from the database | before | after |
| --- | --- | --- |
| `00:01:54` | `NaN:NaN:NaN` | `00:01:54` |
| `0 days 00:01:54` | `NaN:NaN:NaN` | `0 days 00:01:54` |
<!-- PASTE BEFORE/AFTER SCREENSHOTS HERE -->
### TESTING INSTRUCTIONS
From `superset-frontend/`:
```
npx jest packages/superset-ui-core/test/time-format
plugins/plugin-chart-table plugins/plugin-chart-ag-grid-table
```
44 suites, 611 tests. The three test files added here fail on `master` and
pass with this change.
To check by hand: put a column of duration strings (`00:01:54`) in a Table
chart, either typed as temporal in the dataset or typed as string with a time
format set under Customize → column config. Before this change the column
renders `NaN:NaN:NaN`; after it renders the stored value.
### ADDITIONAL INFORMATION
- [x] Has associated issue:
- Fixes #34328
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
