SEPURI-SAI-KRISHNA opened a new pull request, #42408:
URL: https://github.com/apache/superset/pull/42408
### SUMMARY
`rankedSearchCompare` is the default sort comparator for every Select and
AsyncSelect in Superset (`DEFAULT_SORT_COMPARATOR` in
`packages/superset-ui-core/src/components/Select/constants.ts`). Its
case-insensitive substring tier had a copy-paste error:
```ts
Number(bLower.includes(searchLower)) - Number(a.includes(searchLower))
// ^ raw `a`, should be `aLower`
```
The left operand is lowercased, the right one is not, so the two sides of the
comparison are not comparable. This has two effects:
1. **The tier is effectively dead.** An option that matches the search only
when case is ignored is not ranked above an option that does not match at
all — the comparison yields 0 and falls through to `localeCompare`.
2. **The comparator is not antisymmetric.** For `a = "zzABCzz"`,
`b = "aaaa"`, `search = "abc"`, both `compare(a, b)` and `compare(b, a)`
return `1`. `Array.prototype.sort` requires a consistent comparator; with
an inconsistent one the resulting order is implementation-defined.
Where this surfaces: `Select` filters options case-insensitively before
sorting (`handleFilterOptionHelper`), and `optionFilterProps` defaults to
`['label', 'value']`, so an option can enter the sorted set by matching on
its
`value` while the comparator inspects its `label`. Those options are the ones
misranked.
The fix compares `aLower` against `bLower`, consistent with every other tier
in the chain.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not a visual change; the observable difference is ordering. With
`search = "abc"`:
| | `compare("zzABCzz", "aaaa")` | `compare("aaaa", "zzABCzz")` | sorted |
|---|---|---|---|
| before | `1` | `1` | `["aaaa", "zzABCzz"]` |
| after | `-1` | `1` | `["zzABCzz", "aaaa"]` |
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npm run test --
packages/superset-ui-core/src/utils/rankedSearchCompare.test.ts
```
Two tests are added:
- `ranks a case-insensitive substring match above a non-match`
- `is antisymmetric so Array.prototype.sort stays well defined`
To confirm they are genuine regression tests, revert the one-line change and
re-run — the first fails with `Expected: < 0, Received: 1` and the second
with
`Expected: 0, Received: 2`.
Also verified no regressions in the consumers:
```bash
npm run test -- packages/superset-ui-core/src/components/Select
packages/superset-ui-core/test/utils
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] 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: [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]
