From 87e597c406e7ae2b13828e07f33dd59f95f66780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Edeg=C3=A5rd?= Date: Wed, 1 Oct 2025 09:10:45 +0000 Subject: [PATCH] Allows for a opt-in strategy for notifications rather than opt-out. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Henrik Edegård --- .changeset/fancy-years-camp.md | 9 + docs/notifications/index.md | 55 +++ plugins/notifications-backend/config.d.ts | 7 + .../src/service/router.test.ts | 316 +++++++++++++++++- .../src/service/router.ts | 75 ++++- plugins/notifications-common/report.api.md | 1 + plugins/notifications-common/src/types.ts | 6 + plugins/notifications-common/src/utils.ts | 20 +- 8 files changed, 476 insertions(+), 13 deletions(-) create mode 100644 .changeset/fancy-years-camp.md diff --git a/.changeset/fancy-years-camp.md b/.changeset/fancy-years-camp.md new file mode 100644 index 0000000000..41782d4e0c --- /dev/null +++ b/.changeset/fancy-years-camp.md @@ -0,0 +1,9 @@ +--- +'@backstage/plugin-notifications-backend': minor +'@backstage/plugin-notifications-common': minor +--- + +Adds support for default configuration for an entire notification channel. +This setting will also be inherited down to origins and topics while still respecting the users individual choices. + +This will be handy if you want to use a "opt-in" strategy. diff --git a/docs/notifications/index.md b/docs/notifications/index.md index 59ff2962ed..dce58d3214 100644 --- a/docs/notifications/index.md +++ b/docs/notifications/index.md @@ -164,6 +164,61 @@ You can customize the origin names shown in the UI by passing an object where th Each notification processor will receive its own row in the settings page, where the user can enable or disable notifications from that processor. +### Default notification settings + +You can configure default notification settings for all users in your `app-config.yaml` file. This allows you to set up notification preferences globally, such as disabling specific channels or origins by default, implementing an opt-in strategy instead of opt-out. + +#### Channel-level defaults + +You can set a default enabled state for an entire channel. When set to `false`, the channel uses an opt-in strategy where notifications are disabled by default unless explicitly enabled by the user or for specific origins. + +```yaml +notifications: + defaultSettings: + channels: + - id: 'Web' + enabled: false # Opt-in strategy: channel disabled by default + - id: 'Email' + enabled: true # Opt-out strategy: channel enabled by default (default behavior) +``` + +#### Origin-level defaults + +You can also configure defaults for specific origins within a channel: + +```yaml +notifications: + defaultSettings: + channels: + - id: 'Web' + enabled: true # Channel is enabled by default + origins: + - id: 'plugin:scaffolder' + enabled: false # Disable scaffolder notifications by default + - id: 'plugin:catalog' + enabled: true # Enable catalog notifications by default +``` + +#### Topic-level defaults + +For even more granular control, you can set defaults for specific topics within origins: + +```yaml +notifications: + defaultSettings: + channels: + - id: 'Email' + enabled: false # Email is opt-in by default + origins: + - id: 'plugin:catalog' + enabled: true # But catalog notifications are enabled + topics: + - id: 'entity:validation:error' + enabled: false # Except validation errors +``` + +**Note:** If a channel's `enabled` flag is not set, it defaults to `true` for backwards compatibility. When a channel is set to `enabled: false`, all origins within that channel default to disabled unless explicitly enabled. + ### Automatic notification cleanup Notifications are deleted automatically after a certain period of time to prevent the database from growing indefinitely diff --git a/plugins/notifications-backend/config.d.ts b/plugins/notifications-backend/config.d.ts index 2b6f874f72..d03baaec94 100644 --- a/plugins/notifications-backend/config.d.ts +++ b/plugins/notifications-backend/config.d.ts @@ -34,6 +34,13 @@ export interface Config { defaultSettings?: { channels?: { id: string; + /** + * Optional flag to enable/disable the channel by default. + * If not set, defaults to true for backwards compatibility. + * When set to false, the channel uses an opt-in strategy where + * origins are disabled by default unless explicitly enabled. + */ + enabled?: boolean; origins?: { id: string; enabled: boolean; diff --git a/plugins/notifications-backend/src/service/router.test.ts b/plugins/notifications-backend/src/service/router.test.ts index 3f71377151..62a37acbbb 100644 --- a/plugins/notifications-backend/src/service/router.test.ts +++ b/plugins/notifications-backend/src/service/router.test.ts @@ -32,7 +32,7 @@ import { import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; import { DatabaseService } from '@backstage/backend-plugin-api'; import { v4 as uuid } from 'uuid'; -import { DatabaseNotificationsStore } from '../database'; +import { DatabaseNotificationsStore, generateSettingsHash } from '../database'; const databases = TestDatabases.create(); let store: DatabaseNotificationsStore; @@ -581,6 +581,157 @@ describe.each(databases.eachSupportedId())('createRouter (%s)', databaseId => { expect(response.status).toEqual(400); }); + + it('should not send notification when channel is disabled and user has no settings', async () => { + // Create a new config with channel disabled + const configWithChannelDisabled = mockServices.rootConfig({ + data: { + app: { baseUrl: 'http://localhost' }, + notifications: { + defaultSettings: { + channels: [ + { + id: 'Web', + enabled: false, // Channel disabled by default (opt-in) + }, + ], + }, + }, + }, + }); + + const routerWithChannelDisabled = await createRouter({ + logger: mockServices.logger.mock(), + store, + signals: signalService, + userInfo, + config: configWithChannelDisabled, + httpAuth, + auth, + catalog, + }); + const appWithChannelDisabled = express() + .use(routerWithChannelDisabled) + .use(mockErrorHandler()); + + const sendNotificationToDisabledChannel = ( + opts: NotificationSendOptions, + ) => + request(appWithChannelDisabled) + .post('/notifications') + .send(opts) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + + const response = await sendNotificationToDisabledChannel({ + recipients: { + type: 'entity', + entityRef: ['user:default/mock'], + }, + payload: { + title: 'test notification', + topic: 'test-topic', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); // No notifications sent + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(0); // No notifications created + }); + + it('should send notification when user enabled specific topic even if channel is disabled', async () => { + // Create a new config with channel disabled + const configWithChannelDisabled = mockServices.rootConfig({ + data: { + app: { baseUrl: 'http://localhost' }, + notifications: { + defaultSettings: { + channels: [ + { + id: 'Web', + enabled: false, // Channel disabled by default (opt-in) + }, + ], + }, + }, + }, + }); + + const routerWithChannelDisabled = await createRouter({ + logger: mockServices.logger.mock(), + store, + signals: signalService, + userInfo, + config: configWithChannelDisabled, + httpAuth, + auth, + catalog, + }); + const appWithChannelDisabled = express() + .use(routerWithChannelDisabled) + .use(mockErrorHandler()); + + const sendNotificationToDisabledChannel = ( + opts: NotificationSendOptions, + ) => + request(appWithChannelDisabled) + .post('/notifications') + .send(opts) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + + // User explicitly enables a specific topic + const client = await database.getClient(); + await client('user_settings').insert({ + settings_key_hash: generateSettingsHash( + 'user:default/mock', + 'Web', + 'external:test-service', + 'important-topic', + ), + user: 'user:default/mock', + channel: 'Web', + origin: 'external:test-service', + topic: 'important-topic', + enabled: true, + }); + + const response = await sendNotificationToDisabledChannel({ + recipients: { + type: 'entity', + entityRef: ['user:default/mock'], + }, + payload: { + title: 'important notification', + topic: 'important-topic', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + severity: 'normal', + title: 'important notification', + topic: 'important-topic', + }, + user: 'user:default/mock', + }, + ]); + + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(1); // Notification created for enabled topic + }); }); describe('POST /notifications with custom receiver resolver', () => { @@ -932,6 +1083,169 @@ describe.each(databases.eachSupportedId())('createRouter (%s)', databaseId => { ], }); }); + + it('should respect channel-level enabled flag from config', async () => { + // Create a new config with channel-level enabled flag + const configWithChannelEnabled = mockServices.rootConfig({ + data: { + app: { baseUrl: 'http://localhost' }, + notifications: { + defaultSettings: { + channels: [ + { + id: 'Web', + enabled: false, // Channel disabled by default (opt-in) + }, + ], + }, + }, + }, + }); + + const routerWithChannelDisabled = await createRouter({ + logger: mockServices.logger.mock(), + store, + signals: signalService, + userInfo, + config: configWithChannelEnabled, + httpAuth, + auth, + catalog, + }); + const appWithChannelDisabled = express() + .use(routerWithChannelDisabled) + .use(mockErrorHandler()); + + const response = await request(appWithChannelDisabled).get('/settings'); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + channels: [ + { + id: 'Web', + enabled: false, + origins: expect.arrayContaining([ + { + enabled: false, + id: 'external:test-service', + topics: [{ enabled: false, id: 'test-topic' }], + }, + { + enabled: false, + id: 'external:test-service2', + topics: [{ enabled: false, id: 'test-topic2' }], + }, + ]), + }, + ], + }); + }); + + it('should allow user to enable specific topic even when channel is disabled', async () => { + // Create a new config with channel disabled + const configWithChannelDisabled = mockServices.rootConfig({ + data: { + app: { baseUrl: 'http://localhost' }, + notifications: { + defaultSettings: { + channels: [ + { + id: 'Web', + enabled: false, // Channel disabled by default (opt-in) + }, + ], + }, + }, + }, + }); + + const routerWithChannelDisabled = await createRouter({ + logger: mockServices.logger.mock(), + store, + signals: signalService, + userInfo, + config: configWithChannelDisabled, + httpAuth, + auth, + catalog, + }); + const appWithChannelDisabled = express() + .use(routerWithChannelDisabled) + .use(mockErrorHandler()); + + const client = await database.getClient(); + + // Clear existing notifications from beforeEach + await client('notification').del(); + + // Create notifications with multiple topics for the same origin + await client('notification').insert({ + id: uuid(), + user: 'user:default/mock', + origin: 'external:test-service', + topic: 'topic-build-failed', + title: 'Build Failed', + created: new Date(), + severity: 'high', + }); + + await client('notification').insert({ + id: uuid(), + user: 'user:default/mock', + origin: 'external:test-service', + topic: 'topic-deployment-success', + title: 'Deployment Success', + created: new Date(), + severity: 'normal', + }); + + await client('notification').insert({ + id: uuid(), + user: 'user:default/mock', + origin: 'external:test-service', + topic: 'topic-security-alert', + title: 'Security Alert', + created: new Date(), + severity: 'critical', + }); + + // User explicitly enables only one specific topic (build failures) + // The other topics are NOT in the database, so they should inherit from channel default (false) + await client('user_settings').insert({ + settings_key_hash: generateSettingsHash( + 'user:default/mock', + 'Web', + 'external:test-service', + 'topic-build-failed', + ), + user: 'user:default/mock', + channel: 'Web', + origin: 'external:test-service', + topic: 'topic-build-failed', + enabled: true, + }); + + const response = await request(appWithChannelDisabled).get('/settings'); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + channels: [ + { + id: 'Web', + enabled: false, + origins: [ + { + enabled: true, // Origin gets enabled when user enables a topic + id: 'external:test-service', + topics: expect.arrayContaining([ + { enabled: true, id: 'topic-build-failed' }, // User explicitly enabled this + { enabled: false, id: 'topic-deployment-success' }, // Inherits from channel default (false) + { enabled: false, id: 'topic-security-alert' }, // Inherits from channel default (false) + ]), + }, + ], + }, + ], + }); + }); }); describe('POST /settings', () => { diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 77f2685ebf..63d974e708 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -122,7 +122,7 @@ export async function createRouter( topic: any, existingOrigin: OriginSetting | undefined, defaultOriginSettings: OriginSetting | undefined, - defaultEnabled: boolean, + channelDefaultEnabled: boolean, ) => { const existingTopic = existingOrigin?.topics?.find( t => t.id.toLowerCase() === topic.topic.toLowerCase(), @@ -131,11 +131,14 @@ export async function createRouter( t => t.id.toLowerCase() === topic.topic.toLowerCase(), ); + // If topic has explicit setting, use it + // Otherwise check default topic settings from config + // Otherwise use channel default (not origin enabled state) return { id: topic.topic, enabled: existingTopic ? existingTopic.enabled - : defaultTopicSettings?.enabled ?? defaultEnabled, + : defaultTopicSettings?.enabled ?? channelDefaultEnabled, }; }; @@ -144,6 +147,8 @@ export async function createRouter( existingChannel: ChannelSetting | undefined, defaultChannelSettings: ChannelSetting | undefined, topics: { origin: string; topic: string }[], + channelDefaultEnabled: boolean, + channelHasExplicitEnabled: boolean, ) => { const existingOrigin = existingChannel?.origins?.find( o => o.id.toLowerCase() === originId.toLowerCase(), @@ -155,7 +160,7 @@ export async function createRouter( const defaultEnabled = existingOrigin ? existingOrigin.enabled - : defaultOriginSettings?.enabled ?? true; + : defaultOriginSettings?.enabled ?? channelDefaultEnabled; return { id: originId, @@ -167,7 +172,7 @@ export async function createRouter( t, existingOrigin, defaultOriginSettings, - defaultEnabled, + channelHasExplicitEnabled ? channelDefaultEnabled : defaultEnabled, ), ), }; @@ -186,14 +191,29 @@ export async function createRouter( c => c.id.toLowerCase() === channelId.toLowerCase(), ); + // Determine channel enabled state + const channelEnabled = + existingChannel?.enabled ?? defaultChannelSettings?.enabled; + + // Use channel's enabled flag as the default for origins if not explicitly set + const defaultEnabledForOrigins = channelEnabled ?? true; + + // Check if channel has explicit enabled flag (either from user settings or config) + const channelHasExplicitEnabled = + existingChannel?.enabled !== undefined || + defaultChannelSettings?.enabled !== undefined; + return { id: channelId, + enabled: channelEnabled, origins: origins.map(originId => getOriginSettings( originId, existingChannel, defaultChannelSettings, topics, + defaultEnabledForOrigins, + channelHasExplicitEnabled, ), ), }; @@ -241,7 +261,52 @@ export async function createRouter( origin: string; topic: string | null; }) => { - const settings = await getNotificationSettings(opts.user); + // Get user's explicit settings from database + const userSettings = await store.getNotificationSettings({ + user: opts.user, + }); + + // Build a minimal settings object with user settings and config defaults + const settings: NotificationSettings = { + channels: [ + { + id: opts.channel, + enabled: defaultNotificationSettings?.channels?.find( + c => c.id.toLowerCase() === opts.channel.toLowerCase(), + )?.enabled, + origins: [], + }, + ], + }; + + // Add user's channel if it exists + const userChannel = userSettings.channels.find( + c => c.id.toLowerCase() === opts.channel.toLowerCase(), + ); + if (userChannel) { + settings.channels[0] = { + ...settings.channels[0], + enabled: userChannel.enabled ?? settings.channels[0].enabled, + origins: userChannel.origins, + }; + } + + // Add config default origins if not in user settings + const defaultChannelSettings = defaultNotificationSettings?.channels?.find( + c => c.id.toLowerCase() === opts.channel.toLowerCase(), + ); + if (defaultChannelSettings?.origins) { + for (const defaultOrigin of defaultChannelSettings.origins) { + if ( + !settings.channels[0].origins.some( + o => o.id.toLowerCase() === defaultOrigin.id.toLowerCase(), + ) + ) { + settings.channels[0].origins.push(defaultOrigin); + } + } + } + return isNotificationsEnabledFor( settings, opts.channel, diff --git a/plugins/notifications-common/report.api.md b/plugins/notifications-common/report.api.md index 337c41b7b5..ecc0ec8ef0 100644 --- a/plugins/notifications-common/report.api.md +++ b/plugins/notifications-common/report.api.md @@ -9,6 +9,7 @@ import { JsonValue } from '@backstage/types'; // @public (undocumented) export type ChannelSetting = { id: string; + enabled?: boolean; origins: OriginSetting[]; }; diff --git a/plugins/notifications-common/src/types.ts b/plugins/notifications-common/src/types.ts index 4e49de79de..0653007637 100644 --- a/plugins/notifications-common/src/types.ts +++ b/plugins/notifications-common/src/types.ts @@ -154,6 +154,12 @@ export type OriginSetting = { */ export type ChannelSetting = { id: string; + /** + * Optional flag to enable/disable the channel by default. + * If not set, defaults to true for backwards compatibility. + * When set to false, the channel uses an opt-in strategy. + */ + enabled?: boolean; origins: OriginSetting[]; }; diff --git a/plugins/notifications-common/src/utils.ts b/plugins/notifications-common/src/utils.ts index b95df5bea5..02d840e34f 100644 --- a/plugins/notifications-common/src/utils.ts +++ b/plugins/notifications-common/src/utils.ts @@ -29,14 +29,20 @@ export const isNotificationsEnabledFor = ( const origin = channel.origins.find(o => o.id === originId); if (!origin) { - return true; + // If no origin is found, use channel's enabled flag (defaults to true if not set) + return channel.enabled ?? true; } - if (topicId === null) { + + // If topic is specified, check topic-level setting + if (topicId !== null) { + const topic = origin.topics?.find(t => t.id === topicId); + if (topic) { + return topic.enabled; + } + // No explicit topic setting, check origin return origin.enabled; } - const topic = origin.topics?.find(t => t.id === topicId); - if (!topic) { - return origin.enabled; - } - return topic.enabled; + + // No topic specified, check origin-level setting + return origin.enabled; };