codeant-ai-for-open-source[bot] commented on code in PR #40905:
URL: https://github.com/apache/superset/pull/40905#discussion_r3471355847
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx:
##########
@@ -249,3 +319,75 @@ describe('ColumnSelect filterValues behavior', () => {
expect(screen.queryByText('col2')).not.toBeInTheDocument();
});
});
+
+// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
+describe('plugin column-picker control (isColumnSelect)', () => {
+ function renderColumnPicker(
+ controlItemConfig: Record<string, unknown> = {},
+ propsOverrides: Partial<ControlItemsProps> = {},
+ ) {
+ (getControlItems as jest.Mock).mockReturnValue([
+ {
+ name: 'myPluginColumn',
+ config: { isColumnSelect: true, label: 'My Column',
...controlItemConfig },
+ },
+ ]);
+ const props = { ...createProps(), datasetId: 1, ...propsOverrides };
+ const map = getControlItemsMap(props);
+ render(map.mainControlItems.myPluginColumn.element as React.ReactElement);
+ return props;
+ }
+
+ test('populates options after a successful fetch, dropping blank column
names', async () => {
+ const fetchPromise = Promise.resolve({
+ json: {
+ result: {
+ columns: [
+ { column_name: 'col_a' },
+ { column_name: '' },
+ { column_name: null },
+ ],
+ },
+ },
+ } as any);
Review Comment:
**Suggestion:** Remove the `as any` cast and type the mocked fetch response
with an explicit interface or a narrowed mock response type that matches
`cachedSupersetGet`. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The added test code uses an `as any` assertion for the mocked fetch
response, which is a direct violation of the rule against using `any` in
modified TypeScript code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=89211a7382244b998c1602be50efe23e&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=89211a7382244b998c1602be50efe23e&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/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
**Line:** 352:352
**Comment:**
*Custom Rule: Remove the `as any` cast and type the mocked fetch
response with an explicit interface or a narrowed mock response type that
matches `cachedSupersetGet`.
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%2F40905&comment_hash=61983440a9b764591e8ed0190c514622c50b4147807e80b3e1666800d84ee45c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=61983440a9b764591e8ed0190c514622c50b4147807e80b3e1666800d84ee45c&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx:
##########
@@ -32,12 +39,19 @@ jest.mock('./utils', () => ({
doesColumnMatchFilterType: jest.fn(),
}));
-// Mock ColumnSelect to test filterValues logic
+jest.mock('src/utils/cachedSupersetGet');
+const mockCachedSupersetGet = cachedSupersetGet as jest.MockedFunction<
+ typeof cachedSupersetGet
+>;
+
+// Mock ColumnSelect to test filterValues logic and expose onChange
jest.mock('./ColumnSelect', () => ({
ColumnSelect: ({
filterValues,
+ onChange,
}: {
filterValues: (column: any) => boolean;
Review Comment:
**Suggestion:** Replace the `any` parameter type with a concrete column
shape (for example a minimal object type containing the fields used in the
test) so the callback is type-safe. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a new TypeScript test file and the mock component explicitly types
the callback parameter as `any`, which matches the custom rule forbidding `any`
in new or modified TS/TSX code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b10f3c18c5f04708b0c42fcf3c6a060a&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=b10f3c18c5f04708b0c42fcf3c6a060a&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/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
**Line:** 53:53
**Comment:**
*Custom Rule: Replace the `any` parameter type with a concrete column
shape (for example a minimal object type containing the fields used in the
test) so the callback is type-safe.
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%2F40905&comment_hash=86d7cd52915d1b94e9d945a981be0aeff1d53d1de7c8d14b7d22717a356e3686&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=86d7cd52915d1b94e9d945a981be0aeff1d53d1de7c8d14b7d22717a356e3686&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx:
##########
@@ -249,3 +319,75 @@ describe('ColumnSelect filterValues behavior', () => {
expect(screen.queryByText('col2')).not.toBeInTheDocument();
});
});
+
+// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
+describe('plugin column-picker control (isColumnSelect)', () => {
+ function renderColumnPicker(
+ controlItemConfig: Record<string, unknown> = {},
+ propsOverrides: Partial<ControlItemsProps> = {},
+ ) {
+ (getControlItems as jest.Mock).mockReturnValue([
+ {
+ name: 'myPluginColumn',
+ config: { isColumnSelect: true, label: 'My Column',
...controlItemConfig },
+ },
+ ]);
+ const props = { ...createProps(), datasetId: 1, ...propsOverrides };
+ const map = getControlItemsMap(props);
+ render(map.mainControlItems.myPluginColumn.element as React.ReactElement);
+ return props;
+ }
+
+ test('populates options after a successful fetch, dropping blank column
names', async () => {
+ const fetchPromise = Promise.resolve({
+ json: {
+ result: {
+ columns: [
+ { column_name: 'col_a' },
+ { column_name: '' },
+ { column_name: null },
+ ],
+ },
+ },
+ } as any);
+ mockCachedSupersetGet.mockReturnValue(fetchPromise);
+ renderColumnPicker();
+ expect(mockCachedSupersetGet).toHaveBeenCalledWith({
+ endpoint: expect.stringContaining('/api/v1/dataset/1?q='),
+ });
+ await act(async () => {
+ await fetchPromise;
+ });
+ userEvent.click(screen.getByRole('combobox'));
+ expect(await screen.findByRole('option', { name: 'col_a'
})).toBeInTheDocument();
+ expect(screen.getAllByRole('option')).toHaveLength(1);
+ });
+
+ test('falls back to no options when the fetch rejects', async () => {
+ mockCachedSupersetGet.mockRejectedValue(new Error('boom'));
+ renderColumnPicker();
+ await waitFor(() => expect(mockCachedSupersetGet).toHaveBeenCalled());
+ userEvent.click(screen.getByRole('combobox'));
+ expect(screen.queryAllByRole('option')).toHaveLength(0);
+ });
+
+ test('onChange resets defaultDataMask and notifies change', async () => {
+ const fetchPromise = Promise.resolve({
+ json: { result: { columns: [{ column_name: 'col_a' }] } },
+ } as any);
Review Comment:
**Suggestion:** Replace this `as any` assertion with a concrete typed mock
payload so the test verifies response structure with compile-time checks.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is the same `as any` pattern in another newly added test block. It is
present in the final TypeScript file and violates the custom rule requiring a
concrete or narrower type instead of `any`.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b885ac9c1c1d44cb95b78b8afacf3dec&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=b885ac9c1c1d44cb95b78b8afacf3dec&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/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
**Line:** 377:377
**Comment:**
*Custom Rule: Replace this `as any` assertion with a concrete typed
mock payload so the test verifies response structure with compile-time checks.
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%2F40905&comment_hash=2edac89f68eeae481083fb904f99b9a589baa1b17c68de30a8f283b18d7d8b78&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=2edac89f68eeae481083fb904f99b9a589baa1b17c68de30a8f283b18d7d8b78&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]
