Repository navigation
Incident groups filter bar and pagination (superseded by #34599) - #34588
ShaileshParmar11 wants to merge 8 commits into
Conversation
github-actions
Bot
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
| Count | Rule |
|---|---|
| 1 | react-hooks/exhaustive-deps |
| 1 | openmetadata-ui-patterns/no-non-adaptive-palette |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/observability/DataQuality/Dashboard/DqDateRangeFilter.tsx:72:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'buildValue'. Either include it or remove the dependency array. |
| 🟡 | src/components/observability/IncidentManager/IncidentManagerPage.tsx:39:3 |
openmetadata-ui-patterns/no-non-adaptive-palette |
Raw palette class "tw:[&_.custom-chart-background]:bg-gray-blue-25" is static — it does not flip in dark mode. Use the theme-adapting "utility-" variant (tw:bg- |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Jest test CoverageUI tests summary
|
Add the filter row of the design above the incident groups (test case, assignee, open statuses, date field and date range) and page the groups on the endpoint's opaque cursors. - Filters persist in the query string under the params the incident listing on the same page already reads, so one filter set drives both; the listing's own FilterBar is removed. - Any change to the filters, dimension, ordering or page size starts over from the first page; a background refresh keeps the current page. - Cursors are passed back verbatim as `offset`, never decoded. - Empty and error states move to the core EmptyPlaceholder. - DqDateRangeFilter gains an opt-in `fullWidth` so the range trigger spans its field like the inputs beside it.
- The pager jumps to the page picked, through the new `page` parameter, instead of stepping one page towards it. - The assignee filter searches users and teams, lists them on open, and names a selected assignee by display name, also after a reload. The test case search waits for a pause in typing. - Filters that match nothing say so and offer to clear them, instead of the "no active incidents" placeholder. - A failed read offers Retry, and raises a toast only when the rows stay on screen. - The loader keeps the section's height, so the page does not jump.
A background refresh that came back empty stepped back a page, but the read of that page ran in the foreground: had it failed, the rows on screen were wiped for the error state instead of staying with a toast. The step-back now reads the earlier page the way the refresh was read.
Hand the navigation back from the filter change instead of leaving it floating, test the status against its options with includes, and dedupe it with a Set.
The test case filter searched through a debounced call whose responses landed in whatever order they came back, so an older, slower search could replace the results of a newer one. The search now runs as one query per debounced text, as the assignee picker does, and closing the menu drops the pending text. Each filter caption now labels the group its control sits in, so a trigger that shows only the picked value is still announced with its field.
…ad of overriding its button
Code Review ✅ Approved 2 closed / 2 findings🟡 Medium risk · Incident filtering and page-number navigation change UI requests and shared controls. Adds incident groups filter bar with debounced test case search, assignee selection, status filtering, and date range; implements page-number-based pagination with automatic reset to page 1 on filter changes. Resolves background refresh on empty result and step-back behavior; comprehensive unit tests cover filters, paging, empty and error states with 94% statement coverage. No issues found. ✅ 2 closed✅ Edge Case: Refresh past the last page shows 'No active incidents' with no pager
✅ Edge Case: Step-back after empty refresh reloads in the foreground, not background
OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
|
Combined into #34599, which carries the whole grouped incident manager (backend and frontend) as one PR against |
Describe your changes:
Was for #30417 — superseded by #34599
The filter row of the design above the incident groups, and paging by page number.
pageparameter added in Incident groups backend: related entities, drill-down parity, bulk severity #34549 (the next PR in this stack), instead of stepping one page towards it.Type of change:
High-level design:
useIncidentGroupsowns the dimension, the filters (parsed from the URL, with invalid values dropped), the sort and the page. The page state is keyed on the query, so any change resets it to 1.useIncidentGroups. The shareduseIncidentPaginghook the drawer and drill-down lists use arrives with them in Incident group drawer and drill-down (superseded by #34599) #34591.IncidentGroupsFiltersuses the coreFilterSelectand the existingDqDateRangeFilter(which gains an opt-infullWidth). No antd.EmptyPlaceholder.Tests:
Use cases covered
Unit tests
IncidentGroupsFilters.test.tsx,IncidentGroupsView.test.tsx,IncidentGroups.utils.test.ts.jest --coverage, limited to this PR's files):incidentManagerAPI.ts: this PR only adds request param types; its uncovered lines are existing request helpers.Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
/observability/incident-manager.UI screen recording / screenshots:
TODO: attach a recording (opened as draft until it is added).
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.