You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "prathamesh04 (via GitHub)" <gi...@apache.org> on 2026/07/26 13:55:49 UTC
[PR] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
prathamesh04 opened a new pull request, #42440:
URL: https://github.com/apache/superset/pull/42440
## Summary
The `expose_in_sqllab` filter in `DatabaseSelector` was tied to `sqlLabMode`, which controls the UI rendering (compact button vs full dropdown). When the SQL Lab popover passed `sqlLabMode={false}` to render the dropdown, the `expose_in_sqllab` filter was also disabled, allowing databases with `expose_in_sqllab=false` to appear in SQL Lab.
## Before / After
**Before**: The popover's `DatabaseSelector` in SQL Lab passed `sqlLabMode={false}` (to render the dropdown instead of a compact button). The `loadDatabases` function checked `formMode || !sqlLabMode` -- since `sqlLabMode=false`, this evaluated to `true`, skipping the `expose_in_sqllab` filter.
**After**: Added a new `filterBySqlLab` prop that controls the `expose_in_sqllab` filter independently from the UI rendering mode. The popover's `DatabaseSelector` now passes `filterBySqlLab={true}` to apply the filter while still rendering the dropdown.
## Testing Instructions
1. Create a database with `expose_in_sqllab = false`
2. Create another database with `expose_in_sqllab = true`
3. Open SQL Lab -- the database with `expose_in_sqllab = false` should NOT appear in the dropdown
## Additional Information
- Fixes #40850
- Regression introduced in 6.1.0 when the SQL Lab left bar was refactored to use a popover instead of inline dropdown
- Added unit test verifying the `expose_in_sqllab` filter is included in API calls when `filterBySqlLab={true}`
--
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [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 #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: 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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5083793346
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42440?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 55.04%. Comparing base ([`ba0900d`](https://app.codecov.io/gh/apache/superset/commit/ba0900d2b32fc4b294d5192312b136cd1851b349?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a9c8002`](https://app.codecov.io/gh/apache/superset/commit/a9c800285f48bd6782421725c0ca5db17c0226d3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (ba0900d) and HEAD (a9c8002). Click for more details.
>
> <details><summary>HEAD has 84 uploads less than BASE</summary>
>
>| Flag | BASE (ba0900d) | HEAD (a9c8002) |
>|------|------|------|
>|python|74|1|
>|hive|12|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42440 +/- ##
===========================================
- Coverage 65.23% 55.04% -10.19%
===========================================
Files 2795 2795
Lines 157694 157692 -2
Branches 36067 36066 -1
===========================================
- Hits 102871 86804 -16067
- Misses 52847 70470 +17623
+ Partials 1976 418 -1558
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.39% <ø> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.39% <ø> (-20.62%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/42440/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/42440?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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #42440:
URL: https://github.com/apache/superset/pull/42440#discussion_r3672033690
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete assertion on filter value</b></div>
<div id="fix">
The assertion only checks for the presence of `'expose_in_sqllab'` in the URL string, but doesn't verify the value is `true`. This allows a subtle regression where the filter might be added with an incorrect value (e.g., `false` or missing) without the test catching it.
</div>
</div>
<small><i>Code Review Run #a0754a</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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5121226916
@prathamesh04 if you can assess and either apply or refute the newest bot findings, we'll get this merged :) Just want to make sure it's all tidied up to not leave follow-up work.
--
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #42440:
URL: https://github.com/apache/superset/pull/42440
--
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5083781590
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5113138343
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 0d883e7ddd0f3ceeaabcda4eeef61f92840cbdb0 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a6980e4f68e8a000896621b |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42440--superset-docs-preview.netlify.app](https://deploy-preview-42440--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#Ncs7DoAgEADRu2wtEY3ZgDfwGCBLJOEnUGiMd5fGbqZ4D-SSQm6wAgygi4r70du6a6Qrp0rMRVZP75Vm1vlG5b-QDHViKPt0b6YjVCgFp8WiIKE450IizpOG9wM) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42440--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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "prathamesh04 (via GitHub)" <gi...@apache.org>.
prathamesh04 commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5113096609
Hi @rusackas — I've removed the language_pack commit from this branch. The branch now only contains the sqllab `filterBySqlLab` fix. The language pack change is tracked separately in PR #42438. Thanks!
--
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5114580339
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 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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5109551072
Hey @prathamesh04, thanks for the fix! CI's failing on `test_views_are_secured`, and it's not from the sqllab change, it's the second commit dropping `@has_access` from `language_pack`, which isn't mentioned in the PR description. Mind splitting that off into its own PR? An auth change like that needs to stand on its own for review.
--
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5123415586
@prathamesh04 don't sweat the bot nit, that's just test strictness and the underlying fix is correct. Not worth holding up a real regression over. Merging!
--
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] fix(sqllab): add filterBySqlLab prop to decouple expose_in_sqllab filter from UI rendering mode [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42440:
URL: https://github.com/apache/superset/pull/42440#issuecomment-5113169232
<!-- Bito Reply -->
The flagged issue is correct. The current test only verifies the presence of the field name in the URL, which is insufficient to ensure the filter is correctly applied with the expected operator and value. To resolve this, you should parse the Rison-encoded filters from the request URL and assert the specific filter object.
Here is the corrected test implementation:
```typescript
import { parseRison } from 'src/utils/rison';
// ... inside the test
const lastCall = calls[calls.length - 1];
const url = new URL(lastCall.url, window.location.origin);
const filters = parseRison(url.searchParams.get('filters') || '');
expect(filters).toContainEqual({
col: 'expose_in_sqllab',
opr: 'eq',
value: true,
});
```
There are no other comments on this PR to address.
**superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx**
```
const lastCall = calls[calls.length - 1];
const url = new URL(lastCall.url, window.location.origin);
const filters = parseRison(url.searchParams.get('filters') || '');
expect(filters).toContainEqual({
col: 'expose_in_sqllab',
opr: 'eq',
value: true,
});
```
--
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