EnxDev commented on PR #41386:
URL: https://github.com/apache/superset/pull/41386#issuecomment-4877027441
Follow-up on the review findings — addressed in 968e78b…0e3a23c.
**Function \`aggFunc\` guard** — added, with one correction to the premise:
without the guard, a function aggFunc serializes as a *dropped key*
(\`{"colId":"sales"}\`) while "None" serializes as explicit \`"aggFunc":null\`,
so the two never actually collapsed. The guard's value is that the signature no
longer depends on \`JSON.stringify\` silently dropping function values:
functions map to a fixed \`'custom'\` marker. The comment documents this as
defensive-only — AG Grid requires registered string names in serialized column
state, so this plugin only ever persists strings. A unit test pins
function-vs-None distinctness.
**Typed fixtures** — done; fixtures are contextually checked against
\`ColumnState[]\`, all \`as any\` removed.
**Round-trip coverage** — implemented at different seams, deliberately. A
jsdom save→reconcile→applyColumnState round-trip cannot observe aggFunc
restoration in this repo: aggregation column state requires AG Grid's
enterprise \`SharedAggregation\` module (RowGrouping/Pivot/TreeData/SSRM), and
both the jest harness and \`App.tsx\` register community modules only. Probed
empirically: a restored \`aggFunc: 'sum'\` reads back as \`null\` from
\`getColumnState()\`, so the literal test would pass whether or not the fix
works. Instead: (a) a unit test pins aggFunc pass-through (incl. explicit
\`null\`) in \`reconcileColumnState\` — the restore leg this codebase owns; (b)
a component test drives restore → debounced save end to end and asserts every
persisted column entry carries an explicit \`aggFunc\` key, which also covers
the codecov-flagged \`getColumnStateSignature(...)\` line. The apply/read-back
boundary is AG Grid enterprise's contract, only exercisable in enterprise-en
abled builds.
--
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]
