bito-code-review[bot] commented on code in PR #39906:
URL: https://github.com/apache/superset/pull/39906#discussion_r3199840464
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/buildQuery.ts:
##########
@@ -61,6 +63,10 @@ export default function buildQuery(formData:
DeckGeoJsonFormData) {
const withJsColumns = addJsColumnsToColumns(columnStrings, js_columns);
columns = withJsColumns as QueryFormColumn[];
+ if (cross_filter_column && !columns.includes(cross_filter_column)) {
+ columns.push(cross_filter_column);
+ }
Review Comment:
<!-- Bito Reply -->
The reasoning is sound: `addJsColumnsToColumns` returns `string[]`, so
`columns` is a string array at the `includes` check. The `as QueryFormColumn[]`
cast is for TypeScript only. Tooltip columns (as `QueryFormColumn` objects) are
added afterward, so the original code works correctly without the suggested
`some` check.
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.ts:
##########
@@ -32,14 +32,25 @@ export default function transformProps(chartProps:
ChartProps) {
}
const records = getRecordsFromQuery(chartProps.queriesData);
+ const crossFilterCol = formData.cross_filter_column || undefined;
// Parse each record's geojson column value (replicates backend
DeckGeoJson.get_properties)
const features = records
.map((record: DataRecord) => {
const geojsonStr = record[geojsonCol];
if (geojsonStr == null) return null;
try {
- return JSON.parse(String(geojsonStr));
+ const feature = JSON.parse(String(geojsonStr));
+ // Surface cross_filter_column from the row onto feature.properties so
+ // that picking can emit a dimension filter even when the GeoJSON blob
+ // doesn't carry the column itself.
+ if (crossFilterCol && record[crossFilterCol] !== undefined) {
+ feature.properties = {
+ ...feature.properties,
+ [crossFilterCol]: record[crossFilterCol],
+ };
Review Comment:
<!-- Bito Reply -->
The code in transformProps.ts does wire cross_filter_column into
feature.properties using a computed property name, addressing the contract
mismatch. Your explanation on prototype pollution is accurate: computed keys
don't mutate __proto__, and spread is safe. Since crossFilterCol comes from
authenticated column-picker, __proto__ injection isn't possible. The concern is
resolved.
--
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]
