Allows for a opt-in strategy for notifications rather than opt-out.
Signed-off-by: Henrik Edegård <henrik.edegard@fortnox.se>
This commit is contained in:
Henrik Edegård
+7
@@ -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;
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user