You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "SkinnyPigeon (via GitHub)" <gi...@apache.org> on 2026/07/27 11:35:41 UTC
[PR] fix(chart) dataset dropdown sorted by name [superset]
SkinnyPigeon opened a new pull request, #42475:
URL: https://github.com/apache/superset/pull/42475
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
The dataset dropdown in the **Create a new chart** screen was not sorted alphabetically by name. Although `/api/v1/dataset/` already returns results ordered by `table_name asc`, `AsyncSelect` re-sorts options internally using `DEFAULT_SORT_COMPARATOR`. Because `DatasetSelectLabel` returns a React node (a `<Tooltip>` wrapper) rather than a plain string, the comparator fell back to `(a.value as number) - (b.value as number)` on string values like `"5__table"`, producing `NaN` and effectively randomising the order.
**Fix:** Pass a custom `sortComparator` to the `AsyncSelect` in `ChartCreation` that reads `table_name` from each option object (already present in the options data) and sorts with `localeCompare`. This is a targeted, low-risk change isolated to this one component and does not affect any other `Select` usage across the app.
Fixes #42466
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:** datasets appear in creation/ID order (e.g. id 2, 5, 4, 3, 6) regardless of name.
**After:** datasets appear alphabetically by `table_name`, consistent with the Dataset list view.
### TESTING INSTRUCTIONS
1. Sign in to Superset.
2. Go to **Charts → + Chart**.
3. Open the **Choose a dataset** dropdown.
4. Verify datasets are listed alphabetically by table name.
Automated: `npx jest --testPathPatterns="ChartCreation.test"` — all 14 tests pass, including a new test that asserts correct alphabetical DOM order when the API returns datasets with IDs deliberately in non-alphabetical order.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #42466
- [ ] Required feature flags:
- [x] 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
--
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(chart): dataset dropdown sorted by name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5144266246
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(chart): dataset dropdown sorted by name [superset]
Posted by "SkinnyPigeon (via GitHub)" <gi...@apache.org>.
SkinnyPigeon commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5118218704
Well, we've got 2 competing AIs asking for conflicting ways of handling the ordering. Happy to take any suggestions on which one to believe
--
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(chart): dataset dropdown sorted by name [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 #42475:
URL: https://github.com/apache/superset/pull/42475#discussion_r3672056019
##########
superset-frontend/src/pages/ChartCreation/index.tsx:
##########
@@ -343,6 +343,13 @@ export const ChartCreation = ({
optionFilterProps={['id', 'table_name']}
placeholder={t('Choose a %s', datasetLabelLower())}
showSearch
+ sortComparator={(a, b) => {
+ const aName =
+ (a as { table_name?: string }).table_name ?? '';
+ const bName =
+ (b as { table_name?: string }).table_name ?? '';
+ return aName.localeCompare(bName);
+ }}
Review Comment:
**Suggestion:** The API request already orders datasets by `table_name`, but this comparator replaces that ordering with the browser's default-locale `localeCompare` result. Names containing uppercase or accented characters can therefore appear in a different order from the server result, and the order can vary with the user's locale/browser. Use the same deterministic ordering semantics as the API, or avoid re-sorting the server-ordered page when the comparator cannot preserve that contract. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dataset order varies across browser locales.
- ⚠️ Mixed-case or accented names can disagree with API ordering.
- ⚠️ Users may see inconsistent Chart Creation dropdown ordering.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Chart Creation dataset selector rendered by `ChartCreation` at
`superset-frontend/src/pages/ChartCreation/index.tsx:337-355`.
2. The selector requests `/api/v1/dataset/` through `loadDatasources()` at
`superset-frontend/src/pages/ChartCreation/index.tsx:227-252`, explicitly requesting
`order_column: 'table_name'` and `order_direction: 'asc'` at lines 243-244.
3. Return or create datasets whose names include case or accents, such as `apple`, `Zoo`,
or an accented table name. The backend applies its database/SQL collation through the
dataset API, whose allowed ordering includes `table_name` at
`superset/datasets/api.py:183-185`.
4. `AsyncSelect` then invokes the new comparator at
`superset-frontend/src/pages/ChartCreation/index.tsx:346-352`; `String.localeCompare()`
uses the browser's default locale and collation, which can rank those names differently
from the server. The visible dropdown order can consequently vary by browser/user locale
and disagree with the API's `table_name asc` order. Use deterministic comparison semantics
matching the backend, or preserve the server ordering.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5c53e4f24f8a49f983e5e4599b598cf7&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=5c53e4f24f8a49f983e5e4599b598cf7&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/pages/ChartCreation/index.tsx
**Line:** 346:352
**Comment:**
*Logic Error: The API request already orders datasets by `table_name`, but this comparator replaces that ordering with the browser's default-locale `localeCompare` result. Names containing uppercase or accented characters can therefore appear in a different order from the server result, and the order can vary with the user's locale/browser. Use the same deterministic ordering semantics as the API, or avoid re-sorting the server-ordered page when the comparator cannot preserve that contract.
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%2F42475&comment_hash=8faba6a22957ed6f02cb7f41b69ccc63d5a0cb44165a4ab331377a88e24aeb9e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42475&comment_hash=8faba6a22957ed6f02cb7f41b69ccc63d5a0cb44165a4ab331377a88e24aeb9e&reaction=dislike'>👎</a>
##########
superset-frontend/src/pages/ChartCreation/ChartCreation.test.tsx:
##########
@@ -333,6 +333,63 @@ test('shows loading spinner when dataset parameter is present in URL', async ()
locationSpy.mockRestore();
});
+test('dataset dropdown sorts options alphabetically by table name regardless of id order', async () => {
+ fetchMock.clearHistory().removeRoutes();
+ // IDs are deliberately NOT in alphabetical order of table_name to prove
+ // that sorting is by name, not by creation/id order.
+ fetchMock.get(/\/api\/v1\/dataset\/\?q=.*/, {
+ body: {
+ result: [
+ {
+ id: 3,
+ table_name: 'alpha_table',
+ datasource_type: 'table',
+ database: { database_name: 'test_db' },
+ schema: 'public',
+ },
+ {
+ id: 1,
+ table_name: 'beta_table',
+ datasource_type: 'table',
+ database: { database_name: 'test_db' },
+ schema: 'public',
+ },
+ {
+ id: 2,
+ table_name: 'gamma_table',
+ datasource_type: 'table',
+ database: { database_name: 'test_db' },
+ schema: 'public',
+ },
+ ],
Review Comment:
**Suggestion:** The mocked response is already ordered as `alpha_table`, `beta_table`, `gamma_table`, so these assertions pass even when `sortComparator` is ignored and the component simply preserves the server response order. Supply the fixture in a deliberately unsorted `table_name` order (while keeping IDs independently unordered) so the test actually verifies client-side sorting. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ The regression test can pass when sorting is broken.
- ⚠️ Future comparator regressions may reach production undetected.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the new test `dataset dropdown sorts options alphabetically by table name
regardless of id order` at
`superset-frontend/src/pages/ChartCreation/ChartCreation.test.tsx:336`.
2. The mocked `GET /api/v1/dataset/` response at
`superset-frontend/src/pages/ChartCreation/ChartCreation.test.tsx:340-368` returns
`alpha_table`, `beta_table`, and `gamma_table` already in alphabetical order.
3. `ChartCreation.loadDatasources()` at
`superset-frontend/src/pages/ChartCreation/index.tsx:227-252` maps `response.json.result`
without changing its order, so the dropdown receives the same alphabetical sequence even
if the new comparator is removed or ignored.
4. The DOM-order assertions at
`superset-frontend/src/pages/ChartCreation/ChartCreation.test.tsx:384-390` therefore pass
for an implementation that merely preserves server order; change the fixture to an
unsorted table-name sequence while retaining independently unordered IDs to verify
client-side sorting.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8d3b2fff23cb4ec8b37cce21fe0a163f&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=8d3b2fff23cb4ec8b37cce21fe0a163f&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/pages/ChartCreation/ChartCreation.test.tsx
**Line:** 342:364
**Comment:**
*Possible Bug: The mocked response is already ordered as `alpha_table`, `beta_table`, `gamma_table`, so these assertions pass even when `sortComparator` is ignored and the component simply preserves the server response order. Supply the fixture in a deliberately unsorted `table_name` order (while keeping IDs independently unordered) so the test actually verifies client-side sorting.
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%2F42475&comment_hash=f6aff889bc8d1faa96d2209dfac485f88650a645b04caa865882a9171bbd976a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42475&comment_hash=f6aff889bc8d1faa96d2209dfac485f88650a645b04caa865882a9171bbd976a&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(chart) dataset dropdown sorted by name [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5090811484
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 90c9c013873148cc7d7923f434f5587d34d1041f |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a67428da773700008a42270 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42475--superset-docs-preview.netlify.app](https://deploy-preview-42475--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#DYtBCoAwDMD-0rMFmWLFH_iMaicKuo46UBH_bnNJLnkhmx65wABQwWSc5tV72W6cV7aCwoXP6DbNolfCU61EwenBxEf0SWLe9RnFt447akMvTNRQ7fTchkA1fD8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42475--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(chart): dataset dropdown sorted by name [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5258652670
Thanks @SkinnyPigeon! Good call reusing `propertyComparator` instead of hand-rolling it, keeps this consistent with how sorting works everywhere else in the app. Test fixture change addresses the earlier concern about it not actually discriminating sorted vs unsorted output. LGTM, approving.
--
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(chart): dataset dropdown sorted by name [superset]
Posted by "SkinnyPigeon (via GitHub)" <gi...@apache.org>.
SkinnyPigeon commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5142575323
Hey @EnxDev, thanks for taking a look. I've applied the suggested changes
--
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(chart): dataset dropdown sorted by name [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5141247693
## EnxDev's Review Agent — apache/superset#42475 · HEAD 335a118
**comment** — the fix works for the reported case, but the hand-rolled `<`/`>` comparator sorts by UTF-16 code point, which diverges from the API's `table_name asc` ordering for mixed-case names.
### On the two conflicting bot suggestions
@SkinnyPigeon — Bito is right, CodeAnt is not, and there's a third option that settles it.
CodeAnt's objection is that `localeCompare` is locale-dependent and can disagree with the server's collation. True, but it doesn't favour `<`/`>`: exact parity with the metadata DB's collation is unreachable from the browser regardless, and code-point ordering is *further* from every common server collation, not closer. Postgres (`en_US.UTF-8`) and MySQL (`utf8mb4_*_ci`) both order `apple` before `Zebra`; `<`/`>` orders `Zebra` before `apple` because `Z` is 0x5A and `a` is 0x61. Only SQLite's `BINARY` collation matches code-point order.
Superset already settled this internally: `DEFAULT_SORT_COMPARATOR` uses `localeCompare` (`Select/constants.ts:101`), as does the exported `propertyComparator` helper (`Select/utils.tsx:89`). Use the helper instead of hand-rolling — one line, and it's the established pattern (`AlertReportModal.tsx:2649` does `sortComparator={propertyComparator('value')}`):
```tsx
import { propertyComparator } from '@superset-ui/core/components/Select/utils';
// ...
sortComparator={propertyComparator('table_name')}
```
### 🔴 Functional
- **`superset-frontend/src/pages/ChartCreation/index.tsx:346`** · _High_ — Code-point comparison groups every capitalized dataset name ahead of every lowercase one (`Orders`, `Products`, `customers`, `flights`), so the dropdown contradicts both the API's `table_name asc` result and the Datasets list view — the consistency #42466 asks for. Second effect: `AsyncSelect` re-sorts the merged set on every page fetch (`AsyncSelect.tsx:382-392`, `DEFAULT_PAGE_SIZE = 100`), so past 100 datasets, page-2 items whose case ranks them ahead of already-rendered page-1 items get inserted *above* the visible list and rows jump while scrolling. Both go away if the client comparator agrees with the server ordering: `sortComparator={propertyComparator('table_name')}`. **regression test:** fixture with `table_name` values `Zebra`, `apple`, `Mango` and asserts `apple` renders before `Mango` before `Zebra` — that fails on `<`/`>` and passes on `localeCompare`.
### 🟡 Should-fix
- **`superset-frontend/src/pages/ChartCreation/ChartCreation.test.tsx:336`** — the fixture is all-lowercase, so it passes under either comparator and can't tell the two apart. Add a mixed-case name; that's the only case that discriminates. Codecov also reports 83.3% patch coverage with 1 uncovered line in `index.tsx` — one of the comparator's branches is never exercised.
- **PR description** — the stated root cause is wrong, which matters because it's what motivated the hand-rolled comparator. `DEFAULT_SORT_COMPARATOR` doesn't fall through to `(a.value as number) - (b.value as number)`: when the label isn't a string it checks `typeof a.value === 'string'` first (`Select/constants.ts:92-95`), and `value` *is* a string (`"5__table"`), so it does `localeCompare("2__table", "5__table")`. No `NaN` — the options are sorted lexicographically by `"{id}__{type}"`, which is why they come out in ID order (and why with ≥10 datasets `10__table` sorts before `2__table`).
### 🔵 Nits
- `superset-frontend/src/pages/ChartCreation/index.tsx:346` — the comparator drops the third `search` argument that `AsyncSelect` passes through (`Select/utils.tsx:127`). Not a regression — pre-PR the ranking ran against `"{id}__table"` values and was meaningless — but now that a real name is available, `rankedSearchCompare(aName, bName, search ?? '')` would put prefix matches first while typing.
### 🙌 Praise
- `ChartCreation.test.tsx:336` — the fixture (`gamma`/`alpha`/`beta` at ids 2/3/1) is a genuine regression guard: without the comparator the default sorts by value to `beta, gamma, alpha` and the DOM-order assertions fail. It addresses the earlier review comment properly rather than just reordering to look unsorted.
<!-- enxdev-review-agent:335a118 -->
_Reviewed by EnxDev's Review Agent — @EnxDev · HEAD 335a118._
--
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(chart): dataset dropdown sorted by name [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #42475:
URL: https://github.com/apache/superset/pull/42475
--
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(chart): dataset dropdown sorted by name [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5091011346
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42475?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 65.24%. Comparing base ([`6856d0f`](https://app.codecov.io/gh/apache/superset/commit/6856d0fc671dda62f4f305212b66e1bef8e8203e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`90c9c01`](https://app.codecov.io/gh/apache/superset/commit/90c9c013873148cc7d7923f434f5587d34d1041f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42475 +/- ##
==========================================
- Coverage 65.25% 65.24% -0.01%
==========================================
Files 2795 2795
Lines 157639 157643 +4
Branches 36052 36054 +2
==========================================
- Hits 102869 102862 -7
- Misses 52793 52804 +11
Partials 1977 1977
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42475/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/42475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.32% <100.00%> (-0.02%)` | :arrow_down: |
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/42475?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(chart): dataset dropdown sorted by name [superset]
Posted by "SkinnyPigeon (via GitHub)" <gi...@apache.org>.
SkinnyPigeon commented on code in PR #42475:
URL: https://github.com/apache/superset/pull/42475#discussion_r3672569750
##########
superset-frontend/src/pages/ChartCreation/ChartCreation.test.tsx:
##########
@@ -333,6 +333,63 @@ test('shows loading spinner when dataset parameter is present in URL', async ()
locationSpy.mockRestore();
});
+test('dataset dropdown sorts options alphabetically by table name regardless of id order', async () => {
+ fetchMock.clearHistory().removeRoutes();
+ // IDs are deliberately NOT in alphabetical order of table_name to prove
+ // that sorting is by name, not by creation/id order.
+ fetchMock.get(/\/api\/v1\/dataset\/\?q=.*/, {
+ body: {
+ result: [
+ {
+ id: 3,
+ table_name: 'alpha_table',
+ datasource_type: 'table',
+ database: { database_name: 'test_db' },
+ schema: 'public',
+ },
+ {
+ id: 1,
+ table_name: 'beta_table',
+ datasource_type: 'table',
+ database: { database_name: 'test_db' },
+ schema: 'public',
+ },
+ {
+ id: 2,
+ table_name: 'gamma_table',
+ datasource_type: 'table',
+ database: { database_name: 'test_db' },
+ schema: 'public',
+ },
+ ],
Review Comment:
Makes sense
--
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(chart) dataset dropdown sorted by name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5090796201
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(chart): dataset dropdown sorted by name [superset]
Posted by "SkinnyPigeon (via GitHub)" <gi...@apache.org>.
SkinnyPigeon commented on code in PR #42475:
URL: https://github.com/apache/superset/pull/42475#discussion_r3672570912
##########
superset-frontend/src/pages/ChartCreation/index.tsx:
##########
@@ -343,6 +343,13 @@ export const ChartCreation = ({
optionFilterProps={['id', 'table_name']}
placeholder={t('Choose a %s', datasetLabelLower())}
showSearch
+ sortComparator={(a, b) => {
+ const aName =
+ (a as { table_name?: string }).table_name ?? '';
+ const bName =
+ (b as { table_name?: string }).table_name ?? '';
+ return aName.localeCompare(bName);
+ }}
Review Comment:
Sounds good
--
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(chart): dataset dropdown sorted by name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42475:
URL: https://github.com/apache/superset/pull/42475#issuecomment-5117006683
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