Skip to content

Fixes #32658: glossary single-select editor shows display name, not raw FQN - #32726

Closed
detail-app[bot] wants to merge 10 commits into
mainfrom
detail/bug-fix/fixes-32658-glossary-single-select-editor-shows-di-9fe988
Closed

detail-app[bot] wants to merge 10 commits into
mainfrom
detail/bug-fix/fixes-32658-glossary-single-select-editor-shows-di-9fe988

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Describe your changes:

Fixes #32658

I worked on the glossary term editor in single-select mode (reached when the admin-enabled SINGLE_GLOSSARY_TERM_FOR_TABLE Data Asset Rule applies to Tables) because, on a Table that already has one glossary term assigned, the editor opened showing the raw fully-qualified name (e.g. Glossary.term1) instead of the friendly display name (Term 1), and antd logged a dev-mode Warning: \value` should not be array when `TreeSelect` is single mode` warning.

Root cause:

  • TagsSelectForm always set Form's initialValues.tags to defaultValue (a string[]), even in single-select mode, so the antd Form.Item injected an array value into a single-select TreeSelect — producing the rc-tree-select warning.
  • The single-select TreeSelect renders via antd's SingleSelector, whose label is resolved from treeData's valueEntities. But treeData is built from getGlossariesList, which returns only glossary roots (the term children are lazily loaded on expand). So the assigned leaf was missing on first render and the selector fell back to the verbatim FQN — persisting for the whole session until the user expanded the parent glossary or searched for the term.

Type of change:

  • Bug fix

High-level design:

N/A — small change.

Tests:

Use cases covered

  • Opening the glossary editor on a Table that already has one assigned term, with SINGLE_GLOSSARY_TERM_FOR_TABLE enabled, shows the friendly display name immediately (no expand/search needed) and emits no array-value warning.
  • Opening the glossary editor on a Table with no assigned term (still under the single-select rule) renders no selection and emits no array-value warning.
  • Multi-select mode (rule disabled, the default) still renders the assigned term as a styled TagsV1 chip and the save path is unaffected.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files added/updated:
    • openmetadata-ui/src/main/resources/ui/src/components/common/AsyncSelectList/TreeAsyncSelectList.singleselect.test.tsx — new jest spec mounting the real TreeAsyncSelectList through TagsSelectForm exactly as production does, with getGlossariesList mocked to return only a glossary root (the production lazy-load precondition) and a faithful inline reimplementation of convertGlossaryTermsToTreeOptions (avoiding loading GlossaryUtils.tsx, whose module-level lazy() imports hang the jsdom renderer). Three cases: single-select shows friendly name + zero array warnings; empty single-select shows no value + zero warnings; multi-select still renders the chip. The single-select case is verified to fail against the unpatched source (Expected: "Term 1", Received: "Glossary.term1" + 2× array-value warnings), so the test is a genuine regression guard.

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • I added Playwright E2E tests under openmetadata-ui/.../ui/playwright/ for UI changes.
  • Files added/updated:
    • openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/GlossarySingleSelectDisplay.spec.ts — enables SINGLE_GLOSSARY_TERM_FOR_TABLE via the settings API, creates a glossary + term + table, assigns the term via JSON-Patch, opens the editor, and asserts the editor's selection-item shows the friendly displayName (not the raw FQN) and that no value should not be array when ... warning is emitted. A second test case covers the empty single-select path (no assigned term, opens via the add-tag control, asserts no selection and no warning).

Manual testing performed

  1. Brought up a full local stack from scratch — pulled postgres:15 and opensearchproject/opensearch:3.4.0, started them with a compose override supplying POSTGRES_USER/PASSWORD/DB and host port mappings; built the full OpenMetadata distribution with mvn -pl openmetadata-dist -am -DskipTests install; ran DB migrations via ./bootstrap/openmetadata-ops.sh migrate; started the server detached and waited for /api/v1/system/health to return 200; reset the admin password via reset-password. (The default 2 GB heap OOMs tsc:check on this project — 8 GB is required, and NODE_OPTIONS='--max-old-space-size=8192' was used.)
  2. Installed Playwright chromium and ran the setup and bundle-smoke projects to populate playwright/.auth/admin.json and confirm the built app mounts with no bootstrap errors.
  3. Ran the existing DataAssetRulesEnabled Table test (the existing spec exercising SINGLE_GLOSSARY_TERM_FOR_TABLE with assignGlossaryTerm on Tables, lines 198-221 of DataAssetRulesEnabled.spec.ts) — passed.
  4. Ran the existing DataAssetRulesDisabled Table test (multi-select save path: assignGlossaryTerm Add-then-Edit on Tables, lines 270-281 of DataAssetRulesDisabled.spec.ts) — passed (33/33 in the disabled/enabled dependency chain).
  5. Ran the new GlossarySingleSelectDisplay.spec.ts — both test cases passed.
  6. Confirmed the new jest spec fails against the unpatched source (raw FQN displayed + 2× array-value warnings) and passes against the patched source.
  7. Unit tests, typecheck, lint, and build all pass — yarn test --testPathPattern="(AsyncSelectList/TreeAsyncSelectList|TagsSelectForm)" (8/8), yarn tsc:check (0 errors referencing the changed files; the ~604 baseline errors are all in unrelated files), and the ui-checkstyle sequence (organize-imports → eslint → prettier → license) on the changed files (0 new errors, 0 new warnings; the 6 warnings on TreeAsyncSelectList.tsx are line-for-line the pre-existing baseline).

UI screen recording / screenshots:

Not applicable — the visible change is purely a label swap inside an existing dropdown (raw FQN → friendly name) that requires a specific admin-gated rule to surface, for which a screen recording adds little over the E2E test coverage. The new Playwright spec asserts the exact text rendered in the selection-item.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Automatic Fixes PRs can be configured here.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — workflow failed

Validated commit 1cb24e25609ac11c1337718f36060846125a8f79 in Playwright run 34340085588, attempt 2.

✅ 4476 passed · ❌ 3 failed · 🟡 11 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky

Action needed: test(s) failed on every attempt against this PR’s validated commit — see Genuine Failures below. These are test failures, not CI budget or infrastructure issues.

Performance

Blocking 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) 1h 47m 35s

⏱️ Max setup 5m 3s · max shard execution 21m 0s · max shard-job elapsed before upload 24m 34s · reporting 26s

🌐 218.13 requests/attempt · 2.31 app boots/UI scenario · 33.25% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 33.25% (convergence target: at most 15%).
  • Browser traffic was 218.13 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.31 per UI scenario (10960 boots / 4746 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard advanced-search-01 130 0 0 0 0 0
🟡 Shard chromium-01 141 0 1 0 0 0
🟡 Shard chromium-02 131 0 1 0 0 0
✅ Shard chromium-03 196 0 0 0 0 0
✅ Shard chromium-04 113 0 0 0 0 0
✅ Shard chromium-05 144 0 0 0 0 0
✅ Shard chromium-06 146 0 0 0 0 0
🟡 Shard chromium-07 173 0 1 1 0 0
🟡 Shard chromium-08 139 0 2 0 0 0
🟡 Shard chromium-09 137 0 1 0 0 0
✅ Shard chromium-10 130 0 0 0 0 0
🔴 Shard chromium-11 164 3 0 0 0 0
🟡 Shard chromium-12 160 0 1 0 0 0
✅ Shard chromium-13 189 0 0 0 0 0
✅ Shard chromium-14 151 0 0 0 0 0
✅ Shard chromium-15 152 0 0 0 0 0
✅ Shard chromium-16 188 0 0 0 0 0
✅ Shard chromium-17 178 0 0 0 0 0
✅ Shard chromium-18 153 0 0 0 0 0
✅ Shard chromium-19 186 0 0 0 0 0
🟡 Shard chromium-20 180 0 1 0 0 0
✅ Shard chromium-21 162 0 0 0 0 0
🟡 Shard chromium-22 155 0 1 0 0 0
✅ Shard chromium-23 148 0 0 0 0 0
🟡 Shard chromium-24 154 0 1 0 0 0
🟡 Shard chromium-25 156 0 1 0 0 0
✅ Shard data-asset-rules-01 65 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard import-export-01 43 0 0 0 0 0
✅ Shard import-export-02 107 0 0 0 0 0
✅ Shard ingestion-01 35 0 0 0 0 0
✅ Shard ingestion-02 51 0 0 0 0 0
✅ Shard reindex-01 28 0 0 0 0 0
✅ Shard search-01 12 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

Genuine Failures (failed on all attempts)

Pages/Entity.spec.tsDomain Add, Update and Remove (shard chromium-11)
�[31mTest timeout of 180000ms exceeded.�[39m
Pages/ServiceEntity.spec.tsDomain Add, Update and Remove (shard chromium-11)
�[31mTest timeout of 180000ms exceeded.�[39m
Pages/TeamAssetsRightPanel.spec.tsShould edit glossary terms from team assets context (shard chromium-11)
�[31mTest timeout of 180000ms exceeded.�[39m
🟡 11 flaky test(s) (passed on retry)
  • Features/ContextCenterArticles.spec.tsRelated assets, activity feed, user mentions, and article mentions work (shard chromium-01, 1 retry)
  • Pages/Domains.spec.tsCreate domains and add assets (shard chromium-02, 1 retry)
  • Pages/TasksUIFlow.spec.tsCreate and resolve description task for Pipeline via UI (shard chromium-07, 1 retry)
  • Pages/ExplorePageRightPanel.spec.tsShould verify deleted tag not visible in tag selection for mlmodel (shard chromium-08, 1 retry)
  • Pages/UserCreationWithPersona.spec.tsCreate user with persona and verify on profile (shard chromium-08, 1 retry)
  • Pages/ExplorePageRightPanel.spec.tsShould perform CRUD and Removal operations for topic (shard chromium-09, 1 retry)
  • Pages/ServiceEntity.spec.tsDelete Messaging Service (shard chromium-12, 1 retry)
  • Pages/Entity.spec.tsTier Add, Update and Remove (shard chromium-20, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.tsColumn lineage for searchIndex -> container (shard chromium-22, 1 retry)
  • Pages/DescriptionVisibility.spec.tsCustomized Table detail page Description widget shows long description (shard chromium-24, 1 retry)
  • Pages/ExplorePageRightPanel.spec.tsShould perform CRUD and Removal operations for mlmodel (shard chromium-25, 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

@detail-app detail-app Bot added the safe to test Add this label to run secure Github workflows on PRs label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 70%
70.49% (95297/135188) 55.04% (56403/102458) 56.4% (18826/33374)

anuj-kumary and others added 4 commits September 9, 2026 11:42
…h developer handbook

- Add domain tags (@Features, @Governance) to test.describe
- Move entity setup to test.beforeAll and cleanup to test.afterAll
- Add test.step() for logical grouping within each test
- Use authenticateAdminPage for consistent auth pattern
- Use test.slow() instead of test.slow(true)
- Reset entity rules in afterAll to avoid state leaking

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… spec

Fixes the UI checkstyle CI failure — prettier reformats the
test.beforeAll callback indentation and collapses single-line ternaries.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🔄 Playwright impact map auto-refreshed

This PR touched specs or UI source that changed the source→spec routing map. I regenerated .github/playwright/impact-map.generated.json and pushed the diff to this branch.

- source entries: 553 → 553
- 0 added, 0 removed, 9 changed spec-list

Entries whose spec list changed:
  openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityRightPanel/EntityRightPanel.tsx
  openmetadata-ui/src/main/resources/ui/src/components/Glossary/AddGlossaryTermForm/AddGlossaryTermForm.component.tsx
  openmetadata-ui/src/main/resources/ui/src/components/Modals/StyleModal/StyleModal.component.tsx
  openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsContainerV2/TagsContainerV2.tsx
  openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsV1/TagsV1.component.tsx
  openmetadata-ui/src/main/resources/ui/src/components/common/AsyncSelectList/AsyncSelectList.tsx
  openmetadata-ui/src/main/resources/ui/src/components/common/AsyncSelectList/TreeAsyncSelectList.tsx
  openmetadata-ui/src/main/resources/ui/src/components/common/GlossaryTermsSection/GlossaryTermsSection.tsx
  openmetadata-ui/src/main/resources/ui/src/generated/entity/data/table.ts

What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing getByTestId strings. Hand-authored routing in impact-map.json always wins on conflict.

What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit:

python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit  # or a separate commit

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 5 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 5 warning(s) across 1 changed file(s).

Count Rule
4 openmetadata-imports/no-circular-imports
1 react-hooks/exhaustive-deps
All findings
Location Rule Message
🟡 src/components/common/AsyncSelectList/TreeAsyncSelectList.tsx:57: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/common/AsyncSelectList/TreeAsyncSelectList.tsx:62: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/common/AsyncSelectList/TreeAsyncSelectList.tsx:63: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/common/AsyncSelectList/TreeAsyncSelectList.tsx:66: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/common/AsyncSelectList/TreeAsyncSelectList.tsx:296:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchGlossaryListInternal'. Either include it or remove the dependency array.

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

anuj-kumary and others added 2 commits September 9, 2026 15:54
The beforeAll was enabling ALL entity rules (DATA_ASSET_RULES defaults
are all enabled: true), and the afterAll was "resetting" them to the
same enabled state. This poisoned the server-side entityRulesSettings
for subsequent tests in chromium-11 — the Domain Add/Update/Remove and
TeamAssetsRightPanel glossary tests timed out because Data Product
Domain Validation and single-selection rules were unexpectedly active.

Fix: beforeAll now enables ONLY the single-glossary-term rule we need;
afterAll disables all rules to restore default state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@anuj-kumary

Copy link
Copy Markdown
Member

Already addressed here - #33059

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

Labels

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.

[Detail Bug] UI/Glossary: Single-select term editor shows raw FQN and logs TreeSelect value-shape warning

1 participant