Fixes #26805: proper casing for Explore quick filters via sourceFields - #31307
Fixes #26805: proper casing for Explore quick filters via sourceFields#31307Rohit0301 wants to merge 10 commits into
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 ✅ 1043 passed · ❌ 0 failed · 🟡 0 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) 54m 26s ⏱️ Max setup 3m 56s · max shard execution 17m 36s · max shard-job elapsed before upload 22m 23s · reporting 5s 🌐 197.85 requests/attempt · 2.20 app boots/UI scenario · 24.42% common-shard skew Optimization targets still in progress:
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
|
|
| Count | Rule |
|---|---|
| 23 | sonarjs/no-duplicate-string |
| 4 | i18next/no-literal-string |
| 1 | jsx-a11y/no-autofocus |
| 1 | sonarjs/no-nested-conditional |
| 1 | sonarjs/cyclomatic-complexity |
| 1 | sonarjs/cognitive-complexity |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:108:58 |
i18next/no-literal-string |
disallow literal string: <button data-testid={onGetInitialOptions-${searchKey}} onClick={() => onGetInitialOptions?.(searchKey)}> Get Initial Options </button |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:113:53 |
i18next/no-literal-string |
disallow literal string: <button data-testid={onSearch-${searchKey}} onClick={() => onSearch('test', searchKey)}> Search |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:119:47 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 3 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:120:11 |
i18next/no-literal-string |
disallow literal string: <button data-testid={onChange-${searchKey}} onClick={() => onChange([{ key: 'test-key', label: 'test-label' }], searchKey) }> Change |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:150:10 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 15 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:329:9 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 6 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:357:47 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 8 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:482:32 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 6 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:489:31 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 4 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:685:16 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 4 times. |
| 🟡 | src/components/Explore/ExploreQuickFilters.test.tsx:1050:48 |
i18next/no-literal-string |
disallow literal string: Extra Action |
| 🟡 | src/components/Explore/QuickFilterDropdown.tsx:184:17 |
jsx-a11y/no-autofocus |
The autoFocus prop should not be used, as it can reduce usability and accessibility for users. |
| 🟡 | src/components/Explore/QuickFilterDropdown.tsx:221:17 |
sonarjs/no-nested-conditional |
Extract this nested ternary operation into an independent statement. |
| 🟡 | 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.test.ts:173:14 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 4 times. |
| 🟡 | src/utils/AdvancedSearchPureUtils.test.ts:177:52 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 4 times. |
| 🟡 | 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: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 —
|
🚦 Removed from the merge queue —
|
Code Review ✅ Approved 3 resolved / 3 findingsThreads sourceFields through the Explore quick filter pipeline to extract original-cased labels from top_hits aggregations, addressing top-hit label matching and owner test definition findings. ✅ 3 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
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 normalized aggregation keys for filtering while deriving quick-filter labels from original-cased Elasticsearch
_sourcevalues.sourceFieldsmetadata to affected Explore and column-grid filters.top_hitsdata and bypasses cached aggregations when source-backed labels are required.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (3): Last reviewed commit: "lint fix" | Re-trigger Greptile