codeant-ai-for-open-source[bot] commented on code in PR #42440:
URL: https://github.com/apache/superset/pull/42440#discussion_r3670980701
##########
superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx:
##########
@@ -390,3 +390,21 @@ test('Sends the correct schema when changing the schema',
async () => {
);
expect(props.onSchemaChange).toHaveBeenCalledTimes(1);
});
+
+test('should include expose_in_sqllab filter when filterBySqlLab is true',
async () => {
+ const props = createProps();
+ render(<DatabaseSelector {...props} sqlLabMode={false} filterBySqlLab />, {
+ useRedux: true,
+ store,
+ });
+ const select = screen.getByRole('combobox', {
+ name: 'Select database or type to search databases',
+ });
+ await userEvent.click(select);
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls(databaseApiRoute);
+ expect(calls.length).toBeGreaterThanOrEqual(1);
+ const lastCall = calls[calls.length - 1];
+ expect(lastCall.url).toContain('expose_in_sqllab');
Review Comment:
**Suggestion:** The assertion only verifies that the URL contains the field
name, so it passes even if the filter uses the wrong operator or a
false/non-boolean value. Parse the request query and assert that the filter
contains `col: expose_in_sqllab`, `opr: eq`, and `value: true`; otherwise this
test does not verify the behavior introduced by the production change. [code
quality]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Regression test can accept an incorrect SQL Lab filter.
- ❌ Future changes could expose non-SQL-Lab databases.
- ⚠️ The test does not validate operator or boolean value.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the new test `should include expose_in_sqllab filter when
filterBySqlLab is true`
in
`superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx:394-410`;
it renders `DatabaseSelector` with `sqlLabMode={false}` and `filterBySqlLab`
enabled at
lines 396-398.
2. The component builds the database API filters in
`superset-frontend/src/components/DatabaseSelector/index.tsx:232-240`, where
the intended
filter is `{ col: 'expose_in_sqllab', opr: 'eq', value: true }`.
3. The test obtains the last database API request at
`DatabaseSelector.test.tsx:404-408`
but only checks that the URL contains the field name at line 408. A request
containing
`expose_in_sqllab` with a wrong operator, false value, or malformed filter
would therefore
still pass.
4. Parse the request query's Rison-encoded filters and assert an object
containing `col:
'expose_in_sqllab'`, `opr: 'eq'`, and `value: true`; this will verify the
behavior
implemented by `index.tsx:232-240` rather than merely checking the field
name.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a3d9686597d04d2d99f928de50ace150&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=a3d9686597d04d2d99f928de50ace150&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/components/DatabaseSelector/DatabaseSelector.test.tsx
**Line:** 408:408
**Comment:**
*Code Quality: The assertion only verifies that the URL contains the
field name, so it passes even if the filter uses the wrong operator or a
false/non-boolean value. Parse the request query and assert that the filter
contains `col: expose_in_sqllab`, `opr: eq`, and `value: true`; otherwise this
test does not verify the behavior introduced by the production change.
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%2F42440&comment_hash=6a6ea4e03ae9c442c9c01470afed1cee647683091a1fbb75bb99ee03dfdc230d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42440&comment_hash=6a6ea4e03ae9c442c9c01470afed1cee647683091a1fbb75bb99ee03dfdc230d&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]
