Skip to content

refactor(ui): split alerts into reusable hooks and components#29708

Merged
shah-harshit merged 7 commits into
mainfrom
codex/alerts-ui-hooks-refactor
Jul 3, 2026
Merged

refactor(ui): split alerts into reusable hooks and components#29708
shah-harshit merged 7 commits into
mainfrom
codex/alerts-ui-hooks-refactor

Conversation

@shah-harshit

@shah-harshit shah-harshit commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Ref: openmetadata-collate#4839

Refactors alert UI pages by moving listing, details, create, and edit logic into reusable hooks and smaller components while keeping the existing OpenMetadata/Collate-side UI behavior unchanged.

  • Extracts observability alert listing state/actions into useObservabilityAlerts with smaller header, table, and row-action components.
  • Extracts add/edit alert form state/actions into useObservabilityAlertForm, resource/template hooks, and reusable form components.
  • Extracts alert details state/actions into useAlertDetailsPage, permission hooks, and reusable details content components.
  • Keeps alert source/filter/action typing focused on the UI contracts consumed by the reusable components.
  • Leaves the AskCollate AI-mode visual redesign for alerts to a separate PR.

Type of change:

  • Improvement

High-level design:

The refactor keeps the existing route/page entry points intact while moving reusable alert page logic into hooks. Page components now compose smaller presentational components with hook outputs, making the alert UI easier to reuse and customize in future AskCollate AI-mode flows without duplicating fetch, permission, pagination, save, update, owner, description, and delete behavior.

Tests:

Use cases covered

  • Observability alerts list renders data, empty state, create action, edit/delete permissions, pagination, and delete modal behavior.
  • Add/edit observability alert page renders breadcrumbs, form sections, source/filter/action fields, destination fields, save/cancel actions, and extension points.
  • Alert details page renders details, permissions, edit/delete navigation, owner updates, description content, diagnostics, and tabs.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files covered:
    • src/pages/ObservabilityAlertsPage/ObservabilityAlertsPage.test.tsx
    • src/pages/AddObservabilityPage/AddObservabilityPage.test.tsx
    • src/pages/AlertDetailsPage/AlertDetailsPage.test.tsx
    • src/components/Alerts/AlertFormSourceItem/AlertFormSourceItem.test.tsx
    • src/components/Alerts/ObservabilityFormFiltersItem/ObservabilityFormFiltersItem.test.tsx

Backend integration tests

  • Not applicable. No backend API changes.

Ingestion integration tests

  • Not applicable. No ingestion changes.

Playwright (UI) tests

  • Not applicable. Structural UI refactor covered by focused unit tests.

Manual testing performed

  • ./node_modules/.bin/jest src/pages/ObservabilityAlertsPage/ObservabilityAlertsPage.test.tsx src/pages/AddObservabilityPage/AddObservabilityPage.test.tsx src/pages/AlertDetailsPage/AlertDetailsPage.test.tsx src/components/Alerts/AlertFormSourceItem/AlertFormSourceItem.test.tsx src/components/Alerts/ObservabilityFormFiltersItem/ObservabilityFormFiltersItem.test.tsx --runInBand --silent
  • Targeted ESLint on changed UI files with --max-warnings=0.
  • UI checkstyle sequence on changed UI files.
  • git diff --check.
  • TypeScript diagnostics reviewed for changed alert files; no diagnostics reported for the changed alert paths.

UI screen recording / screenshots:

Not applicable. Structural UI refactor with no intended visual behavior change.

Checklist:

  • I have read the CONTRIBUTING document.
  • This PR references the Collate tracking ticket using Ref only.
  • For JSON Schema changes: Not applicable.
  • For UI visual changes: Not applicable. This PR is a structural refactor with no intended visual behavior change.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Summary by Gitar

  • Refactored UI logic:
    • Split alert details, list, and form logic into custom hooks useAlertDetailsPage, useObservabilityAlerts, and useObservabilityAlertForm to improve maintainability and reuse.
    • Decoupled presentational logic by introducing focused components like AlertDetailsContent and ObservabilityAlertsTable.
  • Fixed UI behavior:
    • Enabled proper re-fetching of alert details in AlertDetailsPage when the fqn prop changes by updating the hook dependencies.
    • Added unit test in AlertDetailsPage.test.tsx to verify correct data refetching on fqn updates.

This will update automatically on new commits.

Greptile Summary

This PR refactors the observability alert UI pages by extracting listing, create/edit form, and details logic into reusable hooks (useObservabilityAlerts, useObservabilityAlertForm, useAlertDetailsPage) and smaller presentational components, while keeping the existing route entry-points and visible behavior identical.

  • Three new hook families are introduced under their respective page directories — each hook encapsulates data-fetching, permissions, pagination, and action handlers that were previously inlined in the page component.
  • Three page components (ObservabilityAlertsPage, AddObservabilityPage, AlertDetailsPage) now simply call a hook and spread the result into a single presentational child component.
  • Previously flagged issues (stale fqn on prop change, as unknown as double-cast, unused viewPermission dep) are resolved by the useCallback/[fqn] dep pattern, explicit enum mapping in ObservabilityAlertForm.utils.ts, and a cleaned-up tabItems memo.

Confidence Score: 5/5

Safe to merge — the change is a pure structural refactor with no functional modifications; all existing alert page behavior is preserved through thin shell components that compose the extracted hooks.

The three previously reported bugs (stale fqn on prop change, double-cast type suppression, spurious viewPermission memo dep) are all addressed. The remaining findings are exhaustive-deps omissions in useObservabilityAlerts that have no observable runtime impact given the stability of the affected dependencies.

openmetadata-ui/src/main/resources/ui/src/pages/ObservabilityAlertsPage/hooks/useObservabilityAlerts.ts has two useEffect dependency arrays that are missing their respective useCallback references.

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/pages/AlertDetailsPage/hooks/useAlertDetailsPage.tsx Core hook for alert details; previously reported stale-fqn bug is fixed — fetchAlertDetails and fetchAlertEventDiagnosticCounts are now useCallbacks with [fqn] deps so the effect re-runs on fqn changes. breadcrumb and callback deps also corrected.
openmetadata-ui/src/main/resources/ui/src/pages/AlertDetailsPage/hooks/useAlertDetailsPermissions.ts Extracts permission fetching into its own hook using the ref-escape-hatch pattern; fetchResourcePermission re-runs when fqn changes, cleanly exposing a loading flag to the caller.
openmetadata-ui/src/main/resources/ui/src/pages/ObservabilityAlertsPage/hooks/useObservabilityAlerts.ts Extracts all listing state/actions; fetchAlerts is missing from the second useEffect's dependency array, which will trigger react-hooks/exhaustive-deps warnings even though the practical impact is minimal.
openmetadata-ui/src/main/resources/ui/src/pages/AddObservabilityPage/hooks/useObservabilityAlertForm.ts Replaces the monolithic AddObservabilityPage logic with a composable hook; correctly delegates to useObservabilityAlertResources and useObservabilityAlertTemplates, and handles both fqn-prop and route-param sourcing.
openmetadata-ui/src/main/resources/ui/src/pages/AddObservabilityPage/ObservabilityAlertForm.utils.ts Resolves the previously reported double-cast issue with explicit enum-to-enum mapping functions; all EventFilterRule fields are mapped exhaustively with optional spreading.
openmetadata-ui/src/main/resources/ui/src/pages/AlertDetailsPage/AlertDetailsPage.test.tsx Adds a regression test that re-renders with a different fqn prop and asserts that both API calls fire with the new fqn, directly covering the previously reported stale-data bug.
openmetadata-ui/src/main/resources/ui/src/pages/ObservabilityAlertsPage/components/ObservabilityAlertsTable.tsx Extracted table rendering with new onViewAlert support for Collate AI-mode; falls back to Link navigation when onViewAlert is not provided, preserving existing behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ObservabilityAlertsPage] --> B[useObservabilityAlerts]
    A --> C[ObservabilityAlertsHeader]
    A --> D[ObservabilityAlertsTable]
    D --> E[ObservabilityAlertActions]
    B --> F[usePaging]
    B --> G[usePermissionProvider]
    B --> H[useLimitStore]

    I[AddObservabilityPage] --> J[useObservabilityAlertForm]
    I --> K[ObservabilityAlertForm]
    K --> L[ObservabilityAlertFormFields]
    J --> M[useObservabilityAlertResources]
    J --> N[useObservabilityAlertTemplates]
    M --> O[ObservabilityAlertForm.utils: toObservabilityFilterResourceDescriptor]

    P[AlertDetailsPage] --> Q[useAlertDetailsPage]
    P --> R[AlertDetailsContent]
    Q --> S[useAlertDetailsPermissions]
    Q --> T[observabilityAPI / alertsAPI]
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"}}}%%
flowchart TD
    A[ObservabilityAlertsPage] --> B[useObservabilityAlerts]
    A --> C[ObservabilityAlertsHeader]
    A --> D[ObservabilityAlertsTable]
    D --> E[ObservabilityAlertActions]
    B --> F[usePaging]
    B --> G[usePermissionProvider]
    B --> H[useLimitStore]

    I[AddObservabilityPage] --> J[useObservabilityAlertForm]
    I --> K[ObservabilityAlertForm]
    K --> L[ObservabilityAlertFormFields]
    J --> M[useObservabilityAlertResources]
    J --> N[useObservabilityAlertTemplates]
    M --> O[ObservabilityAlertForm.utils: toObservabilityFilterResourceDescriptor]

    P[AlertDetailsPage] --> Q[useAlertDetailsPage]
    P --> R[AlertDetailsContent]
    Q --> S[useAlertDetailsPermissions]
    Q --> T[observabilityAPI / alertsAPI]
Loading

Reviews (7): Last reviewed commit: "fix(ui): refetch alert details on fqn ch..." | Re-trigger Greptile

@shah-harshit shah-harshit requested a review from a team as a code owner July 2, 2026 14:05
@shah-harshit shah-harshit added UI UI specific issues safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check labels Jul 2, 2026
@shah-harshit shah-harshit self-assigned this Jul 2, 2026
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
63.29% (71361/112750) 46.32% (41448/89464) 47.59% (12723/26734)

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (25 flaky)

✅ 4480 passed · ❌ 0 failed · 🟡 25 flaky · ⏭️ 38 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 438 0 5 16
🟡 Shard 2 806 0 4 8
🟡 Shard 3 806 0 3 7
🟡 Shard 4 810 0 3 5
🟡 Shard 5 853 0 4 0
🟡 Shard 6 767 0 6 2
🟡 25 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the Drive Service entity item action after rules disabled (shard 1, 1 retry)
  • Features/Glossary/GlossaryPagination.spec.ts › should check for nested glossary term search (shard 1, 1 retry)
  • Features/Glossary/GlossaryPagination.spec.ts › should filter by InReview status (shard 1, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: metric (shard 1, 1 retry)
  • Flow/SearchRBAC.spec.ts › the browse tree only shows the asset-type categories a user can access (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/BulkImport.spec.ts › Database Schema (shard 2, 2 retries)
  • Features/ContextCenterPermission.spec.ts › user with deleteAll permission can see delete action but not restore action on an archived document, and can delete it (shard 2, 2 retries)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/KnowledgeCenterList.spec.ts › Knowledge Center List - Verify Recently Viewed widget (shard 3, 1 retry)
  • Features/KnowledgeCenterTextEditor.spec.ts › Rich Text Editor - Text Formatting (shard 3, 1 retry)
  • Features/SearchExport.spec.ts › Export queues a background job and downloads from the jobs tray (shard 3, 1 retry)
  • Flow/ServiceCreationPermissions.spec.ts › User with service creation permission can create a new database service (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Should clear search and show all properties for apiCollection in right panel (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Spreadsheet (shard 4, 1 retry)
  • Pages/ExploreBrowse.spec.ts › service type drill-down disables unrelated roots and query-panel Clear resets it (shard 5, 2 retries)
  • Pages/ExplorePageRightPanel.spec.ts › Should perform CRUD and Removal operations for searchIndex (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel.spec.ts › Should display and verify schema fields for dashboardDataModel (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel.spec.ts › Should navigate to lineage and test controls for dashboardDataModel (shard 5, 1 retry)
  • Features/AutoPilot.spec.ts › Create Service and check the AutoPilot status (shard 6, 1 retry)
  • Pages/GlossaryImportExport.spec.ts › Glossary Bulk Import Export (shard 6, 1 retry)
  • Pages/GlossaryImportExport.spec.ts › Import partial success - some terms pass, some fail (shard 6, 1 retry)
  • Pages/InputOutputPorts.spec.ts › Output ports section collapse/expand (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab is NOT visible for pipelineService in platform lineage (shard 6, 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

@shah-harshit shah-harshit changed the title refactor(ui): split alert page logic into hooks refactor(ui): split alerts into reusable hooks and components Jul 3, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jul 3, 2026

Copy link
Copy Markdown

@shah-harshit shah-harshit merged commit cb623f6 into main Jul 3, 2026
62 of 66 checks passed
@shah-harshit shah-harshit deleted the codex/alerts-ui-hooks-refactor branch July 3, 2026 18:42
@gitar-bot

gitar-bot Bot commented Jul 3, 2026

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

Refactors observability alert logic into reusable hooks and components, improving maintainability while fixing a detail refetching issue upon FQN changes. Addresses the ComponentProps casting finding by updating the filter-item prop type checks.

✅ 1 resolved
Quality: as ComponentProps cast suppresses filter-item prop type checks

📄 openmetadata-ui/src/main/resources/ui/src/pages/AddObservabilityPage/components/ObservabilityAlertFormFields.tsx:46-49 📄 openmetadata-ui/src/main/resources/ui/src/pages/AddObservabilityPage/components/ObservabilityAlertFormFields.tsx:84-88
In ObservabilityAlertFormFields.tsx, observabilityFormFiltersItemProps is assembled from the locally-narrowed containerEntities/supportedFilters types and then cast with as ComponentProps<typeof ObservabilityFormFiltersItem> before being spread into <ObservabilityFormFiltersItem />. If the component's declared prop types are stricter than the local ObservabilityAlertFormFieldsProps shapes (e.g. supportedFilters expects the full generated EventFilterRule shape rather than the narrowed local one), the cast will silently suppress a genuine type mismatch that TypeScript would otherwise catch. Prefer passing the props inline (<ObservabilityFormFiltersItem containerEntities={containerEntities} supportedFilters={supportedFilters} />) so the compiler verifies the shapes at the call site; if that surfaces an incompatibility, reconcile the types rather than casting.

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants