Skip to content

Fixes 34616: Grouped incidents release-testing fixes (sort in URL, status menu, translations) - #36397

Merged
anuj-kumary merged 7 commits into
mainfrom
ShaileshParmar11/grouped-incidents-ai-feedback
Oct 28, 2026
Merged

anuj-kumary merged 7 commits into
mainfrom
ShaileshParmar11/grouped-incidents-ai-feedback

Conversation

@ShaileshParmar11

@ShaileshParmar11 ShaileshParmar11 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Describe your changes:

Fixes #34616

Follow-up to #34599 (grouped incidents in the AI-mode Incident Manager). It fixes the frontend issues found in the 2.1.0 release testing of that feature:

  1. Sort is not kept in the URL. The groups' order is now read from and written to the URL as sortField and sortType (the same names as the API params), so a reload or a shared link keeps the order. Group by, filters and the open group already lived in the URL; the page and page size stay in component state. An unknown field or direction falls back to its default independently. Recorded in ADR:2026-10-28-incident-group-sort-lives-in-the-url.
  2. The inline status menu offers transitions the backend rejects. On an Assigned incident the menu offered Ack and New, and picking Ack failed with Transition 'ack' is not available for task .... The menu now offers only what TestCaseResolutionTaskWorkflow allows from each stage: an Assigned incident can be reassigned or resolved. New and Ack still offer every status, and a Resolved incident is still reopened through reopenResolvedIncident.
  3. Group rows are mistranslated in non-English locales. "4 open" rendered in French as "4 ouvrir" (the verb), the status mix was not pluralised ("3 assigné") and left "new" in English, and severities showed as the English enum name ("Severity 2"). The group row caption and the status mix now use plural keys, translated in every locale ("4 ouverts", "3 assignés · 1 nouveau"). Severities show the translated word with the level ("Sévérité 2") everywhere the incident manager, the severity modal and the Inbox incident tile render them. English output is unchanged ("Severity 2", "open", "3 assigned").

Out of scope, tracked for the backend in the release-testing notes:

  • A group's count can be higher than its drawer: the group counts open incidents by stateId, while the drawer's latest=true listing returns one record per test case.
  • The classic Incident Manager lists incidents of soft-deleted test cases: the testCaseResolutionStatus search listing has no deleted-test-case condition.

The header stat cards not matching the groups, and not following the filters, are by design and unchanged.

Type of change:

  • Bug fix

High-level design:

Frontend only; no schema, API or backend change.

  • Sort in the URL: useIncidentGroups derives the sort from the query string with parseIncidentGroupSort (IncidentGroups.utils.ts) instead of useState. handleSortChange writes sortField and sortType with navigate(..., { replace: true }), the same way the group-by and filter handlers do, so they survive each other's updates. The page reset on a sort change is unchanged: the paging key already includes the sort.
  • Status transitions: INCIDENT_STATUS_MENU_OPTIONS (constants/TestSuite.constant.ts) lists, for each status, the statuses its menu offers, mirroring the edges of TestCaseResolutionTaskWorkflow. InlineTestCaseIncidentStatus and TestCaseStatusModal both read it, so the inline menu and the modal cannot drift apart. The modal already had this rule hard-coded; it now shares the constant.
  • Translations:
    • New plural keys label.incident-open-lowercase and label.count-incident-{new,ack,assigned}, each with all six plural forms in every locale (yarn i18n), with real translations.
    • New getSeverityLabel (Severity/Severity.utils.ts) builds the label from label.severity and the level. It replaces startCase(severity) in the severity badge, chip, modal, filters, bulk selection bar and the Inbox incident tile.

Tests:

Use cases covered

  • Sorting the groups writes sortField/sortType to the URL; opening a URL that carries them loads the groups in that order; an unknown value falls back to the default.
  • The status menu of an Assigned incident offers only Assigned and Resolved.
  • French and Russian pick the right plural form for the new group-row counts (read through i18next itself, against the real locale files).
  • Severity labels render through label.severity in the badge, chip, modal, bulk dialog, group table, incident list and Inbox tile.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files added/updated:
    • IncidentGroupsView.test.tsx: sort kept in the URL; sort read from a link
    • IncidentGroups.utils.test.ts: parseIncidentGroupSort
    • InlineTestCaseIncidentStatus.test.tsx: Assigned incident menu offers only Assigned and Resolved
    • IncidentGroupsPlurals.test.ts: French and Russian plurals for the new keys
    • IncidentStatusBreakdown.test.tsx, IncidentGroupsTable.test.tsx: plural status mix
    • InlineSeverity.test.tsx, Severity.test.tsx, SeverityModal.test.tsx, IncidentGroupBulkStatusModal.test.tsx, IncidentList.test.tsx, taskStatTiles.utils.test.ts: severity label expectations
  • Jest, run locally on the final commit: src/components/observability/IncidentManager, src/components/DataQuality/IncidentManager, src/components/DataQuality/TestCaseStatusModal, src/components/discovery/personal-space/InboxPage. 95 suites, 1464 tests passed.
  • Coverage on the changed source files (Jest, measured on the first commit, before the follow-up that touched TestCaseStatusModal and taskStatTiles): 95.75% lines, 96.03% statements, 87.32% branches, 97.05% functions.
  • yarn tsc --noEmit: 0 errors. ESLint on changed files: 0 errors. The warnings it reports were already there before this change.

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.
  • Ran the impacted incident specs locally against this branch's UI (Vite dev server on d59151e2526) and a local Docker server built from main (3e14a887471), with PLAYWRIGHT_IS_OSS=1. Without that variable the setup step calls /api/v1/ingestionRunners, which is a Collate-only endpoint. 26 passed, 0 failed, 0 flaky, 0 skipped (23 spec tests plus 3 setup/teardown steps):
    • Features/AIMode/IncidentGroups.spec.ts: 10 passed. After 71823b6655d added "keeps the sort in the URL across a reload", I ran this spec again: 11 passed.
    • Features/DataQuality/IncidentManagerLocaleLayout.spec.ts: 7 passed
    • Features/IncidentManager.spec.ts: 6 passed
  • The other specs that make playwright_affected selects were not run locally (about 70, mostly in the wider data quality area). The merge queue runs the full suite. The command also reports impact-map gaps for several changed files, so the merge queue runs the full suite for those files anyway.
  • English labels are unchanged, so the existing text matches in these specs ('Severity 4' and so on) still hold.
  • Added keeps the sort in the URL across a reload to IncidentGroups.spec.ts. The restricted menu on an Assigned incident is covered by Jest only.

Playwright report: 26 passed

Manual testing performed

Ran this branch's UI (yarn start, port 3001) against a local Docker backend with open incidents, logged in as admin:

  1. /observability/incident-manager: sorted by Severity; the URL gained sortField=severity&sortType=desc; after a reload and in a new tab, the same order and header arrow.
  2. Opened an Assigned incident's status menu, in the group drawer and in the classic /incident-manager: it offers only Assigned and Resolved. A New or Ack incident still offers every status.
  3. Switched the language to Français: the group rows read "N ouverts", "3 assignés · 1 nouveau", and the severities read "Sévérité N" in the badge, filter, bulk bar, chip and modal. English is unchanged.

UI screen recording:

Recorded on d59151e2526. The UI is this branch on the Vite dev server (port 3001), proxied to a local Docker stack built from main (3e14a887471): server, PostgreSQL and OpenSearch all healthy, and /api/v1/system/version reports that revision. Data comes from the repo's sample data, plus a fixture seeded through the API with the Playwright seedIncidents helper (one table, three failing test cases: two Assigned, one New, one with Severity 2). The fixture was removed afterwards. Every step in the clip is checked with Playwright assertions against real server responses.

grouped-incidents-fixes.mp4
  1. Sort is kept in the URL. Sorting by Severity adds sortField=severity&sortType=desc to the URL. After a reload, the groups are still sorted by Severity and the column shows the descending arrow.

Sort written to the URL
Sort kept after a reload

  1. Status menu of an Assigned incident. In the group drawer, an Assigned incident's menu offers only Assigned (reassign) and Resolved. Ack and New are not offered.

Assigned incident status menu

  1. French group rows (?lng=fr-FR). The seeded table's row reads "3 ouverts", "2 assignés · 1 nouveau" and "Sévérité 2". Other rows show the singular forms ("1 ouvert", "1 nouveau").

French group rows

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. (No schema change.)

  • For a UI feature or feature task: as part of PR creation, I ran the PR build in Docker with sample data and embedded a verified GitHub-hosted recording plus build/health evidence above (or explained why this requirement is not applicable).

  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

  • Self-review: I reviewed the final diff against REVIEW.md (the pr-checklist skill runs it) and fixed every 🔴 finding, or answered it on the line below.

    • Reviewed 8c0c7b88926: no 🔴. Fixed in 60f888d800a:
      • the Inbox severity tile now uses the shared severity label;
      • TestCaseStatusModal now shares the transition map;
      • a doc comment that had moved to the wrong function is back in place.
    • Answered, not changed:
      • The restricted status menu has no Playwright spec. It is covered by Jest, and is shown in the recording.
      • The now-unused keys label.{open,ack,assigned}-lowercase are left in place. They are shared locale keys, and removing them from 21 files is a separate cleanup.
  • Decision check: adds docs/decisions/2026-10-28-incident-group-sort-lives-in-the-url.md (ADR:2026-10-28-incident-group-sort-lives-in-the-url), since keeping the order in the URL reverses the earlier local-state choice and makes sortField/sortType part of the page's URL contract. The status transition map needs no record: it mirrors the server's TestCaseResolutionTaskWorkflow rather than deciding a new rule.

  • I have added a test that covers the exact scenario we are fixing.

RetriggerView in GreptileConfidence Score: 5/5

The changes since the previous review appear safe to merge.

Summary

The PR keeps incident-group sorting in the URL, shares the allowed status choices, and translates incident counts and severity labels.

  • Grouped incidents keep their sort when people reload or share a link.
  • Incident status menus offer choices allowed from the current stage.
  • Incident counts and severity levels use the selected language across incident views.

Reviews (5) · Last reviewed commit: "ui(incidents): organise imports in bulk ..." · Reviewed by Greptile

…tions

- Keep the grouped view's sort in the URL as sortField/sortType so a
  reload or a shared link keeps the order.
- Offer only the transitions TestCaseResolutionTaskWorkflow allows in the
  inline status menu: an Assigned incident can be reassigned or resolved.
- Use plural keys for the group row counts and status mix, translated in
  every locale, and show severities as the translated label with its level.
Copilot AI balanced review requested due to automatic review settings October 28, 2026 06:24
@ShaileshParmar11
ShaileshParmar11 requested a review from a team as a code owner October 28, 2026 06:24
@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Oct 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 28, 2026 06:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 28, 2026 06:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 79%
79.75% (131285/164612) 66.26% (46305/69874) 69.22% (26279/37963)

- Bulk status changes now skip any incident whose stage has no edge for
  the new status, using the same INCIDENT_STATUS_MENU_OPTIONS map as the
  inline menu and the status modal.
- One replaceSearch helper writes the group-by, filter and sort params.
  Group-by changes now also stringify with arrayFormat: 'repeat', so array
  filters keep round-tripping as repeated params.
- Cover the independent field/direction fallback of parseIncidentGroupSort,
  add a Playwright test for the sort surviving a reload, query the popover
  Back button by test id, and correct the decision record.
Copilot AI balanced review requested due to automatic review settings October 28, 2026 07:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

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: 996 → 996
- 0 added, 0 removed, 1 changed spec-list

Entries whose spec list changed:
  openmetadata-ui/src/main/resources/ui/playwright/utils/waitHelpers.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

Copilot AI balanced review requested due to automatic review settings October 28, 2026 07:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

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

Count Rule
4 openmetadata-imports/no-lower-layer-page-imports
3 react-hooks/exhaustive-deps
2 openmetadata-imports/review-sequential-api-calls
All findings
Location Rule Message
🟡 src/components/DataQuality/IncidentManager/Severity/Severity.component.tsx:59:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'onCancel'. Either include it or remove the dependency array.
🟡 src/components/DataQuality/IncidentManager/TestCaseStatus/InlineTestCaseIncidentStatus.component.tsx:40:1 openmetadata-imports/no-lower-layer-page-imports Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.
🟡 src/components/DataQuality/IncidentManager/TestCaseStatus/InlineTestCaseIncidentStatus.component.tsx:299:27 openmetadata-imports/review-sequential-api-calls Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent
🟡 src/components/DataQuality/TestCaseStatusModal/TestCaseStatusModal.component.tsx:31:1 openmetadata-imports/no-lower-layer-page-imports Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.
🟡 src/components/DataQuality/TestCaseStatusModal/TestCaseStatusModal.component.tsx:32:1 openmetadata-imports/no-lower-layer-page-imports Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.
🟡 src/components/DataQuality/TestCaseStatusModal/TestCaseStatusModal.component.tsx:242:25 openmetadata-imports/review-sequential-api-calls Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent
🟡 src/components/DataQuality/TestCaseStatusModal/TestCaseStatusModal.component.tsx:275:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchInitialAssign'. Either include it or remove the dependency array.
🟡 src/components/DataQuality/TestCaseStatusModal/TestCaseStatusModal.component.tsx:290:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'form'. Either include it or remove the dependency array.
🟡 src/constants/TestSuite.constant.ts:16:1 openmetadata-imports/no-lower-layer-page-imports Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.

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

make ui-checkstyle-changed

Copilot AI balanced review requested due to automatic review settings October 28, 2026 08:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@gitar-bot

gitar-bot Bot commented Oct 28, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 closed / 1 findings

🟡 Medium risk · Incident menus now use a shared transition map and sorting persists in URLs; a map mismatch could hide valid status actions.

Keeps the grouped incident sort in the URL as sortField and sortType, so reloads and shared links preserve order. No open issues remain; the earlier missing decision record for the sort change has been resolved.

✅ 1 closed
✅ Quality: Group sort moved into the URL with no decision record

📄 openmetadata-ui/src/main/resources/ui/src/components/observability/IncidentManager/IncidentGroups/useIncidentGroups.ts:282-287 📄 openmetadata-ui/src/main/resources/ui/src/components/observability/IncidentManager/IncidentGroups/useIncidentGroups.ts:381-395 📄 openmetadata-ui/src/main/resources/ui/src/components/observability/IncidentManager/IncidentGroups/IncidentGroups.constants.ts:63-68 📄 openmetadata-ui/src/main/resources/ui/src/components/observability/IncidentManager/IncidentGroups/IncidentGroups.utils.ts:307-320
This PR reverses a choice the code used to document: the removed comment in useIncidentGroups said the ordering was "Local rather than in the URL: … a shared link carries the groups without having to carry their order too." It also adds a new query-string contract that shared links and reloads now rely on: sortField/sortType, named after the API params, with unknown values falling back to DEFAULT_INCIDENT_GROUP_SORT. The old reasoning now lives only in a deleted comment, the new rule only in the PR description, and no record in docs/decisions/ covers either. Because the repo squash-merges, the reason for the reversal will be lost.

The record would say: "The grouped incidents' order is part of the page URL as sortField and sortType, named after the groups API params. Sort changes replace the history entry and keep the other params. An unknown field or direction falls back to the default order. This supersedes keeping the order in local state, because release testing showed users expect a reload or shared link to keep the order." The description's "keep group sort in URL" / "An unknown sortField or sortType value in the URL falls back to the default order" is where this comes from.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Display: compact → Showing compact verification results.

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

@sonarqubecloud

Copy link
Copy Markdown

@anuj-kumary
anuj-kumary added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 53671c0 Oct 28, 2026
86 checks passed
@anuj-kumary
anuj-kumary deleted the ShaileshParmar11/grouped-incidents-ai-feedback branch October 28, 2026 11:31
ShaileshParmar11 added a commit that referenced this pull request Oct 28, 2026
…atus menu, translations) (#36397) (#36450)

* ui(incidents): keep group sort in URL, limit status menu, fix translations

- Keep the grouped view's sort in the URL as sortField/sortType so a
  reload or a shared link keeps the order.
- Offer only the transitions TestCaseResolutionTaskWorkflow allows in the
  inline status menu: an Assigned incident can be reassigned or resolved.
- Use plural keys for the group row counts and status mix, translated in
  every locale, and show severities as the translated label with its level.

* ui(incidents): share severity wording and status transitions with Inbox and status modal

* docs(decisions): record that the incident group sort lives in the URL

* ui(incidents): organise imports

* ui(incidents): share transition map with bulk, tidy sort URL handling

- Bulk status changes now skip any incident whose stage has no edge for
  the new status, using the same INCIDENT_STATUS_MENU_OPTIONS map as the
  inline menu and the status modal.
- One replaceSearch helper writes the group-by, filter and sort params.
  Group-by changes now also stringify with arrayFormat: 'repeat', so array
  filters keep round-tripping as repeated params.
- Cover the independent field/direction fallback of parseIncidentGroupSort,
  add a Playwright test for the sort surviving a reload, query the popover
  Back button by test id, and correct the decision record.

* chore(playwright): auto-refresh impact-map.generated.json

* ui(incidents): organise imports in bulk utils

---------


(cherry picked from commit 53671c0)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ShaileshParmar11 added a commit that referenced this pull request Oct 29, 2026
…nt status menu (#36538)

Backport of the status-menu part of #36397 to 2.0. An Assigned incident's
menu offered New and Ack, which TestCaseResolutionTaskWorkflow has no edge
for, so picking either failed with "Transition 'new' is not available for
task". The menu now offers only reassign and resolve from Assigned.
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.

Grouped incidents in the AI-mode Incident Manager

3 participants