-
Notifications
You must be signed in to change notification settings - Fork 16.3k
fix(dashboard): ensure world map chart uses correct country code format in crossfilter #35919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(dashboard): ensure world map chart uses correct country code format in crossfilter #35919
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
|
Heads up to @villebro since we're touching his arch-nemesis file, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
| Duplicated country field resolution logic ▹ view | ||
| Incorrect mapData key lookup for new country field types ▹ view | ||
| Duplicated Country Resolution Logic ▹ view | ||
| Inefficient dictionary creation in loop ▹ view | ||
| Missing key validation for country codes ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js | ✅ |
| superset/viz.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #35919 +/- ##
===========================================
+ Coverage 0 68.74% +68.74%
===========================================
Files 0 622 +622
Lines 0 45718 +45718
Branches 0 4975 +4975
===========================================
+ Hits 0 31428 +31428
- Misses 0 13045 +13045
- Partials 0 1245 +1245
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
villebro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fix!
…at in crossfilter (apache#35919)
SUMMARY
This PR fixes a bug where cross filtering on a world map chart always uses the
cca3country code format, even if other codes (cca2,cioc) were selected as thecountryFieldTypeduring chart creation.superset/viz.pyThe backend currently provides country results to the frontend in the format:
{ "country": "string", "name": "string", // … other fields }The country field is the
cca3code of the country, and name is the country’s full name. country must be sent as acca3code, because the frontend plugin used for maps only recognizescca3country codes. To improve interoperability and make additional code formats (e.g.,cca2,cioc) available without altering the existing response structure, we introduce an additional object fieldcode:{ "country": "string", "name": "string", "code": "string" // … other fields }This approach was chosen because it preserves backward compatibility with existing frontend expectations (country remains
cca3). Thecodefield is retrieved from thecountrymodule like all the other country data, and it usesform_data["country_fieldtype"]to know which field type the user requested.superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.jsThis frontend component handles the world map chart and manages cross-filter interactions through the
getCrossFilterDataMaskfunction, which is triggered when a user clicks on a country. Previously,getCrossFilterDataMaskonly checked ifcountryFieldType == "name".If
countryFieldType == "name", it used the backendnamefield (e.g., "Canada"). Otherwise, it used thecountryfield, which was always incca3format (e.g., "CAN"). Now, instead of falling back tocountry, we fall back tocode, which will be in the format the user selected when the chart was created.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
I created a dataset with
cca2country codes, and created 2 charts with it on the same dashboard.Before
When selecting Canada on chart

CA2-1, nothing is shown on chartCA2-2, even though it also has Canada in the dataset. You can also see on the left sidebar thatCANis used to filter the charts, even thoughcca2is set as the country field type.After
When selecting Canada on chart

CA2-1, it is also filtered on chartCA2-2. On the left sidebar, you can also see thatCAis used to filter the charts.TESTING INSTRUCTIONS
Steps to Verify:
cca2).cca2.Expected Result:
Observe that when you click on a country in one map, the corresponding country is highlighted in all other maps.
ADDITIONAL INFORMATION