Skip to content

fix(ui): show -- for empty owner/experts/glossary/tags in Domain and … - #33038

Merged
anuj-kumary merged 10 commits into
open-metadata:mainfrom
Vansh0310:fix/domain-dataproduct-table-empty-placeholder-33036
Sep 11, 2026
Merged

anuj-kumary merged 10 commits into
open-metadata:mainfrom
Vansh0310:fix/domain-dataproduct-table-empty-placeholder-33036

Conversation

@Vansh0310

@Vansh0310 Vansh0310 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-09-10 at 2 23 08 PM Screenshot 2026-09-10 at 2 23 49 PM

Describe your changes:

Fixes #33036

I worked on replacing the empty-state text in the Domain and Data Product list (table) views with --,
because it was inconsistent with the grid card, which already shows -- for empty Owners, Experts,
Glossary Terms, and Tags. Previously the table views showed "No Owners" for an empty owner, and a single
- for empty glossary terms / tags. I reused two already-optional props added earlier for the grid card
(OwnerLabel's showDashPlaceholder and TagBadgeList's emptyPlaceholder) and threaded them through
the two table-column renderers (useDomainTableColumns.tsx for Domain, DataProductListPage.tsx for
Data Product) — nothing else about either component or either card view changes, and no other list/table
pages are affected. As a follow-up cleanup on the same branch, I also changed the three
domainFieldRenderers.tsx cell functions from taking a bundled options object to direct parameters
(renderDomainOwnersCell(entity, showDashPlaceholder?), and both tag renderers as
(entity, size?, emptyPlaceholder?)) — a pure signature simplification, no behavior change.

Type of change:

  • Bug fix

High-level design:

N/A — small change. Four files touched in the main fix, all additive: OwnerLabel's
showDashPlaceholder prop and TagBadgeList's emptyPlaceholder prop already existed (added for the
Data Product grid card), so this just passes them through from two table-column renderers that
previously called those components without those options. A follow-up commit simplified
domainFieldRenderers.tsx's three cell-renderer signatures from an options object to direct parameters,
purely for call-site readability — verified the one caller with real arguments
(useDomainTableColumns.tsx) updated correctly, and that the 6 other call sites
(useDomainCardTemplates.tsx, all passing no second argument) are unaffected either way.

Tests:

Use cases covered

  • Domain table view: a Domain with no owner shows -- in the Owner column; no glossary terms / no tags
    shows -- in those columns.
  • Data Product table view: a Data Product with no owner shows -- in the Owner column; no experts shows
    -- in Experts; no glossary terms / no tags shows -- in those columns.
  • Domain and Data Product grid card views are unaffected (Data Product's card already showed -- from
    earlier work; Domain's card keeps its existing behavior, out of scope for this fix).
  • TagBadgeList's emptyPlaceholder prop itself renders the custom value when provided.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files added/updated: useDomainTableColumns.test.tsx (3 new tests — owners/glossaryTerms/tags cells
    pass the right placeholder arguments through to domainFieldRenderers), TagBadgeList.test.tsx (1 new
    test — custom emptyPlaceholder renders). No new test files.
  • DataProductListPage.tsx's owners/experts/glossaryTerms/tags cell wiring is untested — it has no
    existing test file, and creating one wasn't approved for this pass.
  • Coverage %: not measured.

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not yet decided — flag for review.

Manual testing performed

  1. tsc typecheck — no new errors (same 2 pre-existing, unrelated errors before and after each commit).
  2. make ui-checkstyle-changed — exit 0, no reformatting.
  3. Mutation-tested the new positional-argument tests: dropped the true flag and swapped
    tagSize/NO_DATA_PLACEHOLDER's argument order — each change failed its own test while the untouched
    tags case stayed green, confirming the tests actually catch a wrong-order call (the main new risk
    the options→direct-params refactor introduces). Source restored afterward.
  4. Verified via temporary local tests (since removed, not committed): Domain table
    owners/glossaryTerms/tags all render --; OwnerLabel with the Data Product table's exact props
    renders --, and without the flag still renders "No Owners"; both grid-card paths still render
    - / "No Owners" (unaffected); existing TagBadgeList tests still pass.
  5. Browser end-to-end check on /domain and /dataProduct table views — pending, blocked on sign-in.

UI screen recording / screenshots:

Pending — blocked on the same sign-in issue as manual testing above; will attach before/after of both
table views once verified.

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: not applicable, no schema changes.
  • 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.
  • I have added a test that covers the exact scenario we are fixing.
  • I have added a test that covers the exact scenario we are fixing. For complex issues, comment the issue number in the test for future reference.
  • I have added a test that covers the exact scenario we are fixing. For complex issues, comment the issue number in the test for future reference.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This 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 skip-pr-checks label.

@github-actions

github-actions Bot commented Sep 9, 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 9, 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!

@anuj-kumary anuj-kumary added the safe to test Add this label to run secure Github workflows on PRs label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 70%
70.92% (96322/135811) 55.6% (57209/102888) 56.94% (19063/33479)

@github-actions github-actions Bot removed the safe to test Add this label to run secure Github workflows on PRs label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 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!

@anuj-kumary anuj-kumary added the safe to test Add this label to run secure Github workflows on PRs label Sep 9, 2026
Drops the explicit undefined size argument at call sites that only set
emptyPlaceholder. renderDomainOwnersCell stays positional.

Refs open-metadata#33036
…ty-placeholder-33036' into fix/domain-dataproduct-table-empty-placeholder-33036
@github-actions

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!

… size

No call site passes size without emptyPlaceholder, so making size the
trailing optional lets every caller omit what it does not set - no
options object, no placeholder undefined.

Refs open-metadata#33036
@github-actions

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

Copy link
Copy Markdown
Contributor

🚦 Removed from the merge queue — failed_checks (2026-09-10T13:12:54Z)

Blocked the queue: playwright-summary

@github-actions

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!

@gitar-bot

gitar-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Standardizes empty-state placeholders in Domain and Data Product table views to show -- instead of "No Owners" and single -, matching the existing grid card behavior. Reuses existing optional props (showDashPlaceholder and emptyPlaceholder) and includes a follow-up refactor of cell-renderer signatures from bundled options to direct parameters for improved readability. Unit tests added for placeholder wiring and TagBadgeList custom empty values. No issues found.

Options

Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 2 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), 2 warning(s) across 1 changed file(s).

Count Rule
1 no-restricted-imports
1 react-hooks/exhaustive-deps
All findings
Location Rule Message
🟡 src/components/DataProduct/DataProductListPage.tsx:24:1 no-restricted-imports '@untitledui/icons' import is restricted from being used by a pattern. Import icons from @openmetadata/ui-core-components/icons, not directly from @untitledui/i
🟡 src/components/DataProduct/DataProductListPage.tsx:408:6 react-hooks/exhaustive-deps React Hook useMemo has missing dependencies: 'dataProductColumns', 'dataProductListing.handleSelect', and 'dataProductListing.handleSelectAll'. Either include t

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

make ui-checkstyle-changed

@sonarqubecloud

Copy link
Copy Markdown

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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Domain and Data Product list pages: drop the "no owner" placeholder, leave the cell with hyphen

3 participants