From 61b26441ab94a68a84ac6a563eeb88a88f3a7eba Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 14:41:54 -0600 Subject: [PATCH 01/11] Add inert hook skeletons for reply polling and notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create two new service packages following the established Jira polling pattern. Each provides a module-level register function, a Celery task with Redis lock, and APScheduler wiring — all no-ops until Fidesplus registers an implementation. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/api/main.py | 4 + src/fides/config/execution_settings.py | 8 ++ src/fides/service/correspondence/__init__.py | 6 + .../correspondence/reply_polling_task.py | 77 ++++++++++++ src/fides/service/notifications/__init__.py | 6 + .../notifications/notification_task.py | 75 ++++++++++++ tests/service/correspondence/__init__.py | 0 .../correspondence/test_reply_polling_task.py | 114 ++++++++++++++++++ tests/service/notifications/__init__.py | 0 .../notifications/test_notification_task.py | 114 ++++++++++++++++++ 10 files changed, 404 insertions(+) create mode 100644 src/fides/service/correspondence/__init__.py create mode 100644 src/fides/service/correspondence/reply_polling_task.py create mode 100644 src/fides/service/notifications/__init__.py create mode 100644 src/fides/service/notifications/notification_task.py create mode 100644 tests/service/correspondence/__init__.py create mode 100644 tests/service/correspondence/test_reply_polling_task.py create mode 100644 tests/service/notifications/__init__.py create mode 100644 tests/service/notifications/test_notification_task.py diff --git a/src/fides/api/main.py b/src/fides/api/main.py index c737124a122..4521773d0fc 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -62,7 +62,9 @@ from fides.api.util.rate_limit import safe_rate_limit_key from fides.cli.utils import FIDES_ASCII_ART from fides.config import CONFIG, check_required_webserver_config_values +from fides.service.correspondence.reply_polling_task import initiate_reply_polling from fides.service.jira.polling_task import initiate_jira_ticket_polling +from fides.service.notifications.notification_task import initiate_notification_task NEXT_JS_CATCH_ALL_SEGMENTS_RE = r"^\[{1,2}\.\.\.\w+\]{1,2}" # https://nextjs.org/docs/pages/building-your-application/routing/dynamic-routes#catch-all-segments # Turbopack (Next.js 16+) chunk filenames can embed ".." anywhere, e.g. @@ -107,6 +109,8 @@ async def lifespan(wrapped_app: FastAPI) -> AsyncGenerator[None, None]: initiate_interrupted_task_requeue_poll() initiate_polling_task_requeue() initiate_jira_ticket_polling() + initiate_reply_polling() + initiate_notification_task() initiate_bcrypt_migration_task() initiate_post_upgrade_index_creation() initiate_post_upgrade_backfill() diff --git a/src/fides/config/execution_settings.py b/src/fides/config/execution_settings.py index 7861a54e319..6a1846b04af 100644 --- a/src/fides/config/execution_settings.py +++ b/src/fides/config/execution_settings.py @@ -103,4 +103,12 @@ class ExecutionSettings(FidesSettings): default=3, description="Minutes between polling Jira for ticket status updates.", ) + reply_polling_interval_minutes: int = Field( + default=3, + description="Minutes between polling the IMAP mailbox for DSR reply messages.", + ) + notification_interval_minutes: int = Field( + default=5, + description="Minutes between processing pending DSR lifecycle notifications.", + ) model_config = SettingsConfigDict(env_prefix=ENV_PREFIX) diff --git a/src/fides/service/correspondence/__init__.py b/src/fides/service/correspondence/__init__.py new file mode 100644 index 00000000000..629cfc6ce9a --- /dev/null +++ b/src/fides/service/correspondence/__init__.py @@ -0,0 +1,6 @@ +"""Correspondence services for Fides OSS. + +This package contains the Celery task skeleton and scheduler wiring for +polling an IMAP mailbox for DSR reply messages. The actual polling +implementation is registered by Fidesplus. +""" diff --git a/src/fides/service/correspondence/reply_polling_task.py b/src/fides/service/correspondence/reply_polling_task.py new file mode 100644 index 00000000000..0b889dd1bd7 --- /dev/null +++ b/src/fides/service/correspondence/reply_polling_task.py @@ -0,0 +1,77 @@ +"""Celery task and scheduler wiring for reply mailbox polling. + +The ``poll_reply_mailbox`` task acquires a Redis lock, then delegates to +a registered polling service callable. Until Fidesplus registers an +implementation, the task is a no-op. + +The ``initiate_reply_polling`` function adds the task to the APScheduler +on application startup. +""" + +from collections.abc import Callable + +from loguru import logger +from sqlalchemy.orm import Session + +from fides.api.tasks import DatabaseTask, celery_app +from fides.api.tasks.scheduled.scheduler import scheduler +from fides.api.util.lock import redis_lock +from fides.config import CONFIG + +REPLY_POLLING_JOB = "reply_mailbox_polling" +REPLY_POLLING_LOCK = "reply_mailbox_polling_lock" +REPLY_POLLING_LOCK_TIMEOUT = 600 + +_service_fn: Callable[[Session], None] | None = None + + +def register_reply_poll_service(fn: Callable[[Session], None]) -> None: + """Register the actual polling implementation (called by Fidesplus).""" + global _service_fn # noqa: PLW0603 + _service_fn = fn + logger.info("Reply mailbox polling service registered") + + +@celery_app.task(base=DatabaseTask, bind=True) +def poll_reply_mailbox(self: DatabaseTask) -> None: + """Poll an IMAP mailbox for DSR reply messages. + + Acquires a Redis lock to prevent concurrent execution. Delegates to + the registered polling service; if none is registered the task is a + no-op. + """ + with redis_lock(REPLY_POLLING_LOCK, REPLY_POLLING_LOCK_TIMEOUT) as lock: + if not lock: + return + + if _service_fn is None: + logger.debug("Reply mailbox polling: no service registered, skipping") + return + + with self.get_new_session() as db: + _service_fn(db) + + +def initiate_reply_polling() -> None: + """Add the reply mailbox polling job to the APScheduler. + + Called during application startup from ``main.py``. Skipped in + test mode. + """ + if CONFIG.test_mode: + return + + if not scheduler.running: + raise RuntimeError( + "Scheduler is not running! Cannot add reply mailbox polling job." + ) + + logger.info("Initiating scheduler for reply mailbox polling") + scheduler.add_job( + func=poll_reply_mailbox.delay, + trigger="interval", + id=REPLY_POLLING_JOB, + coalesce=True, + replace_existing=True, + minutes=CONFIG.execution.reply_polling_interval_minutes, + ) diff --git a/src/fides/service/notifications/__init__.py b/src/fides/service/notifications/__init__.py new file mode 100644 index 00000000000..4bcbab374bc --- /dev/null +++ b/src/fides/service/notifications/__init__.py @@ -0,0 +1,6 @@ +"""Notification services for Fides OSS. + +This package contains the Celery task skeleton and scheduler wiring for +sending DSR lifecycle notifications. The actual notification +implementation is registered by Fidesplus. +""" diff --git a/src/fides/service/notifications/notification_task.py b/src/fides/service/notifications/notification_task.py new file mode 100644 index 00000000000..94683a7f061 --- /dev/null +++ b/src/fides/service/notifications/notification_task.py @@ -0,0 +1,75 @@ +"""Celery task and scheduler wiring for DSR lifecycle notifications. + +The ``send_notifications`` task acquires a Redis lock, then delegates to +a registered notification service callable. Until Fidesplus registers an +implementation, the task is a no-op. + +The ``initiate_notification_task`` function adds the task to the +APScheduler on application startup. +""" + +from collections.abc import Callable + +from loguru import logger +from sqlalchemy.orm import Session + +from fides.api.tasks import DatabaseTask, celery_app +from fides.api.tasks.scheduled.scheduler import scheduler +from fides.api.util.lock import redis_lock +from fides.config import CONFIG + +NOTIFICATION_JOB = "dsr_notifications" +NOTIFICATION_LOCK = "dsr_notifications_lock" +NOTIFICATION_LOCK_TIMEOUT = 600 + +_service_fn: Callable[[Session], None] | None = None + + +def register_notification_service(fn: Callable[[Session], None]) -> None: + """Register the actual notification implementation (called by Fidesplus).""" + global _service_fn # noqa: PLW0603 + _service_fn = fn + logger.info("DSR notification service registered") + + +@celery_app.task(base=DatabaseTask, bind=True) +def send_notifications(self: DatabaseTask) -> None: + """Process and send pending DSR lifecycle notifications. + + Acquires a Redis lock to prevent concurrent execution. Delegates to + the registered notification service; if none is registered the task is a + no-op. + """ + with redis_lock(NOTIFICATION_LOCK, NOTIFICATION_LOCK_TIMEOUT) as lock: + if not lock: + return + + if _service_fn is None: + logger.debug("DSR notifications: no service registered, skipping") + return + + with self.get_new_session() as db: + _service_fn(db) + + +def initiate_notification_task() -> None: + """Add the DSR notification job to the APScheduler. + + Called during application startup from ``main.py``. Skipped in + test mode. + """ + if CONFIG.test_mode: + return + + if not scheduler.running: + raise RuntimeError("Scheduler is not running! Cannot add DSR notification job.") + + logger.info("Initiating scheduler for DSR notifications") + scheduler.add_job( + func=send_notifications.delay, + trigger="interval", + id=NOTIFICATION_JOB, + coalesce=True, + replace_existing=True, + minutes=CONFIG.execution.notification_interval_minutes, + ) diff --git a/tests/service/correspondence/__init__.py b/tests/service/correspondence/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/service/correspondence/test_reply_polling_task.py b/tests/service/correspondence/test_reply_polling_task.py new file mode 100644 index 00000000000..ea7ea59548a --- /dev/null +++ b/tests/service/correspondence/test_reply_polling_task.py @@ -0,0 +1,114 @@ +"""Tests for the reply mailbox polling task skeleton.""" + +from contextlib import contextmanager +from unittest.mock import MagicMock, patch + +import pytest + +from fides.config import CONFIG +from fides.service.correspondence import reply_polling_task +from fides.service.correspondence.reply_polling_task import ( + REPLY_POLLING_JOB, + initiate_reply_polling, + poll_reply_mailbox, + register_reply_poll_service, +) + + +class TestRegisterReplyPollService: + def test_register_sets_service_fn(self, monkeypatch): + mock_fn = MagicMock() + monkeypatch.setattr(reply_polling_task, "_service_fn", None) + register_reply_poll_service(mock_fn) + assert reply_polling_task._service_fn is mock_fn + + +class TestPollReplyMailboxTask: + def test_no_op_when_no_service_registered(self, monkeypatch): + """Lock is acquired but no service fn is registered — task skips.""" + monkeypatch.setattr(reply_polling_task, "_service_fn", None) + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield MagicMock() # truthy lock + + with patch.object(reply_polling_task, "redis_lock", _fake_lock): + poll_reply_mailbox.apply().get() + + def test_delegates_to_registered_service(self, monkeypatch): + """Lock is acquired and a service fn is registered — task calls it with a DB session.""" + mock_service = MagicMock() + monkeypatch.setattr(reply_polling_task, "_service_fn", mock_service) + + mock_session = MagicMock() + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield MagicMock() + + @contextmanager + def _fake_get_new_session(_self): + yield mock_session + + with ( + patch.object(reply_polling_task, "redis_lock", _fake_lock), + patch( + "fides.service.correspondence.reply_polling_task.DatabaseTask.get_new_session", + _fake_get_new_session, + ), + ): + poll_reply_mailbox.apply().get() + + mock_service.assert_called_once_with(mock_session) + + def test_skips_when_lock_not_acquired(self, monkeypatch): + """Another worker holds the lock — task exits without calling the service.""" + mock_service = MagicMock() + monkeypatch.setattr(reply_polling_task, "_service_fn", mock_service) + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield None # lock not acquired + + with patch.object(reply_polling_task, "redis_lock", _fake_lock): + poll_reply_mailbox.apply().get() + + mock_service.assert_not_called() + + +class TestInitiateReplyPolling: + def test_skips_in_test_mode(self, monkeypatch): + monkeypatch.setattr(CONFIG, "test_mode", True) + mock_scheduler = MagicMock() + with patch.object(reply_polling_task, "scheduler", mock_scheduler): + initiate_reply_polling() + mock_scheduler.add_job.assert_not_called() + + def test_raises_when_scheduler_not_running(self, monkeypatch): + monkeypatch.setattr(CONFIG, "test_mode", False) + mock_scheduler = MagicMock() + mock_scheduler.running = False + with ( + patch.object(reply_polling_task, "scheduler", mock_scheduler), + pytest.raises(RuntimeError, match="Scheduler is not running"), + ): + initiate_reply_polling() + + def test_adds_scheduler_job(self, monkeypatch): + monkeypatch.setattr(CONFIG, "test_mode", False) + mock_scheduler = MagicMock() + mock_scheduler.running = True + with patch.object(reply_polling_task, "scheduler", mock_scheduler): + initiate_reply_polling() + + mock_scheduler.add_job.assert_called_once() + call_kwargs = mock_scheduler.add_job.call_args[1] + assert call_kwargs["id"] == REPLY_POLLING_JOB + assert call_kwargs["trigger"] == "interval" + assert call_kwargs["func"] == poll_reply_mailbox.delay + assert call_kwargs["minutes"] == CONFIG.execution.reply_polling_interval_minutes + + +class TestReplyPollingConfig: + def test_default_polling_interval(self): + assert CONFIG.execution.reply_polling_interval_minutes == 3 diff --git a/tests/service/notifications/__init__.py b/tests/service/notifications/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/service/notifications/test_notification_task.py b/tests/service/notifications/test_notification_task.py new file mode 100644 index 00000000000..a6f82895f4d --- /dev/null +++ b/tests/service/notifications/test_notification_task.py @@ -0,0 +1,114 @@ +"""Tests for the DSR notification task skeleton.""" + +from contextlib import contextmanager +from unittest.mock import MagicMock, patch + +import pytest + +from fides.config import CONFIG +from fides.service.notifications import notification_task +from fides.service.notifications.notification_task import ( + NOTIFICATION_JOB, + initiate_notification_task, + register_notification_service, + send_notifications, +) + + +class TestRegisterNotificationService: + def test_register_sets_service_fn(self, monkeypatch): + mock_fn = MagicMock() + monkeypatch.setattr(notification_task, "_service_fn", None) + register_notification_service(mock_fn) + assert notification_task._service_fn is mock_fn + + +class TestSendNotificationsTask: + def test_no_op_when_no_service_registered(self, monkeypatch): + """Lock is acquired but no service fn is registered — task skips.""" + monkeypatch.setattr(notification_task, "_service_fn", None) + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield MagicMock() # truthy lock + + with patch.object(notification_task, "redis_lock", _fake_lock): + send_notifications.apply().get() + + def test_delegates_to_registered_service(self, monkeypatch): + """Lock is acquired and a service fn is registered — task calls it with a DB session.""" + mock_service = MagicMock() + monkeypatch.setattr(notification_task, "_service_fn", mock_service) + + mock_session = MagicMock() + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield MagicMock() + + @contextmanager + def _fake_get_new_session(_self): + yield mock_session + + with ( + patch.object(notification_task, "redis_lock", _fake_lock), + patch( + "fides.service.notifications.notification_task.DatabaseTask.get_new_session", + _fake_get_new_session, + ), + ): + send_notifications.apply().get() + + mock_service.assert_called_once_with(mock_session) + + def test_skips_when_lock_not_acquired(self, monkeypatch): + """Another worker holds the lock — task exits without calling the service.""" + mock_service = MagicMock() + monkeypatch.setattr(notification_task, "_service_fn", mock_service) + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield None # lock not acquired + + with patch.object(notification_task, "redis_lock", _fake_lock): + send_notifications.apply().get() + + mock_service.assert_not_called() + + +class TestInitiateNotificationTask: + def test_skips_in_test_mode(self, monkeypatch): + monkeypatch.setattr(CONFIG, "test_mode", True) + mock_scheduler = MagicMock() + with patch.object(notification_task, "scheduler", mock_scheduler): + initiate_notification_task() + mock_scheduler.add_job.assert_not_called() + + def test_raises_when_scheduler_not_running(self, monkeypatch): + monkeypatch.setattr(CONFIG, "test_mode", False) + mock_scheduler = MagicMock() + mock_scheduler.running = False + with ( + patch.object(notification_task, "scheduler", mock_scheduler), + pytest.raises(RuntimeError, match="Scheduler is not running"), + ): + initiate_notification_task() + + def test_adds_scheduler_job(self, monkeypatch): + monkeypatch.setattr(CONFIG, "test_mode", False) + mock_scheduler = MagicMock() + mock_scheduler.running = True + with patch.object(notification_task, "scheduler", mock_scheduler): + initiate_notification_task() + + mock_scheduler.add_job.assert_called_once() + call_kwargs = mock_scheduler.add_job.call_args[1] + assert call_kwargs["id"] == NOTIFICATION_JOB + assert call_kwargs["trigger"] == "interval" + assert call_kwargs["func"] == send_notifications.delay + assert call_kwargs["minutes"] == CONFIG.execution.notification_interval_minutes + + +class TestNotificationConfig: + def test_default_notification_interval(self): + assert CONFIG.execution.notification_interval_minutes == 5 From 5e3fe1c61f8a1ebe2e06e7cf0c21dc630e988c1f Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 14:42:18 -0600 Subject: [PATCH 02/11] Add changelog for ENG-3302 Co-Authored-By: Claude Opus 4.6 (1M context) --- changelog/8158-hook-skeleton-polling-notifications.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/8158-hook-skeleton-polling-notifications.yaml diff --git a/changelog/8158-hook-skeleton-polling-notifications.yaml b/changelog/8158-hook-skeleton-polling-notifications.yaml new file mode 100644 index 00000000000..9f3885e8b75 --- /dev/null +++ b/changelog/8158-hook-skeleton-polling-notifications.yaml @@ -0,0 +1,8 @@ +--- +ticket: ENG-3302 +type: added +message: >- + Added inert hook skeletons for reply mailbox polling and DSR + notifications. These Celery tasks are no-ops until Fidesplus + registers an implementation. +pr: 8158 From ead0c17188a7f8cf2dbce6f3f0fb35036b13d938 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 14:49:28 -0600 Subject: [PATCH 03/11] Fix changelog format to match template Co-Authored-By: Claude Opus 4.6 (1M context) --- .../8158-hook-skeleton-polling-notifications.yaml | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/changelog/8158-hook-skeleton-polling-notifications.yaml b/changelog/8158-hook-skeleton-polling-notifications.yaml index 9f3885e8b75..bc25fb893b4 100644 --- a/changelog/8158-hook-skeleton-polling-notifications.yaml +++ b/changelog/8158-hook-skeleton-polling-notifications.yaml @@ -1,8 +1,4 @@ ---- -ticket: ENG-3302 -type: added -message: >- - Added inert hook skeletons for reply mailbox polling and DSR - notifications. These Celery tasks are no-ops until Fidesplus - registers an implementation. +type: Added +description: Added inert hook skeletons for reply mailbox polling and DSR notifications pr: 8158 +labels: [] From 957fcf9eaf16493825529292483153c76569bc3f Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 15:04:08 -0600 Subject: [PATCH 04/11] Refactor notification hook to Option C: event-driven + sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notify task is now the primary delivery path — called directly by DSR lifecycle code with a privacy_request_id and event_type. The sweep_notifications task runs on a scheduled interval as a catch-all for missed or failed notifications. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/service/notifications/__init__.py | 12 ++- .../notifications/notification_task.py | 94 ++++++++++++---- .../notifications/test_notification_task.py | 102 +++++++++++++----- 3 files changed, 157 insertions(+), 51 deletions(-) diff --git a/src/fides/service/notifications/__init__.py b/src/fides/service/notifications/__init__.py index 4bcbab374bc..3988bf58716 100644 --- a/src/fides/service/notifications/__init__.py +++ b/src/fides/service/notifications/__init__.py @@ -1,6 +1,12 @@ """Notification services for Fides OSS. -This package contains the Celery task skeleton and scheduler wiring for -sending DSR lifecycle notifications. The actual notification -implementation is registered by Fidesplus. +This package provides two Celery tasks for DSR lifecycle notifications: + +1. ``notify`` — event-driven task called directly when a DSR state change + occurs. This is the primary delivery path for immediate notifications. + +2. ``sweep_notifications`` — scheduled task that runs on an interval to + catch any notifications that were missed or failed on the primary path. + +Both are no-ops until Fidesplus registers implementations. """ diff --git a/src/fides/service/notifications/notification_task.py b/src/fides/service/notifications/notification_task.py index 94683a7f061..8043dcb8164 100644 --- a/src/fides/service/notifications/notification_task.py +++ b/src/fides/service/notifications/notification_task.py @@ -1,10 +1,18 @@ -"""Celery task and scheduler wiring for DSR lifecycle notifications. +"""Celery tasks for DSR lifecycle notifications. -The ``send_notifications`` task acquires a Redis lock, then delegates to -a registered notification service callable. Until Fidesplus registers an -implementation, the task is a no-op. +Two delivery paths (Option C — hybrid): -The ``initiate_notification_task`` function adds the task to the +1. **Event-driven (primary):** The ``notify`` task is called directly by + DSR lifecycle code when a state change occurs (e.g., request completed). + It delivers the notification immediately via the registered handler. + +2. **Sweep (secondary):** The ``sweep_notifications`` task runs on a + scheduled interval via APScheduler. It catches any notifications that + were missed or failed on the primary path. + +Both paths are no-ops until Fidesplus registers implementations. + +The ``initiate_notification_task`` function adds the sweep job to the APScheduler on application startup. """ @@ -22,38 +30,76 @@ NOTIFICATION_LOCK = "dsr_notifications_lock" NOTIFICATION_LOCK_TIMEOUT = 600 -_service_fn: Callable[[Session], None] | None = None +_sweep_fn: Callable[[Session], None] | None = None +_notify_fn: Callable[[Session, str, str], None] | None = None + + +def register_notification_sweep(fn: Callable[[Session], None]) -> None: + """Register the sweep implementation (called by Fidesplus). + + The sweep function receives a DB session and should query for any + pending unsent notifications and process them. + """ + global _sweep_fn # noqa: PLW0603 + _sweep_fn = fn + logger.info("DSR notification sweep service registered") + +def register_notification_handler(fn: Callable[[Session, str, str], None]) -> None: + """Register the event-driven notification handler (called by Fidesplus). + + The handler receives a DB session, a privacy_request_id, and an + event_type string (e.g. "request_completed", "request_approved"). + """ + global _notify_fn # noqa: PLW0603 + _notify_fn = fn + logger.info("DSR notification handler registered") + + +@celery_app.task(base=DatabaseTask, bind=True) +def notify(self: DatabaseTask, privacy_request_id: str, event_type: str) -> None: + """Send a notification for a specific DSR lifecycle event. + + Called directly by DSR lifecycle code (e.g. after a request is + completed). Delegates to the registered handler; if none is + registered the task is a no-op. + """ + if _notify_fn is None: + logger.debug( + "DSR notification handler not registered, skipping notify " + "for request={} event={}", + privacy_request_id, + event_type, + ) + return -def register_notification_service(fn: Callable[[Session], None]) -> None: - """Register the actual notification implementation (called by Fidesplus).""" - global _service_fn # noqa: PLW0603 - _service_fn = fn - logger.info("DSR notification service registered") + with self.get_new_session() as db: + _notify_fn(db, privacy_request_id, event_type) @celery_app.task(base=DatabaseTask, bind=True) -def send_notifications(self: DatabaseTask) -> None: - """Process and send pending DSR lifecycle notifications. +def sweep_notifications(self: DatabaseTask) -> None: + """Sweep for pending unsent notifications and process them. - Acquires a Redis lock to prevent concurrent execution. Delegates to - the registered notification service; if none is registered the task is a - no-op. + Runs on a scheduled interval as a catch-all for notifications that + failed or were missed on the event-driven path. Acquires a Redis + lock to prevent concurrent execution. Delegates to the registered + sweep function; if none is registered the task is a no-op. """ with redis_lock(NOTIFICATION_LOCK, NOTIFICATION_LOCK_TIMEOUT) as lock: if not lock: return - if _service_fn is None: - logger.debug("DSR notifications: no service registered, skipping") + if _sweep_fn is None: + logger.debug("DSR notification sweep: no service registered, skipping") return with self.get_new_session() as db: - _service_fn(db) + _sweep_fn(db) def initiate_notification_task() -> None: - """Add the DSR notification job to the APScheduler. + """Add the DSR notification sweep job to the APScheduler. Called during application startup from ``main.py``. Skipped in test mode. @@ -62,11 +108,13 @@ def initiate_notification_task() -> None: return if not scheduler.running: - raise RuntimeError("Scheduler is not running! Cannot add DSR notification job.") + raise RuntimeError( + "Scheduler is not running! Cannot add DSR notification sweep job." + ) - logger.info("Initiating scheduler for DSR notifications") + logger.info("Initiating scheduler for DSR notification sweep") scheduler.add_job( - func=send_notifications.delay, + func=sweep_notifications.delay, trigger="interval", id=NOTIFICATION_JOB, coalesce=True, diff --git a/tests/service/notifications/test_notification_task.py b/tests/service/notifications/test_notification_task.py index a6f82895f4d..7744cefdb82 100644 --- a/tests/service/notifications/test_notification_task.py +++ b/tests/service/notifications/test_notification_task.py @@ -1,4 +1,4 @@ -"""Tests for the DSR notification task skeleton.""" +"""Tests for the DSR notification task skeleton (Option C: event-driven + sweep).""" from contextlib import contextmanager from unittest.mock import MagicMock, patch @@ -10,35 +10,81 @@ from fides.service.notifications.notification_task import ( NOTIFICATION_JOB, initiate_notification_task, - register_notification_service, - send_notifications, + notify, + register_notification_handler, + register_notification_sweep, + sweep_notifications, ) +# ── Registration ───────────────────────────────────────────────────── -class TestRegisterNotificationService: - def test_register_sets_service_fn(self, monkeypatch): + +class TestRegisterNotificationSweep: + def test_register_sets_sweep_fn(self, monkeypatch): + mock_fn = MagicMock() + monkeypatch.setattr(notification_task, "_sweep_fn", None) + register_notification_sweep(mock_fn) + assert notification_task._sweep_fn is mock_fn + + +class TestRegisterNotificationHandler: + def test_register_sets_notify_fn(self, monkeypatch): mock_fn = MagicMock() - monkeypatch.setattr(notification_task, "_service_fn", None) - register_notification_service(mock_fn) - assert notification_task._service_fn is mock_fn + monkeypatch.setattr(notification_task, "_notify_fn", None) + register_notification_handler(mock_fn) + assert notification_task._notify_fn is mock_fn + + +# ── Event-driven notify task ───────────────────────────────────────── + + +class TestNotifyTask: + def test_no_op_when_no_handler_registered(self, monkeypatch): + """No handler registered — task skips without touching the DB.""" + monkeypatch.setattr(notification_task, "_notify_fn", None) + notify.apply(args=["req-123", "request_completed"]).get() + + def test_delegates_to_registered_handler(self, monkeypatch): + """Handler registered — task calls it with session, request ID, and event type.""" + mock_handler = MagicMock() + monkeypatch.setattr(notification_task, "_notify_fn", mock_handler) + + mock_session = MagicMock() + + @contextmanager + def _fake_get_new_session(_self): + yield mock_session + + with patch( + "fides.service.notifications.notification_task.DatabaseTask.get_new_session", + _fake_get_new_session, + ): + notify.apply(args=["req-123", "request_completed"]).get() + + mock_handler.assert_called_once_with( + mock_session, "req-123", "request_completed" + ) -class TestSendNotificationsTask: - def test_no_op_when_no_service_registered(self, monkeypatch): - """Lock is acquired but no service fn is registered — task skips.""" - monkeypatch.setattr(notification_task, "_service_fn", None) +# ── Sweep task ─────────────────────────────────────────────────────── + + +class TestSweepNotificationsTask: + def test_no_op_when_no_sweep_registered(self, monkeypatch): + """Lock is acquired but no sweep fn is registered — task skips.""" + monkeypatch.setattr(notification_task, "_sweep_fn", None) @contextmanager def _fake_lock(*_args, **_kwargs): yield MagicMock() # truthy lock with patch.object(notification_task, "redis_lock", _fake_lock): - send_notifications.apply().get() + sweep_notifications.apply().get() - def test_delegates_to_registered_service(self, monkeypatch): - """Lock is acquired and a service fn is registered — task calls it with a DB session.""" - mock_service = MagicMock() - monkeypatch.setattr(notification_task, "_service_fn", mock_service) + def test_delegates_to_registered_sweep(self, monkeypatch): + """Lock is acquired and a sweep fn is registered — task calls it with a DB session.""" + mock_sweep = MagicMock() + monkeypatch.setattr(notification_task, "_sweep_fn", mock_sweep) mock_session = MagicMock() @@ -57,23 +103,26 @@ def _fake_get_new_session(_self): _fake_get_new_session, ), ): - send_notifications.apply().get() + sweep_notifications.apply().get() - mock_service.assert_called_once_with(mock_session) + mock_sweep.assert_called_once_with(mock_session) def test_skips_when_lock_not_acquired(self, monkeypatch): - """Another worker holds the lock — task exits without calling the service.""" - mock_service = MagicMock() - monkeypatch.setattr(notification_task, "_service_fn", mock_service) + """Another worker holds the lock — task exits without calling the sweep.""" + mock_sweep = MagicMock() + monkeypatch.setattr(notification_task, "_sweep_fn", mock_sweep) @contextmanager def _fake_lock(*_args, **_kwargs): yield None # lock not acquired with patch.object(notification_task, "redis_lock", _fake_lock): - send_notifications.apply().get() + sweep_notifications.apply().get() + + mock_sweep.assert_not_called() - mock_service.assert_not_called() + +# ── Scheduler wiring ───────────────────────────────────────────────── class TestInitiateNotificationTask: @@ -105,10 +154,13 @@ def test_adds_scheduler_job(self, monkeypatch): call_kwargs = mock_scheduler.add_job.call_args[1] assert call_kwargs["id"] == NOTIFICATION_JOB assert call_kwargs["trigger"] == "interval" - assert call_kwargs["func"] == send_notifications.delay + assert call_kwargs["func"] == sweep_notifications.delay assert call_kwargs["minutes"] == CONFIG.execution.notification_interval_minutes +# ── Config ─────────────────────────────────────────────────────────── + + class TestNotificationConfig: def test_default_notification_interval(self): assert CONFIG.execution.notification_interval_minutes == 5 From 2bb61578e4ead58620d674e8ecae01a7390ba218 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 15:32:40 -0600 Subject: [PATCH 05/11] Address PR review feedback - Add "set once at startup" comments on module-level hook variables - Add exception propagation tests for all registered service functions - Clarify relationship to messaging/ package in __init__.py docstrings Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/service/correspondence/__init__.py | 6 +++ .../correspondence/reply_polling_task.py | 2 + src/fides/service/notifications/__init__.py | 4 ++ .../notifications/notification_task.py | 2 + .../correspondence/test_reply_polling_task.py | 28 ++++++++++ .../notifications/test_notification_task.py | 51 +++++++++++++++++++ 6 files changed, 93 insertions(+) diff --git a/src/fides/service/correspondence/__init__.py b/src/fides/service/correspondence/__init__.py index 629cfc6ce9a..89c6af4c9e9 100644 --- a/src/fides/service/correspondence/__init__.py +++ b/src/fides/service/correspondence/__init__.py @@ -3,4 +3,10 @@ This package contains the Celery task skeleton and scheduler wiring for polling an IMAP mailbox for DSR reply messages. The actual polling implementation is registered by Fidesplus. + +Note: This is a domain feature (two-way communication with data subjects), +not a transport layer. Correspondence *uses* the ``messaging`` package +to deliver emails but is separate from it — ``messaging`` handles how to +send; ``correspondence`` handles what to send, threading, and inbound +reply processing. """ diff --git a/src/fides/service/correspondence/reply_polling_task.py b/src/fides/service/correspondence/reply_polling_task.py index 0b889dd1bd7..4bf6cdf62ab 100644 --- a/src/fides/service/correspondence/reply_polling_task.py +++ b/src/fides/service/correspondence/reply_polling_task.py @@ -22,6 +22,8 @@ REPLY_POLLING_LOCK = "reply_mailbox_polling_lock" REPLY_POLLING_LOCK_TIMEOUT = 600 +# Set once at startup by Fidesplus via register_reply_poll_service(); +# only read thereafter by Celery workers. Safe under CPython's GIL. _service_fn: Callable[[Session], None] | None = None diff --git a/src/fides/service/notifications/__init__.py b/src/fides/service/notifications/__init__.py index 3988bf58716..2dbc84db984 100644 --- a/src/fides/service/notifications/__init__.py +++ b/src/fides/service/notifications/__init__.py @@ -9,4 +9,8 @@ catch any notifications that were missed or failed on the primary path. Both are no-ops until Fidesplus registers implementations. + +Note: This is distinct from the ``messaging`` package, which is a +transport layer (email/SMS delivery via SES, Twilio, etc.). This package +handles *when* and *why* to notify; ``messaging`` handles *how*. """ diff --git a/src/fides/service/notifications/notification_task.py b/src/fides/service/notifications/notification_task.py index 8043dcb8164..a3a77ae1f40 100644 --- a/src/fides/service/notifications/notification_task.py +++ b/src/fides/service/notifications/notification_task.py @@ -30,6 +30,8 @@ NOTIFICATION_LOCK = "dsr_notifications_lock" NOTIFICATION_LOCK_TIMEOUT = 600 +# Set once at startup by Fidesplus via the register_* functions; +# only read thereafter by Celery workers. Safe under CPython's GIL. _sweep_fn: Callable[[Session], None] | None = None _notify_fn: Callable[[Session, str, str], None] | None = None diff --git a/tests/service/correspondence/test_reply_polling_task.py b/tests/service/correspondence/test_reply_polling_task.py index ea7ea59548a..a444982b7f6 100644 --- a/tests/service/correspondence/test_reply_polling_task.py +++ b/tests/service/correspondence/test_reply_polling_task.py @@ -75,6 +75,34 @@ def _fake_lock(*_args, **_kwargs): mock_service.assert_not_called() + def test_service_exception_propagates(self, monkeypatch): + """Exception from the registered service fn propagates as a Celery task failure.""" + monkeypatch.setattr( + reply_polling_task, + "_service_fn", + MagicMock(side_effect=RuntimeError("boom")), + ) + + mock_session = MagicMock() + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield MagicMock() + + @contextmanager + def _fake_get_new_session(_self): + yield mock_session + + with ( + patch.object(reply_polling_task, "redis_lock", _fake_lock), + patch( + "fides.service.correspondence.reply_polling_task.DatabaseTask.get_new_session", + _fake_get_new_session, + ), + pytest.raises(RuntimeError, match="boom"), + ): + poll_reply_mailbox.apply().get() + class TestInitiateReplyPolling: def test_skips_in_test_mode(self, monkeypatch): diff --git a/tests/service/notifications/test_notification_task.py b/tests/service/notifications/test_notification_task.py index 7744cefdb82..60ab9942599 100644 --- a/tests/service/notifications/test_notification_task.py +++ b/tests/service/notifications/test_notification_task.py @@ -65,6 +65,29 @@ def _fake_get_new_session(_self): mock_session, "req-123", "request_completed" ) + def test_handler_exception_propagates(self, monkeypatch): + """Exception from the registered handler propagates as a Celery task failure.""" + monkeypatch.setattr( + notification_task, + "_notify_fn", + MagicMock(side_effect=RuntimeError("boom")), + ) + + mock_session = MagicMock() + + @contextmanager + def _fake_get_new_session(_self): + yield mock_session + + with ( + patch( + "fides.service.notifications.notification_task.DatabaseTask.get_new_session", + _fake_get_new_session, + ), + pytest.raises(RuntimeError, match="boom"), + ): + notify.apply(args=["req-123", "request_completed"]).get() + # ── Sweep task ─────────────────────────────────────────────────────── @@ -121,6 +144,34 @@ def _fake_lock(*_args, **_kwargs): mock_sweep.assert_not_called() + def test_sweep_exception_propagates(self, monkeypatch): + """Exception from the registered sweep fn propagates as a Celery task failure.""" + monkeypatch.setattr( + notification_task, + "_sweep_fn", + MagicMock(side_effect=RuntimeError("boom")), + ) + + mock_session = MagicMock() + + @contextmanager + def _fake_lock(*_args, **_kwargs): + yield MagicMock() + + @contextmanager + def _fake_get_new_session(_self): + yield mock_session + + with ( + patch.object(notification_task, "redis_lock", _fake_lock), + patch( + "fides.service.notifications.notification_task.DatabaseTask.get_new_session", + _fake_get_new_session, + ), + pytest.raises(RuntimeError, match="boom"), + ): + sweep_notifications.apply().get() + # ── Scheduler wiring ───────────────────────────────────────────────── From 8e2ab834a9cfd1e84bebfd6598e5fe89465a6866 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 16:08:05 -0600 Subject: [PATCH 06/11] Address round 2 PR review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename notify → send_dsr_notification for clarity in Celery logs/queues - Document fork vs thread propagation constraint on hook registration - Document why send_dsr_notification intentionally has no Redis lock Co-Authored-By: Claude Opus 4.6 (1M context) --- .../service/correspondence/reply_polling_task.py | 5 ++++- src/fides/service/notifications/notification_task.py | 11 +++++++++-- tests/service/notifications/test_notification_task.py | 10 +++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/fides/service/correspondence/reply_polling_task.py b/src/fides/service/correspondence/reply_polling_task.py index 4bf6cdf62ab..744f42ff1a5 100644 --- a/src/fides/service/correspondence/reply_polling_task.py +++ b/src/fides/service/correspondence/reply_polling_task.py @@ -23,7 +23,10 @@ REPLY_POLLING_LOCK_TIMEOUT = 600 # Set once at startup by Fidesplus via register_reply_poll_service(); -# only read thereafter by Celery workers. Safe under CPython's GIL. +# only read thereafter by Celery workers. Safe under CPython's GIL +# for threaded workers. For forked workers (default), registration +# MUST occur at module import time (before fork) — see +# reply_polling_registration.py in fidesplus. _service_fn: Callable[[Session], None] | None = None diff --git a/src/fides/service/notifications/notification_task.py b/src/fides/service/notifications/notification_task.py index a3a77ae1f40..95e79a04cd8 100644 --- a/src/fides/service/notifications/notification_task.py +++ b/src/fides/service/notifications/notification_task.py @@ -31,7 +31,10 @@ NOTIFICATION_LOCK_TIMEOUT = 600 # Set once at startup by Fidesplus via the register_* functions; -# only read thereafter by Celery workers. Safe under CPython's GIL. +# only read thereafter by Celery workers. Safe under CPython's GIL +# for threaded workers. For forked workers (default), registration +# MUST occur at module import time (before fork) — see registration.py +# in fidesplus. _sweep_fn: Callable[[Session], None] | None = None _notify_fn: Callable[[Session, str, str], None] | None = None @@ -58,8 +61,12 @@ def register_notification_handler(fn: Callable[[Session, str, str], None]) -> No logger.info("DSR notification handler registered") +# No lock: concurrent execution is expected — each invocation targets a +# distinct privacy_request_id and these can legitimately run in parallel. @celery_app.task(base=DatabaseTask, bind=True) -def notify(self: DatabaseTask, privacy_request_id: str, event_type: str) -> None: +def send_dsr_notification( + self: DatabaseTask, privacy_request_id: str, event_type: str +) -> None: """Send a notification for a specific DSR lifecycle event. Called directly by DSR lifecycle code (e.g. after a request is diff --git a/tests/service/notifications/test_notification_task.py b/tests/service/notifications/test_notification_task.py index 60ab9942599..38ad85ef553 100644 --- a/tests/service/notifications/test_notification_task.py +++ b/tests/service/notifications/test_notification_task.py @@ -10,9 +10,9 @@ from fides.service.notifications.notification_task import ( NOTIFICATION_JOB, initiate_notification_task, - notify, register_notification_handler, register_notification_sweep, + send_dsr_notification, sweep_notifications, ) @@ -38,11 +38,11 @@ def test_register_sets_notify_fn(self, monkeypatch): # ── Event-driven notify task ───────────────────────────────────────── -class TestNotifyTask: +class TestSendDsrNotificationTask: def test_no_op_when_no_handler_registered(self, monkeypatch): """No handler registered — task skips without touching the DB.""" monkeypatch.setattr(notification_task, "_notify_fn", None) - notify.apply(args=["req-123", "request_completed"]).get() + send_dsr_notification.apply(args=["req-123", "request_completed"]).get() def test_delegates_to_registered_handler(self, monkeypatch): """Handler registered — task calls it with session, request ID, and event type.""" @@ -59,7 +59,7 @@ def _fake_get_new_session(_self): "fides.service.notifications.notification_task.DatabaseTask.get_new_session", _fake_get_new_session, ): - notify.apply(args=["req-123", "request_completed"]).get() + send_dsr_notification.apply(args=["req-123", "request_completed"]).get() mock_handler.assert_called_once_with( mock_session, "req-123", "request_completed" @@ -86,7 +86,7 @@ def _fake_get_new_session(_self): ), pytest.raises(RuntimeError, match="boom"), ): - notify.apply(args=["req-123", "request_completed"]).get() + send_dsr_notification.apply(args=["req-123", "request_completed"]).get() # ── Sweep task ─────────────────────────────────────────────────────── From 77fd8193c95f5c4bc1e5d1dd6845a0ee22ebebc9 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 16:31:03 -0600 Subject: [PATCH 07/11] ENG-3303: Generate TS types for correspondence API contract Add frontend type definitions for the correspondence and notification schemas so FE development can start in parallel with MSW mocks. New types: CorrespondenceDeliveryStatus, NotificationResponse, UnreadCountResponse, Page_NotificationResponse_ Updated: CommentResponse (6 new optional fields), index.ts exports Co-Authored-By: Claude Opus 4.6 (1M context) --- clients/admin-ui/src/types/api/index.ts | 4 ++ .../src/types/api/models/CommentResponse.ts | 35 ++++++++++++ .../models/CorrespondenceDeliveryStatus.ts | 14 +++++ .../types/api/models/NotificationResponse.ts | 57 +++++++++++++++++++ .../api/models/Page_NotificationResponse_.ts | 29 ++++++++++ .../types/api/models/UnreadCountResponse.ts | 15 +++++ 6 files changed, 154 insertions(+) create mode 100644 clients/admin-ui/src/types/api/models/CorrespondenceDeliveryStatus.ts create mode 100644 clients/admin-ui/src/types/api/models/NotificationResponse.ts create mode 100644 clients/admin-ui/src/types/api/models/Page_NotificationResponse_.ts create mode 100644 clients/admin-ui/src/types/api/models/UnreadCountResponse.ts diff --git a/clients/admin-ui/src/types/api/index.ts b/clients/admin-ui/src/types/api/index.ts index 365a6fdf93f..96fb06f66f1 100644 --- a/clients/admin-ui/src/types/api/index.ts +++ b/clients/admin-ui/src/types/api/index.ts @@ -155,6 +155,7 @@ export * from "./models/ColumnSort"; export type * from "./models/CommentResponse"; export * from "./models/CommentType"; export * from "./models/ComponentType"; +export * from "./models/CorrespondenceDeliveryStatus"; export type * from "./models/ConditionGroup"; export type * from "./models/ConditionLeaf"; export type * from "./models/ConditionalTotalCursorPage_DatastoreStagedResourceTreeAPIResponse_"; @@ -550,6 +551,7 @@ export type * from "./models/NoticeTranslation"; export type * from "./models/NoticeTranslationCreate"; export type * from "./models/NoticeTranslationResponse"; export type * from "./models/NotificationApplicationConfig"; +export type * from "./models/NotificationResponse"; export type * from "./models/OAuthConfigSchema"; export * from "./models/OAuthGrantType"; export type * from "./models/OktaConfig"; @@ -603,6 +605,7 @@ export type * from "./models/Page_MonitorConfigStagedResourcesAggregateRecord_"; export type * from "./models/Page_MonitorExecution_"; export type * from "./models/Page_MonitorStatusResponse_"; export type * from "./models/Page_MonitorTaskResponse_"; +export type * from "./models/Page_NotificationResponse_"; export type * from "./models/Page_PolicyResponse_"; export type * from "./models/Page_PolicyWebhookResponse_"; export type * from "./models/Page_PreApprovalWebhookResponse_"; @@ -924,6 +927,7 @@ export type * from "./models/TrendMetric"; export * from "./models/TrendPeriod"; export type * from "./models/TrendsResponse"; export type * from "./models/UnlabeledIdentities"; +export type * from "./models/UnreadCountResponse"; export type * from "./models/UnverifiedIdentity"; export type * from "./models/UnverifiedPrivacyPreferencesRequest"; export type * from "./models/UpdateAnswerRequest"; diff --git a/clients/admin-ui/src/types/api/models/CommentResponse.ts b/clients/admin-ui/src/types/api/models/CommentResponse.ts index 2646b64f4be..12c24cb07c5 100644 --- a/clients/admin-ui/src/types/api/models/CommentResponse.ts +++ b/clients/admin-ui/src/types/api/models/CommentResponse.ts @@ -2,6 +2,7 @@ import type { AttachmentResponse } from "./AttachmentResponse"; import { CommentType } from "./CommentType"; +import { CorrespondenceDeliveryStatus } from "./CorrespondenceDeliveryStatus"; /** * CommentResponse @@ -67,4 +68,38 @@ export type CommentResponse = { * The comment type */ comment_type: CommentType; + /** + * Delivery status for correspondence messages + */ + delivery_status?: CorrespondenceDeliveryStatus | null; + /** + * Parent Id + * + * Parent comment ID for threaded replies + */ + parent_id?: string | null; + /** + * Sender Email + * + * Sender email address for correspondence messages + */ + sender_email?: string | null; + /** + * Recipient Email + * + * Recipient email address for correspondence messages + */ + recipient_email?: string | null; + /** + * Bounce Reason + * + * Bounce reason if delivery failed + */ + bounce_reason?: string | null; + /** + * Reply To Address + * + * Reply-to address for correspondence threading + */ + reply_to_address?: string | null; }; diff --git a/clients/admin-ui/src/types/api/models/CorrespondenceDeliveryStatus.ts b/clients/admin-ui/src/types/api/models/CorrespondenceDeliveryStatus.ts new file mode 100644 index 00000000000..10a9369aaec --- /dev/null +++ b/clients/admin-ui/src/types/api/models/CorrespondenceDeliveryStatus.ts @@ -0,0 +1,14 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * CorrespondenceDeliveryStatus + * + * Delivery status for correspondence messages. + */ +export enum CorrespondenceDeliveryStatus { + PENDING = "pending", + SENT = "sent", + DELIVERED = "delivered", + BOUNCED = "bounced", + FAILED = "failed", +} diff --git a/clients/admin-ui/src/types/api/models/NotificationResponse.ts b/clients/admin-ui/src/types/api/models/NotificationResponse.ts new file mode 100644 index 00000000000..e7c37b0604b --- /dev/null +++ b/clients/admin-ui/src/types/api/models/NotificationResponse.ts @@ -0,0 +1,57 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * NotificationResponse + * + * Response schema for a single notification. + */ +export type NotificationResponse = { + /** + * Id + * + * The notification ID + */ + id: string; + /** + * Notification Type + * + * The type of notification + */ + notification_type: string; + /** + * Title + * + * Notification title + */ + title: string; + /** + * Body + * + * Notification body text + */ + body?: string | null; + /** + * Resource Type + * + * Type of the linked resource + */ + resource_type?: string | null; + /** + * Resource Id + * + * ID of the linked resource + */ + resource_id?: string | null; + /** + * Is Read + * + * Whether the notification has been read + */ + is_read?: boolean; + /** + * Created At + * + * When the notification was created + */ + created_at: string; +}; diff --git a/clients/admin-ui/src/types/api/models/Page_NotificationResponse_.ts b/clients/admin-ui/src/types/api/models/Page_NotificationResponse_.ts new file mode 100644 index 00000000000..2ba48efc21c --- /dev/null +++ b/clients/admin-ui/src/types/api/models/Page_NotificationResponse_.ts @@ -0,0 +1,29 @@ +// This file is auto-generated by @hey-api/openapi-ts + +import type { NotificationResponse } from "./NotificationResponse"; + +/** + * Page[NotificationResponse] + */ +export type Page_NotificationResponse_ = { + /** + * Items + */ + items: Array; + /** + * Total + */ + total: number; + /** + * Page + */ + page: number; + /** + * Size + */ + size: number; + /** + * Pages + */ + pages: number; +}; diff --git a/clients/admin-ui/src/types/api/models/UnreadCountResponse.ts b/clients/admin-ui/src/types/api/models/UnreadCountResponse.ts new file mode 100644 index 00000000000..5c262a52288 --- /dev/null +++ b/clients/admin-ui/src/types/api/models/UnreadCountResponse.ts @@ -0,0 +1,15 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * UnreadCountResponse + * + * Response schema for unread notification count. + */ +export type UnreadCountResponse = { + /** + * Count + * + * Number of unread notifications + */ + count: number; +}; From f63d18b396ac56fd001d51ec1ae472fcddcbe33b Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 11 May 2026 16:33:15 -0600 Subject: [PATCH 08/11] Add changelog entry for ENG-3303 codegen Co-Authored-By: Claude Opus 4.6 (1M context) --- changelog/8159-codegen-correspondence-ts-types.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/8159-codegen-correspondence-ts-types.yaml diff --git a/changelog/8159-codegen-correspondence-ts-types.yaml b/changelog/8159-codegen-correspondence-ts-types.yaml new file mode 100644 index 00000000000..d250f524aa6 --- /dev/null +++ b/changelog/8159-codegen-correspondence-ts-types.yaml @@ -0,0 +1,4 @@ +type: Added +description: Generated TypeScript types for correspondence and notification API contract (ENG-3303). +pr: 8159 +labels: [] From 84513c986355fb8def04afd1dcf9ee1b7dd1a555 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 12 May 2026 16:01:59 -0600 Subject: [PATCH 09/11] Regenerate TS types for contract review changes New types: MarkAllReadResponse, NotificationType enum, NotificationResourceType enum, ErrorResponse, Body_send_correspondence (multipart FormData shape). Updated: NotificationResponse now uses typed enums instead of plain strings for notification_type and resource_type. Co-Authored-By: Claude Opus 4.6 (1M context) --- clients/admin-ui/src/types/api/index.ts | 5 +++ ...privacy_request_id__correspondence_post.ts | 32 +++++++++++++++++++ .../src/types/api/models/ErrorResponse.ts | 13 ++++++++ .../types/api/models/MarkAllReadResponse.ts | 15 +++++++++ .../api/models/NotificationResourceType.ts | 10 ++++++ .../types/api/models/NotificationResponse.ts | 11 +++---- .../src/types/api/models/NotificationType.ts | 15 +++++++++ 7 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 clients/admin-ui/src/types/api/models/Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post.ts create mode 100644 clients/admin-ui/src/types/api/models/ErrorResponse.ts create mode 100644 clients/admin-ui/src/types/api/models/MarkAllReadResponse.ts create mode 100644 clients/admin-ui/src/types/api/models/NotificationResourceType.ts create mode 100644 clients/admin-ui/src/types/api/models/NotificationType.ts diff --git a/clients/admin-ui/src/types/api/index.ts b/clients/admin-ui/src/types/api/index.ts index 96fb06f66f1..52e7245070f 100644 --- a/clients/admin-ui/src/types/api/index.ts +++ b/clients/admin-ui/src/types/api/index.ts @@ -80,6 +80,7 @@ export type * from "./models/Body_create_comment_api_v1_plus_privacy_request__pr export type * from "./models/Body_create_comment_api_v1_plus_privacy_request__privacy_request_id__comment_post"; export type * from "./models/Body_create_comment_api_v1_plus_privacy_request__privacy_request_id__erasure_manual_webhook__connection_key__comment_post"; export type * from "./models/Body_export_minimal_datamap_with_format_api_v1_plus_datamap_minimal__export_format__post"; +export type * from "./models/Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post"; export type * from "./models/Body_skip_single_manual_field_api_v1_privacy_request__privacy_request_id__manual_field__manual_field_id__skip_post"; export type * from "./models/Body_submit_single_manual_field_api_v1_privacy_request__privacy_request_id__manual_field__manual_field_id__complete_post"; export type * from "./models/Body_upload_data_api_v1_storage__request_id__post"; @@ -328,6 +329,7 @@ export * from "./models/EnforcementLevel"; export type * from "./models/EntraDocsSchema"; export type * from "./models/EntraMonitorConfig"; export * from "./models/ErrorNotificationMode"; +export type * from "./models/ErrorResponse"; export * from "./models/ErrorType"; export type * from "./models/Evaluation"; export type * from "./models/EventAuditResponse"; @@ -490,6 +492,7 @@ export type * from "./models/ManualWebhookDocsSchema"; export type * from "./models/ManualWebhookField"; export type * from "./models/MappedPurpose"; export type * from "./models/MariaDBDocsSchema"; +export type * from "./models/MarkAllReadResponse"; export type * from "./models/MaskingAPIRequest"; export type * from "./models/MaskingAPIResponse"; export * from "./models/MaskingStrategies"; @@ -551,7 +554,9 @@ export type * from "./models/NoticeTranslation"; export type * from "./models/NoticeTranslationCreate"; export type * from "./models/NoticeTranslationResponse"; export type * from "./models/NotificationApplicationConfig"; +export type * from "./models/NotificationResourceType"; export type * from "./models/NotificationResponse"; +export * from "./models/NotificationType"; export type * from "./models/OAuthConfigSchema"; export * from "./models/OAuthGrantType"; export type * from "./models/OktaConfig"; diff --git a/clients/admin-ui/src/types/api/models/Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post.ts b/clients/admin-ui/src/types/api/models/Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post.ts new file mode 100644 index 00000000000..1704f6aaae3 --- /dev/null +++ b/clients/admin-ui/src/types/api/models/Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post.ts @@ -0,0 +1,32 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post + */ +export type Body_send_correspondence_api_v1_plus_privacy_request__privacy_request_id__correspondence_post = + { + /** + * Subject + * + * Email subject line + */ + subject: string; + /** + * Body + * + * Email body content (HTML supported) + */ + body: string; + /** + * Template Id + * + * Optional template ID + */ + template_id?: string | null; + /** + * Attachments + * + * Optional file attachments + */ + attachments?: Array; + }; diff --git a/clients/admin-ui/src/types/api/models/ErrorResponse.ts b/clients/admin-ui/src/types/api/models/ErrorResponse.ts new file mode 100644 index 00000000000..0b8ca7e39b0 --- /dev/null +++ b/clients/admin-ui/src/types/api/models/ErrorResponse.ts @@ -0,0 +1,13 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * ErrorResponse + * + * Model for error responses. + */ +export type ErrorResponse = { + /** + * Detail + */ + detail: string; +}; diff --git a/clients/admin-ui/src/types/api/models/MarkAllReadResponse.ts b/clients/admin-ui/src/types/api/models/MarkAllReadResponse.ts new file mode 100644 index 00000000000..e6f263025e1 --- /dev/null +++ b/clients/admin-ui/src/types/api/models/MarkAllReadResponse.ts @@ -0,0 +1,15 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * MarkAllReadResponse + * + * Response for bulk mark-all-as-read. + */ +export type MarkAllReadResponse = { + /** + * Marked Count + * + * Number of notifications marked as read + */ + marked_count: number; +}; diff --git a/clients/admin-ui/src/types/api/models/NotificationResourceType.ts b/clients/admin-ui/src/types/api/models/NotificationResourceType.ts new file mode 100644 index 00000000000..53f0051d6f8 --- /dev/null +++ b/clients/admin-ui/src/types/api/models/NotificationResourceType.ts @@ -0,0 +1,10 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * NotificationResourceType + * + * Resource types that notifications can link to. + */ +export enum NotificationResourceType { + PRIVACY_REQUEST = "privacy_request", +} diff --git a/clients/admin-ui/src/types/api/models/NotificationResponse.ts b/clients/admin-ui/src/types/api/models/NotificationResponse.ts index e7c37b0604b..3428497fcb8 100644 --- a/clients/admin-ui/src/types/api/models/NotificationResponse.ts +++ b/clients/admin-ui/src/types/api/models/NotificationResponse.ts @@ -1,5 +1,8 @@ // This file is auto-generated by @hey-api/openapi-ts +import { NotificationResourceType } from "./NotificationResourceType"; +import { NotificationType } from "./NotificationType"; + /** * NotificationResponse * @@ -13,11 +16,9 @@ export type NotificationResponse = { */ id: string; /** - * Notification Type - * * The type of notification */ - notification_type: string; + notification_type: NotificationType; /** * Title * @@ -31,11 +32,9 @@ export type NotificationResponse = { */ body?: string | null; /** - * Resource Type - * * Type of the linked resource */ - resource_type?: string | null; + resource_type?: NotificationResourceType | null; /** * Resource Id * diff --git a/clients/admin-ui/src/types/api/models/NotificationType.ts b/clients/admin-ui/src/types/api/models/NotificationType.ts new file mode 100644 index 00000000000..b0042ac2a9d --- /dev/null +++ b/clients/admin-ui/src/types/api/models/NotificationType.ts @@ -0,0 +1,15 @@ +// This file is auto-generated by @hey-api/openapi-ts + +/** + * NotificationType + * + * Types of in-app notifications. + * + * String constants — the DB column is a plain String (no migration + * for new values), but the API contract enumerates valid values so + * FE can switch on them. + */ +export enum NotificationType { + CORRESPONDENCE_REPLY = "correspondence_reply", + CORRESPONDENCE_BOUNCE = "correspondence_bounce", +} From 3115be781845401b75474c779f4c3d6b252abc62 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Fri, 15 May 2026 10:01:45 -0600 Subject: [PATCH 10/11] Fix N+1 query on Comment.correspondence_metadata relationship Add lazy="selectin" so listing comments batches metadata loading into a single IN query instead of one query per comment. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/api/models/comment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fides/api/models/comment.py b/src/fides/api/models/comment.py index 9fc9f1dff32..2f37ba8f294 100644 --- a/src/fides/api/models/comment.py +++ b/src/fides/api/models/comment.py @@ -161,6 +161,7 @@ class Comment(Base): "CorrespondenceMetadata", back_populates="comment", uselist=False, + lazy="selectin", cascade="all, delete-orphan", ) From 586e0e86f687d0b29759ce058fd2c99481e8dfc7 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Fri, 15 May 2026 10:04:52 -0600 Subject: [PATCH 11/11] Add read_at field to NotificationResponse TS type Matches the read_at: datetime | None field added to the Pydantic schema in fidesplus PR #3554. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../admin-ui/src/types/api/models/NotificationResponse.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clients/admin-ui/src/types/api/models/NotificationResponse.ts b/clients/admin-ui/src/types/api/models/NotificationResponse.ts index 3428497fcb8..cfd9fa518ee 100644 --- a/clients/admin-ui/src/types/api/models/NotificationResponse.ts +++ b/clients/admin-ui/src/types/api/models/NotificationResponse.ts @@ -47,6 +47,12 @@ export type NotificationResponse = { * Whether the notification has been read */ is_read?: boolean; + /** + * Read At + * + * When the notification was read + */ + read_at?: string | null; /** * Created At *