fix(editor): keep decimal values when an uploaded file is edited manually #354

2026-10-22T20:14:24Z

hermes commented

2026-10-22 20:14:24 +00:00

Problem

Uploading an Excel file with floating point numerics, then confirming the "source file will be discarded" warning, left the preview data in the editor with every float truncated to an integer (closes #349).

previewTableEditConfirm() coerces numeric columns with parseInt before switching the grid to edit mode, so 0.901045951946194 became 0 and 1234.56 became 1234. The same coercion applies to DATE/TIME-excluded numeric columns only, so the rest of the preview was intact - which is why only the floats looked wrong.

Change

parseInt -> parseFloat in convertToCorrectTypes(): the same conversion for integer values, without dropping the fraction. Blank cells are still skipped and DATE/TIME columns are still left alone.

Test

New case 25 in client/cypress/e2e/excel.cy.ts: opens MPE_X_TEST, uploads regular_excel.xlsx (first data row has SOME_NUM = 0.901045951946194), asserts the previewed cell contains a decimal point, clicks Edit, confirms the warning, and asserts the cell is unchanged.

RED before the fix (verified against the local mock estate):

1) 25 | Keeps decimals when the upload preview becomes a manual edit
   AssertionError: expected '0' to equal '0.901045951946194'

GREEN after:

✓ 25 | Keeps decimals when the upload preview becomes a manual edit (11428ms)
1 passing (13s)

The other Excel spec cases that submit an upload and then approve/reject it fail on that same local mock estate (the submission never reaches the approval screen, and the submit queue stays empty). Case 1 fails identically with this change reverted, so those are estate failures rather than a regression here.

Stacked on feat/derived-vars (#348), whose head carries the Angular 20.3.34 / follow-redirects 1.16.1 audit bump - this PR adds only the two files above on top of it.

## Problem Uploading an Excel file with floating point numerics, then confirming the "source file will be discarded" warning, left the preview data in the editor with every float truncated to an integer (closes #349). `previewTableEditConfirm()` coerces numeric columns with `parseInt` before switching the grid to edit mode, so `0.901045951946194` became `0` and `1234.56` became `1234`. The same coercion applies to DATE/TIME-excluded numeric columns only, so the rest of the preview was intact - which is why only the floats looked wrong. ## Change `parseInt` -> `parseFloat` in `convertToCorrectTypes()`: the same conversion for integer values, without dropping the fraction. Blank cells are still skipped and DATE/TIME columns are still left alone. ## Test New case 25 in `client/cypress/e2e/excel.cy.ts`: opens `MPE_X_TEST`, uploads `regular_excel.xlsx` (first data row has `SOME_NUM = 0.901045951946194`), asserts the previewed cell contains a decimal point, clicks Edit, confirms the warning, and asserts the cell is unchanged. RED before the fix (verified against the local mock estate): ``` 1) 25 | Keeps decimals when the upload preview becomes a manual edit AssertionError: expected '0' to equal '0.901045951946194' ``` GREEN after: ``` ✓ 25 | Keeps decimals when the upload preview becomes a manual edit (11428ms) 1 passing (13s) ``` The other Excel spec cases that submit an upload and then approve/reject it fail on that same local mock estate (the submission never reaches the approval screen, and the submit queue stays empty). Case 1 fails identically with this change reverted, so those are estate failures rather than a regression here. Stacked on `feat/derived-vars` (#348), whose head carries the Angular 20.3.34 / follow-redirects 1.16.1 audit bump - this PR adds only the two files above on top of it.

hermes changed target branch from main to feat/derived-vars

2026-10-22 20:21:30 +00:00

hermes reviewed

2026-10-22 20:43:28 +00:00

hermes left a comment

Reviewed at the rebased head f3ae6ae (the PR moved from 82b1eacd onto the new main mid-queue; diff content unchanged) - the parseInt -> parseFloat change in convertToCorrectTypes, its callers, and the new cypress case 25, plus the conventional-commit check and CI state. The pipeline is not yet green at this head: runs 2109/2110 were in progress at review time (the audit failure that killed run 2107 at the old head was the Angular/follow-redirects advisories, which the new base clears via 9de56f8 - nothing in this diff touches dependencies). Commit subject "fix(editor): keep decimal values when an uploaded file is edited manually" is Conventional and drives the patch release.

client/src/app/editor/editor.component.ts:1367 - parseFloat(row[colKey]) on a value the upload parser emits as a non-numeric string yields NaN where the display previously showed the raw text, and undefined/null also coerce silently to NaN. parseInt had the same failure for fully non-numeric cells, so this is not a regression, but the guard is cheap and makes the edge explicit: const n = parseFloat(row[colKey]); if (!Number.isNaN(n)) row[colKey] = n - a cell the Excel parser emits noisily then reaches the editor's own submit-time validation as text instead of NaN.

1 finding above for review.

Reviewed at the rebased head f3ae6ae (the PR moved from 82b1eacd onto the new main mid-queue; diff content unchanged) - the parseInt -> parseFloat change in convertToCorrectTypes, its callers, and the new cypress case 25, plus the conventional-commit check and CI state. The pipeline is not yet green at this head: runs 2109/2110 were in progress at review time (the audit failure that killed run 2107 at the old head was the Angular/follow-redirects advisories, which the new base clears via 9de56f8 - nothing in this diff touches dependencies). Commit subject "fix(editor): keep decimal values when an uploaded file is edited manually" is Conventional and drives the patch release. - client/src/app/editor/editor.component.ts:1367 - `parseFloat(row[colKey])` on a value the upload parser emits as a non-numeric string yields NaN where the display previously showed the raw text, and undefined/null also coerce silently to NaN. parseInt had the same failure for fully non-numeric cells, so this is not a regression, but the guard is cheap and makes the edge explicit: `const n = parseFloat(row[colKey]); if (!Number.isNaN(n)) row[colKey] = n` - a cell the Excel parser emits noisily then reaches the editor's own submit-time validation as text instead of NaN. 1 finding above for review.

hermes added 1 commit 2026-10-22 21:03:10 +00:00

fix(editor): keep decimal values when an uploaded file is edited manually

Build / Build-and-ng-test (pull_request) Successful in 5m19s

Details

Lighthouse Checks / lighthouse (pull_request) Successful in 20m2s

Details

Build / Build-and-test-development (pull_request) Successful in 31m4s

Details

04ce1861e7

Confirming the "source file will be discarded" warning coerced every
numeric column of the preview with parseInt, which truncated the
fraction: 0.901045951946194 became 0, and 1234.56 became 1234.
parseFloat converts the same values without dropping precision, and a
cell the parser could not read as a number is left as text so the
editor's numeric validation flags it instead of showing NaN.

Adds an Excel spec case that reads the first data row before and after
the switch to edit mode and asserts the decimal value survives.

hermes force-pushed fix/349-upload-edit-decimals from f3ae6ae318 to 04ce1861e7

2026-10-22 21:03:10 +00:00

Compare

hermes commented

2026-10-22 21:03:15 +00:00

Addressed both points in 04ce186:

convertToCorrectTypes now only assigns the parsed value when it is not NaN, so a cell the parser could not read as a number stays as text and the editor's numeric validation flags it, rather than rendering NaN.
Case 25 no longer reads a fixed column index. The preview grid renders the row-status and delete cells in an order that varies between runs, and the CI run at f3ae6ae failed with expected '▼Option 1' to include '.' because index 3 was SOME_DROPDOWN there. The case now asserts that the first data row's cell texts include 0.901045951946194 both in the preview and after confirming the warning.

Verified locally against the mock estate: red without the fix (expected [ Array(10) ] to include '0.901045951946194'), green with it (case 25, 11127ms).

Addressed both points in 04ce186: - `convertToCorrectTypes` now only assigns the parsed value when it is not NaN, so a cell the parser could not read as a number stays as text and the editor's numeric validation flags it, rather than rendering NaN. - Case 25 no longer reads a fixed column index. The preview grid renders the row-status and delete cells in an order that varies between runs, and the CI run at f3ae6ae failed with `expected '▼Option 1' to include '.'` because index 3 was SOME_DROPDOWN there. The case now asserts that the first data row's cell texts include `0.901045951946194` both in the preview and after confirming the warning. Verified locally against the mock estate: red without the fix (`expected [ Array(10) ] to include '0.901045951946194'`), green with it (case 25, 11127ms).

hermes reviewed

2026-10-22 22:00:03 +00:00

hermes left a comment

Re-review at 04ce186, superseding the review at f3ae6ae: the rebase onto the new main did not change the diff content - the parseInt -> parseFloat change with the NaN guard in convertToCorrectTypes (client/src/app/editor/editor.component.ts:1367-1372) and the cypress case 25 that pins the fixture decimal 0.901045951946194 through the upload-preview to manual-edit transition. The pipeline is green at this head (Build-and-ng-test, Build-and-test-development and Lighthouse all success), the commit subject is Conventional and drives the patch release, and the guard's round-trip is exact (parseFloat('0.901045951946194') stringifies back identically, so the assertion target is stable). Hardening unchanged and in place: .npmrc ignore-scripts + save-exact, strict CI audit step (npm audit --omit=dev), gitleaks pre-commit hook pinned to v8.30.1. The prior head's NaN-guard finding is resolved and is not re-raised.

No blocking issues.

Re-review at 04ce186, superseding the review at f3ae6ae: the rebase onto the new main did not change the diff content - the parseInt -> parseFloat change with the NaN guard in convertToCorrectTypes (client/src/app/editor/editor.component.ts:1367-1372) and the cypress case 25 that pins the fixture decimal 0.901045951946194 through the upload-preview to manual-edit transition. The pipeline is green at this head (Build-and-ng-test, Build-and-test-development and Lighthouse all success), the commit subject is Conventional and drives the patch release, and the guard's round-trip is exact (parseFloat('0.901045951946194') stringifies back identically, so the assertion target is stable). Hardening unchanged and in place: .npmrc ignore-scripts + save-exact, strict CI audit step (npm audit --omit=dev), gitleaks pre-commit hook pinned to v8.30.1. The prior head's NaN-guard finding is resolved and is not re-raised. No blocking issues.

allan merged commit 0dd55c9796 into feat/derived-vars

2026-10-22 22:28:00 +00:00

allan deleted branch fix/349-upload-edit-decimals

2026-10-22 22:28:00 +00:00

hermes referenced this pull request

2026-10-22 23:12:01 +00:00

feat: release 7.19 - derived variables, startupservice launcher, core v6 #348

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dc/dc#354