Skip to content

feat(ui): Explore redesign - stacking browse+filter query bar, chips, cards#28892

Open
harshach wants to merge 16 commits into
mainfrom
harshach/explore-page-redesign
Open

feat(ui): Explore redesign - stacking browse+filter query bar, chips, cards#28892
harshach wants to merge 16 commits into
mainfrom
harshach/explore-page-redesign

Conversation

@harshach

@harshach harshach commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #28930

This PR redesigns the Explore page around one idea: browsing and filtering compose into a single, visible, removable query. The tree's browse location now lives in its own browsePath URL param and ANDs with the dropdown quickFilter, so clicking a service never clears your Type filter (and vice versa); everything you pick shows in a persistent QUERY bar as removable chips. Filter dropdowns apply immediately (no Update button), show facet options that exclude their own field, and use human-readable labels; incompatible tree categories gray out and collapse when an asset type is selected. Result cards get a compact white layout (5–6 per viewport), single-line middle-ellipsized breadcrumbs, a border-only selected state, and a stroke icon set scoped to explore surfaces.

Type of change:

  • Improvement

High-level design:

  • Composable query model: query = browsePath (tree location) AND quickFilter (facets) AND queryFilter (advanced). browsePath serializes the exact ExploreQuickFilterField[] shape the tree already builds per level; it is combined at a single injection point in ExplorePageV1.performFetch (also export modal + facet fetches + the SWR cache key), keeping quickFilter 100% backward compatible for deep links.
  • Tree selection split (ExploreTree): hierarchical levels (category/serviceType/service/database/schema) emit the browse path; entity-type leaves emit path prefix + a Type upsert into the Data Assets slot — both land in ONE navigation. Highlight follows the chips (deepest-loaded-ancestor fallback), disabled categories collapse via controlled expandedKeys, counts render at every level.
  • QUERY bar (ExploreQueryFilterChips): persistent; tinted browse chips with hierarchical removal (truncateBrowsePath), filter chips, right-aligned Clear, empty-state placeholder.
  • Facets (SearchDropdown/ExploreQuickFilters): opt-in immediateApply (default off — 14 other consumers unchanged); per-facet filter excludes the facet's own field (design's facetOptions semantics); canonical entity-type casing resolver bridges lowercase aggregation keys vs camelCase enum.
  • Alternatives considered: merging tree state into the shared quickFilter param — rejected because a flat ES bool query loses level provenance/ordering, making browse chips and hierarchical removal unrecoverable, and DB/schema terms aren't dropdown items so they'd be silently dropped on the next facet apply.
  • No backend changes needed: existing /v1/search/query + /v1/search/aggregate cover everything (verified against SearchResourceIT coverage).

Tests:

Use cases covered

  • Apply Data Assets → Table, then click a service in the tree: both stack as chips; results scope to tables in that service; removing the Service chip falls back to the category; Clear All resets everything.
  • Selecting an asset type grays out and collapses categories that cannot contain it (a Table can only live under Database services).
  • With a service selected, the Data Assets facet lists every type available in that scope and keeps the list stable while selecting/unselecting.
  • Deep links / reloads with an active browsePath highlight the deepest loaded ancestor; legacy quickFilter-only URLs behave unchanged.

Unit tests

  • Added unit tests for the new/changed logic.
  • Files: ExplorePureUtils.test.ts (browsePath parse/build/truncate, canonical casing, node-by-path, graying), ExploreQueryFilterChips.test.tsx, AdvancedSearchPureUtils.test.ts, extended SearchDropdown.test.tsx, ExploreTree.test.tsx, ExploreSearchCard.test.tsx, ExploreQuickFilters.test.tsx — 124 tests green across 11 suites.

Backend integration tests

  • Not applicable (no backend API changes; existing SearchResourceIT already covers the aggregate/query_filter paths used).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Added playwright/e2e/Features/ExploreQueryBar.spec.ts (persistent bar, filter-survives-tree-click with chip removal + Clear, asset-type graying — 3/3 green locally). Updated explore helpers/specs for the immediate-apply UX and testid-based tree locators (count badges broke exact-text matching).

Manual testing performed

  1. Ran the dockerized backend with sample data and the UI via yarn start; verified the full hero flow end-to-end in Chromium (scripted + interactive): persistent QUERY bar, no Update button, chip stacking/removal, graying + collapse, facet refresh scoped to the selected service, card density/breadcrumb ellipsis, icons.
  2. Verified URL round-trips: browsePath + quickFilter coexist; reload restores chips, results, and tree highlight.

UI screen recording / screenshots:

Will attach before/after screenshots and a short recording as a PR comment.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR is linked via Fixes #28930 (title kept descriptive per feat-style convention)
  • My PR is linked to a GitHub issue via Fixes #28930 above
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: not applicable (no schema changes).
  • For UI changes: I attached a screen recording and/or screenshots above (to follow as comment).
  • I have added tests (unit / Playwright) and listed them above.

🤖 Generated with Claude Code

Greptile Summary

This PR redesigns the Explore page around a composable query model: browsing the left-hand tree and selecting facet filters now compose into a single persistent QUERY bar of removable chips, with browse location kept in a dedicated browsePath URL param that ANDs with the existing quickFilter without clobbering it.

  • New browsePath URL param & pure utils (ExplorePureUtils.ts): parseBrowsePathFields, getBrowsePathQueryFilter, truncateBrowsePath, findTreeNodeKeyByBrowsePath, getDisabledExploreTreeKeys, and getCanonicalEntityType implement the full browse-path lifecycle; injected at a single point in performFetch so tab counts, search, and NLQ queries are all scoped to the browse location.
  • ExploreQueryFilterChips (new component): persistent bar rendering tinted hierarchical browse chips and quick-filter chips with individual removal and a right-aligned Clear All.
  • SearchDropdown immediateApply mode: opt-in flag fires onChange on every checkbox click and hides the Update button; all 14 existing consumers are unchanged (default false).

Confidence Score: 4/5

Safe to merge after fixing the isBrowsePathField validator; all other changes are additive or behind opt-in flags.

The isBrowsePathField validator in parseBrowsePathFields checks that value is an array but never validates its elements. Downstream functions (getExploreQueryFilterMust, getBrowsePathSignature) call .key.toLowerCase() on each element without a null guard, so a shared URL containing browsePath=[{"key":"entityType","value":[null]}] crashes the Explore page with a TypeError for every user who opens it. The fix is small and confined to a single type guard, and every other part of the redesign is correct.

openmetadata-ui/src/main/resources/ui/src/utils/ExplorePureUtils.ts — specifically the isBrowsePathField element validation gap

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/utils/ExplorePureUtils.ts Core new logic for browsePath lifecycle — isBrowsePathField validator doesn't check value array element types, breaking stated malformed-URL robustness
openmetadata-ui/src/main/resources/ui/src/pages/ExplorePage/ExplorePageV1.component.tsx Adds browseFields/browseQueryFilter split and handleTreeSelect single-navigate — one minor missing dep in useCallback
openmetadata-ui/src/main/resources/ui/src/components/ExploreV1/ExploreV1.component.tsx Adds ExploreQueryFilterChips bar, handleExploreTreeSelect, handleRemoveBrowseLevel, selectedEntityTypes — logic sound
openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreTree/ExploreTree.tsx Controlled expandedKeys, disabledRootKeys for type-graying, highlight sync via findTreeNodeKeyByBrowsePath
openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQueryFilterChips/ExploreQueryFilterChips.component.tsx New component: browse-level and filter chips with Clear All — clean implementation
openmetadata-ui/src/main/resources/ui/src/components/SearchDropdown/SearchDropdown.tsx Adds immediateApply opt-in; 14 existing consumers unaffected
openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx Per-facet filter exclusion, immediateApply forwarding, human-readable entity-type labels
openmetadata-ui/src/main/resources/ui/src/components/ExploreV1/ExploreSearchCard/ExploreSearchCard.tsx Card wrapper, collapsible breadcrumb ellipsis, stroke icons — ellipsis textContent check is fragile
openmetadata-ui/src/main/resources/ui/src/utils/SearchClassBase.ts Replaces legacy SVG imports with getExploreAssetIcon stroke icons
openmetadata-ui/src/main/resources/ui/src/utils/ExploreIconUtils.tsx New explore-scoped stroke icon map; returns null for unknowns preserving fallback chain

Comments Outside Diff (1)

  1. openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreTree/ExploreTree.tsx, line 446-461 (link)

    P2 Disabled category expansion not preserved on re-enable

    When a type filter disables a root category, visibleExpandedKeys filters out those root keys before passing them to the Tree. Ant Design calls onExpand with only the keys visible to the Tree, so setExpandedKeys(keys) permanently removes the disabled keys from state. Once the type filter is cleared and the category is re-enabled, the category stays collapsed — the user must manually re-expand it.

    A minimal fix is to preserve the removed keys across the disable/re-enable cycle by merging them back in onExpand:

Reviews (4): Last reviewed commit: "fix(ui): restore explore toolbar Clear A..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Browse location and quick filters now stack into one removable query:
- New browsePath URL param holds the tree location (category/serviceType/
  service/database/schema); it ANDs with the dropdown quickFilter so browsing
  never clears filters and vice versa (single combine point in performFetch;
  export + facet fetches included).
- Persistent QUERY bar (ExploreQueryFilterChips): tinted browse chips +
  filter chips, each removable (browse removal truncates deeper levels),
  right-aligned Clear, empty-state placeholder.
- Filter dropdowns apply immediately (no Update button) with helper text,
  compact menu, "+" selected state, and facet options that exclude their own
  field so unselecting a type reveals the other available types in scope.
- Tree: categories that cannot contain the selected asset types gray out and
  collapse; counts render at every level; selection highlight follows the
  browse chips (deepest-loaded-ancestor fallback for deep links/reloads).
- Cards: compact white layout (5-6 per viewport), single-line breadcrumb with
  middle-ellipsis click-to-expand, border-only selected state, stroke icon
  set (ExploreIconUtils) scoped to explore surfaces.
- Human-readable entity-type labels via canonical-casing resolver
  (aggregations return lowercase keys, EntityType enum is camelCase).
- Defensive aggregation access so malformed search responses degrade to
  empty lists instead of error toasts.
- Tests: new ExploreQueryBar.spec (persistent bar, filter-survives-tree-click,
  graying); explore spec helpers switched to testid locators (count badges
  broke exact-text matching); 124 unit tests green across 11 suites.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@harshach harshach requested a review from a team as a code owner June 10, 2026 04:53
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Jun 10, 2026
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/ExplorePureUtils.ts
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 3 failure(s), 11 flaky

✅ 4307 passed · ❌ 3 failed · 🟡 11 flaky · ⏭️ 88 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 301 0 1 4
🟡 Shard 2 812 0 2 9
✅ Shard 3 813 0 0 8
🟡 Shard 4 864 0 1 12
🟡 Shard 5 731 0 2 47
🔴 Shard 6 786 3 5 8

Genuine Failures (failed on all attempts)

Pages/InputOutputPorts.spec.ts › Input ports list displays entity cards (shard 6)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoBeVisible�[2m(�[22m�[2m)�[22m failed

Locator: locator('text=pw-table-7147fc28-974f-44e1-bdb8-2a1edd786016').first()
Expected: visible
Timeout: 15000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toBeVisible" with timeout 15000ms�[22m
�[2m  - waiting for locator('text=pw-table-7147fc28-974f-44e1-bdb8-2a1edd786016').first()�[22m

Pages/InputOutputPorts.spec.ts › Lineage displays input and output ports (shard 6)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoBeVisible�[2m(�[22m�[2m)�[22m failed

Locator: locator('text=pw-table-65024410-a982-4212-a75c-fb79c528101b').first()
Expected: visible
Timeout: 15000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toBeVisible" with timeout 15000ms�[22m
�[2m  - waiting for locator('text=pw-table-65024410-a982-4212-a75c-fb79c528101b').first()�[22m

Pages/InputOutputPorts.spec.ts › Lineage with only input ports (shard 6)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoBeVisible�[2m(�[22m�[2m)�[22m failed

Locator: locator('text=pw-table-858668ce-477a-4aaa-98e4-603a3a76df28').first()
Expected: visible
Timeout: 15000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toBeVisible" with timeout 15000ms�[22m
�[2m  - waiting for locator('text=pw-table-858668ce-477a-4aaa-98e4-603a3a76df28').first()�[22m

🟡 11 flaky test(s) (passed on retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should select and edit nested STRUCT field (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 4, 1 retry)
  • Pages/EntityDataSteward.spec.ts › Tier Add, Update and Remove (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 5, 1 retry)
  • Pages/InputOutputPorts.spec.ts › Cancel port removal (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify Impact Analysis service filter selection (shard 6, 1 retry)
  • Pages/Tag.spec.ts › Verify Owner Add Delete (shard 6, 1 retry)
  • Pages/UserDetails.spec.ts › My Data Tab - AssetsTabs search functionality (shard 6, 1 retry)
  • Pages/Users.spec.ts › Token generation & revocation for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

- Restore the dropdown search input unconditionally: hiding it for small
  option sets broke checkExploreSearchFilter/searchAndClickOnOption, which
  every Entity.spec Tier/Certification/Tag/Glossary flow fills (~100 tests).
- Keep selected facet values visible in immediate-apply mode: own-field-
  excluded aggregations may not return the selected value in the top-N, and
  non-independent mode dropped selections missing from options (ExploreTree
  glossary/columns specs).
- Make every explore-context update-btn click conditional via a shared
  clickUpdateButtonIfVisible helper, arming response waits before the option
  click so both legacy and immediate-apply modes are satisfied
  (checkExploreSearchFilter, ExploreSortOrderFilter, DomainFilterQueryFilter,
  SearchExport, NestedColumnsExpandCollapse).
- Mock the new @untitledui icons in ExploreV1.test (XClose/XCircle/
  FilterFunnel01/Trash01) — the always-rendered QUERY bar needs them.
- Add a regression unit test for selected-option visibility.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 62%
62.33% (66805/107173) 44.14% (37438/84812) 45.45% (11241/24732)

Adds coverage for the composable-query semantics of the redesigned
Explore page, anchored on the Tier1-OR-Tier2 scenario:

- ExploreFilterComposition.spec.ts (new E2E): tiered fixtures across
  table/dashboard/topic verify OR within a field (Tier1 OR Tier2 union
  fires ONE must clause with two should terms and surfaces assets of
  either tier), chip removal narrowing the union, AND across fields
  (tier + tag), and asset-type union composing with tier union. Facet
  options re-fetch only on dropdown open, so option picks retry by
  reopening to absorb indexing lag.
- ExplorePureUtils.test.ts: unit-level OR-within / AND-across / empty-
  field assertions on getBrowsePathQueryFilter.
- ExploreQuickFilters.test.tsx: facet aggregations keep sibling-field
  selections (tier terms scope the tags facet) while excluding the
  facet's own field.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sv-se (Swedish) locale arrived from main (#28973) after the
explore-redesign keys were added, so it lacked browse-estate, in,
browse-estate-query-placeholder, and pick-values-to-refine. Running
`yarn i18n` backfills them (English fallback for later translation),
clearing the I18n Sync checkstyle diff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ExploreDiscovery "showDeleted is on" (shard 3): the owner and domain
  facet steps still clicked the removed update-btn (armed the query
  response after the option click). Arm before the option click, use
  clickUpdateButtonIfVisible, and Escape to close each dropdown before
  opening the next — immediate-apply keeps the menu open and each open
  menu owns its own search-input.
- Users "Check permissions for Data Steward" (shard 6): the prior fix
  closed the Data Assets dropdown with Escape, but ExploreV1's
  document-level Escape handler also closes the auto-opened summary
  panel, so the entity-link this step clicks vanished and the test hung
  to timeout. Close the dropdown by toggling its trigger instead, which
  leaves the summary panel (driven by searchResults) intact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chirag-madlani and others added 4 commits June 17, 2026 17:12
…-redesign

# Conflicts:
#	openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx
#	openmetadata-ui/src/main/resources/ui/src/components/ExploreV1/ExploreV1.component.tsx
#	openmetadata-ui/src/main/resources/ui/src/pages/ExplorePage/ExplorePageV1.component.tsx
#	openmetadata-ui/src/main/resources/ui/src/utils/ExploreUtils.tsx
…se i18n

The redesign dropped the explore toolbar's "Clear All" button, but the
shared checkExploreSearchFilter helper (Entity.spec across every entity
type) and SearchSeparationSuite still click data-testid="clear-filters"
after applying a Tier/Certification/Tag/Glossary filter. With it gone,
~80 tests across CI shards 3/4/5 hung to timeout on the missing locator.

Restore the toolbar Clear All verbatim from main (gated on active quick
filters / browse path / SQL query), wired to the same clearFilters the
QUERY-bar Clear uses. The design keeps both controls. Add ExploreV1
unit tests locking the clear-filters contract (absent with no filters,
present with an active browse filter).

Also sync the new SSO i18n keys into the sv-se locale (yarn i18n),
clearing the I18n Sync checkstyle diff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Jun 17, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 4 resolved / 5 findings

Redesigns the Explore page with a unified, composable query bar for tree browsing and facet filtering. Refined the query response waiter to use strict matching instead of loose substring checks for improved test reliability.

💡 Edge Case: Loose substring match in query_filter response waiter

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreFilterComposition.spec.ts:153-162

In selectOptionAndWaitForQuery, the response is matched via queryFilter.includes(optionKey). For short/generic option keys such as 'table' (used in the "Narrow the domain to tables only" step), the substring can incidentally appear in unrelated parts of the serialized query_filter (e.g. field names like table.*, index names, or nested clause keys), so the waiter may resolve on the wrong request and weaken the assertion or cause flakiness. Consider matching on a more specific token — e.g. the field/key pair ("${fieldKey}" together with the value) or a quoted value "${optionKey}" — to ensure the awaited response is the one produced by the option just clicked.

✅ 4 resolved
Edge Case: Browsing a category root grays out & collapses sibling categories

📄 openmetadata-ui/src/main/resources/ui/src/components/ExploreV1/ExploreV1.component.tsx:472-486 📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreTree/ExploreTree.tsx:303-315 📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreTree/ExploreTree.tsx:382-396
selectedEntityTypes (ExploreV1.component.tsx:472-486) is the union of the Data Assets quick-filter value AND the entityType browse field value. When the user clicks a category root in the tree (e.g. "Databases"), onNodeSelect builds a browse field whose value is the category's full childEntities list (database, databaseSchema, storedProcedure, table, tableColumn). That entire set then flows into selectedEntityTypes, so getDisabledExploreTreeKeys disables — and visibleExpandedKeys collapses — every sibling category that contains none of those types.

The result is that merely browsing into a category (not selecting a specific asset type) grays out and collapses all other top-level categories. The PR description scopes the gray-out/collapse behavior to "when an asset type is selected," so triggering it on a category-level browse click — including auto-collapsing an already-expanded sibling subtree — may be surprising. Please confirm this is intended; if not, exclude the browse-field category value (or only the multi-type category roots) from selectedEntityTypes.

Bug: Domain facet key may not be lowercased like tagFQN buckets

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreFilterComposition.spec.ts:484 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreFilterComposition.spec.ts:101-103
domainKey = lowercaseKey(assetDomain.responseData.displayName) is used as both the dropdown option testid and the query-chip testid for the Domains facet (field domains.displayName.keyword). The fixture's displayName is PW Domain <uuid> (mixed case, see support/domain/Domain.ts). The inline comment justifies lowercasing because 'tagFQN-style aggregation buckets are lowercase-normalized', but displayName.keyword is NOT a tagFQN field — its aggregation bucket key depends on whether the backend keyword mapping applies a lowercase normalizer. If it does not, the rendered option/chip testid will retain the original casing (PW Domain ...) and ensureFilterOptionVisible will spin for its full 90s budget and then fail the test. This was unverifiable from the diff alone; please confirm the actual bucket casing for domains.displayName.keyword (e.g. run the domain test in isolation). If domain buckets are NOT lowercased, drop lowercaseKey() for the domain key.

Quality: parseBrowsePathFields accepts any JSON array without shape validation

📄 openmetadata-ui/src/main/resources/ui/src/utils/ExplorePureUtils.ts:418-432
parseBrowsePathFields (ExplorePureUtils.ts:418-434) only checks Array.isArray(parsed) before casting the parsed JSON to ExploreQuickFilterField[]. A crafted/legacy browsePath URL param such as [1,2,3] or [{}] would be accepted and propagated into chip rendering and query building. I verified the downstream consumers (getExploreQueryFilterMust, getBrowsePathSignature, the chips component) are defensive (field.value ?? [], isEmpty(filter.value) guards) so this does not currently crash, but the cast silently trusts untrusted URL input. Consider validating that each element has at least a string key (and array/undefined value) and dropping the param otherwise, so malformed deep links degrade to an empty browse path rather than rendering empty/garbage chips.

Edge Case: Gold certification step omits Escape before asserting visibility

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreFilterComposition.spec.ts:531-542
In the 'certification union' test, the first step ('Filter by the gold certification') selects the facet option and then immediately calls searchAndExpectEntityVisible/NotVisible WITHOUT first pressing page.keyboard.press('Escape'). Every other facet step in this file (term test L443/463, domain test L493/508, tier helper L199) closes the dropdown before asserting on result cards. searchAndExpectEntityVisible interacts with the top searchBox (fill + Enter) while the Certification dropdown menu is still open, which can intercept input or overlay the cards and cause intermittent flakiness. The poll-with-retries in the helper mitigates but does not eliminate this. Add await page.keyboard.press('Escape'); after the gold selection, consistent with the other steps.

🤖 Prompt for agents
Code Review: Redesigns the Explore page with a unified, composable query bar for tree browsing and facet filtering. Refined the query response waiter to use strict matching instead of loose substring checks for improved test reliability.

1. 💡 Edge Case: Loose substring match in query_filter response waiter
   Files: openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreFilterComposition.spec.ts:153-162

   In `selectOptionAndWaitForQuery`, the response is matched via `queryFilter.includes(optionKey)`. For short/generic option keys such as `'table'` (used in the "Narrow the domain to tables only" step), the substring can incidentally appear in unrelated parts of the serialized `query_filter` (e.g. field names like `table.*`, index names, or nested clause keys), so the waiter may resolve on the wrong request and weaken the assertion or cause flakiness. Consider matching on a more specific token — e.g. the field/key pair (`"${fieldKey}"` together with the value) or a quoted value `"${optionKey}"` — to ensure the awaited response is the one produced by the option just clicked.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Comment on lines 409 to +431
);
};

const isBrowsePathField = (
field: unknown
): field is ExploreQuickFilterField => {
const record =
field && typeof field === 'object'
? (field as Record<string, unknown>)
: undefined;

return (
typeof record?.key === 'string' &&
(record.value === undefined || Array.isArray(record.value))
);
};

/**
* The browse location selected in the explore tree, kept in its own
* `browsePath` URL param (ordered ExploreQuickFilterField[] — category,
* serviceType, service, database, schema). It ANDs with the dropdown
* `quickFilter`, so browsing never clears filters and vice versa.
* The param is untrusted URL input — malformed elements are dropped so

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 isBrowsePathField doesn't validate value array elements

The validator confirms value is an array but never checks its elements. Any downstream call that reads option.key — including getExploreQueryFilterMust (filterValue.key.toLowerCase()) and getBrowsePathSignature (option.key.toLowerCase()) — will throw a TypeError: Cannot read properties of null (reading 'key') when an element is null, a number, or any non-object primitive.

The function comment ("malformed elements are dropped") is the stated contract but is not delivered. If a user shares a crafted explore URL such as browsePath=[{"key":"entityType","value":[null]}], anyone opening it will see the tree/page crash with an unhandled exception. Adding an element-level type guard (e.g., typeof item === 'object' && item !== null && typeof item.key === 'string') inside isBrowsePathField would close the gap.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explore page redesign: browsable, composable query experience

2 participants