Skip to content

fix(chart): include semantic views in datasource filter - #46021

Open
mikebridge wants to merge 2 commits into
apache:masterfrom
mikebridge:sc-131919-charts-datasource-filter
Open

mikebridge wants to merge 2 commits into
apache:masterfrom
mikebridge:sc-131919-charts-datasource-filter

Conversation

@mikebridge

Copy link
Copy Markdown
Contributor

SUMMARY

TL;DR: The Charts list Datasource filter includes semantic views when Semantic Layers is enabled, and selecting a view shows only charts built from that view.

Background: The Charts list already displays and links semantic views in its Datasource column.

Problem: The filter loaded options only from the dataset endpoint. A semantic view could not be found by name, and a numeric datasource ID alone could not distinguish a view from a dataset with the same ID.

Changes: With Semantic Layers enabled, the filter uses the combined datasource list and labels dataset and semantic-view options separately. The selected value includes both datasource type and ID; an additive chart-list search operator matches both. Invalid typed values return no charts. With Semantic Layers disabled, the existing dataset endpoint and numeric equality filter remain in use. The user guide documents the expanded filter.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not included; the filter options and request are covered by RTL tests.

TESTING INSTRUCTIONS

  1. Enable Semantic Layers. Open Charts and search the Datasource filter for a semantic view. Confirm it appears beside dataset options with a type label.
  2. Select the view. Confirm the list contains charts from that view and excludes charts from a dataset with the same numeric ID.
  3. Disable Semantic Layers. Confirm the filter still lists datasets and filters them as before.
  4. Automated checks: four ChartList Jest suites (85 tests), chart-filter Python tests (19 tests), translation-template drift check, and changed-file pre-commit hooks passed.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags: SEMANTIC_LAYERS for semantic-view options
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API: additive chart-list search operator
  • Removes existing feature or API

@github-actions github-actions Bot added api Related to the REST API doc Namespace | Anything related to documentation labels Nov 4, 2026
@netlify

netlify Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 521ba0c
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6aeb97785bc35e9fddbe1c36
😎 Deploy Preview https://deploy-preview-46021--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.10345% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.01%. Comparing base (c0b27ee) to head (521ba0c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
superset/charts/filters.py 88.88% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #46021   +/-   ##
=======================================
  Coverage   85.00%   85.01%           
=======================================
  Files        3080     3080           
  Lines      209252   209278   +26     
  Branches    49292    49300    +8     
=======================================
+ Hits       177881   177910   +29     
+ Misses      27990    27986    -4     
- Partials     3381     3382    +1     
Flag Coverage Δ
hive 34.19% <33.33%> (-0.01%) ⬇️
javascript 81.31% <100.00%> (+<0.01%) ⬆️
mysql 52.01% <33.33%> (-0.01%) ⬇️
postgres 52.03% <33.33%> (-0.01%) ⬇️
presto 35.85% <33.33%> (-0.01%) ⬇️
python 87.96% <88.88%> (-0.01%) ⬇️
sqlite 51.78% <33.33%> (-0.01%) ⬇️
unit 82.90% <88.88%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mikebridge
mikebridge marked this pull request as ready for review November 5, 2026 02:09
Comment on lines +859 to 862
operator: isFeatureEnabled(FeatureFlag.SemanticLayers)
? FilterOperator.ChartDatasource
: FilterOperator.Equals,
unfilteredLabel: t('All'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: With Semantic Layers enabled, saved URLs still pass numeric datasource_id values; ChartDatasourceFilter rejects them, so bookmarked dataset filters return no charts.

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Api mismatch

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset-frontend/src/pages/ChartList/index.tsx
**Line:** 859:862
**Comment:**
	*Api Mismatch: With Semantic Layers enabled, saved URLs still pass numeric `datasource_id` values; `ChartDatasourceFilter` rejects them, so bookmarked dataset filters return no charts.

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.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@codeant-ai-for-open-source

Copy link
Copy Markdown
Contributor

CodeAnt  CodeAnt PR Risk

🟡  Medium Risk

  • The PR needs attention before merging.
  • With Semantic Layers enabled, existing chart-list URLs carrying numeric datasource_id values are rejected, so bookmarked filters show no charts.

Assessed commit: 521ba0caa08d

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the REST API doc Namespace | Anything related to documentation size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant