Skip to content

Fix glossary export playwright - #30585

Merged
chirag-madlani merged 8 commits into
mainfrom
fix-glossary-export-playwright
Jul 29, 2026
Merged

Fix glossary export playwright#30585
chirag-madlani merged 8 commits into
mainfrom
fix-glossary-export-playwright

Conversation

@shrabantipaul-collate

@shrabantipaul-collate shrabantipaul-collate commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

  • Fixed a flaky getByTestId('glossary-terms-popover').getByText(...) failure in GlossaryMutualExclusivityDataProductTree.spec.ts (ME-T01), caused by the top-level glossary tree being virtualized — a newly created glossary with a random name could sort outside the initially-rendered rows and never mount in the DOM.
  • expandTreeNodeByName (playwright/utils/glossary.ts) now filters through the tree's search input before asserting visibility, so the virtualized list is forced to render the matching row regardless of scroll position.
  • Search is only applied at the top-level glossary lookup. Nested (parent-term) lookups pass { search: false } and instead resolve within the subtree already loaded when the glossary was expanded — searching a second time returns matched terms one level deep, making their expand chevron permanently non-interactive.
  • Removed the duplicated if/else search logic in expandToGlossaryTermChildren.

Fixes #

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

High-level design:

N/A — small change.

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Not applicable.

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.

Greptile Summary

Updates the glossary Playwright helper to make virtualized tree expansion more reliable.

  • Filters the tree before locating a top-level glossary.
  • Disables repeated search when expanding an already-loaded parent term.
  • Consolidates duplicated search and expansion logic.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts Adds optional search behavior to glossary tree expansion and uses it only for top-level glossary lookup.

Reviews (3): Last reviewed commit: "Merge branch 'main' into fix-glossary-ex..." | Re-trigger Greptile

@shrabantipaul-collate
shrabantipaul-collate requested a review from a team as a code owner July 28, 2026 12:17
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 added the UI UI specific issues label Jul 28, 2026
@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!

1 similar comment
@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

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 Jul 28, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 18fdcc0a31e124be80522c55860bdecbb8adfe06 in Playwright run 30428389322, attempt 1.

✅ 536 passed · ❌ 0 failed · 🟡 1 flaky · ⏭️ 5 skipped · 🧰 0 lifecycle flaky

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) 56m 53s

⏱️ Max setup 3m 0s · max shard execution 17m 28s · max shard-job elapsed before upload 20m 32s · reporting 5s

🌐 203.07 requests/attempt · 2.86 app boots/UI scenario · 3.24% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 203.07 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.86 per UI scenario (1599 boots / 559 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
🟡 Shard chromium-01 121 0 1 0 0 0
✅ Shard chromium-02 129 0 0 0 0 0
✅ Shard chromium-03 148 0 0 3 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 23 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 27 0 0 2 0 0
🟡 1 flaky test(s) (passed on retry)
  • Pages/Entity.spec.tsDomain Propagation (shard chromium-01, 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

@shrabantipaul-collate
shrabantipaul-collate added this pull request to the merge queue Jul 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 29, 2026
@shrabantipaul-collate
shrabantipaul-collate added this pull request to the merge queue Jul 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 29, 2026
@shrabantipaul-collate shrabantipaul-collate added the skip-pr-checks Bypass PR metadata validation check label Jul 29, 2026
@shrabantipaul-collate shrabantipaul-collate added the To release Will cherry-pick this PR into the release branch label Jul 29, 2026
@chirag-madlani
chirag-madlani enabled auto-merge July 29, 2026 06:45
@chirag-madlani
chirag-madlani added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 00a7b1d Jul 29, 2026
74 checks passed
@chirag-madlani
chirag-madlani deleted the fix-glossary-export-playwright branch July 29, 2026 11:45
@github-actions

Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 2.0 branch.

github-actions Bot pushed a commit that referenced this pull request Jul 29, 2026
* fix flaky searchIndexApplication

* fix flaky glossary terms popover playwright

* revert unwanted changes

---------

Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
Co-authored-by: Rohit Jain <60229265+Rohit0301@users.noreply.github.com>
(cherry picked from commit 00a7b1d)
@github-actions

Copy link
Copy Markdown
Contributor

Failed to cherry-pick changes to the 1.13 branch.
Please cherry-pick the changes manually.
You can find more details here.

@gitar-bot

gitar-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Updates the glossary Playwright utility to filter through the tree search input for top-level glossary lookups, resolving flaky test failures caused by virtualized row rendering. No issues found.

Options

Display: compact → Showing less information.

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

Compact
gitar display:verbose         

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

Rohit0301 added a commit that referenced this pull request Jul 30, 2026
…right test (#30680)

* fix(ui): center domain/data-product list row items & fix DP detail header title width (#30510)

* fix(ui): vertically center domain & data product list row items

The AI-mode Domain and Data Product list tables rendered their name cells with
`<Box align="start">` (align-items: flex-start), pinning the icon + name to
the top of the row instead of centering it — most visible when the name wraps
to multiple lines. Use `align="center"` on the name/Domains cells so the icon
lines up with the middle of the (single- or multi-line) name.

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

* fix(ui): don't reserve badge space in the DP header when the status badge is empty

DataProductsDetailsPage passed `badge={statusBadge}` to EntityHeader. A truthy
`badge` applies `w-max-full-200` (max-width: calc(100% - 200px)) to the title
column — reserving 200px even when EntityStatusBadge renders nothing (the common
`Unprocessed` status), which squeezed the title into a narrow box and forced
long names to wrap/break mid-word. The Domain header passes no badge and is
unaffected.

Move `statusBadge` into `suffix` (next to the LearningIcon): it still renders
inline when a status exists, but no longer reserves 200px, so the title uses its
natural width.

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

* ci(playwright): give the domain impact-map mapping the Reindex lane

The domain/dataProduct mapping selects SearchSeparation `*Domain*` specs
(e.g. DomainRenamePrefixCascade.spec.ts) via its `**/*Domain*.spec.ts`
glob, but assigned only [chromium, Basic, DomainIsolation]. Those specs
are testIgnored from chromium/Basic and only run under the `Reindex`
project, so build_playwright_shards.py found no runnable unit and aborted
plan-playwright with "matched no runnable tests" on every PR touching
Domain/DataProduct UI sources. Add `Reindex` to the mapping's projects so
the spec resolves to its real lane and runs. Side-effect is contained:
of this mapping's specs, only the SearchSeparation one yields a Reindex
unit.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Fixed the knowledge center translations (#30521)

* Fixed the knowledge center translations

* minor fix

* playwright fix

* lint fix

* ci(playwright): move import/export specs into a dedicated lane (#30617)

* ci(playwright): move import/export specs into a dedicated lane

Add an `import-export` planner lane for the ~61 minutes of bulk
import/export test content that currently rides in the chromium lane.
Chromium is running at (or just under) its shard-budget ceiling in
merge_queue; peeling this content off gives the common lane real
headroom instead of relying on the 20-min bump alone.

- Planner: add `ImportExport` project → `import-export` lane, 2 workers,
  same (1, 8) shard bounds as ingestion / search-rbac.
- playwright.config.ts: new `ImportExport` project (grep `@import-export`,
  `fullyParallel: true`, `workers: 2`, entity setup + teardown). Chromium's
  `grepInvert` excludes `@import-export` when the dedicated lane is on,
  mirroring the `@ingestion` pattern.
- Workflow: set `PW_DEDICATED_IMPORT_EXPORT: "true"` on discover +
  seed-state steps.
- Tag 12 specs with `@import-export` (TestCaseImportExport{Basic,E2eFlow},
  ODCSImportExport{,Permissions}, GlossaryImportExport, BulkImport{,
  WithDotInName}, MetricBulkImportExportEdit, BulkEditImportPermissions,
  CSVImportWithQuotesAndCommas, SearchExport, LineageExportPNGSnapshot).

Verified locally:
- `pytest .github/scripts/tests/test_playwright_ci_planning.py` — 56 passed.
- `playwright test --list --project=ImportExport` enumerates all 12 files.
- `playwright test --list --project=chromium` — 0 import/export leaks;
  3637 tests remain in chromium.

* ci: fix ui-checkstyle formatting on tagged import/export specs

Running the full ui-checkstyle-changed chain (organize-imports → eslint
--fix → prettier --write) re-indents the describe bodies of the specs
whose top-level test.describe I converted to multi-line when adding the
@Import-Export tag. Local prettier --check accepted both single-line
and multi-line describe forms, but CI's checkstyle wants the
prettier-canonical form (body at 4 spaces relative to file when the
arrow function opens on its own line).

* ci(playwright): include ImportExport project in discovery command

Greptile P1: when the dedicated lane is on, chromium's grepInvert
excludes @Import-Export specs, but the planner discovery command
didn't request the ImportExport project — so those specs were absent
from the plan entirely and would never run in CI.

Add --project=ImportExport to the discovery list, matching the
Ingestion / DomainIsolation / SearchRBAC / etc pattern.

* ci(playwright): restore fork PR support on knowledge-graph workflow (#30561)

* ci(playwright): restore fork PR support on knowledge-graph workflow

Mirrors the pattern #30485 established on playwright-postgresql-e2e.yml
to the RDF/knowledge-graph workflow, which #30310 also regressed to
pull_request-only. Fork PRs against RDF/ontology paths were previously
short-circuited by the `!fork` check on the second job — they built
the distribution but silently skipped the entire Playwright run, so
RDF work from external contributors ran zero E2E coverage.

Changes:

  - Add pull_request_target trigger with the same paths filter used by
    pull_request so the scope stays RDF-focused.
  - Partition the concurrency group by github.event_name so pull_request
    and pull_request_target runs on the same PR don't cancel each other.
  - Introduce a `gate` job that waits for Team Label reconciliation on
    fork synchronize events, reads the reconciled live PR labels via
    the GitHub API, and computes a should_run decision:
      workflow_dispatch                          → run
      draft                                       → skip
      labeled event, label != "safe to test"     → skip
      same-repo PR + pull_request                 → run
      fork PR + pull_request_target + reconciled
        "safe to test" label                      → run
      everything else                             → skip
  - Wire `build` and `playwright-rdf-postgresql` through `needs: gate`
    with a `needs.gate.outputs.should_run == 'true'` guard. Removes the
    now-redundant `!github.event.pull_request.head.repo.fork` short-
    circuit from the RDF job.
  - Fix the two `actions/checkout@v7` steps to explicitly check out
    `github.event.pull_request.head.sha` under pull_request_target
    (base.sha is wrong for testing fork code) and set
    `allow-unsafe-pr-checkout: true` — the gate already restricted this
    event to safe-to-test-labelled forks, and the build/test steps
    consume the fork tree inside docker containers.

The workflow doesn't have a playwright-summary job or a named
required-check consumer, so no name-collision rename (#30476) is
needed here. `pull_request` vs `pull_request_target` runs on the same
PR each report their own separate check-run status; gate-skipped runs
finish trivially with `should_run=false` and don't perform any docker
work.

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

* add merge_group trigger + name-collision facade for required check

Per @chirag Madlani review — two follow-up gaps confirmed:

  1. This workflow is required in the merge queue.
  2. `Playwright RDF (Knowledge Graph + Ontology)` is a required
     branch-protection check.

Add merge_group support:

  - Add `merge_group:` to on: triggers.
  - Add merge_group to the gate's always-run case: workflow_dispatch |
    schedule | merge_group.

Add name-collision handling (same pattern #30476 established for
playwright-postgresql-e2e.yml):

  - Rename the existing execution job's display name from
    `Playwright RDF (Knowledge Graph + Ontology)` to
    `RDF Playwright execution` — internal identity only, no longer
    the required check name.
  - Add a `playwright-rdf-summary` job with `if: always() && !cancelled()`
    that publishes a dynamic check name:
      gate succeeds, should_run=true  → `Playwright RDF (Knowledge Graph + Ontology)`
      gate fails (crash/cancelled)     → `Playwright RDF (Knowledge Graph + Ontology)` + hard fail
      labeled event, non-safe-to-test → `... (label ignored)`
      gate succeeds, should_run=false  → `... (skipped)`
  - Three steps inside the summary job:
      "Guard against missing gate decision" — fails loudly when gate
        itself did not produce a valid should_run
      "Report gate-skipped run as green" — no-op success for legitimate
        skips (redundant sibling event, fork without safe-to-test)
      "Fail on upstream failure" — propagates build / execution job
        failures under the required check name so the PR fails visibly

Without this, both events on a same-repo PR would publish check-runs
under `Playwright RDF (Knowledge Graph + Ontology)`; the
pull_request_target sibling finishes in ~15s with a green skip while
the real pull_request run is still compiling, making the PR appear
mergeable before any RDF test executes — the same class of hole
#30476 fixed for playwright-summary on postgresql-e2e.

No branch-protection update needed — the required check name is
preserved end-to-end; only its publisher moved from the underlying
execution job to the always-running facade.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* resolve entity schema path from the classpath scan instead of a hardcoded map (#30618)

* Fixes 30533: derive entity schema subdirectory from the classpath scan instead of a hardcoded map

* gitarbot feedback: make duplicate entity type schema resolution independent of classpath scan order

* fix(settings): don't log an unset optional setting as an error (#28960) (#30625)

* fix(ui): keep GenericTab's widget grid inside its container (#30622)

react-grid-layout derives each grid item's `left` and `width` in pixels and
rounds the two independently. Their exact values always sum to the container
width, but each rounding can add just under half a pixel, so the right edge of
the last column can land 1px outside the container.

That pixel is the widget card's right border. An ancestor that scrolls turns it
into a horizontal scrollbar; one that clips swallows the border outright. On
entity detail pages both happen, because the active tab pane scrolls vertically
and CSS promotes the cross axis to `auto` once one axis is not `visible`.

Give the grid 1px of horizontal containerPadding. That pulls the exact right
edge in to `width - 1`, which the rounding -- bounded at 1px, since the two
exact edges sum to the width and each rounding adds under half a pixel -- can
never exceed.

Across widths 600-2400 and every column split that reaches the right edge, the
last column escapes the container at 1575 of 1801 widths today and at none of
them with this change. For the 6 + 2 split the entity detail pages actually use,
it is one width in four.

Adjusting `margin` instead does not work: it is the gutter between items and
sits inside the sum that equals the container width, so changing it only moves
which widths collide (still 175/701 at every integer value from 0 to 24).

* Improve bulk export action stats (#30615)

* Improve bulk export action stats

* Add auto opened when the export jobs is success

* nit

* fix stylling

* nit

* addressed comments

* fix lint checks

* nit

* fix failing unit test

* Fixes #30117: hide soft-deleted owners on the Test Suite detail page (#30520)

* Fixes #30117: keep soft-deleted owners in PATCH base to fix index crash

Editing owners failed with "array item index out of range" when an owner
user was soft-deleted. Detail pages fetch with include=all (soft-deleted
owner present) and compute a positional JSON Patch, but the server loaded
the PATCH base with NON_DELETED, which dropped that owner and shrank the
array so the positional op referenced an index past the end. The dangling
ownership also could not be removed.

Both patch() entry points now load the original with a RelationIncludes
that keeps the entity at NON_DELETED but resolves owners/experts/reviewers/
followers with ALL, so the positional patch stays in range and the ID-based
updateOwners diff removes the dangling OWNS row. Adds two integration tests
reproducing the reported crash (deleted owner sorting last) and the silent
wrong-owner removal (deleted owner in the middle).

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

* Fixes #30117: tolerate existing soft-deleted owners on PATCH; generic test

The first pass loaded the PATCH base with owner/expert/reviewer/follower refs
resolved as ALL. That fixed the positional "array item index out of range"
crash on owner removal, but getValidatedOwners (which runs on every PATCH)
re-validates the whole owner list against NON_DELETED, so ANY edit (e.g. a
description change) on an entity carrying a dangling soft-deleted owner then
failed with 404 "user instance ... not found".

- Narrow the base override to owners only. Experts/reviewers/followers are
  validated on update against NON_DELETED and would hit the same 404; they are
  left unchanged as a separate follow-up, per the issue.
- Add getValidatedOwnersForPatch: retain owners already assigned in the base
  (a soft-deleted user is still a valid, existing assignment) and validate only
  newly added owners against NON_DELETED, so a deleted user still cannot be
  freshly assigned.
- Add a generic BaseEntityIT test so every owner-supporting entity is covered,
  plus a TestSuite regression test asserting a non-owner edit succeeds and
  preserves the soft-deleted owner.

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

* Fixes #30117: address review - retain authoritative owner refs; document tradeoff

- getValidatedOwnersForPatch: retained (already-assigned) owners now use the
  authoritative ref from the base (resolved with ALL, so names are populated)
  instead of the client-supplied ref, and the sort uses a null-safe comparator.
  Prevents a possible NPE/500 when a client sends a minimal (id+type, no name)
  ref for an already-assigned owner, and keeps response metadata authoritative.
- Document the known trade-off on PATCH_ORIGINAL_INCLUDES: callers that diff
  owners against a NON_DELETED view (a GET sending owners:non-deleted) can
  misalign against the ALL base; the representation-aware fix is the follow-up
  the issue flags as a design call.

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

* Fixes #30117: hide soft-deleted owners on Test Suite page (align with data assets)

Revert the server-side PATCH-base change. Loading the base with owners=ALL
fixed the Test Suite crash but regressed the 17+ data-asset detail pages that
fetch owners with includeRelations=owners:non-deleted: a positional owner
remove computed against the filtered array removed the wrong owner when applied
to the longer ALL base. A single server base cannot align with both client
owner representations, so the patch itself must carry it - a separate design
call the issue flags.

Fix the reported crash where the divergence actually is: the Test Suite detail
page was the only owner-editing page fetching owners with include=all and no
includeRelations override. getTestSuiteByName now defaults owners/experts to
non-deleted like every data-asset page, so a soft-deleted owner is not surfaced
and the owner-edit diff stays aligned with the server's NON_DELETED PATCH base -
no "array item index out of range". Showing and removing soft-deleted owners
remains the issue's separate design call.

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

* Fixes #30117: honor includeRelations on Test Suite getByName (frontend fix was a no-op)

getTestSuiteByName sends includeRelations=owners:non-deleted, but the Test
Suite getByName endpoint - unlike getById and every other entity's getByName
(e.g. TableResource) - did not accept the param, so the server still resolved
owners with include=all and surfaced the soft-deleted owner. The owner-edit
crash therefore remained.

Wire includeRelations through getByName (same pattern as getById /
TableResource.getByName), and add an IT that proves it: an include=all fetch
still surfaces the soft-deleted owner, but includeRelations=owners:non-deleted
hides it. Verified RED (without the wiring the assertion fails - owner still
surfaced) and GREEN.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Sid <30566406+siddhant1@users.noreply.github.com>

* Fix glossary export playwright (#30585)

* fix flaky searchIndexApplication

* fix flaky glossary terms popover playwright

* revert unwanted changes

---------

Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
Co-authored-by: Rohit Jain <60229265+Rohit0301@users.noreply.github.com>

* ISSUE #30151 - Incident Grouping Endpoint (#30367)

* feat: added incident group endpoint

* feat: added owner, status, assignee, severiuty and bucket for client metrics

* feat: added owner, status, assignee, severiuty and bucket for client metrics

* empty commit for CI trigger

* feat: remove n+1 in entity ref. retrieval when resolving group references

* feat: ignore optional argumewnt static check error

* feat: address comments and fix failing integration tests

* feat: revert pre existing migrations to 2.0

* feat: address first pass review

* feat: move endpoint data to summary table

* feat: address comments

* feat: address comments

* chore: fix migration

* fix Column Bulk Operations tag & glossary dropdowns not selectable in the edit drawer (#30634)

* fix Column Bulk Operations tag & glossary dropdowns not selectable in the edit drawer

* fix lint checks

* fix(test): correct EditAll-deny test assertions in EntityPermissions

When EditAll is in a DENY rule it subsumes all Edit* operations via
matchesBySubsumption(), so EditTier/EditOwners/EditCertification buttons
are not visible even when those operations have explicit ALLOW rules.
Updated assertions from toBeVisible() to not.toBeVisible() and unskipped
the test to reflect actual backend deny-first policy semantics.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Sid <30566406+siddhant1@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
Co-authored-by: Vishnu Jain <121681876+Vishnuujain@users.noreply.github.com>
Co-authored-by: Anujkumar Yadav <anujf0510@gmail.com>
Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>
Co-authored-by: shrabantipaul-collate <shrabanti.paul@getcollate.io>
Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
Co-authored-by: Teddy <teddy.crepineau@gmail.com>
timothybrush pushed a commit to timothybrush/OpenMetadata that referenced this pull request Jul 30, 2026
…ne merge-queue entry (open-metadata#30699)

* fix(playwright): stabilize flaky online users and explore specs

* fix(playwright): assert empty-placeholder after last tag deletion

After deleting the last tag, ClassificationDetails renders an
EmptyPlaceholder instead of the Table, so the old not.toContainText
assertion on [data-testid="table"] always failed. Assert the empty
placeholder is visible instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(playwright): stabilize owner selector opening

* fix(playwright): accept unmapped explore tab response

* fix(playwright): remove owner selector retry loop

* test(playwright): fix four flaky lineage/persona/customize specs

- persona.ts: wait for confirmation modal, scope waitForResponse to PATCH
  /personas/{id}, and move it right before the "Yes" click so the response
  race in set/remove default persona cannot fire before the listener attaches.
- DashboardDataModelClass.create: on 409 from the dashboard service POST,
  GET the existing service and reuse it so a rare uuid collision or leftover
  service no longer breaks the LineageFilters beforeAll.
- CustomizeDetailPage.spec: wrap the add-widget click + modal visibility in
  expect.toPass so a click that lands on a still-settling grid tile is
  retried until add-widget-modal actually appears.
- TableClass.visitEntityPage: reconstruct the FQN from service/db/schema/
  entity names and refetch by name when fullyQualifiedName is missing, so
  we never fall through to the flaky global-search visitEntityPage path.

* test(playwright): retry until widget-info-tabs shows, not modal root

The previous retry checked add-widget-modal's visibility, but that testid
lands on the antd .ant-modal-root wrapper which is 0x0 — toBeVisible on
it always reports hidden, so expect.toPass would keep clicking (breaking
the modal state) until the 30s timeout. Check widget-info-tabs (the
tabbed content inside the modal) instead.

* fix(playwright): wait for the newly triggered pipeline run, not the previous one

triggerTestSuitePipelineAndWaitForSuccess polled /pipelineStatus?limit=1 and
accepted whatever the latest record was, with no notion of which run it had
triggered. Airflow queues a DAG asynchronously, so ~2s after the trigger the
previous run's `success` record is still the latest one: the helper returned
immediately and callers went on to assert against stale results.

In CI run 30439518657 that surfaced as "Resolving incident & re-run pipeline"
reading an incident status of "Resolved" where it expected "New" — the poll
had read run 178b5ca8 (success, recorded 2m17s before the trigger) while the
new DAG was still queued, so no new failure or incident existed yet.

Snapshot the run recorded before triggering, then poll until a record with a
different runId (or a later timestamp) reaches success.

Also fold the two fixed 5s "deployment settling time" sleeps into a bounded
retry of the trigger request itself: the orchestrator rejects a trigger until
the workflow is registered, so retry until it is accepted rather than paying
5s on every call.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(playwright): treat runId or timestamp as sufficient evidence of a new run

isNewRun required BOTH a different runId AND a later-or-equal timestamp.
`runId` is optional in the generated PipelineStatus schema, so when neither
the baseline nor the polled record carries one, `undefined !== undefined` is
always false and the conjunction can never become true: the poll spins on
Queued until the 300s timeout and the test fails — the opposite of the
reliability this helper is meant to provide.

Either signal alone is sufficient, and both still reject the stale record
(same runId, same timestamp), so use a disjunction with a strict timestamp
comparison.

Reported by gitar-bot and greptile on open-metadata#30655.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix issue with AssetHealthWidget

* ci(playwright): run Data Quality + Observability specs on DQ/observability changes

IncidentManager.spec.ts was reachable in the impact map from only one mapping,
whose sources are ingestion/** and the Java pipeline resources. Changing the
Data Quality UI, the DQ backend, the observability pages, or the Playwright
helper the spec itself depends on selected neither it nor most of the DQ suite,
so PRs in this area went green without ever exercising it. That is how open-metadata#30654
survived: the fix touched only playwright/utils/incidentManager.ts, and the
Ingestion lane ran one unrelated canary test.

Extend the Data Quality mapping to the whole Data Quality + Observability
surface and add the specs that belong to it:

- sources: DataQuality/IncidentManager/Profiler/Alerts components, the
  DataQuality/IncidentManager/TestSuite*/Observability*/AlertDetails pages,
  the DQ + alerts rest clients and utils, playwright/utils/incidentManager.ts,
  resources/dqtests/**, resources/events/** + events/subscription/**, the
  TestCase*/TestSuite/TestDefinition repositories, and schema/tests/**
- specs: Features/IncidentManager.spec.ts, Flow/ObservabilityAlerts.spec.ts,
  Flow/NotificationAlerts.spec.ts

Two globs in the mapping were dead and had never matched anything:
pages/TestSuite/** (no such directory) and resources/tests/** (the real path
is resources/dqtests/**), so the backend DQ surface had no coverage at all.

Verified with select_playwright_tests.py: each of the four representative
paths above now selects IncidentManager.spec.ts for the Ingestion project and
none did before; an unrelated Glossary change still does not select it; every
glob in the mapping resolves to at least one real file.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(ui): make the Asset Health widget E2E spec seed its own tables

The spec used sample_data.ecommerce_db.shopify.fact_sale as its
"unconfigured" fixture. 6e5bc80 (open-metadata#30367) added a native test suite for
that table, so the data quality and observability rows now render status
badges instead of setup CTAs and their testids no longer exist.

Create both fixtures over the API in beforeAll instead, reuse
TableClass.visitEntityPage in place of the hand-rolled page.goto helper,
and assert the badge branch of the data quality row, which nothing covered
before.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(playwright): paginate across pages to find rows in GlossaryTermRelationSettings tests

Parallel workers accumulate relation types that exceed PAGE_SIZE_BASE (15),
pushing target rows to page 2+. The edits, deletes, and locks tests were
failing because they looked only at the first page.

Added findRowAcrossPages helper that navigates Next Page until the target
testId is visible, then each affected test calls it before interacting with
the row.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(playwright): use waitFor in findRowAcrossPages to avoid stale-DOM race

isVisible() is instantaneous — after clicking Next Page and awaiting the
network response, it reads the old rows before React flushes the new ones,
causing the loop to skip the target page and throw.

Switch to waitFor({ state: 'visible', timeout: 3_000 }) which retries
until the element appears or the timeout expires, giving React time to
render before we decide to advance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(ci): run DQ import/export specs in the dedicated ImportExport lane

open-metadata#30617 moved every `@import-export` spec into a dedicated ImportExport
project and grepInvert-ed them out of chromium/Basic/Ingestion/Reindex,
but impact-map.json was never taught about that lane.

The DQ/observability mapping's `Features/DataQuality/**/*.spec.ts` glob
still swept TestCaseImportExportBasic and TestCaseImportExportE2eFlow in
and requested them under the four lanes that can no longer run them, so
build_playwright_shards.py aborted with "Playwright selectors matched no
runnable tests". This surfaced here because this branch added
playwright/utils/incidentManager.ts to that mapping's sources, which is
what makes the mapping fire.

selector_matches_unit is any-of-projects, so adding the lane resolves the
two tagged specs without pulling the other 24 DataQuality specs into it.

Co-Authored-By: Claude <noreply@anthropic.com>

* test(playwright): wait for the modal's Yes button, not the antd root

default-persona-confirmation-modal is the antd .ant-modal-root wrapper
(0x0), so expect(modal).toBeVisible() always reports hidden and the
step hits the 15s timeout. Wait for the inner Yes button instead — its
visibility is the real signal that the modal is rendered and clickable.

* Revert AssetHealthWidget EMPTY_TABLE_FQN change

* test(playwright): wait for ES to index 60 subdomains before UI check

The subdomains tab label reads its count from an ES aggregation filtered
by parent.fullyQualifiedName.keyword. With 60 subdomains created in a
parallel Promise.all, ES was frequently still indexing when the test
opened the UI, so the tab rendered a stale count (as low as 1) and
toContainText('60') hit the 15s timeout. Poll the same aggregation the
UI uses until the total reaches 60 (up to 60s) before releasing the
beforeAll, and fail fast with a clear message if indexing lags.

* fix(e2e): converge SubDomainPagination on search indexing instead of racing it

Both failure modes come from the same thing: the test asserts on state that is
served from Elasticsearch, which is eventually consistent, using assertions
that cannot cause a refetch.

The CI access log for run 30523193756 shows it exactly. The sub-domain is
created at 08:21:52 (POST /api/v1/domains -> 201). The count query runs at
08:21:52 and again at 08:21:58 when the domain page mounts. Then no further
search query is issued until the afterAll delete at 08:22:14 — the entire
15s assertion window elapsed with zero refetches, so
`expect(tab).toContainText('61')` could only re-read a DOM frozen at 60.

The retry then failed differently but from the same cause: Playwright re-runs
beforeAll on retry, so a brand-new domain was created (60 POSTs at 08:22:17)
and selectDomain immediately searched for it before it was indexed. The row
never appeared, so the click auto-waited until the 180s test timeout.

Changes:
- checkSubDomainCount, alongside the existing checkAssetsCount and
  checkDataProductCount, asserts the tab count on the `count` testid and
  reloads between attempts so the one-shot count query is actually re-issued.
- selectDomain retries the search until the row is visible before clicking,
  rather than clicking into a list that may not contain it yet. This is the
  root cause of the retry hang and is latent for every spec that creates a
  domain and immediately selects it.
- Drop the inline cleanup that looked up a hardcoded "NewTestSubDomain".
  SubDomain generates `PW%Subdomain.<uuid>`, so that lookup has 404'd since
  the test was added; afterAll already deletes the parent recursively.
- Replace the magic 9/60/61 literals with SUBDOMAIN_COUNT / PAGE_SIZE.

Both waits are driven through the UI — no search-index queries or field names
are duplicated into the test layer.

Co-Authored-By: Claude <noreply@anthropic.com>

* Revert "test(playwright): wait for ES to index 60 subdomains before UI check"

This reverts commit 08ce53b.

* fix(playwright): re-trigger test-suite pipeline when Airflow runs an empty DAG

Airflow 3.x accepts a trigger (HTTP 200) even when its dag-processor has not
serialized the freshly deployed DAG yet. That trigger creates an empty DagRun
that finishes in ~0.03s, runs no task, and never writes a pipelineStatus, so
the helper polled the full 300s and IncidentManager timed out in CI. The
refactor's "retry until the trigger is accepted" assumption does not hold on
Airflow 3.x: the trigger is accepted regardless of serialization state, so a
rejected-trigger retry never fires and the empty run slips through. CI (slow
dag-processor) loses this race; a fast local box usually wins it.

Reproduced deterministically on a real Airflow 3.2.2 stack: the first trigger
of a fresh DAG materializes no run within 60s (empty run, no status), and a
plain re-trigger of the now-serialized DAG runs for real and reaches success in
~6s.

Fix: after triggering, wait up to 60s for a genuinely new run to surface; if
none does (the empty-run signature, since it writes no status), re-trigger up
to three times, then poll for success. A run that never materializes now fails
loudly instead of hanging. Validated: full IncidentManager spec 10/10, 0 flaky,
against real Airflow 3.2.2.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix lint checks

* fix(test): correct EditAll-deny assertions in EntityPermissions Playwright test (open-metadata#30680)

* fix(ui): center domain/data-product list row items & fix DP detail header title width (open-metadata#30510)

* fix(ui): vertically center domain & data product list row items

The AI-mode Domain and Data Product list tables rendered their name cells with
`<Box align="start">` (align-items: flex-start), pinning the icon + name to
the top of the row instead of centering it — most visible when the name wraps
to multiple lines. Use `align="center"` on the name/Domains cells so the icon
lines up with the middle of the (single- or multi-line) name.

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

* fix(ui): don't reserve badge space in the DP header when the status badge is empty

DataProductsDetailsPage passed `badge={statusBadge}` to EntityHeader. A truthy
`badge` applies `w-max-full-200` (max-width: calc(100% - 200px)) to the title
column — reserving 200px even when EntityStatusBadge renders nothing (the common
`Unprocessed` status), which squeezed the title into a narrow box and forced
long names to wrap/break mid-word. The Domain header passes no badge and is
unaffected.

Move `statusBadge` into `suffix` (next to the LearningIcon): it still renders
inline when a status exists, but no longer reserves 200px, so the title uses its
natural width.

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

* ci(playwright): give the domain impact-map mapping the Reindex lane

The domain/dataProduct mapping selects SearchSeparation `*Domain*` specs
(e.g. DomainRenamePrefixCascade.spec.ts) via its `**/*Domain*.spec.ts`
glob, but assigned only [chromium, Basic, DomainIsolation]. Those specs
are testIgnored from chromium/Basic and only run under the `Reindex`
project, so build_playwright_shards.py found no runnable unit and aborted
plan-playwright with "matched no runnable tests" on every PR touching
Domain/DataProduct UI sources. Add `Reindex` to the mapping's projects so
the spec resolves to its real lane and runs. Side-effect is contained:
of this mapping's specs, only the SearchSeparation one yields a Reindex
unit.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Fixed the knowledge center translations (open-metadata#30521)

* Fixed the knowledge center translations

* minor fix

* playwright fix

* lint fix

* ci(playwright): move import/export specs into a dedicated lane (open-metadata#30617)

* ci(playwright): move import/export specs into a dedicated lane

Add an `import-export` planner lane for the ~61 minutes of bulk
import/export test content that currently rides in the chromium lane.
Chromium is running at (or just under) its shard-budget ceiling in
merge_queue; peeling this content off gives the common lane real
headroom instead of relying on the 20-min bump alone.

- Planner: add `ImportExport` project → `import-export` lane, 2 workers,
  same (1, 8) shard bounds as ingestion / search-rbac.
- playwright.config.ts: new `ImportExport` project (grep `@import-export`,
  `fullyParallel: true`, `workers: 2`, entity setup + teardown). Chromium's
  `grepInvert` excludes `@import-export` when the dedicated lane is on,
  mirroring the `@ingestion` pattern.
- Workflow: set `PW_DEDICATED_IMPORT_EXPORT: "true"` on discover +
  seed-state steps.
- Tag 12 specs with `@import-export` (TestCaseImportExport{Basic,E2eFlow},
  ODCSImportExport{,Permissions}, GlossaryImportExport, BulkImport{,
  WithDotInName}, MetricBulkImportExportEdit, BulkEditImportPermissions,
  CSVImportWithQuotesAndCommas, SearchExport, LineageExportPNGSnapshot).

Verified locally:
- `pytest .github/scripts/tests/test_playwright_ci_planning.py` — 56 passed.
- `playwright test --list --project=ImportExport` enumerates all 12 files.
- `playwright test --list --project=chromium` — 0 import/export leaks;
  3637 tests remain in chromium.

* ci: fix ui-checkstyle formatting on tagged import/export specs

Running the full ui-checkstyle-changed chain (organize-imports → eslint
--fix → prettier --write) re-indents the describe bodies of the specs
whose top-level test.describe I converted to multi-line when adding the
@Import-Export tag. Local prettier --check accepted both single-line
and multi-line describe forms, but CI's checkstyle wants the
prettier-canonical form (body at 4 spaces relative to file when the
arrow function opens on its own line).

* ci(playwright): include ImportExport project in discovery command

Greptile P1: when the dedicated lane is on, chromium's grepInvert
excludes @Import-Export specs, but the planner discovery command
didn't request the ImportExport project — so those specs were absent
from the plan entirely and would never run in CI.

Add --project=ImportExport to the discovery list, matching the
Ingestion / DomainIsolation / SearchRBAC / etc pattern.

* ci(playwright): restore fork PR support on knowledge-graph workflow (open-metadata#30561)

* ci(playwright): restore fork PR support on knowledge-graph workflow

Mirrors the pattern open-metadata#30485 established on playwright-postgresql-e2e.yml
to the RDF/knowledge-graph workflow, which open-metadata#30310 also regressed to
pull_request-only. Fork PRs against RDF/ontology paths were previously
short-circuited by the `!fork` check on the second job — they built
the distribution but silently skipped the entire Playwright run, so
RDF work from external contributors ran zero E2E coverage.

Changes:

  - Add pull_request_target trigger with the same paths filter used by
    pull_request so the scope stays RDF-focused.
  - Partition the concurrency group by github.event_name so pull_request
    and pull_request_target runs on the same PR don't cancel each other.
  - Introduce a `gate` job that waits for Team Label reconciliation on
    fork synchronize events, reads the reconciled live PR labels via
    the GitHub API, and computes a should_run decision:
      workflow_dispatch                          → run
      draft                                       → skip
      labeled event, label != "safe to test"     → skip
      same-repo PR + pull_request                 → run
      fork PR + pull_request_target + reconciled
        "safe to test" label                      → run
      everything else                             → skip
  - Wire `build` and `playwright-rdf-postgresql` through `needs: gate`
    with a `needs.gate.outputs.should_run == 'true'` guard. Removes the
    now-redundant `!github.event.pull_request.head.repo.fork` short-
    circuit from the RDF job.
  - Fix the two `actions/checkout@v7` steps to explicitly check out
    `github.event.pull_request.head.sha` under pull_request_target
    (base.sha is wrong for testing fork code) and set
    `allow-unsafe-pr-checkout: true` — the gate already restricted this
    event to safe-to-test-labelled forks, and the build/test steps
    consume the fork tree inside docker containers.

The workflow doesn't have a playwright-summary job or a named
required-check consumer, so no name-collision rename (open-metadata#30476) is
needed here. `pull_request` vs `pull_request_target` runs on the same
PR each report their own separate check-run status; gate-skipped runs
finish trivially with `should_run=false` and don't perform any docker
work.

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

* add merge_group trigger + name-collision facade for required check

Per @chirag Madlani review — two follow-up gaps confirmed:

  1. This workflow is required in the merge queue.
  2. `Playwright RDF (Knowledge Graph + Ontology)` is a required
     branch-protection check.

Add merge_group support:

  - Add `merge_group:` to on: triggers.
  - Add merge_group to the gate's always-run case: workflow_dispatch |
    schedule | merge_group.

Add name-collision handling (same pattern open-metadata#30476 established for
playwright-postgresql-e2e.yml):

  - Rename the existing execution job's display name from
    `Playwright RDF (Knowledge Graph + Ontology)` to
    `RDF Playwright execution` — internal identity only, no longer
    the required check name.
  - Add a `playwright-rdf-summary` job with `if: always() && !cancelled()`
    that publishes a dynamic check name:
      gate succeeds, should_run=true  → `Playwright RDF (Knowledge Graph + Ontology)`
      gate fails (crash/cancelled)     → `Playwright RDF (Knowledge Graph + Ontology)` + hard fail
      labeled event, non-safe-to-test → `... (label ignored)`
      gate succeeds, should_run=false  → `... (skipped)`
  - Three steps inside the summary job:
      "Guard against missing gate decision" — fails loudly when gate
        itself did not produce a valid should_run
      "Report gate-skipped run as green" — no-op success for legitimate
        skips (redundant sibling event, fork without safe-to-test)
      "Fail on upstream failure" — propagates build / execution job
        failures under the required check name so the PR fails visibly

Without this, both events on a same-repo PR would publish check-runs
under `Playwright RDF (Knowledge Graph + Ontology)`; the
pull_request_target sibling finishes in ~15s with a green skip while
the real pull_request run is still compiling, making the PR appear
mergeable before any RDF test executes — the same class of hole
open-metadata#30476 fixed for playwright-summary on postgresql-e2e.

No branch-protection update needed — the required check name is
preserved end-to-end; only its publisher moved from the underlying
execution job to the always-running facade.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* resolve entity schema path from the classpath scan instead of a hardcoded map (open-metadata#30618)

* Fixes 30533: derive entity schema subdirectory from the classpath scan instead of a hardcoded map

* gitarbot feedback: make duplicate entity type schema resolution independent of classpath scan order

* fix(settings): don't log an unset optional setting as an error (open-metadata#28960) (open-metadata#30625)

* fix(ui): keep GenericTab's widget grid inside its container (open-metadata#30622)

react-grid-layout derives each grid item's `left` and `width` in pixels and
rounds the two independently. Their exact values always sum to the container
width, but each rounding can add just under half a pixel, so the right edge of
the last column can land 1px outside the container.

That pixel is the widget card's right border. An ancestor that scrolls turns it
into a horizontal scrollbar; one that clips swallows the border outright. On
entity detail pages both happen, because the active tab pane scrolls vertically
and CSS promotes the cross axis to `auto` once one axis is not `visible`.

Give the grid 1px of horizontal containerPadding. That pulls the exact right
edge in to `width - 1`, which the rounding -- bounded at 1px, since the two
exact edges sum to the width and each rounding adds under half a pixel -- can
never exceed.

Across widths 600-2400 and every column split that reaches the right edge, the
last column escapes the container at 1575 of 1801 widths today and at none of
them with this change. For the 6 + 2 split the entity detail pages actually use,
it is one width in four.

Adjusting `margin` instead does not work: it is the gutter between items and
sits inside the sum that equals the container width, so changing it only moves
which widths collide (still 175/701 at every integer value from 0 to 24).

* Improve bulk export action stats (open-metadata#30615)

* Improve bulk export action stats

* Add auto opened when the export jobs is success

* nit

* fix stylling

* nit

* addressed comments

* fix lint checks

* nit

* fix failing unit test

* Fixes open-metadata#30117: hide soft-deleted owners on the Test Suite detail page (open-metadata#30520)

* Fixes open-metadata#30117: keep soft-deleted owners in PATCH base to fix index crash

Editing owners failed with "array item index out of range" when an owner
user was soft-deleted. Detail pages fetch with include=all (soft-deleted
owner present) and compute a positional JSON Patch, but the server loaded
the PATCH base with NON_DELETED, which dropped that owner and shrank the
array so the positional op referenced an index past the end. The dangling
ownership also could not be removed.

Both patch() entry points now load the original with a RelationIncludes
that keeps the entity at NON_DELETED but resolves owners/experts/reviewers/
followers with ALL, so the positional patch stays in range and the ID-based
updateOwners diff removes the dangling OWNS row. Adds two integration tests
reproducing the reported crash (deleted owner sorting last) and the silent
wrong-owner removal (deleted owner in the middle).

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

* Fixes open-metadata#30117: tolerate existing soft-deleted owners on PATCH; generic test

The first pass loaded the PATCH base with owner/expert/reviewer/follower refs
resolved as ALL. That fixed the positional "array item index out of range"
crash on owner removal, but getValidatedOwners (which runs on every PATCH)
re-validates the whole owner list against NON_DELETED, so ANY edit (e.g. a
description change) on an entity carrying a dangling soft-deleted owner then
failed with 404 "user instance ... not found".

- Narrow the base override to owners only. Experts/reviewers/followers are
  validated on update against NON_DELETED and would hit the same 404; they are
  left unchanged as a separate follow-up, per the issue.
- Add getValidatedOwnersForPatch: retain owners already assigned in the base
  (a soft-deleted user is still a valid, existing assignment) and validate only
  newly added owners against NON_DELETED, so a deleted user still cannot be
  freshly assigned.
- Add a generic BaseEntityIT test so every owner-supporting entity is covered,
  plus a TestSuite regression test asserting a non-owner edit succeeds and
  preserves the soft-deleted owner.

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

* Fixes open-metadata#30117: address review - retain authoritative owner refs; document tradeoff

- getValidatedOwnersForPatch: retained (already-assigned) owners now use the
  authoritative ref from the base (resolved with ALL, so names are populated)
  instead of the client-supplied ref, and the sort uses a null-safe comparator.
  Prevents a possible NPE/500 when a client sends a minimal (id+type, no name)
  ref for an already-assigned owner, and keeps response metadata authoritative.
- Document the known trade-off on PATCH_ORIGINAL_INCLUDES: callers that diff
  owners against a NON_DELETED view (a GET sending owners:non-deleted) can
  misalign against the ALL base; the representation-aware fix is the follow-up
  the issue flags as a design call.

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

* Fixes open-metadata#30117: hide soft-deleted owners on Test Suite page (align with data assets)

Revert the server-side PATCH-base change. Loading the base with owners=ALL
fixed the Test Suite crash but regressed the 17+ data-asset detail pages that
fetch owners with includeRelations=owners:non-deleted: a positional owner
remove computed against the filtered array removed the wrong owner when applied
to the longer ALL base. A single server base cannot align with both client
owner representations, so the patch itself must carry it - a separate design
call the issue flags.

Fix the reported crash where the divergence actually is: the Test Suite detail
page was the only owner-editing page fetching owners with include=all and no
includeRelations override. getTestSuiteByName now defaults owners/experts to
non-deleted like every data-asset page, so a soft-deleted owner is not surfaced
and the owner-edit diff stays aligned with the server's NON_DELETED PATCH base -
no "array item index out of range". Showing and removing soft-deleted owners
remains the issue's separate design call.

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

* Fixes open-metadata#30117: honor includeRelations on Test Suite getByName (frontend fix was a no-op)

getTestSuiteByName sends includeRelations=owners:non-deleted, but the Test
Suite getByName endpoint - unlike getById and every other entity's getByName
(e.g. TableResource) - did not accept the param, so the server still resolved
owners with include=all and surfaced the soft-deleted owner. The owner-edit
crash therefore remained.

Wire includeRelations through getByName (same pattern as getById /
TableResource.getByName), and add an IT that proves it: an include=all fetch
still surfaces the soft-deleted owner, but includeRelations=owners:non-deleted
hides it. Verified RED (without the wiring the assertion fails - owner still
surfaced) and GREEN.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Sid <30566406+siddhant1@users.noreply.github.com>

* Fix glossary export playwright (open-metadata#30585)

* fix flaky searchIndexApplication

* fix flaky glossary terms popover playwright

* revert unwanted changes

---------

Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
Co-authored-by: Rohit Jain <60229265+Rohit0301@users.noreply.github.com>

* ISSUE open-metadata#30151 - Incident Grouping Endpoint (open-metadata#30367)

* feat: added incident group endpoint

* feat: added owner, status, assignee, severiuty and bucket for client metrics

* feat: added owner, status, assignee, severiuty and bucket for client metrics

* empty commit for CI trigger

* feat: remove n+1 in entity ref. retrieval when resolving group references

* feat: ignore optional argumewnt static check error

* feat: address comments and fix failing integration tests

* feat: revert pre existing migrations to 2.0

* feat: address first pass review

* feat: move endpoint data to summary table

* feat: address comments

* feat: address comments

* chore: fix migration

* fix Column Bulk Operations tag & glossary dropdowns not selectable in the edit drawer (open-metadata#30634)

* fix Column Bulk Operations tag & glossary dropdowns not selectable in the edit drawer

* fix lint checks

* fix(test): correct EditAll-deny test assertions in EntityPermissions

When EditAll is in a DENY rule it subsumes all Edit* operations via
matchesBySubsumption(), so EditTier/EditOwners/EditCertification buttons
are not visible even when those operations have explicit ALLOW rules.
Updated assertions from toBeVisible() to not.toBeVisible() and unskipped
the test to reflect actual backend deny-first policy semantics.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Sid <30566406+siddhant1@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
Co-authored-by: Vishnu Jain <121681876+Vishnuujain@users.noreply.github.com>
Co-authored-by: Anujkumar Yadav <anujf0510@gmail.com>
Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>
Co-authored-by: shrabantipaul-collate <shrabanti.paul@getcollate.io>
Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
Co-authored-by: Teddy <teddy.crepineau@gmail.com>

---------

Co-authored-by: Harsh Vador <harsh.vador@somaiya.edu>
Co-authored-by: Rohit0301 <rj03012002@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Harsh Vador <58542468+harsh-vador@users.noreply.github.com>
Co-authored-by: Rohit Jain <60229265+Rohit0301@users.noreply.github.com>
Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
Co-authored-by: Shailesh Parmar <shailesh.parmar.webdev@gmail.com>
Co-authored-by: anuj-kumary <anujf0510@gmail.com>
Co-authored-by: Vishnu Jain <121681876+Vishnuujain@users.noreply.github.com>
Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>
Co-authored-by: shrabantipaul-collate <shrabanti.paul@getcollate.io>
Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
Co-authored-by: Teddy <teddy.crepineau@gmail.com>
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 skip-pr-checks Bypass PR metadata validation check To release Will cherry-pick this PR into the release branch UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants