Feat/risk notification config support#377
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds risk notification configuration support by migrating away from the legacy risk_notifications_subscribed boolean into the unified notification subscriptions model, and extends risk notifications/digests to support Slack delivery alongside email.
Changes:
- Replace legacy risk subscription boolean with
risk_notificationsnotification type + per-user delivery channels (email/slack), including migration/backfill. - Add Slack delivery paths + Slack message formatters for risk lifecycle notifications and the risk open digest.
- Add a
riskdigestCLI command to run the risk digest worker directly, and update tests/docs to match the new subscription model.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/service/worker/user_repository.go | Stops returning legacy risk subscription boolean; relies on notification subscriptions + Slack link. |
| internal/service/worker/risk_workers.go | Sends risk lifecycle notifications via user-selected channels (email/Slack); adds Slack formatting. |
| internal/service/worker/risk_workers_test.go | Updates fixtures to seed notification subscriptions; adds Slack channel test coverage. |
| internal/service/worker/risk_digest_worker.go | Risk digest worker now supports email + Slack deliveries and exposes a reusable “plan” builder. |
| internal/service/worker/risk_digest_worker_test.go | Updates mocked users to use notification subscriptions; adds Slack digest test. |
| internal/service/worker/risk_digest_worker_integration_test.go | Seeds subscription rows instead of legacy booleans; updates worker construction. |
| internal/service/worker/poam_workers_integration_test.go | Seeds shared notification subscriptions; mostly formatting/indentation fixes in tests. |
| internal/service/worker/poam_digest_worker.go | POAM digest now checks subscription via notification channels (email) instead of legacy boolean. |
| internal/service/worker/jobs.go | Removes RiskNotificationsSubscribed from NotificationUser; wires Slack service into risk workers. |
| internal/service/slack/formatters/workflow_task_digest.go | Limits number of overdue tasks shown in Slack and adds “more” notice/link. |
| internal/service/slack/formatters/workflow_task_digest_test.go | Adds tests for overdue-task limiting behavior and “more” notice/link. |
| internal/service/slack/formatters/risk_notifications.go | New Slack message formatters for risk lifecycle notifications and risk digest. |
| internal/service/slack/formatters/risk_notifications_test.go | Unit tests for new risk Slack formatters. |
| internal/service/relational/ccf_internal.go | Removes legacy RiskNotificationsSubscribed field from User model. |
| internal/service/notification/constants.go | Adds risk_notifications notification type + wire mapping (riskNotifications). |
| internal/service/notification/constants_test.go | Tests normalization/wire mapping for risk notifications type. |
| internal/service/migrator.go | Adds migration to backfill legacy risk subscription into subscription rows and drop the column. |
| internal/service/migrator_test.go | Verifies legacy risk subscription backfill + idempotency. |
| internal/api/handler/users.go | Removes legacy boolean from subscriptions API; documents riskNotifications as supported type. |
| internal/api/handler/users_integration_test.go | Updates API tests to validate risk notification channels via the notifications map. |
| docs/swagger.yaml | Removes legacy boolean field from schema; updates descriptions for supported notification types. |
| docs/swagger.json | Same as YAML: removes legacy boolean and updates descriptions. |
| cmd/root.go | Registers new riskdigest command. |
| cmd/riskdigest.go | Adds riskdigest test command to run digest worker directly (email + Slack initialized). |
| cmd/riskdigest_direct.go | Implements direct-run digest execution and aggregates per-recipient errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.