codeant-ai-for-open-source[bot] commented on code in PR #41509:
URL: https://github.com/apache/superset/pull/41509#discussion_r3630190843
##########
superset-frontend/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx:
##########
@@ -89,6 +94,48 @@ describe('DataTablesPane', () => {
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
});
+ test('Hides Samples tab when datasource opts out via
supports_samples=false', async () => {
+ const props = createDataTablesPaneProps(0);
+ const propsWithoutSamples = {
+ ...props,
+ datasource: { ...props.datasource, supports_samples: false },
+ };
+ render(<DataTablesPane {...propsWithoutSamples} />, { useRedux: true });
+ expect(await screen.findByText('Results')).toBeVisible();
+ expect(screen.queryByText('Samples')).not.toBeInTheDocument();
+ });
+
+ test('Falls back to Results when active Samples tab disappears mid-session',
async () => {
+ // Regression for codeant Major finding on PR #41509: a datasource swap
+ // that hides the Samples tab while it was the active tab used to leave
+ // ``activeTabKey === 'samples'`` orphaned, rendering a blank panel.
+ const props = createDataTablesPaneProps(0);
+ const { rerender } = render(<DataTablesPane {...props} />, {
+ useRedux: true,
+ });
+
+ // Open the panel and pick the Samples tab.
+ userEvent.click(screen.getByLabelText('Expand data panel'));
+ userEvent.click(await screen.findByText('Samples'));
+ expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
+
+ // Swap to a datasource that doesn't support samples (e.g. a semantic
+ // view). The Samples tab should disappear and the panel should land on
+ // Results with content still rendered.
+ rerender(
+ <DataTablesPane
+ {...props}
+ datasource={{ ...props.datasource, supports_samples: false }}
+ />,
+ );
+ await waitFor(() => {
+ expect(screen.queryByText('Samples')).not.toBeInTheDocument();
+ });
+ expect(screen.getByText('Results')).toBeVisible();
+ // Panel stays expanded and renders Results content rather than going
blank.
+ expect(screen.getByLabelText('Collapse data panel')).toBeVisible();
Review Comment:
**Suggestion:** This regression test does not actually verify that the pane
switched away from an orphaned tab and rendered fallback content; it only
checks that the tab label and collapse button are visible, which can still be
true when the content area is blank. Add an assertion on concrete Results-pane
content (or active tab state) after rerender so the original blank-panel bug
cannot pass unnoticed. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Explore Data panel regression may bypass this specific test.
- ⚠️ Blank Results content on datasource swap might reoccur undetected.
- ⚠️ Regression coverage for semantic view datasources remains incomplete.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open
`superset-frontend/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx`
and
locate the test `Falls back to Results when active Samples tab disappears
mid-session` at
lines 108–137, which documents a prior bug leaving `activeTabKey ===
'samples'` and
rendering a blank panel (comment at lines 109–112).
2. Assume the original bug in `DataTablesPane` reappears so that, after a
datasource swap
that removes the Samples tab, the tab strip (including the `Results` label)
and the panel
chrome (including the `Collapse data panel` button) still render, but the
content area
remains blank because the active tab key no longer matches any rendered tab
(as described
in the test comment).
3. Run the frontend test suite (e.g. `npm test -- DataTablesPane.test.tsx`)
so this
regression test mounts `DataTablesPane`, clicks `Expand data panel` and
`Samples`, then
rerenders with `supports_samples: false`, following the exact sequence coded
at lines
113–133.
4. Observe that the assertions at lines 134–136 only verify that the
`Results` tab label
and `Collapse data panel` button are visible, without asserting any concrete
Results
content (such as table rows) or the active tab state; in the bug scenario
described in
step 2 these UI chrome elements can still be present while the content area
is blank, so
the test still passes and fails to catch the regression.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=88203ce1d697483782014a2fc7e55ee3&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=88203ce1d697483782014a2fc7e55ee3&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/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx
**Line:** 134:136
**Comment:**
*Logic Error: This regression test does not actually verify that the
pane switched away from an orphaned tab and rendered fallback content; it only
checks that the tab label and collapse button are visible, which can still be
true when the content area is blank. Add an assertion on concrete Results-pane
content (or active tab state) after rerender so the original blank-panel bug
cannot pass unnoticed.
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%2F41509&comment_hash=1b5c619447d6d6a7d50f4594af2f9f4779cef79606280c27e2d3c60ce4902457&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41509&comment_hash=1b5c619447d6d6a7d50f4594af2f9f4779cef79606280c27e2d3c60ce4902457&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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
