codeant-ai-for-open-source[bot] commented on code in PR #42601:
URL: https://github.com/apache/superset/pull/42601#discussion_r3707209611
##########
superset-frontend/src/utils/downloadAsPivotExcel.ts:
##########
@@ -17,12 +17,77 @@
* under the License.
*/
import { utils, writeFile } from 'xlsx';
+import type { WorkSheet } from 'xlsx';
+
+// ISO 8601 date (and optional time) form, e.g. "2024-01-01" or
+// "2024-01-01 00:00:00". This layout is unambiguous under any locale
+// (unlike "1/2/2024", which means different dates depending on the
+// reader), so it's safe to restore as a native Excel date.
+const ISO_DATE_RE = /^(\d{4})-(\d{2})-(\d{2})(?:[
T](\d{2}):(\d{2}):(\d{2}))?$/;
+
+// `raw: true` (used below) keeps every table cell as text, so ordinary
+// numbers and dates lose their native Excel type along with the
+// locale-formatted values. A cell's text is only restored to a real number
+// or date when it is unambiguous under any locale: a plain number that
+// round-trips losslessly through Number() (e.g. "42" or "-3.5"), or an
+// ISO 8601 date/datetime string. Restoring those can't reintroduce the
+// misparsing raw: true guards against. Anything else (grouped thousands,
+// percent suffixes, trailing zero padding, other D3_FORMAT output, etc.)
+// stays as text, exactly as rendered.
+function restoreUnambiguousNumbers(sheet: WorkSheet): void {
+ Object.keys(sheet).forEach(cellRef => {
+ if (cellRef.startsWith('!')) {
+ return;
+ }
+ const cell = sheet[cellRef];
+ if (!cell || cell.t !== 's' || typeof cell.v !== 'string') {
+ return;
+ }
+ const isoMatch = cell.v.match(ISO_DATE_RE);
+ if (isoMatch) {
+ const [y, mo, d, h, mi, s] = isoMatch
+ .slice(1)
+ .map((part: string | undefined) => Number(part ?? 0));
+ const date = new Date(y, mo - 1, d, h, mi, s);
+ // The Date constructor rolls invalid components over into the next
+ // unit (e.g. day 40 becomes the 10th of the following month, minute
+ // 60 becomes the top of the next hour) instead of rejecting them, so
+ // confirm every part - date and time - round-trips before trusting
+ // the result.
+ const isValid =
+ date.getFullYear() === y &&
+ date.getMonth() === mo - 1 &&
+ date.getDate() === d &&
+ date.getHours() === h &&
+ date.getMinutes() === mi &&
+ date.getSeconds() === s;
+ if (isValid) {
+ cell.t = 'd';
+ cell.v = date;
+ return;
+ }
+ }
+ const value = Number(cell.v);
+ if (cell.v !== '' && Number.isFinite(value) && String(value) === cell.v) {
+ cell.t = 'n';
+ cell.v = value;
+ }
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag converting plain numeric strings to numeric Excel cells in
pivot exports; numeric dimension labels are an accepted cosmetic tradeoff when
column metadata is unavailable.
**Applied to:**
- `**/downloadAsPivotExcel.ts`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset-frontend/src/utils/downloadAsPivotExcel.ts:
##########
@@ -17,12 +17,77 @@
* under the License.
*/
import { utils, writeFile } from 'xlsx';
+import type { WorkSheet } from 'xlsx';
+
+// ISO 8601 date (and optional time) form, e.g. "2024-01-01" or
+// "2024-01-01 00:00:00". This layout is unambiguous under any locale
+// (unlike "1/2/2024", which means different dates depending on the
+// reader), so it's safe to restore as a native Excel date.
+const ISO_DATE_RE = /^(\d{4})-(\d{2})-(\d{2})(?:[
T](\d{2}):(\d{2}):(\d{2}))?$/;
+
+// `raw: true` (used below) keeps every table cell as text, so ordinary
+// numbers and dates lose their native Excel type along with the
+// locale-formatted values. A cell's text is only restored to a real number
+// or date when it is unambiguous under any locale: a plain number that
+// round-trips losslessly through Number() (e.g. "42" or "-3.5"), or an
+// ISO 8601 date/datetime string. Restoring those can't reintroduce the
+// misparsing raw: true guards against. Anything else (grouped thousands,
+// percent suffixes, trailing zero padding, other D3_FORMAT output, etc.)
+// stays as text, exactly as rendered.
+function restoreUnambiguousNumbers(sheet: WorkSheet): void {
+ Object.keys(sheet).forEach(cellRef => {
+ if (cellRef.startsWith('!')) {
+ return;
+ }
+ const cell = sheet[cellRef];
+ if (!cell || cell.t !== 's' || typeof cell.v !== 'string') {
+ return;
+ }
+ const isoMatch = cell.v.match(ISO_DATE_RE);
+ if (isoMatch) {
+ const [y, mo, d, h, mi, s] = isoMatch
+ .slice(1)
+ .map((part: string | undefined) => Number(part ?? 0));
+ const date = new Date(y, mo - 1, d, h, mi, s);
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag exact ISO-date restoration in this utility; rendered HTML
lacks cell metadata needed to distinguish date dimensions from text labels, and
the conservative check is intentional to preserve actual date-column conversion.
**Applied to:**
- `superset-frontend/src/utils/downloadAsPivotExcel.ts`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
--
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]
