Fixes #26805: proper casing for Explore quick filters via sourceFields - #31307
Conversation
…s top_hits
Thread `sourceFields` through the quick-filter pipeline so each
aggregation request triggers a `top_hits` sub-aggregation, and option
labels are read from `_source` (original case) rather than the
lowercase bucket key.
- Add `sourceFields?: string` to `ExploreQuickFilterField` interface
- Add `sourceFields` to all affected filter constants (domains, owners,
tags, tier, certification, service, database, schema, charts, tasks,
data models, classification, glossary) across all dropdown lists;
owners uses the flat `ownerDisplayName` field (no array traversal)
- Update `getAggregationOptions` to forward `sourceFields` to GET path
and pass `topHits: { size: 1 }` on POST path
- Refactor `getOptionsFromAggregationBucket` to extract a private
`extractSourceValue` helper with array-aware dot-path traversal
- Thread `sourceFields` through `ExploreQuickFilters` fetch functions
- Add Playwright tests asserting proper casing for domains, tiers, tags
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Rohit0301
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
✅ Playwright Results — workflow succeededValidated commit ✅ 1085 passed · ❌ 0 failed · 🟡 3 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 52m 57s ⏱️ Max setup 4m 46s · max shard execution 18m 12s · max shard-job elapsed before upload 22m 27s · reporting 9s 🌐 201.84 requests/attempt · 2.20 app boots/UI scenario · 22.52% common-shard skew Optimization targets still in progress:
🟡 3 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
- ExploreQuickFilters.test.tsx: add missing 10th arg (sourceFields=undefined) to all getAggregationOptions toHaveBeenCalledWith assertions - AdvancedSearchPureUtils.ts: fix extractSourceValue to match the correct array element by bucket key (case-insensitive) rather than always taking [0] — fixes the case where an asset has multiple domains/tags and [0] doesn't correspond to the current bucket - AdvancedSearchPureUtils.test.ts: add unit tests covering flat field extraction, nested single-object path, array element matching by key, and fallback when no top_hits data is present Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Jest test CoverageUI tests summary
|
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
|
| Count | Rule |
|---|---|
| 14 | sonarjs/no-duplicate-string |
| 3 | openmetadata-imports/no-circular-imports |
| 3 | openmetadata-imports/no-lower-layer-page-imports |
| 3 | openmetadata-imports/no-impure-pure-utils |
| 1 | sonarjs/cyclomatic-complexity |
| 1 | sonarjs/cognitive-complexity |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/Explore/ExplorePage.interface.ts:50:1 |
openmetadata-imports/no-circular-imports |
This runtime import participates in a circular dependency. Extract the shared type/constant/utility or invert the dependency. |
| 🟡 | src/components/Explore/ExplorePage.interface.ts:51:1 |
openmetadata-imports/no-lower-layer-page-imports |
Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here. |
| 🟡 | src/components/Explore/ExplorePage.interface.ts:52:1 |
openmetadata-imports/no-circular-imports |
This runtime import participates in a circular dependency. Extract the shared type/constant/utility or invert the dependency. |
| 🟡 | src/components/Explore/ExplorePage.interface.ts:53:1 |
openmetadata-imports/no-circular-imports |
This runtime import participates in a circular dependency. Extract the shared type/constant/utility or invert the dependency. |
| 🟡 | src/components/Explore/ExploreQuickFilters.tsx:23:1 |
openmetadata-imports/no-lower-layer-page-imports |
Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:22:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 7 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:24:19 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 7 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:27:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 8 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:34:19 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 7 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:37:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 5 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:39:19 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 5 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:42:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 5 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:44:19 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 5 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:47:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 5 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:97:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 3 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:102:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 3 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:107:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 4 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:272:12 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 3 times. |
| 🟡 | src/constants/AdvancedSearch.constants.ts:274:15 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 3 times. |
| 🟡 | src/utils/AdvancedSearchPureUtils.ts:16:1 |
openmetadata-imports/no-impure-pure-utils |
Pure utilities must not depend on React, UI, state, hooks, pages, or REST clients. Move orchestration/rendering out or move shared types to a lower layer. |
| 🟡 | src/utils/AdvancedSearchPureUtils.ts:17:1 |
openmetadata-imports/no-impure-pure-utils |
Pure utilities must not depend on React, UI, state, hooks, pages, or REST clients. Move orchestration/rendering out or move shared types to a lower layer. |
| 🟡 | src/utils/AdvancedSearchPureUtils.ts:18:1 |
openmetadata-imports/no-impure-pure-utils |
Pure utilities must not depend on React, UI, state, hooks, pages, or REST clients. Move orchestration/rendering out or move shared types to a lower layer. |
| 🟡 | src/utils/AdvancedSearchPureUtils.ts:214:23 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":214,"column":22,"endLine":214,"endColum |
| 🟡 | src/utils/ExploreUtils.tsx:26:1 |
openmetadata-imports/no-lower-layer-page-imports |
Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here. |
| 🟡 | src/utils/ExploreUtils.tsx:204:4 |
sonarjs/cognitive-complexity |
Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed. |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
|
Code Review ✅ Approved 4 resolved / 4 findingsThreads sourceFields through the Explore quick filter pipeline to fetch original-case labels via top_hits sub-aggregations, resolving lowercase display issues. No issues found. ✅ 4 resolved✅ Bug: top_hits array label may not match the aggregation bucket
✅ Bug: Owner test assumes user.responseData.displayName is defined
✅ Bug: Owner test relies on undefined displayName
✅ Edge Case: toLowerCase() on testid can collide case-differing keys
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
3 participants
Describe your changes:
Fixes #26805
Explore quick-filter dropdowns were showing option labels in lowercase because Elasticsearch
.keywordfields use alowercase_normalizerfor aggregation matching. This PR threads asourceFieldsparameter through the filter pipeline so each aggregation request triggers atop_hitssub-aggregation; labels are then read from_source(original case) rather than the lowercased bucket key.Type of change:
High-level design:
Problem:
getOptionsFromAggregationBucketusedbucket.key(lowercase) as the display label.Solution: The backend already supports a
top_hitssub-aggregation via thesourceFieldsquery param (used by AdvancedSearch autocomplete). This PR reuses the same mechanism for quick filters:ExploreQuickFilterFieldinterface gains an optionalsourceFields?: stringfield (dot-path into_source).sourceFieldsvalues: domains →'domains.displayName', owners →'ownerDisplayName'(flat field, confirmed from ES index mapping), tags →'tags.tagFQN', tier/certification, service/database/schema, charts/tasks/data models, etc.getAggregationOptionsforwardssourceFieldsto the GET path and addstopHits: { size: 1 }on the POST path to trigger the sub-aggregation.getOptionsFromAggregationBucketgains an optionalsourceFieldsparam and a privateextractSourceValuehelper that handles both flat and array-typed_sourcepaths (arrays are dereferenced at[0]mid-traversal).keystays as the lowercase bucket key for filter matching; onlylabelchanges.ExploreQuickFiltersthreadsfield.sourceFieldsthrough all fetch and render paths, and bypasses cached page aggregations (which lack top_hits data) whensourceFieldsis set.Tests:
Use cases covered
ownerDisplayNamesource fieldPersonalData.Personal)Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
playwright/e2e/Features/ExploreQuickFilters.spec.ts— newdescribeblock'Quick filter options - proper casing from top_hits'Manual testing performed
UI screen recording / screenshots:
Not applicable (behavior change in label text, no visual layout changes).
Checklist:
Fixes <issue-number>: <short explanation>Fixes #26805above.Greptile Summary
The PR preserves lowercased aggregation keys for filtering while sourcing quick-filter labels from each bucket's original-cased
_sourcedata.sourceFieldsmetadata through quick-filter loading and aggregation requests.Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains.
No blocking failure remains.
Important Files Changed
Sequence Diagram
Reviews (8): Last reviewed commit: "Merge branch 'main' into explore-quick-f..." | Re-trigger Greptile
Context used: