You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sadpandajoe (via GitHub)" <gi...@apache.org> on 2026/10/01 00:30:43 UTC
[PR] test(explore): cover chart header actions, chart pills, and view container wiring [superset]
sadpandajoe opened a new pull request, #44848:
URL: https://github.com/apache/superset/pull/44848
### SUMMARY
Adds React Testing Library coverage for the Explore page shell: the chart header's action menu, the chart pills, and the view container's wiring. This change touches test files only.
- **Header actions** (`ExploreChartHeader.test.tsx`)
- Copy permalink: the request carries the chart state, the link is copied, and a success toast shows. On a failed request or a missing datasource, a danger toast shows and nothing is copied.
- Share by email: opens a `mailto:` link that contains the permalink. On a failed request, a danger toast shows instead.
- Embed code: shows the chart iframe. It is hidden when the `EmbeddableCharts` flag is off.
- View query: sends the rendered chart's form data, shows the generated SQL, and closes.
- **Chart pills** (`ChartPills.test.tsx`, new)
- Where the row count is read from for table and non-table charts, with `sql_rowcount` preferred over `rowcount`.
- `hideRowCount`.
- The cached label and its refresh.
- Loading state.
- The elapsed timer.
- **View container** (`ExploreViewContainer.test.tsx`)
- The Run button state when controls do or do not have validation errors, including matrixify control errors.
- Ctrl/Cmd+Enter queues the chart query.
- Tooltip template variables get appended. The tests assert the stored template value.
- Datasource metadata is requested only when the datasource control changes.
Each new test was checked by breaking the related product code, and the test failed each time.
Some behavior that looks like a bug was left unpinned on purpose:
- Ctrl/Cmd+Enter runs the query even when a control has validation errors and the Run button is disabled.
- `fetchDatasourceMetadata` is called without `dispatch`, so the datasource-change effect probably never reaches the store. The test checks that the request is made, not what it does to the store.
- The keydown listener is added on `document`, but a changed handler is removed from `window`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A: tests only.
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npx jest \
src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx \
src/explore/components/ChartPills.test.tsx \
src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
- [ ] 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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-6156677025
## CodeAnt PR Risk: Low Risk
- The PR appears safe to merge; the visible changes add frontend regression tests without changing runtime behavior.
**Assessed commit:** `74df936c7e48`
--
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(explore): cover chart header actions, chart pills, and view container wiring [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 #44848:
URL: https://github.com/apache/superset/pull/44848#discussion_r4150664515
##########
superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx:
##########
@@ -1309,3 +1328,297 @@ test('automatic axis title margin adjustment handles both X and Y axis titles be
jest.restoreAllMocks();
}
});
+
+const getChart = (store: Store) =>
+ (
+ store.getState() as {
+ charts: Record<number, { triggerQuery?: boolean }>;
+ }
+ ).charts[1];
+
+test.each([
+ ['Ctrl', '{Control>}{Enter}{/Control}'],
+ ['Cmd', '{Meta>}{Enter}{/Meta}'],
+])('%s+Enter anywhere on the page queues the chart query', async (_, keys) => {
+ setupTableChartControlPanel();
+ try {
+ const store = createStore(reduxState, reducerIndex);
+ renderWithRouter({ initialState: reduxState, store: store as Store });
+ await screen.findByTestId('control-panels-container');
+
+ // Mounting queues a query; clear it so only the shortcut can set it again.
+ act(() => {
+ store.dispatch(chartActions.triggerQuery(false, 1));
+ });
+ expect(getChart(store as Store).triggerQuery).toBe(false);
+
+ await userEvent.keyboard(keys);
+
+ await waitFor(() =>
+ expect(getChart(store as Store).triggerQuery).toBe(true),
+ );
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+test.each([
+ ['Enter without a modifier', '{Enter}'],
+ ['Ctrl with another key', '{Control>}a{/Control}'],
+])('%s does not queue the chart query', async (_, keys) => {
+ setupTableChartControlPanel();
+ try {
+ const store = createStore(reduxState, reducerIndex);
+ renderWithRouter({ initialState: reduxState, store: store as Store });
+ await screen.findByTestId('control-panels-container');
+ act(() => {
+ store.dispatch(chartActions.triggerQuery(false, 1));
+ });
+
+ await userEvent.keyboard(keys);
+
+ expect(getChart(store as Store).triggerQuery).toBe(false);
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+const stateWithControls = (controls: object) => ({
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ controls: { ...reduxState.explore.controls, ...controls },
+ },
+});
+
+test('keeps the Run button enabled when no control has validation errors', async () => {
+ setupTableChartControlPanel();
+ try {
+ renderWithRouter({
+ initialState: stateWithControls({
+ metric: { value: 'count', label: 'Metric' },
+ }),
+ });
+
+ const panel = await screen.findByTestId('control-panels-container');
+ expect(panel).toHaveAttribute('data-has-button-error', 'false');
+ expect(screen.getByRole('button', { name: 'Update chart' })).toBeEnabled();
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+test('disables the Run button and reports the error when a control fails validation', async () => {
+ setupTableChartControlPanel();
+ try {
+ renderWithRouter({
+ initialState: stateWithControls({
+ metric: {
+ value: '',
+ label: 'Metric',
+ validationErrors: ['Metric is required'],
+ },
+ }),
+ });
+
+ const panel = await screen.findByTestId('control-panels-container');
+ expect(panel).toHaveAttribute('data-has-button-error', 'true');
+ expect(panel).toHaveAttribute('data-has-data-tab-error', 'true');
+ expect(screen.getByRole('button', { name: 'Update chart' })).toBeDisabled();
+ expect(screen.getByRole('tooltip')).toHaveTextContent(
+ 'Control labeled Metric: Metric is required',
+ );
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+test('disables the Run button for a matrixify control error without flagging the Data tab', async () => {
+ setupTableChartControlPanel();
+ try {
+ renderWithRouter({
+ initialState: stateWithControls({
+ matrixify_rows: {
+ value: '',
+ label: 'Rows',
+ tabOverride: 'matrixify',
+ validationErrors: ['Rows are required'],
+ },
+ }),
+ });
+
+ const panel = await screen.findByTestId('control-panels-container');
+ expect(panel).toHaveAttribute('data-has-button-error', 'true');
+ expect(panel).toHaveAttribute('data-has-data-tab-error', 'false');
+ expect(screen.getByRole('button', { name: 'Update chart' })).toBeDisabled();
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+const tooltipState = (vizType: string, template = '') => ({
+ ...reduxState,
+ charts: {
+ 1: {
+ ...reduxState.charts[1],
+ latestQueryFormData: {
+ ...reduxState.charts[1].latestQueryFormData,
+ viz_type: vizType,
+ },
+ },
+ },
+ explore: {
+ ...reduxState.explore,
+ form_data: { datasource: '1__table', viz_type: vizType, metrics: [] },
+ controls: {
+ ...reduxState.explore.controls,
+ viz_type: { value: vizType },
+ tooltip_contents: { value: [] },
+ tooltip_template: { value: template },
+ },
+ },
+});
+
+// Renders the container and returns a function that selects tooltip contents
+// the way the control would and reports the template the container wrote back.
+const renderTooltipChart = (vizType: string, template = '') => {
+ getChartControlPanelRegistry().registerValue(vizType, {
+ controlPanelSections: [],
+ });
+ const setControlValueSpy = jest.spyOn(exploreActions, 'setControlValue');
+ const initialState = tooltipState(vizType, template);
+ const store = createStore(initialState, reducerIndex);
+ renderWithRouter({ initialState, store: store as Store });
+ setControlValueSpy.mockClear();
+ return {
+ selectTooltipContents: (contents: unknown[]) => {
+ act(() => {
+ store.dispatch(
+ exploreActions.setControlValue('tooltip_contents', contents),
+ );
+ });
+ },
+ templateWrites: () =>
+ setControlValueSpy.mock.calls
+ .filter(([controlName]) => controlName === 'tooltip_template')
+ .map(([, value]) => value),
+ storedTemplate: () =>
+ ((store as Store).getState() as ExplorePageState).explore.controls
+ .tooltip_template?.value,
+ cleanup: () => getChartControlPanelRegistry().remove(vizType),
+ };
+};
+
+test('appends a {{ field }} variable to the tooltip template for a selected column', () => {
+ const chart = renderTooltipChart('deck_scatter');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual(['{{ name }}']);
+ expect(chart.storedTemplate()).toBe('{{ name }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('appends after the existing tooltip template text', () => {
+ const chart = renderTooltipChart('deck_scatter', 'Details:');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual(['Details: {{ name }}']);
+ expect(chart.storedTemplate()).toBe('Details: {{ name }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('appends a limited variable for a column on an aggregated chart', () => {
+ const chart = renderTooltipChart('heatmap');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual(['{{ limit name 10 }}']);
+ expect(chart.storedTemplate()).toBe('{{ limit name 10 }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('appends a plain variable for a metric even on an aggregated chart', () => {
+ const chart = renderTooltipChart('heatmap');
+ try {
+ chart.selectTooltipContents([
+ { item_type: 'metric', metric_name: 'count' },
+ ]);
+
+ expect(chart.templateWrites()).toEqual(['{{ count }}']);
+ expect(chart.storedTemplate()).toBe('{{ count }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('leaves the tooltip template alone when it already references the field', () => {
+ const chart = renderTooltipChart('deck_scatter', 'Name: {{ name }}');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual([]);
+ expect(chart.storedTemplate()).toBe('Name: {{ name }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+const datasourceState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ form_data: { datasource: '1__table', viz_type: VizType.Table, metrics: [] },
+ controls: { ...reduxState.explore.controls, row_limit: { value: 100 } },
+ },
+};
+
+test('requests the datasource metadata for the new datasource when the datasource control changes', () => {
+ setupTableChartControlPanel();
+ try {
+ const fetchMetadataSpy = jest.spyOn(
+ datasourceActions,
+ 'fetchDatasourceMetadata',
+ );
+ const store = createStore(datasourceState, reducerIndex);
+ renderWithRouter({ initialState: datasourceState, store: store as Store });
+ fetchMetadataSpy.mockClear();
+
+ act(() => {
+ store.dispatch(exploreActions.setControlValue('datasource', '2__table'));
+ });
+
+ expect(fetchMetadataSpy).toHaveBeenCalledTimes(1);
+ expect(fetchMetadataSpy).toHaveBeenCalledWith('2__table');
Review Comment:
**Suggestion:** The effect calls `fetchDatasourceMetadata` with `props.form_data.datasource`, which remains `1__table`; this expectation requires `2__table` and fails against the current caller.
**Assessment:** π `Major` Β· π `Occurrence: Often` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e50ab52bb2a24898b34229ad147cbd24&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e50ab52bb2a24898b34229ad147cbd24&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
**Line:** 1599:1599
**Comment:**
*Api Mismatch: The effect calls `fetchDatasourceMetadata` with `props.form_data.datasource`, which remains `1__table`; this expectation requires `2__table` and fails against the current caller.
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%2F44848&comment_hash=8f61d34de0eed73b98a75c0e3620f8c518cd685411a936c3e99fde22ed8e014e&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44848&comment_hash=8f61d34de0eed73b98a75c0e3620f8c518cd685411a936c3e99fde22ed8e014e&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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-5959768827
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 an incremental AI Review.
- `/review full` - 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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44848:
URL: https://github.com/apache/superset/pull/44848#discussion_r4150805795
##########
superset-frontend/src/explore/components/ChartPills.test.tsx:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 { QueryData, VizType } from '@superset-ui/core';
+import { render, screen, userEvent } from 'spec/helpers/testing-library';
+import { ChartPills, ChartPillsProps } from './ChartPills';
+
+const ROW_LIMIT = 10000;
+
+const renderPills = (props: Partial<ChartPillsProps> = {}) => {
+ const refreshCachedQuery = jest.fn();
+ render(
+ <ChartPills
+ chartStatus="success"
+ chartUpdateStartTime={0}
+ rowLimit={ROW_LIMIT}
+ refreshCachedQuery={refreshCachedQuery}
+ {...props}
+ />,
+ );
+ return { refreshCachedQuery };
+};
+
+// The table viz reports its total row count in a second query when the
+// dataset is paginated on the server.
+const paginatedTableResponses = (
+ pageRowCount: number,
+ totalRowCount: number,
+): QueryData[] => [
+ { rowcount: pageRowCount, data: [] },
+ { data: [{ rowcount: totalRowCount }] },
+];
+
+test.each([VizType.Table, VizType.TableAgGrid])(
+ 'reads the row count of a server-paginated %s from the second query',
+ vizType => {
+ renderPills({
+ queriesResponse: paginatedTableResponses(25, 250),
+ formData: { viz_type: vizType, server_pagination: true },
+ });
+
+ expect(screen.getByText('250 rows')).toBeInTheDocument();
+ expect(screen.queryByText('25 rows')).not.toBeInTheDocument();
+ },
+);
+
+test('reads the row count from the first query for a table with a single query', () => {
+ renderPills({
+ queriesResponse: [{ rowcount: 25, data: [] }],
+ formData: { viz_type: VizType.Table, server_pagination: true },
+ });
+
+ expect(screen.getByText('25 rows')).toBeInTheDocument();
+});
+
+test('reads the row count from the first query for non-table charts even with a second query', () => {
+ renderPills({
+ queriesResponse: paginatedTableResponses(25, 250),
+ formData: { viz_type: VizType.Histogram },
+ });
+
+ expect(screen.getByText('25 rows')).toBeInTheDocument();
+ expect(screen.queryByText('250 rows')).not.toBeInTheDocument();
+});
+
+test('prefers sql_rowcount over rowcount', () => {
+ renderPills({
+ queriesResponse: [{ sql_rowcount: 40, rowcount: 7, data: [] }],
+ formData: { viz_type: VizType.Histogram },
+ });
+
+ expect(screen.getByText('40 rows')).toBeInTheDocument();
+ expect(screen.queryByText('7 rows')).not.toBeInTheDocument();
+});
+
+test('falls back to rowcount when sql_rowcount is absent', () => {
+ renderPills({
+ queriesResponse: [{ rowcount: 7, data: [] }],
+ formData: { viz_type: VizType.Histogram },
+ });
+
+ expect(screen.getByText('7 rows')).toBeInTheDocument();
+});
+
+test('hides the row count but keeps the timer when hideRowCount is set', () => {
+ renderPills({
+ queriesResponse: [{ rowcount: 25, data: [] }],
+ hideRowCount: true,
+ });
+
+ expect(screen.queryByText('25 rows')).not.toBeInTheDocument();
+ expect(screen.getByRole('timer')).toBeInTheDocument();
+});
+
+test('does not show the row count without a query response', () => {
+ renderPills({ queriesResponse: [] });
+
+ expect(screen.queryByText(/rows?$/)).not.toBeInTheDocument();
+});
+
+test('shows the cached label for a cached response and refreshes on click', async () => {
+ const { refreshCachedQuery } = renderPills({
+ queriesResponse: [
+ {
+ rowcount: 25,
+ is_cached: true,
+ cached_dttm: '2024-01-01T00:00:00',
+ data: [],
+ },
+ ],
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicated cached fixture</b></div>
<div id="fix">
The cached-response literal is duplicated verbatim at lines 143-150. Extract a `cachedResponse()` helper beside `paginatedTableResponses` so the two fixtures cannot silently diverge when the cached payload changes.
</div>
</div>
<small><i>Code Review Run #d837aa</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/ChartPills.test.tsx:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 { QueryData, VizType } from '@superset-ui/core';
+import { render, screen, userEvent } from 'spec/helpers/testing-library';
+import { ChartPills, ChartPillsProps } from './ChartPills';
+
+const ROW_LIMIT = 10000;
+
+const renderPills = (props: Partial<ChartPillsProps> = {}) => {
+ const refreshCachedQuery = jest.fn();
+ render(
+ <ChartPills
+ chartStatus="success"
+ chartUpdateStartTime={0}
+ rowLimit={ROW_LIMIT}
+ refreshCachedQuery={refreshCachedQuery}
+ {...props}
+ />,
+ );
+ return { refreshCachedQuery };
+};
+
+// The table viz reports its total row count in a second query when the
+// dataset is paginated on the server.
+const paginatedTableResponses = (
+ pageRowCount: number,
+ totalRowCount: number,
+): QueryData[] => [
+ { rowcount: pageRowCount, data: [] },
+ { data: [{ rowcount: totalRowCount }] },
+];
+
+test.each([VizType.Table, VizType.TableAgGrid])(
+ 'reads the row count of a server-paginated %s from the second query',
+ vizType => {
+ renderPills({
+ queriesResponse: paginatedTableResponses(25, 250),
+ formData: { viz_type: vizType, server_pagination: true },
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inert server_pagination fixture</b></div>
<div id="fix">
`ChartPills` never reads `formData.server_pagination` (only `viz_type`; see `ChartPills.tsx`), so this flag is inert and the test name implies a causality the component does not implement - the second query is used whenever `queriesResponse.length > 1`. Drop the flag (also at line 65) or implement the gate in `ChartPills`.
</div>
</div>
<small><i>Code Review Run #d837aa</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/ChartPills.test.tsx:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 { QueryData, VizType } from '@superset-ui/core';
+import { render, screen, userEvent } from 'spec/helpers/testing-library';
+import { ChartPills, ChartPillsProps } from './ChartPills';
+
+const ROW_LIMIT = 10000;
+
+const renderPills = (props: Partial<ChartPillsProps> = {}) => {
+ const refreshCachedQuery = jest.fn();
+ render(
+ <ChartPills
+ chartStatus="success"
+ chartUpdateStartTime={0}
+ rowLimit={ROW_LIMIT}
+ refreshCachedQuery={refreshCachedQuery}
+ {...props}
+ />,
+ );
+ return { refreshCachedQuery };
+};
+
+// The table viz reports its total row count in a second query when the
+// dataset is paginated on the server.
+const paginatedTableResponses = (
+ pageRowCount: number,
+ totalRowCount: number,
+): QueryData[] => [
+ { rowcount: pageRowCount, data: [] },
+ { data: [{ rowcount: totalRowCount }] },
+];
+
+test.each([VizType.Table, VizType.TableAgGrid])(
+ 'reads the row count of a server-paginated %s from the second query',
+ vizType => {
+ renderPills({
+ queriesResponse: paginatedTableResponses(25, 250),
+ formData: { viz_type: vizType, server_pagination: true },
+ });
+
+ expect(screen.getByText('250 rows')).toBeInTheDocument();
+ expect(screen.queryByText('25 rows')).not.toBeInTheDocument();
+ },
+);
+
+test('reads the row count from the first query for a table with a single query', () => {
+ renderPills({
+ queriesResponse: [{ rowcount: 25, data: [] }],
+ formData: { viz_type: VizType.Table, server_pagination: true },
+ });
+
+ expect(screen.getByText('25 rows')).toBeInTheDocument();
+});
+
+test('reads the row count from the first query for non-table charts even with a second query', () => {
+ renderPills({
+ queriesResponse: paginatedTableResponses(25, 250),
+ formData: { viz_type: VizType.Histogram },
+ });
+
+ expect(screen.getByText('25 rows')).toBeInTheDocument();
+ expect(screen.queryByText('250 rows')).not.toBeInTheDocument();
+});
+
+test('prefers sql_rowcount over rowcount', () => {
+ renderPills({
+ queriesResponse: [{ sql_rowcount: 40, rowcount: 7, data: [] }],
+ formData: { viz_type: VizType.Histogram },
+ });
+
+ expect(screen.getByText('40 rows')).toBeInTheDocument();
+ expect(screen.queryByText('7 rows')).not.toBeInTheDocument();
+});
+
+test('falls back to rowcount when sql_rowcount is absent', () => {
+ renderPills({
+ queriesResponse: [{ rowcount: 7, data: [] }],
+ formData: { viz_type: VizType.Histogram },
+ });
+
+ expect(screen.getByText('7 rows')).toBeInTheDocument();
+});
+
+test('hides the row count but keeps the timer when hideRowCount is set', () => {
+ renderPills({
+ queriesResponse: [{ rowcount: 25, data: [] }],
+ hideRowCount: true,
+ });
+
+ expect(screen.queryByText('25 rows')).not.toBeInTheDocument();
+ expect(screen.getByRole('timer')).toBeInTheDocument();
+});
+
+test('does not show the row count without a query response', () => {
+ renderPills({ queriesResponse: [] });
+
+ expect(screen.queryByText(/rows?$/)).not.toBeInTheDocument();
+});
+
+test('shows the cached label for a cached response and refreshes on click', async () => {
+ const { refreshCachedQuery } = renderPills({
+ queriesResponse: [
+ {
+ rowcount: 25,
+ is_cached: true,
+ cached_dttm: '2024-01-01T00:00:00',
+ data: [],
+ },
+ ],
+ });
+
+ expect(refreshCachedQuery).not.toHaveBeenCalled();
+ await userEvent.click(screen.getByText('Cached'));
+
+ expect(refreshCachedQuery).toHaveBeenCalledTimes(1);
+});
+
+test('does not show the cached label for an uncached response', () => {
+ renderPills({ queriesResponse: [{ rowcount: 25, data: [] }] });
+
+ expect(screen.queryByText('Cached')).not.toBeInTheDocument();
+});
+
+test('shows neither the row count nor the cached label while loading', () => {
+ renderPills({
+ chartStatus: 'loading',
+ queriesResponse: [
+ {
+ rowcount: 25,
+ is_cached: true,
+ cached_dttm: '2024-01-01T00:00:00',
+ data: [],
+ },
+ ],
+ });
+
+ expect(screen.queryByText('25 rows')).not.toBeInTheDocument();
+ expect(screen.queryByText('Cached')).not.toBeInTheDocument();
+ expect(screen.getByRole('timer')).toBeInTheDocument();
+});
+
+test('shows the elapsed time between the update start and end', () => {
+ renderPills({
+ chartUpdateStartTime: 1000,
+ chartUpdateEndTime: 3500,
+ queriesResponse: [{ rowcount: 25, data: [] }],
+ });
+
+ expect(screen.getByRole('timer')).toHaveTextContent('00:00:02.50');
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Weak timer assertion</b></div>
<div id="fix">
`Timer` renders `fDuration(startTime, endTime)` with format `HH:mm:ss.SSS` (`packages/superset-ui-core/src/utils/dates.ts`), so the label shows '00:00:02.500'. `toHaveTextContent` matches substrings, so expecting '00:00:02.50' passes but would not catch a regression to two-digit milliseconds. Pin the full rendered value.
</div>
</div>
<small><i>Code Review Run #d837aa</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-5922387309
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44848?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 81.69%. Comparing base ([`883e43e`](https://app.codecov.io/gh/apache/superset/commit/883e43ee2daca50c92e83d36604f8dd12873434d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4cf1699`](https://app.codecov.io/gh/apache/superset/commit/4cf169966a28e7e9e910c2d1dd9bab569fa5c257?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44848 +/- ##
==========================================
+ Coverage 81.65% 81.69% +0.03%
==========================================
Files 2977 2977
Lines 181253 181253
Branches 41873 41873
==========================================
+ Hits 148003 148070 +67
+ Misses 30522 30455 -67
Partials 2728 2728
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44848/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/44848/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.14% <ΓΈ> (+0.07%)` | :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/44848?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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-5956682090
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 9c64e654e3d0d5ecd53215cd7892b357bf949625 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6abfd9ea45b99700085818b0 |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-44848--superset-docs-preview.netlify.app](https://deploy-preview-44848--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#q1YqKMrPLShRslJS0lFKKkrMS84AslMrCnLyi1J1i0pydIszUnNygJIpqUCxSs8UoLRZYlJaimVqoolpkqWluYGBgYWphaFFkoFSLQA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44848--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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #44848:
URL: https://github.com/apache/superset/pull/44848#discussion_r4167638427
##########
superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx:
##########
@@ -1309,3 +1328,297 @@ test('automatic axis title margin adjustment handles both X and Y axis titles be
jest.restoreAllMocks();
}
});
+
+const getChart = (store: Store) =>
+ (
+ store.getState() as {
+ charts: Record<number, { triggerQuery?: boolean }>;
+ }
+ ).charts[1];
+
+test.each([
+ ['Ctrl', '{Control>}{Enter}{/Control}'],
+ ['Cmd', '{Meta>}{Enter}{/Meta}'],
+])('%s+Enter anywhere on the page queues the chart query', async (_, keys) => {
+ setupTableChartControlPanel();
+ try {
+ const store = createStore(reduxState, reducerIndex);
+ renderWithRouter({ initialState: reduxState, store: store as Store });
+ await screen.findByTestId('control-panels-container');
+
+ // Mounting queues a query; clear it so only the shortcut can set it again.
+ act(() => {
+ store.dispatch(chartActions.triggerQuery(false, 1));
+ });
+ expect(getChart(store as Store).triggerQuery).toBe(false);
+
+ await userEvent.keyboard(keys);
+
+ await waitFor(() =>
+ expect(getChart(store as Store).triggerQuery).toBe(true),
+ );
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+test.each([
+ ['Enter without a modifier', '{Enter}'],
+ ['Ctrl with another key', '{Control>}a{/Control}'],
+])('%s does not queue the chart query', async (_, keys) => {
+ setupTableChartControlPanel();
+ try {
+ const store = createStore(reduxState, reducerIndex);
+ renderWithRouter({ initialState: reduxState, store: store as Store });
+ await screen.findByTestId('control-panels-container');
+ act(() => {
+ store.dispatch(chartActions.triggerQuery(false, 1));
+ });
+
+ await userEvent.keyboard(keys);
+
+ expect(getChart(store as Store).triggerQuery).toBe(false);
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+const stateWithControls = (controls: object) => ({
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ controls: { ...reduxState.explore.controls, ...controls },
+ },
+});
+
+test('keeps the Run button enabled when no control has validation errors', async () => {
+ setupTableChartControlPanel();
+ try {
+ renderWithRouter({
+ initialState: stateWithControls({
+ metric: { value: 'count', label: 'Metric' },
+ }),
+ });
+
+ const panel = await screen.findByTestId('control-panels-container');
+ expect(panel).toHaveAttribute('data-has-button-error', 'false');
+ expect(screen.getByRole('button', { name: 'Update chart' })).toBeEnabled();
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+test('disables the Run button and reports the error when a control fails validation', async () => {
+ setupTableChartControlPanel();
+ try {
+ renderWithRouter({
+ initialState: stateWithControls({
+ metric: {
+ value: '',
+ label: 'Metric',
+ validationErrors: ['Metric is required'],
+ },
+ }),
+ });
+
+ const panel = await screen.findByTestId('control-panels-container');
+ expect(panel).toHaveAttribute('data-has-button-error', 'true');
+ expect(panel).toHaveAttribute('data-has-data-tab-error', 'true');
+ expect(screen.getByRole('button', { name: 'Update chart' })).toBeDisabled();
+ expect(screen.getByRole('tooltip')).toHaveTextContent(
+ 'Control labeled Metric: Metric is required',
+ );
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+test('disables the Run button for a matrixify control error without flagging the Data tab', async () => {
+ setupTableChartControlPanel();
+ try {
+ renderWithRouter({
+ initialState: stateWithControls({
+ matrixify_rows: {
+ value: '',
+ label: 'Rows',
+ tabOverride: 'matrixify',
+ validationErrors: ['Rows are required'],
+ },
+ }),
+ });
+
+ const panel = await screen.findByTestId('control-panels-container');
+ expect(panel).toHaveAttribute('data-has-button-error', 'true');
+ expect(panel).toHaveAttribute('data-has-data-tab-error', 'false');
+ expect(screen.getByRole('button', { name: 'Update chart' })).toBeDisabled();
+ } finally {
+ getChartControlPanelRegistry().remove('table');
+ }
+});
+
+const tooltipState = (vizType: string, template = '') => ({
+ ...reduxState,
+ charts: {
+ 1: {
+ ...reduxState.charts[1],
+ latestQueryFormData: {
+ ...reduxState.charts[1].latestQueryFormData,
+ viz_type: vizType,
+ },
+ },
+ },
+ explore: {
+ ...reduxState.explore,
+ form_data: { datasource: '1__table', viz_type: vizType, metrics: [] },
+ controls: {
+ ...reduxState.explore.controls,
+ viz_type: { value: vizType },
+ tooltip_contents: { value: [] },
+ tooltip_template: { value: template },
+ },
+ },
+});
+
+// Renders the container and returns a function that selects tooltip contents
+// the way the control would and reports the template the container wrote back.
+const renderTooltipChart = (vizType: string, template = '') => {
+ getChartControlPanelRegistry().registerValue(vizType, {
+ controlPanelSections: [],
+ });
+ const setControlValueSpy = jest.spyOn(exploreActions, 'setControlValue');
+ const initialState = tooltipState(vizType, template);
+ const store = createStore(initialState, reducerIndex);
+ renderWithRouter({ initialState, store: store as Store });
+ setControlValueSpy.mockClear();
+ return {
+ selectTooltipContents: (contents: unknown[]) => {
+ act(() => {
+ store.dispatch(
+ exploreActions.setControlValue('tooltip_contents', contents),
+ );
+ });
+ },
+ templateWrites: () =>
+ setControlValueSpy.mock.calls
+ .filter(([controlName]) => controlName === 'tooltip_template')
+ .map(([, value]) => value),
+ storedTemplate: () =>
+ ((store as Store).getState() as ExplorePageState).explore.controls
+ .tooltip_template?.value,
+ cleanup: () => getChartControlPanelRegistry().remove(vizType),
+ };
+};
+
+test('appends a {{ field }} variable to the tooltip template for a selected column', () => {
+ const chart = renderTooltipChart('deck_scatter');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual(['{{ name }}']);
+ expect(chart.storedTemplate()).toBe('{{ name }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('appends after the existing tooltip template text', () => {
+ const chart = renderTooltipChart('deck_scatter', 'Details:');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual(['Details: {{ name }}']);
+ expect(chart.storedTemplate()).toBe('Details: {{ name }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('appends a limited variable for a column on an aggregated chart', () => {
+ const chart = renderTooltipChart('heatmap');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual(['{{ limit name 10 }}']);
+ expect(chart.storedTemplate()).toBe('{{ limit name 10 }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('appends a plain variable for a metric even on an aggregated chart', () => {
+ const chart = renderTooltipChart('heatmap');
+ try {
+ chart.selectTooltipContents([
+ { item_type: 'metric', metric_name: 'count' },
+ ]);
+
+ expect(chart.templateWrites()).toEqual(['{{ count }}']);
+ expect(chart.storedTemplate()).toBe('{{ count }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+test('leaves the tooltip template alone when it already references the field', () => {
+ const chart = renderTooltipChart('deck_scatter', 'Name: {{ name }}');
+ try {
+ chart.selectTooltipContents([{ item_type: 'column', column_name: 'name' }]);
+
+ expect(chart.templateWrites()).toEqual([]);
+ expect(chart.storedTemplate()).toBe('Name: {{ name }}');
+ } finally {
+ chart.cleanup();
+ }
+});
+
+const datasourceState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ form_data: { datasource: '1__table', viz_type: VizType.Table, metrics: [] },
+ controls: { ...reduxState.explore.controls, row_limit: { value: 100 } },
+ },
+};
+
+test('requests the datasource metadata for the new datasource when the datasource control changes', () => {
+ setupTableChartControlPanel();
+ try {
+ const fetchMetadataSpy = jest.spyOn(
+ datasourceActions,
+ 'fetchDatasourceMetadata',
+ );
+ const store = createStore(datasourceState, reducerIndex);
+ renderWithRouter({ initialState: datasourceState, store: store as Store });
+ fetchMetadataSpy.mockClear();
+
+ act(() => {
+ store.dispatch(exploreActions.setControlValue('datasource', '2__table'));
+ });
+
+ expect(fetchMetadataSpy).toHaveBeenCalledTimes(1);
+ expect(fetchMetadataSpy).toHaveBeenCalledWith('2__table');
Review Comment:
This test passes as written. `props.form_data` is rebuilt from the current controls in `mapStateToProps` (`getFormDataFromControls(controls)`), so after the datasource control changes it already holds `2__table`, not the initial `1__table`. I confirmed the test fails if the effect is made to pass a stale value (Expected `2__table`, Received `1__table`), so it guards the behavior. No change needed.
--
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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-5922313189
<!-- Bito Reply -->
The flagged issue regarding the API mismatch in the test file cannot be validated or resolved because the specified file, `superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx`, is not present in the provided pull request diff. The PR appears to introduce a new test file, `ChartPills.test.tsx`, but does not contain the file mentioned in the review comment.
--
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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-5963810408
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 an incremental AI Review.
- `/review full` - 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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44848:
URL: https://github.com/apache/superset/pull/44848#issuecomment-5922259410
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(explore): cover chart header actions, chart pills, and view container wiring [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #44848:
URL: https://github.com/apache/superset/pull/44848
--
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