codeant-ai-for-open-source[bot] commented on code in PR #42601:
URL: https://github.com/apache/superset/pull/42601#discussion_r3707229840
##########
superset-frontend/src/utils/downloadAsPivotExcel.ts:
##########
@@ -17,12 +17,84 @@
* 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));
+ // SheetJS serializes a cell's Date via its absolute (UTC) epoch time,
+ // not its local calendar fields, so building this from local
+ // components would silently shift the exported value by the host's
+ // UTC offset (e.g. midnight in a positive-offset timezone would
+ // export as the previous day). Constructing - and validating - via
+ // UTC keeps the exported date/time identical to the source text
+ // regardless of the host's timezone.
+ const date = new Date(Date.UTC(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.getUTCFullYear() === y &&
+ date.getUTCMonth() === mo - 1 &&
+ date.getUTCDate() === d &&
+ date.getUTCHours() === h &&
+ date.getUTCMinutes() === mi &&
+ date.getUTCSeconds() === 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:
**Suggestion:** The sheet contains both aggregate values and pivot
dimension/header labels, but this conversion treats every matching string cell
as a numeric value. A categorical label such as `42` or `2024-01-01` is
therefore changed into a native Excel number/date, altering its type and
potentially its sorting, filtering, and displayed semantics in the exported
workbook. Restrict restoration to cells known to represent measures, or
preserve all rendered pivot text to avoid changing dimension labels. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Pivot dimension labels can become numeric Excel cells.
- ⚠️ ISO-shaped labels can become native Excel dates.
- ⚠️ Excel sorting and filtering semantics may change.
- ⚠️ Pivoted Excel export no longer preserves all rendered text.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e236e9bea69d4cf09ed369447e826801&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e236e9bea69d4cf09ed369447e826801&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/utils/downloadAsPivotExcel.ts
**Line:** 77:81
**Comment:**
*Logic Error: The sheet contains both aggregate values and pivot
dimension/header labels, but this conversion treats every matching string cell
as a numeric value. A categorical label such as `42` or `2024-01-01` is
therefore changed into a native Excel number/date, altering its type and
potentially its sorting, filtering, and displayed semantics in the exported
workbook. Restrict restoration to cells known to represent measures, or
preserve all rendered pivot text to avoid changing dimension labels.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42601&comment_hash=5b9546fbdedc04bc40acbcf691e9f3ba54fb60836ca306226311bced48bc2ecb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42601&comment_hash=5b9546fbdedc04bc40acbcf691e9f3ba54fb60836ca306226311bced48bc2ecb&reaction=dislike'>👎</a>
--
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]
