Skip to content

Add product name filter to product dashboard#2189

Merged
KatelynGit merged 5 commits into
mainfrom
add-product-name-filter-to-product-dash
Apr 29, 2026
Merged

Add product name filter to product dashboard#2189
KatelynGit merged 5 commits into
mainfrom
add-product-name-filter-to-product-dash

Conversation

@KatelynGit

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

https://github.com/mitodl/hq/issues/9332

Description (What does it do?)

Add product name filter to product dashboard

How can this be tested?

review qa and yaml

Copilot AI review requested due to automatic review settings April 29, 2026 15:25

Copilot AI left a comment

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.

Pull request overview

Adds a new “Product Name” native filter to the existing Superset “Products” dashboard YAML asset, to support filtering the product dashboard by product name (per hq#9332).

Changes:

  • Adds a new NATIVE_FILTER targeting product_name on the marts__combined__products dataset.
  • Updates dashboard chart configuration / chart IDs referenced in metadata and position.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +28
'121':
crossFilters:
chartsInScope:
- 67
- 82
scope: global
id: 112
'67':
id: 121
'138':
crossFilters:
chartsInScope:
- 82
- 67
- 112
scope: global
id: 67
'82':
id: 138
'154':
crossFilters:
chartsInScope:
- 67
- 82
- 112
scope: global
id: 82
id: 154

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

metadata.chart_configuration now uses chart IDs 121/138/154, but the chartsInScope arrays still reference 67/82/112. Those IDs no longer match the charts referenced in this dashboard file (see position.*.meta.chartId), so cross-filter scoping will be incorrect/broken. Please update chartsInScope values to the correct chart IDs (and ensure any other chartId references in this dashboard are updated consistently).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Fixed in dc4dd07. Updated chartsInScope in chart_configuration and global_chart_configuration to use the correct chart IDs (121/138/154 instead of the stale 67/82/112). Also renamed layout component IDs CHART-explore-112-1CHART-explore-121-1 and CHART-explore-82-1CHART-explore-138-1 (with all children/reference updates) so component IDs consistently match their meta.chartId values.

Comment on lines 247 to 252
@@ -217,7 +248,7 @@ position:
children: []
id: CHART-explore-112-1
meta:
chartId: 112
chartId: 121
height: 50

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

The layout component ID CHART-explore-112-1 no longer matches the referenced meta.chartId: 121. In other dashboard YAMLs, the CHART-explore-<chartId>-1 convention matches meta.chartId (e.g., MITx_and_xPro_Products...yaml has CHART-explore-82-1 with chartId: 82). Consider renaming this component (and its references in parents/children) to keep IDs consistent and avoid confusing/fragile exports.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Already addressed in dc4dd07CHART-explore-112-1 was renamed to CHART-explore-121-1 (matching meta.chartId: 121), with all children/id references updated consistently.

Comment on lines 263 to 268
@@ -233,7 +264,7 @@ position:
children: []
id: CHART-explore-82-1
meta:
chartId: 82
chartId: 138
height: 77

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

The layout component ID CHART-explore-82-1 no longer matches the referenced meta.chartId: 138. This breaks the usual CHART-explore-<chartId>-1 naming pattern used across other dashboard YAMLs and makes future diffs/maintenance harder. Update the component ID (and any references to it) to align with the new chartId, or revert chartId if it wasn’t meant to change.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Already addressed in dc4dd07CHART-explore-82-1 was renamed to CHART-explore-138-1 (matching meta.chartId: 138), with all children/id references updated consistently.

KatelynGit and others added 2 commits April 29, 2026 11:29
…be26-c1642d73b288.yaml

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…art IDs (121/138/154)

Agent-Logs-Url: https://github.com/mitodl/ol-data-platform/sessions/a3ec77fc-79da-418e-bfc7-273f8f53b74a

Co-authored-by: KatelynGit <134744245+KatelynGit@users.noreply.github.com>
@KatelynGit KatelynGit merged commit 0a188e8 into main Apr 29, 2026
6 checks passed
@KatelynGit KatelynGit deleted the add-product-name-filter-to-product-dash branch April 29, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants