Skip to content

ENG-177: Replace useAlert with Ant Design useMessage#7697

Merged
gilluminate merged 8 commits intomainfrom
gill/ENG-177/remove-use-alert
Mar 19, 2026
Merged

ENG-177: Replace useAlert with Ant Design useMessage#7697
gilluminate merged 8 commits intomainfrom
gill/ENG-177/remove-use-alert

Conversation

@gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Mar 18, 2026

Ticket ENG-177

Description Of Changes

Replaces the Chakra-based useAlert hook with Ant Design's useMessage across the admin UI. useAPIHelper is preserved but updated to use useMessage internally instead of useAlert. This is PR 2 of 4 for the toast-to-Ant migration (ENG-177)

Code Changes

  • Deleted useAlert.tsx and useQueryResultToast.ts
  • Updated useAPIHelper to use useMessage from fidesui instead of useAlert
  • Migrated all direct useAlert consumers (~30 files) to useMessage for success/error/info toasts
  • Added cy.shouldShowMessage(type, text?) Cypress command in ant-support.ts for asserting Ant Design message toasts
  • Updated Cypress tests in assets-results.cy.ts to use the new helper

Steps to Confirm

  1. Trigger mutations (create, update, delete) across affected features and verify message toasts appear correctly
  2. Trigger API errors and verify error messages are displayed via useAPIHelper
  3. Run Cypress test suite for action center assets
  4. Verify messaging provider configuration flows have no regressions
  5. Verify privacy request configuration toasts work correctly

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Contributor

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Mar 19, 2026 3:14pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Mar 19, 2026 3:14pm

Request Review

gilluminate added a commit that referenced this pull request Mar 18, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gilluminate and others added 4 commits March 18, 2026 16:18
Migrate all consumers to use Ant Design's useMessage hook from fidesui.
Delete the old Chakra-based alert hooks and add a Cypress
shouldShowMessage helper for Ant Design message assertions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gilluminate gilluminate changed the title ENG-177: Remove useAlert, useAPIHelper, and useQueryResultToast hooks ENG-177: Replace useAlert with Ant Design useMessage Mar 18, 2026
@gilluminate gilluminate requested a review from kruulik March 18, 2026 23:48
@gilluminate gilluminate marked this pull request as ready for review March 18, 2026 23:49
@gilluminate gilluminate requested a review from a team as a code owner March 18, 2026 23:49
@gilluminate gilluminate requested review from speaker-ender and removed request for a team and speaker-ender March 18, 2026 23:49
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This PR migrates ~30 admin UI components from the Chakra-based useAlert hook to Ant Design's useMessage, deletes useAlert.tsx and useQueryResultToast.ts, updates useAPIHelper to use useMessage internally, and adds a shouldShowMessage Cypress helper. The migration is largely correct and consistent, but there are a few notable issues to address before merging.

  • Bug: useDSRErrorAlert.tsx — the unmount cleanup effect still calls Chakra's toast.close(), not Ant Design's message.destroy(). Because the persistent error message (duration: 0) is now an Ant Design message, it will never be dismissed on component unmount.
  • Information loss: Several error toasts that previously displayed a descriptive title alongside the error body now only show the body. Most notably, PrivacyRequestDuplicateDetectionSettings.tsx drops getErrorMessage(result.error) entirely from the displayed toast.
  • Residual duplication: useVerifyConfiguration.ts retains a local getErrorMessage helper that was not consolidated with useAPIHelper, unlike the other messaging files updated in this PR.
  • PR size: 46 files changed exceeds the project's 15-file change limit.

Confidence Score: 3/5

  • Safe to merge after fixing the useDSRErrorAlert cleanup bug and addressing the dropped error details in PrivacyRequestDuplicateDetectionSettings.
  • The majority of the migration is mechanical and correct. However, there is one clear functional regression: persistent Ant Design messages in useDSRErrorAlert won't be dismissed on unmount because the cleanup targets the wrong toast system. Additionally, actual API error details are silently dropped in PrivacyRequestDuplicateDetectionSettings, degrading the user's ability to diagnose failures.
  • clients/admin-ui/src/features/privacy-requests/hooks/useDSRErrorAlert.tsx (unmount cleanup targets wrong toast system) and clients/admin-ui/src/features/settings/PrivacyRequestDuplicateDetectionSettings.tsx (API error detail dropped from toast).

Important Files Changed

Filename Overview
clients/admin-ui/src/features/privacy-requests/hooks/useDSRErrorAlert.tsx Migrated from useAlert to useMessage, but the unmount cleanup effect still targets the Chakra toast system rather than the new Ant Design message, causing persistent (duration: 0) error messages to never be dismissed on unmount.
clients/admin-ui/src/features/common/hooks/useAPIHelper.ts Cleanly migrated from useAlert to useMessage; handleError logic preserved and now shows errors via Ant Design message API.
clients/admin-ui/cypress/support/ant-support.ts Added shouldShowMessage Cypress command targeting .ant-message-{type} selectors; straightforward addition with a sensible 10s timeout.
clients/admin-ui/src/features/messaging/useVerifyConfiguration.ts Retains a local getErrorMessage helper that duplicates the logic in useAPIHelper, which was not consolidated during this migration unlike the other messaging form files.
clients/admin-ui/src/features/settings/PrivacyRequestDuplicateDetectionSettings.tsx Migration drops the actual API error message (previously the title) from the error toast — users now only see the generic description without the underlying error detail.
clients/admin-ui/src/features/privacy-requests/buttons/ReprocessButton.tsx Migration removes the descriptive error titles ("DSR batch automation has failed due to the following:"), reducing error context shown to the user.
clients/admin-ui/src/features/privacy-requests/drawers/ConfigureAlerts.tsx Migration drops the "Failed to save notification settings" error title; core logic is otherwise correctly ported.
clients/admin-ui/src/features/integrations/configure-monitor/MonitorConfigActionsCell.tsx Migrated from useQueryResultToast to inline useMessage calls; error and success handling is preserved correctly.

Comments Outside Diff (3)

  1. clients/admin-ui/src/features/privacy-requests/configuration/GoogleCloudStorageConfiguration.tsx, line 1 (link)

    P2 PR exceeds 15-file change limit

    This PR changes 46 files, which is above the 15-file limit. Large PRs are harder to review thoroughly and increase the risk of unintended regressions. Consider splitting this migration into smaller, more targeted PRs — for example, grouping by feature area (privacy-requests, messaging, data-discovery, etc.).

    Rule Used: No PRs over 500 line changes OR 15 files changes s... (source)

  2. clients/admin-ui/src/features/messaging/useVerifyConfiguration.ts, line 22-30 (link)

    P2 Duplicated getErrorMessage logic still present

    This file still contains a local getErrorMessage helper that exactly mirrors the logic in useAPIHelper.handleError. SendTestMessageModal.tsx and TwilioSMSMessagingForm.tsx were correctly migrated in this PR to use useAPIHelper, but useVerifyConfiguration.ts was overlooked. Consider replacing the local helper with useAPIHelper().handleError for consistency and to avoid drift.

  3. clients/admin-ui/src/features/privacy-requests/hooks/useDSRErrorAlert.tsx, line 88-97 (link)

    P1 Cleanup effect targets wrong toast system

    The unmount cleanup still uses useChakraToast (toast.isActive(...) and toast.close(...)) to dismiss the notification, but the alert is now displayed via message.error({ key: ..., ... }) from Ant Design's useMessage. Since the Chakra toast and Ant Design message are entirely separate systems, the cleanup will never find a matching notification — the persistent error message (duration: 0) will remain on screen after the component unmounts.

    The fix is to call message.destroy(key) in the cleanup effect instead, and remove the now-unused useChakraToast import and toast variable.

Last reviewed commit: "Update 7697-remove-u..."

Copy link
Contributor

@kruulik kruulik left a comment

Choose a reason for hiding this comment

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

All very straightforward, looks good!

@gilluminate gilluminate added this pull request to the merge queue Mar 19, 2026
Merged via the queue into main with commit dbf202e Mar 19, 2026
45 of 46 checks passed
@gilluminate gilluminate deleted the gill/ENG-177/remove-use-alert branch March 19, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants