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/06/05 23:58:08 UTC
[PR] test(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
rusackas opened a new pull request, #40818:
URL: https://github.com/apache/superset/pull/40818
### SUMMARY
This is a **test-only PR** opened as a TDD-style validation of issue #29519.
#29519 reports that a dashboard-level filter that is in scope for a **Mixed** (`mixed_timeseries`) chart applies to **Query A** but sometimes not to **Query B**.
A Mixed chart issues a single query context containing two queries (`queries[0]` = A, `queries[1]` = B). This PR adds a Playwright spec that creates a Mixed chart, places it on a dashboard behind a native filter scoped to the chart, loads the dashboard, and inspects the outgoing `POST /api/v1/chart/data` payload to assert the filter appears in **both** queries.
1. **`Mixed chart applies dashboard filter to both queries (#29519)`** — asserts `queries[0].filters` and `queries[1].filters` both contain the in-scope dashboard filter.
**Status:** on current `master` this test **passes** — both queries inherit the dashboard filter — which indicates the reported behavior is resolved at the query-building level (`buildQuery` propagates `extra_form_data` to both queries). This PR therefore validates the fix and locks in a regression guard; merging closes #29519.
### How to interpret CI
- **CI green** → both queries inherit the dashboard filter (contract holds); merging closes #29519 and guards against regressions.
- **CI red** → Query B dropped the filter; the bug is live in the Mixed chart query path (`superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries`).
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npx playwright test playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts
```
### ADDITIONAL INFORMATION
- [x] Has associated issue: closes #29519
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40818:
URL: https://github.com/apache/superset/pull/40818#issuecomment-4636502019
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | ab165c638f288d0d13ae0d1cce1e48241fb47bd2 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a236290d030dc00073d9ebb |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40818--superset-docs-preview.netlify.app](https://deploy-preview-40818--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#DctRDsIgDADQu_TbZhWyGbyBxyi0BpJNSMFkxnh3-Xs_7wvN6tEG3AEuEI1fKU8PkaX0_lZ0Yb0GPMqpgimzDRTuOVY2wWfZh1qfUbTt9fOQWTd2fnOBhDxJIqKbl6Axwu8P) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40818--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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40818:
URL: https://github.com/apache/superset/pull/40818#issuecomment-4651600975
<!-- Bito Reply -->
Extracting `findDatasetIdByName` into a helper function is a good practice, as it improves readability and reusability across tests. Since this is a test file, you could move it to a shared utility file (e.g., `superset-frontend/playwright/helpers/api/dataset.ts`) if other tests need to look up datasets by name.
**superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts**
```
async function findDatasetIdByName(page: any, name: string): Promise<number> {
const query = `(filters:!((col:table_name,opr:eq,value:'${name}')))`;
const resp = await page.request.get(`api/v1/dataset/?q=${query}`);
const body = await resp.json();
if (!body.result?.length) {
throw new Error(`Dataset ${name} not found`);
}
return body.result[0].id;
}
```
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40818:
URL: https://github.com/apache/superset/pull/40818
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40818:
URL: https://github.com/apache/superset/pull/40818#discussion_r3375820692
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,203 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+async function findDatasetIdByName(page: any, name: string): Promise<number> {
Review Comment:
Good call — there was actually already a shared `getDatasetByName` helper in `playwright/helpers/api/dataset.ts` (the same one `createTestChart` uses), so I dropped the inline `findDatasetIdByName` and reused that instead. Bonus: it kills the `page: any`. Done in 31434db.
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,203 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+async function findDatasetIdByName(page: any, name: string): Promise<number> {
+ const query = `(filters:!((col:table_name,opr:eq,value:'${name}')))`;
+ const resp = await page.request.get(`api/v1/dataset/?q=${query}`);
+ const body = await resp.json();
+ if (!body.result?.length) {
+ throw new Error(`Dataset ${name} not found`);
+ }
+ return body.result[0].id;
+}
+
+testWithAssets(
+ 'Mixed chart applies dashboard filter to both queries (#29519)',
+ async ({ page, testAssets }) => {
+ const datasetId = await findDatasetIdByName(page, DATASET_NAME);
+
+ const chartParams = {
Review Comment:
I went back and forth on this one. I looked at how the sibling dashboard specs handle it (e.g. `clear-all-filters.spec.ts`) and they keep the per-test viz `params` inline at the point of chart creation — there is no shared fixtures location for chart params in the playwright suite, and this `chartParams` object is single-use and tightly coupled to this specific mixed_timeseries repro. Pulling it into a separate file would scatter the test's context for no reuse benefit, so I lean toward leaving it inline for locality. Happy to extract it if you feel strongly, though — leaving this thread open for you.
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40818:
URL: https://github.com/apache/superset/pull/40818#discussion_r3375851449
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,200 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { getDatasetByName } from '../../helpers/api/dataset';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+testWithAssets(
+ 'Mixed chart applies dashboard filter to both queries (#29519)',
+ async ({ page, testAssets }) => {
+ const dataset = await getDatasetByName(page, DATASET_NAME);
+ if (!dataset) {
+ throw new Error(
+ `Dataset ${DATASET_NAME} not found — run Superset with --load-examples`,
+ );
+ }
+ const datasetId = dataset.id;
+
+ const chartParams = {
+ datasource: `${datasetId}__table`,
+ viz_type: 'mixed_timeseries',
+ x_axis: 'ds',
+ time_grain_sqla: 'P1Y',
+ metrics: ['count'],
+ groupby: [],
+ adhoc_filters: [],
+ metrics_b: ['count'],
+ groupby_b: [],
+ adhoc_filters_b: [],
+ row_limit: 100,
+ row_limit_b: 100,
+ truncate_metric: true,
+ truncate_metric_b: true,
+ comparison_type: 'values',
+ color_scheme: 'supersetColors',
+ };
+ const chartResp = await apiPost(page, 'api/v1/chart/', {
+ slice_name: `mixed_filter_repro_${Date.now()}`,
+ viz_type: 'mixed_timeseries',
+ datasource_id: datasetId,
+ datasource_type: 'table',
+ params: JSON.stringify(chartParams),
+ });
+ expect(chartResp.ok()).toBe(true);
+ const chartId: number = (await chartResp.json()).id;
+ testAssets.trackChart(chartId);
+
+ const chartLayoutKey = `CHART-${chartId}`;
+ const filterId = `NATIVE_FILTER-${Math.random().toString(36).slice(2, 10)}`;
+ const positionJson = {
+ DASHBOARD_VERSION_KEY: 'v2',
+ ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] },
+ GRID_ID: {
+ type: 'GRID',
+ id: 'GRID_ID',
+ children: ['ROW-1'],
+ parents: ['ROOT_ID'],
+ },
+ 'ROW-1': {
+ type: 'ROW',
+ id: 'ROW-1',
+ children: [chartLayoutKey],
+ parents: ['ROOT_ID', 'GRID_ID'],
+ meta: { background: 'BACKGROUND_TRANSPARENT' },
+ },
+ [chartLayoutKey]: {
+ type: 'CHART',
+ id: chartLayoutKey,
+ children: [],
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-1'],
+ meta: { chartId, width: 8, height: 60, sliceName: 'mixed_filter_repro' },
+ },
+ };
+ const jsonMetadata = {
+ native_filter_configuration: [
+ {
+ id: filterId,
+ name: 'Gender',
+ filterType: 'filter_select',
+ type: 'NATIVE_FILTER',
+ targets: [{ datasetId, column: { name: FILTER_COLUMN } }],
+ controlValues: {
+ multiSelect: false,
+ enableEmptyFilter: false,
+ defaultToFirstItem: false,
+ inverseSelection: false,
+ searchAllOptions: false,
+ },
+ defaultDataMask: {
+ filterState: { value: [FILTER_VALUE] },
+ extraFormData: {
+ filters: [
+ { col: FILTER_COLUMN, op: 'IN', val: [FILTER_VALUE] },
+ ],
+ },
+ },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ chartsInScope: [chartId],
+ },
+ ],
+ chart_configuration: {},
+ cross_filters_enabled: false,
+ global_chart_configuration: {
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ chartsInScope: [chartId],
+ },
+ };
+ const dashResp = await apiPostDashboard(page, {
+ dashboard_title: `mixed_filter_repro_${Date.now()}`,
+ published: true,
+ position_json: JSON.stringify(positionJson),
+ json_metadata: JSON.stringify(jsonMetadata),
+ });
+ expect(dashResp.ok()).toBe(true);
+ const dashBody = await dashResp.json();
+ const dashboardId: number = dashBody.result?.id ?? dashBody.id;
+ testAssets.trackDashboard(dashboardId);
+
+ await apiPut(page, `api/v1/chart/${chartId}`, { dashboards: [dashboardId] });
+
+ // Capture the Mixed chart's data request (the one with two queries).
+ const twoQueryPayloads: any[] = [];
+ page.on('request', req => {
+ if (
+ req.url().includes('/api/v1/chart/data') &&
+ req.method() === 'POST'
+ ) {
+ try {
+ const body = req.postDataJSON();
+ if (body?.queries?.length === 2) {
+ twoQueryPayloads.push(body);
+ }
+ } catch {
+ // ignore non-JSON bodies
+ }
+ }
+ });
+
+ const dashboardPage = new DashboardPage(page);
+ await dashboardPage.gotoById(dashboardId);
+ await dashboardPage.waitForLoad();
+ await dashboardPage.waitForChartsToLoad();
+
+ await expect
+ .poll(() => twoQueryPayloads.length, { timeout: 15_000 })
+ .toBeGreaterThan(0);
+
+ const payload = twoQueryPayloads[twoQueryPayloads.length - 1];
+ const filtersA = JSON.stringify(payload.queries[0].filters || []);
+ const filtersB = JSON.stringify(payload.queries[1].filters || []);
+
+ expect(
+ filtersA.includes(FILTER_COLUMN),
+ 'Query A should inherit the dashboard filter',
+ ).toBe(true);
+ expect(
+ filtersB.includes(FILTER_COLUMN),
+ 'Query B should inherit the dashboard filter (see #29519)',
+ ).toBe(true);
Review Comment:
**Suggestion:** The assertion only checks whether the filter column name string appears in serialized filters, which can pass even when the actual filter payload is wrong (wrong operator/value or unrelated structure containing the same text). Assert against structured filter objects (`col`, `op`, and `val`) in both queries so the regression test actually verifies correct filter propagation semantics. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Mixed chart filter test may miss semantic regressions.
- ⚠️ Query operator/value bugs could ship undetected for Mixed charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. After collecting POST `/api/v1/chart/data` payloads with two queries in
`superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:160-176`,
the test selects the last payload at line 187 and derives `filtersA` and `filtersB` as
JSON strings from `payload.queries[0].filters` and `payload.queries[1].filters` at lines
188-189.
2. The assertions at lines 191-198 use `filtersA.includes(FILTER_COLUMN)` and
`filtersB.includes(FILTER_COLUMN)` to decide whether each query "inherits the dashboard
filter", so they only check that the substring `'gender'` (from `FILTER_COLUMN` at line
43) appears somewhere in the serialized filters.
3. The intended filter semantics are defined in the dashboard's native filter
configuration at lines 112-135, where `defaultDataMask.extraFormData.filters` is
explicitly set to `[ { col: FILTER_COLUMN, op: 'IN', val: [FILTER_VALUE] } ]`, meaning
both Mixed chart queries should receive a structured filter object with the correct `col`,
`op`, and `val`.
4. If the MixedTimeseries query-building path (referenced in the test comment at lines
31-34 as `plugin-chart-echarts/src/MixedTimeseries`) regresses to emitting an incorrect
filter payload—for example, using a wrong operator, omitting `val`, or embedding the
column name in an unrelated structure—while still including the string `"gender"` in
`queries[*].filters`, these substring-based checks at lines 191-198 would still pass, so
the regression test would not catch the broken filter semantics for one or both queries.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9498f66cb2ae433cb6ec28106503de35&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9498f66cb2ae433cb6ec28106503de35&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts
**Line:** 188:198
**Comment:**
*Incomplete Implementation: The assertion only checks whether the filter column name string appears in serialized filters, which can pass even when the actual filter payload is wrong (wrong operator/value or unrelated structure containing the same text). Assert against structured filter objects (`col`, `op`, and `val`) in both queries so the regression test actually verifies correct filter propagation semantics.
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
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40818&comment_hash=98858ea6ef584284f714857bb66481a4643bcb7ffe7bc8f26fb0ea9c44f8f9d0&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40818&comment_hash=98858ea6ef584284f714857bb66481a4643bcb7ffe7bc8f26fb0ea9c44f8f9d0&reaction=dislike'>👎</a>
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,200 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { getDatasetByName } from '../../helpers/api/dataset';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+testWithAssets(
+ 'Mixed chart applies dashboard filter to both queries (#29519)',
+ async ({ page, testAssets }) => {
+ const dataset = await getDatasetByName(page, DATASET_NAME);
+ if (!dataset) {
+ throw new Error(
+ `Dataset ${DATASET_NAME} not found — run Superset with --load-examples`,
+ );
+ }
+ const datasetId = dataset.id;
+
+ const chartParams = {
+ datasource: `${datasetId}__table`,
+ viz_type: 'mixed_timeseries',
+ x_axis: 'ds',
+ time_grain_sqla: 'P1Y',
+ metrics: ['count'],
+ groupby: [],
+ adhoc_filters: [],
+ metrics_b: ['count'],
+ groupby_b: [],
+ adhoc_filters_b: [],
+ row_limit: 100,
+ row_limit_b: 100,
+ truncate_metric: true,
+ truncate_metric_b: true,
+ comparison_type: 'values',
+ color_scheme: 'supersetColors',
+ };
+ const chartResp = await apiPost(page, 'api/v1/chart/', {
+ slice_name: `mixed_filter_repro_${Date.now()}`,
+ viz_type: 'mixed_timeseries',
+ datasource_id: datasetId,
+ datasource_type: 'table',
+ params: JSON.stringify(chartParams),
+ });
+ expect(chartResp.ok()).toBe(true);
+ const chartId: number = (await chartResp.json()).id;
Review Comment:
**Suggestion:** The chart creation response is parsed as if `id` is always top-level, but this API can return `{ result: { id } }` in some environments. When that shape is returned, the computed chart ID becomes `undefined`, which then breaks dashboard linkage and navigation with hard-to-diagnose failures. Parse both response shapes before using the ID. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed-chart dashboard filter regression test fails on some APIs.
- ⚠️ CI for issue #29519 may be flaky across deployments.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:46-84`,
the test creates a Mixed chart by POSTing to `api/v1/chart/` via `apiPost` at lines 75-81
and then parses the response as `const chartId: number = (await chartResp.json()).id;` at
line 83.
2. The same chart creation endpoint is wrapped by `apiPostChart` in
`superset-frontend/playwright/helpers/api/chart.ts:5-9`, and its response is parsed in
`superset-frontend/playwright/tests/chart/chart-test-helpers.ts:29-35` using `const id =
body.result?.id ?? body.id;` with the comment "Handle both response shapes: { id } or {
result: { id } }", demonstrating that in real environments the API can return either `{ id
}` or `{ result: { id } }`.
3. A similar dual-shape handling exists for datasets in
`superset-frontend/playwright/helpers/api/dataset.ts:1-3`, where the helper returns
`body.result?.id ?? body.id ?? null`, further confirming that Superset APIs sometimes wrap
created resource IDs under `result`.
4. When the `api/v1/chart/` POST used in the mixed-chart test returns the `{ result: { id
} }` shape, `(await chartResp.json()).id` at line 83 is `undefined`, so `chartId` becomes
`undefined`; this propagates into `chartLayoutKey` (line 86) and the dashboard linkage
update `await apiPut(page, \`api/v1/chart/${chartId}\`, { dashboards: [dashboardId] });`
at line 158, causing the created chart not to be correctly linked to the dashboard and
leading to no two-query `/api/v1/chart/data` requests and eventual timeout in the
`expect.poll` at lines 183-185.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3440c28b860a42c59285443a4be0f05c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3440c28b860a42c59285443a4be0f05c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts
**Line:** 83:83
**Comment:**
*Api Mismatch: The chart creation response is parsed as if `id` is always top-level, but this API can return `{ result: { id } }` in some environments. When that shape is returned, the computed chart ID becomes `undefined`, which then breaks dashboard linkage and navigation with hard-to-diagnose failures. Parse both response shapes before using the ID.
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
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40818&comment_hash=f5d6dfb2bcd9cb8907310dda912cfcb70373d91fbf655add1fb6f847a0474164&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40818&comment_hash=f5d6dfb2bcd9cb8907310dda912cfcb70373d91fbf655add1fb6f847a0474164&reaction=dislike'>👎</a>
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40818:
URL: https://github.com/apache/superset/pull/40818#issuecomment-4636529702
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40818?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 64.18%. Comparing base ([`3d7021f`](https://app.codecov.io/gh/apache/superset/commit/3d7021fdf95b7ca37fa30c72449ff8d59d4340c2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ab165c6`](https://app.codecov.io/gh/apache/superset/commit/ab165c638f288d0d13ae0d1cce1e48241fb47bd2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40818 +/- ##
==========================================
- Coverage 64.19% 64.18% -0.01%
==========================================
Files 2666 2666
Lines 143991 143991
Branches 33108 33108
==========================================
- Hits 92428 92427 -1
- Misses 49950 49951 +1
Partials 1613 1613
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40818/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/40818/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.86% <ø> (-0.01%)` | :arrow_down: |
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/40818?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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40818:
URL: https://github.com/apache/superset/pull/40818#discussion_r3375125235
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,203 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+async function findDatasetIdByName(page: any, name: string): Promise<number> {
Review Comment:
nit: should this be pulled out as a helper function?
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40818:
URL: https://github.com/apache/superset/pull/40818#discussion_r3375143643
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,203 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+async function findDatasetIdByName(page: any, name: string): Promise<number> {
+ const query = `(filters:!((col:table_name,opr:eq,value:'${name}')))`;
+ const resp = await page.request.get(`api/v1/dataset/?q=${query}`);
+ const body = await resp.json();
+ if (!body.result?.length) {
+ throw new Error(`Dataset ${name} not found`);
+ }
+ return body.result[0].id;
+}
+
+testWithAssets(
+ 'Mixed chart applies dashboard filter to both queries (#29519)',
+ async ({ page, testAssets }) => {
+ const datasetId = await findDatasetIdByName(page, DATASET_NAME);
+
+ const chartParams = {
Review Comment:
nit: Should this just live as other files so it doesn't clutter the test files?
--
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40818:
URL: https://github.com/apache/superset/pull/40818#issuecomment-4636496728
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(mixed-chart): dashboard filters should reach both Mixed chart queries (#29519) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40818:
URL: https://github.com/apache/superset/pull/40818#discussion_r3376102340
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,200 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { getDatasetByName } from '../../helpers/api/dataset';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+testWithAssets(
+ 'Mixed chart applies dashboard filter to both queries (#29519)',
+ async ({ page, testAssets }) => {
+ const dataset = await getDatasetByName(page, DATASET_NAME);
+ if (!dataset) {
+ throw new Error(
+ `Dataset ${DATASET_NAME} not found — run Superset with --load-examples`,
+ );
+ }
+ const datasetId = dataset.id;
+
+ const chartParams = {
+ datasource: `${datasetId}__table`,
+ viz_type: 'mixed_timeseries',
+ x_axis: 'ds',
+ time_grain_sqla: 'P1Y',
+ metrics: ['count'],
+ groupby: [],
+ adhoc_filters: [],
+ metrics_b: ['count'],
+ groupby_b: [],
+ adhoc_filters_b: [],
+ row_limit: 100,
+ row_limit_b: 100,
+ truncate_metric: true,
+ truncate_metric_b: true,
+ comparison_type: 'values',
+ color_scheme: 'supersetColors',
+ };
+ const chartResp = await apiPost(page, 'api/v1/chart/', {
+ slice_name: `mixed_filter_repro_${Date.now()}`,
+ viz_type: 'mixed_timeseries',
+ datasource_id: datasetId,
+ datasource_type: 'table',
+ params: JSON.stringify(chartParams),
+ });
+ expect(chartResp.ok()).toBe(true);
+ const chartId: number = (await chartResp.json()).id;
Review Comment:
Good catch — fixed in ab165c6. The chart-creation response is now parsed with `chartBody.result?.id ?? chartBody.id`, matching the dual-shape handling already used for the dashboard response in this same file and in `chart-test-helpers.ts`.
##########
superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts:
##########
@@ -0,0 +1,200 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression for #29519: a dashboard-level filter that is in scope for a Mixed
+ * (mixed_timeseries) chart should apply to BOTH of the chart's queries — Query
+ * A and Query B — not just Query A.
+ *
+ * A Mixed chart issues a single query context with two queries
+ * (queries[0] = A, queries[1] = B). This test creates a Mixed chart, puts it on
+ * a dashboard behind a native filter scoped to the chart, loads the dashboard,
+ * and inspects the outgoing POST /api/v1/chart/data payload to assert the filter
+ * is present in both queries.
+ *
+ * CI green => both queries inherit the dashboard filter (contract holds);
+ * merging closes #29519 and guards against regressions.
+ * CI red => Query B dropped the filter; the bug is live in the Mixed chart
+ * query-building path (plugin-chart-echarts/src/MixedTimeseries).
+ */
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { getDatasetByName } from '../../helpers/api/dataset';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+const FILTER_VALUE = 'boy';
+
+testWithAssets(
+ 'Mixed chart applies dashboard filter to both queries (#29519)',
+ async ({ page, testAssets }) => {
+ const dataset = await getDatasetByName(page, DATASET_NAME);
+ if (!dataset) {
+ throw new Error(
+ `Dataset ${DATASET_NAME} not found — run Superset with --load-examples`,
+ );
+ }
+ const datasetId = dataset.id;
+
+ const chartParams = {
+ datasource: `${datasetId}__table`,
+ viz_type: 'mixed_timeseries',
+ x_axis: 'ds',
+ time_grain_sqla: 'P1Y',
+ metrics: ['count'],
+ groupby: [],
+ adhoc_filters: [],
+ metrics_b: ['count'],
+ groupby_b: [],
+ adhoc_filters_b: [],
+ row_limit: 100,
+ row_limit_b: 100,
+ truncate_metric: true,
+ truncate_metric_b: true,
+ comparison_type: 'values',
+ color_scheme: 'supersetColors',
+ };
+ const chartResp = await apiPost(page, 'api/v1/chart/', {
+ slice_name: `mixed_filter_repro_${Date.now()}`,
+ viz_type: 'mixed_timeseries',
+ datasource_id: datasetId,
+ datasource_type: 'table',
+ params: JSON.stringify(chartParams),
+ });
+ expect(chartResp.ok()).toBe(true);
+ const chartId: number = (await chartResp.json()).id;
+ testAssets.trackChart(chartId);
+
+ const chartLayoutKey = `CHART-${chartId}`;
+ const filterId = `NATIVE_FILTER-${Math.random().toString(36).slice(2, 10)}`;
+ const positionJson = {
+ DASHBOARD_VERSION_KEY: 'v2',
+ ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] },
+ GRID_ID: {
+ type: 'GRID',
+ id: 'GRID_ID',
+ children: ['ROW-1'],
+ parents: ['ROOT_ID'],
+ },
+ 'ROW-1': {
+ type: 'ROW',
+ id: 'ROW-1',
+ children: [chartLayoutKey],
+ parents: ['ROOT_ID', 'GRID_ID'],
+ meta: { background: 'BACKGROUND_TRANSPARENT' },
+ },
+ [chartLayoutKey]: {
+ type: 'CHART',
+ id: chartLayoutKey,
+ children: [],
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-1'],
+ meta: { chartId, width: 8, height: 60, sliceName: 'mixed_filter_repro' },
+ },
+ };
+ const jsonMetadata = {
+ native_filter_configuration: [
+ {
+ id: filterId,
+ name: 'Gender',
+ filterType: 'filter_select',
+ type: 'NATIVE_FILTER',
+ targets: [{ datasetId, column: { name: FILTER_COLUMN } }],
+ controlValues: {
+ multiSelect: false,
+ enableEmptyFilter: false,
+ defaultToFirstItem: false,
+ inverseSelection: false,
+ searchAllOptions: false,
+ },
+ defaultDataMask: {
+ filterState: { value: [FILTER_VALUE] },
+ extraFormData: {
+ filters: [
+ { col: FILTER_COLUMN, op: 'IN', val: [FILTER_VALUE] },
+ ],
+ },
+ },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ chartsInScope: [chartId],
+ },
+ ],
+ chart_configuration: {},
+ cross_filters_enabled: false,
+ global_chart_configuration: {
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ chartsInScope: [chartId],
+ },
+ };
+ const dashResp = await apiPostDashboard(page, {
+ dashboard_title: `mixed_filter_repro_${Date.now()}`,
+ published: true,
+ position_json: JSON.stringify(positionJson),
+ json_metadata: JSON.stringify(jsonMetadata),
+ });
+ expect(dashResp.ok()).toBe(true);
+ const dashBody = await dashResp.json();
+ const dashboardId: number = dashBody.result?.id ?? dashBody.id;
+ testAssets.trackDashboard(dashboardId);
+
+ await apiPut(page, `api/v1/chart/${chartId}`, { dashboards: [dashboardId] });
+
+ // Capture the Mixed chart's data request (the one with two queries).
+ const twoQueryPayloads: any[] = [];
+ page.on('request', req => {
+ if (
+ req.url().includes('/api/v1/chart/data') &&
+ req.method() === 'POST'
+ ) {
+ try {
+ const body = req.postDataJSON();
+ if (body?.queries?.length === 2) {
+ twoQueryPayloads.push(body);
+ }
+ } catch {
+ // ignore non-JSON bodies
+ }
+ }
+ });
+
+ const dashboardPage = new DashboardPage(page);
+ await dashboardPage.gotoById(dashboardId);
+ await dashboardPage.waitForLoad();
+ await dashboardPage.waitForChartsToLoad();
+
+ await expect
+ .poll(() => twoQueryPayloads.length, { timeout: 15_000 })
+ .toBeGreaterThan(0);
+
+ const payload = twoQueryPayloads[twoQueryPayloads.length - 1];
+ const filtersA = JSON.stringify(payload.queries[0].filters || []);
+ const filtersB = JSON.stringify(payload.queries[1].filters || []);
+
+ expect(
+ filtersA.includes(FILTER_COLUMN),
+ 'Query A should inherit the dashboard filter',
+ ).toBe(true);
+ expect(
+ filtersB.includes(FILTER_COLUMN),
+ 'Query B should inherit the dashboard filter (see #29519)',
+ ).toBe(true);
Review Comment:
Agreed — fixed in ab165c6. Both query assertions now check the structured filter object (`expect.arrayContaining([expect.objectContaining({ col, op: 'IN', val })])`) rather than a substring of the serialized payload, so a wrong-operator/value regression in the MixedTimeseries query-building path would now be caught.
--
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