viharnani commented on PR #43238: URL: https://github.com/apache/superset/pull/43238#issuecomment-5728010371
> @viharnani checking in, remiding that @sadpandajoe's newest review comments remain open: the stale `stackDimension` case where `indexOf` returns -1 and, with multiple metrics, the offset lands on the metric label instead of a real dimension, and using raw dimension values as object keys in both accumulators, so a value like `__proto__` or `constructor` writes onto the prototype instead of creating its own group. Both look like real bugs, not nits, worth fixing before this merges. Holler if you want a hand. @rusackas @sadpandajoe Thanks for following up! Both points have been addressed in the latest push: Stale stackDimension Guard: Added (chartProps.rawFormData.groupby as string[]).includes(stackDimension) in transformProps.ts to ensure that if stackDimension is removed/stale and indexOf returns -1, it falls back safely rather than selecting the metric label. Added a regression test: should fallback gracefully when stackDimension is stale (not in groupby) in transformProps.test.ts. Prototype Safety & Key Collisions (__proto__, constructor, etc.): Initialized accumulators in transformProps.ts (perGroupTotalStackedValues) and series.ts (extractShowValueIndexes) as null-prototype objects via Object.create(null) with Object.prototype.hasOwnProperty.call checks. Hardened transformers.ts with hasOwnProperty and Array.isArray guards before resolving per-group showValue index and total arrays. Added regression unit tests for prototype property names in both series.test.ts and transformProps.test.ts. All unit tests and linters pass cleanly (189/189 tests in targeted suites and 772/772 across plugin-chart-echarts). Ready for your review! -- 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]
