Skip to content

Conversation

@jack-gale-ethyca
Copy link
Contributor

@jack-gale-ethyca jack-gale-ethyca commented Jan 7, 2026

ENG-2303

Updates the privacy request error notification email template with new branding, improved copy, and enhanced UX in the notification configuration drawer.

Ticket ENG-2303

Description Of Changes

Email Template (privacy_request_error_notification.html)

  • Branding: Added Ethyca branded banner at the top with Minos background (#2b2e35) and Corinth logo (#fafafa)
  • Copy improvements:
    • Removed robotic language, made tone more conversational
    • Removed second sentence from intro paragraph
    • Removed red alert box section entirely
    • Removed error count from action paragraph (now focuses on next steps)
  • Styling: Updated to match other email templates with responsive design and dark mode support

Code Changes

Backend (message_dispatch_service.py)

  • Fixed bug where unsent_errors variable wasn't being passed to template
  • Added support for optional organization_name and company_logo_url variables

Frontend (ConfigureAlerts.tsx)

  • Refactored to Ant Design: Migrated from Chakra UI to Ant Design components for consistency
  • UX improvements:
    • Updated header to "DSR notifications"
    • Added description text at top (trimmed from original)
    • Moved placeholder text to InfoTooltip
    • Smaller toggle switch
    • Improved spacing and layout
    • Buttons moved to footer with proper spacing

Email Input Component (EmailChipList.tsx)

  • Refactored to work with new Ant Design form structure
  • Simplified props interface (no longer requires Formik FieldArray)
  • Uses default tag color for email chips

Steps to Confirm

  • Email template renders correctly with new styling
  • All form validation and submission flows work correctly
  • Email is readable when forwarded to Slack

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

ENG-2303

Updates the privacy request error notification email template with new branding, improved copy, and enhanced UX in the notification configuration drawer.
@jack-gale-ethyca jack-gale-ethyca requested review from a team as code owners January 7, 2026 19:18
@jack-gale-ethyca jack-gale-ethyca requested review from galvana and lucanovera and removed request for a team January 7, 2026 19:18
@vercel
Copy link

vercel bot commented Jan 7, 2026

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

Project Deployment Review Updated (UTC)
fides-plus-nightly Error Error Jan 7, 2026 7:28pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
fides-privacy-center Ignored Ignored Jan 7, 2026 7:28pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 7, 2026

Greptile Summary

This PR updates the DSR notification UX with improved branding and migrates the frontend from Chakra UI/Formik to Ant Design/React state.

Key changes:

  • Email template redesigned with Ethyca branding, responsive layout, and dark mode support
  • Backend now passes organization_name and company_logo_url to templates (previously missing unsent_errors)
  • Frontend refactored from Formik to React state management with Ant Design components

Critical issue:

  • The ConfigureAlerts drawer no longer loads existing notification settings from the API. When users open the drawer, they always see empty defaults instead of their current configuration. This breaks the edit flow.

Confidence Score: 3/5

  • Not safe to merge - breaks existing notification configuration editing
  • The PR contains a critical functional regression where users cannot view or edit their existing notification settings. The drawer always shows defaults instead of loading current values from the API.
  • ConfigureAlerts.tsx needs the data loading logic restored before merge

Important Files Changed

Filename Overview
clients/admin-ui/src/features/privacy-requests/drawers/ConfigureAlerts.tsx Migrated from Formik/Chakra to React state/Ant Design, but loses ability to load existing notification settings
src/fides/api/service/messaging/message_dispatch_service.py Added support for optional organization_name and company_logo_url variables, fixed unsent_errors bug
src/fides/api/email_templates/templates/privacy_request_error_notification.html Complete redesign with Ethyca branding, improved styling, responsive design, and dark mode support

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 66 to 74
useEffect(() => {
if (isOpen) {
setSkip(false);
// Reset to defaults when drawer opens
setEmails([]);
setNotify(false);
setMinErrorCount(DEFAULT_MIN_ERROR_COUNT);
}
if (data) {
setFormValues({
emails: data.email_addresses,
notify: data.notify_after_failures !== 0,
minErrorCount: data.notify_after_failures,
});
}
}, [data, isOpen]);
}, [isOpen]);
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Form doesn't load existing notification settings from API. The useEffect resets to defaults when drawer opens, but doesn't populate from data when it's available. This breaks the edit flow - users can't see/modify their current settings.

Previous code set form values from data:

if (data) {
  setFormValues({
    emails: data.email_addresses,
    notify: data.notify_after_failures !== 0,
    minErrorCount: data.notify_after_failures,
  });
}

Add similar logic to populate state from API response:

Suggested change
useEffect(() => {
if (isOpen) {
setSkip(false);
// Reset to defaults when drawer opens
setEmails([]);
setNotify(false);
setMinErrorCount(DEFAULT_MIN_ERROR_COUNT);
}
if (data) {
setFormValues({
emails: data.email_addresses,
notify: data.notify_after_failures !== 0,
minErrorCount: data.notify_after_failures,
});
}
}, [data, isOpen]);
}, [isOpen]);
useEffect(() => {
if (isOpen) {
setSkip(false);
}
if (data) {
setEmails(data.email_addresses);
setNotify(data.notify_after_failures !== 0);
setMinErrorCount(data.notify_after_failures || DEFAULT_MIN_ERROR_COUNT);
}
}, [data, isOpen]);

</FormLabel>
</Form.Item>

<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Using div instead of semantic HTML. Per custom rules (ecc7d51f), avoid div elements when possible.

Suggested change
<div>
<Form.Item label="Notification settings">

Context Used: Rule from dashboard - Avoid using div elements when possible. Use semantic HTML elements or component library alternativ... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

If you saved an email, then closed and reopened the tray, the email would disappear. this Fixes that.
@jack-gale-ethyca
Copy link
Contributor Author

@galvana There is technically a BE change here, cursor found a bug and updated it. If we don't want to include that we don't have to. LMK.

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