You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/08/03 23:06:28 UTC
[PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
rusackas opened a new pull request, #42734:
URL: https://github.com/apache/superset/pull/42734
### SUMMARY
Adds a regression test for #34748, which reports that a Deck.gl GeoJSON chart only ever renders a single feature on the map, no matter how many rows the query returns (the reporter found that capping the row limit to 1 made no visible difference).
Traced the pipeline: `Geojson/transformProps.ts` maps every row in `queriesData[0].data` into its own parsed Feature (`records.map(...)`), and `Geojson.tsx`'s `recurseGeoJson` correctly walks the resulting `{ features: [...] }` object, pushing every feature it finds. This looks like it was fixed by the frontend refactor in #39906, which moved GeoJSON row parsing out of the legacy backend `viz.py` `DeckGeoJson.get_properties` path (what the reporters, on 5.0.0/6.0.0, were most likely hitting) and into this per-row frontend transform.
The new test feeds `transformProps` a 3-row payload, each row a distinct GeoJSON `Feature` string, and asserts all 3 come out the other end. It passes on current `master`, proving the bug is already fixed.
### TESTING INSTRUCTIONS
```
cd superset-frontend
npm run test -- plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.test.ts
```
### ADDITIONAL INFORMATION
- [x] Has associated issue: Closes #34748
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42734:
URL: https://github.com/apache/superset/pull/42734#discussion_r3727842492
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.test.ts:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { ChartProps, DatasourceType } from '@superset-ui/core';
+import transformProps from './transformProps';
+
+// Regression for https://github.com/apache/superset/issues/34748: reporters
+// on 5.0.0/6.0.0 found that only a single row's GeoJSON Feature ever
+// rendered on the map, no matter how many rows the query returned (setting
+// the row limit to 1 made no visible difference).
+test('transformProps builds a feature for every row, not just the first', () => {
Review Comment:
Fair point, added a getLayer test that feeds a 3-feature payload through recurseGeoJson and checks all 3 land in GeoJsonLayer.data, so the render boundary is covered too.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42734:
URL: https://github.com/apache/superset/pull/42734#issuecomment-5172679998
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42734:
URL: https://github.com/apache/superset/pull/42734#issuecomment-5205070154
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42734:
URL: https://github.com/apache/superset/pull/42734#issuecomment-5203261906
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 6b6095184154a6100f506dff19ef133e0fd2041d |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a745d6e657ce600080c8a1b |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-42734--superset-docs-preview.netlify.app](https://deploy-preview-42734--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctLDoMgEADQu8y6RJoikN6gx0Bmglh0CJ8YY3r3snubd0MuvOcGb4AHLMUdfh1uiFOstZN4KaOsQPLfkEQg3iofYu-pxcLnKEg58fXBkbQzakZNejaetJTSSm_dc4HfHw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42734--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #42734:
URL: https://github.com/apache/superset/pull/42734#discussion_r3727121841
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.test.ts:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { ChartProps, DatasourceType } from '@superset-ui/core';
+import transformProps from './transformProps';
+
+// Regression for https://github.com/apache/superset/issues/34748: reporters
+// on 5.0.0/6.0.0 found that only a single row's GeoJSON Feature ever
+// rendered on the map, no matter how many rows the query returned (setting
+// the row limit to 1 made no visible difference).
+test('transformProps builds a feature for every row, not just the first', () => {
Review Comment:
This can close #34748 while leaving the reported failure untested: the legacy backend already emitted one feature per row, and this assertion stops before `recurseGeoJson`/`GeoJsonLayer`, where payloads can still be dropped. Could this exercise the consumer/render boundary, or avoid `Closes` until the actual failing boundary is identified?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42734:
URL: https://github.com/apache/superset/pull/42734#issuecomment-5172753553
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42734?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 65.58%. Comparing base ([`25ab961`](https://app.codecov.io/gh/apache/superset/commit/25ab96188f1bea54029345ac79f4cc3b05a4d1d4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`974a221`](https://app.codecov.io/gh/apache/superset/commit/974a221920f2fb174a3067370ab01ace7b7e206d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42734 +/- ##
==========================================
+ Coverage 65.57% 65.58% +0.01%
==========================================
Files 2818 2818
Lines 160038 160038
Branches 36557 36557
==========================================
+ Hits 104938 104955 +17
+ Misses 53054 53037 -17
Partials 2046 2046
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42734/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42734/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.77% <ΓΈ> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42734?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #42734:
URL: https://github.com/apache/superset/pull/42734
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(deckgl): pin GeoJSON layer against multi-row payloads (#34748) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42734:
URL: https://github.com/apache/superset/pull/42734#issuecomment-5202199862
<!-- Bito Reply -->
The test case provided in the PR specifically targets the `transformProps` function to ensure it correctly processes multiple rows of GeoJSON data, addressing the issue where only the first row was being rendered. While this test verifies that `transformProps` correctly aggregates features from the query data, it does not exercise the subsequent rendering pipeline (e.g., `recurseGeoJson` or the `GeoJsonLayer` component itself).
If the underlying issue involves data being dropped at the consumer/render boundary rather than in the transformation logic, this test will pass while the production failure persists. It is advisable to keep the `Closes` reference if the transformation logic was indeed the root cause, but if there is uncertainty about the boundary, adding an integration test or a component-level test that renders the layer would provide more robust verification.
**superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.test.ts**
```
const result = transformProps(mockChartProps as ChartProps);
const features = result.payload.data.features as Array<{
geometry: { coordinates: number[] };
}>;
expect(features).toHaveLength(3);
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org