codeant-ai-for-open-source[bot] commented on code in PR #37027:
URL: https://github.com/apache/superset/pull/37027#discussion_r2678328013
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/transformProps.test.ts:
##########
@@ -257,4 +288,23 @@ describe('Polygon transformProps', () => {
expect(features).toHaveLength(1);
expect(features[0]?.elevation).toBeUndefined();
});
+
+ test('should handle geohash decoding successfully', () => {
+ const props = {
+ ...mockedChartPropsWithGeoHash,
+ rawFormData: {
+ ...mockedChartPropsWithGeoHash.rawFormData,
+ point_radius_fixed: {
+ type: 'fix',
+ value: '1000',
+ },
+ },
+ };
+
+ const result = transformProps(props as ChartProps);
+
+ const features = result.payload.data.features as PolygonFeature[];
+ expect(features.flatMap(p => p?.polygon)).toHaveLength(20); // 4 geohashes
x 5 corners each
Review Comment:
**Suggestion:** The test uses `features.flatMap(p => p?.polygon)` which can
return `undefined` for features without a `polygon` property; `flatMap` will
include that `undefined` in the result and make the length assertion flaky or
incorrect. Return an empty array when `polygon` is missing so `flatMap` always
receives an array. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
expect(features.flatMap(p => p?.polygon ?? [])).toHaveLength(20); // 4
geohashes x 5 corners each
```
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/transformProps.test.ts
**Line:** 307:307
**Comment:**
*Logic Error: The test uses `features.flatMap(p => p?.polygon)` which
can return `undefined` for features without a `polygon` property; `flatMap`
will include that `undefined` in the result and make the length assertion flaky
or incorrect. Return an empty array when `polygon` is missing so `flatMap`
always receives an array.
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.
```
</details>
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/transformProps.ts:
##########
@@ -163,6 +174,7 @@ function processPolygonData(
}
export default function transformProps(chartProps: ChartProps) {
+ console.log({ chartProps });
Review Comment:
**Suggestion:** Debug logging left in production: `console.log({ chartProps
})` can leak sensitive chart or data properties; remove it or replace with a
gated/debug logger to avoid exposing data in production consoles. [security]
**Severity Level:** Critical 🚨
```suggestion
// debug logging intentionally removed in production; use a debug logger
if needed
```
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/transformProps.ts
**Line:** 177:177
**Comment:**
*Security: Debug logging left in production: `console.log({ chartProps
})` can leak sensitive chart or data properties; remove it or replace with a
gated/debug logger to avoid exposing data in production consoles.
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.
```
</details>
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/transformProps.ts:
##########
@@ -122,6 +123,16 @@ function processPolygonData(
break;
}
case 'geohash':
+ polygonCoords = [];
+ const decoded = decode_bbox(String(rawPolygonData));
+ if (decoded) {
+ polygonCoords.push([decoded[1], decoded[0]]); // SW
+ polygonCoords.push([decoded[3], decoded[0]]); // NW
+ polygonCoords.push([decoded[3], decoded[2]]); // NE
+ polygonCoords.push([decoded[1], decoded[2]]); // SE
+ polygonCoords.push([decoded[1], decoded[0]]); // SW
+ }
+ break;
Review Comment:
**Suggestion:** Incorrect bounding-box index mapping: the code mixes up the
min/max lat/lon indices when building corner coordinates (NW and SE are
swapped). This will produce an incorrectly ordered polygon (corners in wrong
positions), which can display inverted or malformed polygons. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
case 'geohash': {
polygonCoords = [];
const decoded = decode_bbox(String(rawPolygonData));
if (decoded) {
// decode_bbox returns [minLat, minLon, maxLat, maxLon]
polygonCoords.push([decoded[1], decoded[0]]); // SW (minLon,
minLat)
polygonCoords.push([decoded[1], decoded[2]]); // NW (minLon,
maxLat)
polygonCoords.push([decoded[3], decoded[2]]); // NE (maxLon,
maxLat)
polygonCoords.push([decoded[3], decoded[0]]); // SE (maxLon,
minLat)
polygonCoords.push([decoded[1], decoded[0]]); // SW (close
polygon)
}
break;
}
```
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/transformProps.ts
**Line:** 125:135
**Comment:**
*Logic Error: Incorrect bounding-box index mapping: the code mixes up
the min/max lat/lon indices when building corner coordinates (NW and SE are
swapped). This will produce an incorrectly ordered polygon (corners in wrong
positions), which can display inverted or malformed polygons.
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.
```
</details>
--
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]
