ISSUE #4798-CLT

[TeddyCr]

Describe your changes:

Fixes #4798

I worked on removing the N+1 incident-status lookup on the data-quality / test-case pages because the UI was issuing one GET /v1/dataQuality/testCases/{stateId}/testCaseIncidentStatus request per test case with an open incident. On a table with 50 failing test cases that meant 50 sequential round-trips from the browser after the list had already loaded, leaving the Incident column showing skeletons long after the rest of the table had painted.

The fix inlines the incident status into the test case payload behind a new incidentStatus field, so a single list request returns everything the table needs.

Backend — new incidentStatus field on TestCase, resolved via a batched query:

• testCase.json gains an incidentStatus field ($ref → testCaseResolutionStatus.json).
• TestCaseResolutionStatusTimeSeriesDAO.getLatestRecordBatch fetches the latest resolution status for N test case FQNs in one query (chunked via EntityDAO.queryInChunks).
• TestCaseRepository.fetchAndSetIncidentStatuses wires that batch into setFieldsInBulk (DB-backed list) and into a listFromSearchWithOffset override (ES-backed search/list, which builds entities straight from the search doc and therefore never calls setFieldsInBulk).
• setFields keeps a single-entity path for GET /testCases/{id}?fields=incidentStatus.

Frontend — consume the inlined field instead of fetching per row:

• DataQualityTab replaces the Promise.allSettled fan-out (fetchTestCaseStatus) with a synchronous collectInlineIncidentStatuses over testCase.incidentStatus.
• TabSpecificField.INCIDENT_STATUS added to the fields list at the four call sites that render the Incident column.

Type of change:

• Improvement

High-level design:

Approach. The incident status was already derived server-side per test case; the only problem was that it was reachable solely through a separate per-stateId endpoint. Rather than add a new bulk endpoint (which the UI would still have to correlate back to rows), the status is inlined onto the entity behind an opt-in fields=incidentStatus flag. Existing clients that don't request the field see no change in payload or query cost.

Why two backend wiring points. setFieldsInBulk covers the DB-backed list path, but listFromSearchWithOffset reconstructs entities from the ES document via JsonUtils.readOrConvertValueLenient and never routes through setFieldsInBulk. The search/list endpoint — which is what all four UI call sites actually hit — therefore needs the explicit override. This mirrors the existing TestSuiteRepository.listFromSearchWithOffset precedent.

Backward compatibility. incidentStatus is opt-in via fields; it is absent from PATCH_FIELDS/UPDATE_FIELDS, so no write path can persist it. No migration needed — the field is derived at read time from the existing test_case_resolution_status_time_series table and is never stored on the entity row.

Alternative considered. Denormalizing incidentStatus onto the entity row / ES document (as incidentId already is) would make it free to read, but it would need every incident transition to fan out a test-case reindex, and the status would go stale on any missed event. The read-time batch keeps a single source of truth for one extra query per page.

Tests:

Use cases covered

• A table with failing test cases renders the Incident column from a single list request (no per-row incident fetch).
• GET /v1/dataQuality/testCases/{id}?fields=incidentStatus returns the latest incident status for a failed test case.
• GET /v1/dataQuality/testCases/search/list?fields=incidentStatus inlines the incident status for every returned test case.
• A test case with no incident renders no incident status.

Unit tests

• I added unit tests for the new/changed logic.
• Files added/updated:
  • DataQualityTab.test.tsx — new Inline incident status describe block: renders from the inlined field and rebuilds testCaseReference; renders nothing when incidentStatus is absent. Also drops the now-unused incidentManagerAPI mock.
  • useTestCaseList.test.ts, useTestCaseListPage.test.tsx, TestCases.test.tsx, TableProfilerProvider.test.tsx, TestSuiteDetailsPage.test.tsx — updated fields assertions for incidentStatus.
• Result: 85 suites / 1126 tests passing across components/Database/Profiler, components/DataQuality, pages/TestSuiteDetailsPage.

Backend integration tests

• I added integration tests in openmetadata-integration-tests/ for new/changed API endpoints.
• Files added/updated:
  • TestCaseResourceIT.test_incidentStatusInlinedInGetAndSearchList — asserts incidentStatus is populated with TestCaseResolutionStatusTypes.New on both the single get and the ES-backed search/list.

Ingestion integration tests

• Not applicable (no ingestion changes).

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. (Not needed — incidentStatus is derived at read time and never stored; see High-level design.)
• 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 tests around the new logic.
• For connector/ingestion changes: I updated the documentation. (N/A — no connector/ingestion changes.)

Greptile Summary

This PR inlines incident status into test-case list responses to remove per-row requests. The main changes are:

• Adds the optional incidentStatus test-case field.
• Loads latest statuses with a chunked batch query.
• Hydrates both database-backed and search-backed list results.
• Updates the data-quality UI to consume inline statuses.
• Adds backend integration and frontend unit coverage.

Confidence Score: 4/5

Status selection and active-incident filtering need fixes before merging.

• Equal timestamps can make the batch query return the wrong latest transition.
• Resolved incident history can remain visible after the active incident ID is cleared.
• The repository override matches the existing list API and the relevant callers request the new field.

CollectionDAO.java and DataQualityTab.tsx

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java	Adds chunked latest-status retrieval, but timestamp ties can select an arbitrary transition.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java	Hydrates incident status for single entities and both list paths using FQN-hash correlation.
openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java	Registers incidentStatus as a selectable read field.
openmetadata-spec/src/main/resources/json/schema/tests/testCase.json	Adds the optional derived incident-status field to the test-case schema.
openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx	Consumes inline statuses but can display historical resolved incidents after the active incident ID is cleared.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseResourceIT.java	Covers single-get and search-list hydration for a newly failed test case.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as Data Quality UI
    participant API as TestCase search/list
    participant Repo as TestCaseRepository
    participant DB as Status time series
    UI->>API: "GET search/list?fields=incidentStatus"
    API->>Repo: listFromSearchWithOffset
    Repo->>DB: Fetch latest statuses by FQN hash
    DB-->>Repo: Batched status records
    Repo-->>API: Test cases with incidentStatus
    API-->>UI: One list response
    UI->>UI: Match statuses by test-case FQN

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as Data Quality UI
    participant API as TestCase search/list
    participant Repo as TestCaseRepository
    participant DB as Status time series
    UI->>API: "GET search/list?fields=incidentStatus"
    API->>Repo: listFromSearchWithOffset
    Repo->>DB: Fetch latest statuses by FQN hash
    DB-->>Repo: Batched status records
    Repo-->>API: Test cases with incidentStatus
    API-->>UI: One list response
    UI->>UI: Match statuses by test-case FQN

Loading

Comments Outside Diff (2)

1. openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx, line 352 (link)

   Resolved Incidents Reappear

   The old path fetched a status only when incidentId identified an active incident. The new code accepts any historical incidentStatus, so a test case whose incident was resolved and whose incidentId was cleared can still show Resolved in the Incident column. Keep the active-incident check when collecting inline statuses.

2. openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java, line 10320-10322 (link)

   Timestamp Ties Return Arbitrary Status

   When two transitions for one test case share the same millisecond timestamp, both rows match the MAX(timestamp) join. Their order is unspecified, and fetchAndSetIncidentStatuses overwrites the hash-map entry, so callers can receive either transition as the latest status—for example, Assigned instead of Resolved. Add a deterministic tie-breaker consistent with the single-record lookup.

_{Reviews (1): Last reviewed commit: "fix: clean failing tests" | Re-trigger Greptile}

Context used (3)

• Context used - CLAUDE.md (source)
• Context used - openmetadata-ui-core-components/CLAUDE.md (source)
• Context used - AGENTS.md (source)

[github-actions]

❌ 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.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[gitar-bot]

💡 Edge Case: Batch latest-record query can return duplicate rows on timestamp ties

getLatestRecordBatchInternal joins on t1.timestamp = t2.maxTs, so if two resolution-status rows for the same entityFQNHash share the exact same MAX(timestamp) (same millisecond), both rows are returned and fetchAndSetIncidentStatuses keeps whichever the map iteration overwrites last. This is non-deterministic and can differ from the single-entity path (getLatestRecord, which orders by timestamp DESC LIMIT 1). Impact is low but consider disambiguating the join (e.g. add a tiebreaker like MAX(id) or select DISTINCT ON) so the inlined status matches the single-get result.

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Eliminates N+1 incident status lookups on the data-quality page by inlining statuses via batched read-time queries. Ensure the getLatestRecordBatch implementation is hardened against duplicate rows resulting from timestamp ties.

💡 Edge Case: Batch latest-record query can return duplicate rows on timestamp ties

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java:10317-10329 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java:480-494

getLatestRecordBatchInternal joins on t1.timestamp = t2.maxTs, so if two resolution-status rows for the same entityFQNHash share the exact same MAX(timestamp) (same millisecond), both rows are returned and fetchAndSetIncidentStatuses keeps whichever the map iteration overwrites last. This is non-deterministic and can differ from the single-entity path (getLatestRecord, which orders by timestamp DESC LIMIT 1). Impact is low but consider disambiguating the join (e.g. add a tiebreaker like MAX(id) or select DISTINCT ON) so the inlined status matches the single-get result.

🤖 Prompt for agents

Code Review: Eliminates N+1 incident status lookups on the data-quality page by inlining statuses via batched read-time queries. Ensure the `getLatestRecordBatch` implementation is hardened against duplicate rows resulting from timestamp ties.

1. 💡 Edge Case: Batch latest-record query can return duplicate rows on timestamp ties
   Files: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java:10317-10329, openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java:480-494

   getLatestRecordBatchInternal joins on `t1.timestamp = t2.maxTs`, so if two resolution-status rows for the same entityFQNHash share the exact same MAX(timestamp) (same millisecond), both rows are returned and `fetchAndSetIncidentStatuses` keeps whichever the map iteration overwrites last. This is non-deterministic and can differ from the single-entity path (`getLatestRecord`, which orders by timestamp DESC LIMIT 1). Impact is low but consider disambiguating the join (e.g. add a tiebreaker like MAX(id) or select DISTINCT ON) so the inlined status matches the single-get result.

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}