Skip to content

Commit

Permalink
chore(Notifications): Refactor notification settings to be per team
Browse files Browse the repository at this point in the history
Previously although meant to be per team, the settings were still tied
to a specific auth object. This meant that 1 team could have different
conflicting settings.
  • Loading branch information
Dschoordsch committed Feb 19, 2025
1 parent e8c4e70 commit 6c7e914
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 64 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
"husky": "^7.0.4",
"jscodeshift": "^0.14.0",
"kysely": "^0.27.4",
"kysely-codegen": "^0.15.0",
"kysely-codegen": "^0.17.0",
"kysely-ctl": "^0.9.0",
"lerna": "^6.4.1",
"mini-css-extract-plugin": "^2.7.2",
Expand Down
38 changes: 27 additions & 11 deletions packages/server/dataloader/integrationAuthLoaders.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import DataLoader from 'dataloader'
import {Selectable, sql} from 'kysely'
import errorFilter from '../graphql/errorFilter'
import isValid from '../graphql/isValid'
import getKysely from '../postgres/getKysely'
Expand All @@ -8,7 +9,7 @@ import getIntegrationProvidersByIds, {
} from '../postgres/queries/getIntegrationProvidersByIds'
import {selectSlackNotifications, selectTeamMemberIntegrationAuth} from '../postgres/select'
import {SlackAuth, SlackNotification, TeamMemberIntegrationAuth} from '../postgres/types'
import {NotificationSettings} from '../postgres/types/pg'
import {TeamNotificationSettings} from '../postgres/types/pg'
import NullableDataLoader from './NullableDataLoader'
import RootDataLoader from './RootDataLoader'

Expand Down Expand Up @@ -164,23 +165,22 @@ export const slackNotificationsByTeamIdAndEvent = (parent: RootDataLoader) => {
})
}

export const teamMemberIntegrationAuthsByTeamIdAndEvent = (parent: RootDataLoader) => {
export const teamMemberIntegrationAuthsByTeamIdAndService = (parent: RootDataLoader) => {
return new DataLoader<
{teamId: string; service: IntegrationProviderServiceEnum; event: SlackNotification['event']},
{teamId: string; service: IntegrationProviderServiceEnum},
TeamMemberIntegrationAuth[],
string
>(
async (keys) => {
const pg = getKysely()
const res = (await pg
.selectFrom('TeamMemberIntegrationAuth')
.innerJoin('NotificationSettings', 'authId', 'TeamMemberIntegrationAuth.id')
.selectAll()
.where(({eb, refTuple, tuple}) =>
eb(
refTuple('teamId', 'service', 'event'),
refTuple('teamId', 'service'),
'in',
keys.map(({teamId, service, event}) => tuple(teamId, service, event))
keys.map(({teamId, service}) => tuple(teamId, service))
)
)
.execute()) as unknown as TeamMemberIntegrationAuth[]
Expand All @@ -196,17 +196,33 @@ export const teamMemberIntegrationAuthsByTeamIdAndEvent = (parent: RootDataLoade
)
}

export const notificationSettingsByAuthId = (parent: RootDataLoader) => {
return new DataLoader<number, NotificationSettings['event'][], string>(
export const notificationSettingsByProviderIdAndTeamId = (parent: RootDataLoader) => {
return new DataLoader<
{providerId: number; teamId: string},
Selectable<TeamNotificationSettings>['events'],
string
>(
async (keys) => {
const pg = getKysely()
const res = await pg
.selectFrom('NotificationSettings')
.selectFrom('TeamNotificationSettings')
.selectAll()
.where(({eb}) => eb('authId', 'in', keys))
// convert to text[] as kysely would otherwise not parse the array
.select(sql<TeamNotificationSettings['events']>`events::text[]`.as('events'))
.where(({eb, refTuple, tuple}) =>
eb(
refTuple('providerId', 'teamId'),
'in',
keys.map(({providerId, teamId}) => tuple(providerId, teamId))
)
)
.execute()

return keys.map((key) => res.filter(({authId}) => authId === key).map(({event}) => event))
return keys.map(
(key) =>
res.find(({providerId, teamId}) => providerId === key.providerId && teamId === key.teamId)
?.events || []
)
},
{
...parent.dataLoaderOptions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import {IntegrationProviderMSTeams as IIntegrationProviderMSTeams} from '../../.
import {SlackNotification, Team} from '../../../../postgres/types'
import IUser from '../../../../postgres/types/IUser'
import {AnyMeeting, MeetingTypeEnum} from '../../../../postgres/types/Meeting'
import {NotificationSettings} from '../../../../postgres/types/pg'
import MSTeamsServerManager from '../../../../utils/MSTeamsServerManager'
import {analytics} from '../../../../utils/analytics/analytics'
import sendToSentry from '../../../../utils/sendToSentry'
import {DataLoaderWorker} from '../../../graphql'
import isValid from '../../../isValid'
import {SlackNotificationEventEnum} from '../../../public/resolverTypes'
import {NotificationIntegrationHelper} from './NotificationIntegrationHelper'
import {createNotifier} from './Notifier'
import getSummaryText from './getSummaryText'
Expand Down Expand Up @@ -338,22 +339,38 @@ async function getMSTeams(
dataLoader: DataLoaderWorker,
teamId: string,
userId: string,
event: NotificationSettings['event']
event: SlackNotificationEventEnum
) {
const [auths, user] = await Promise.all([
dataLoader
.get('teamMemberIntegrationAuthsByTeamIdAndEvent')
.load({service: 'msTeams', teamId, event}),
.get('teamMemberIntegrationAuthsByTeamIdAndService')
.load({service: 'msTeams', teamId}),
dataLoader.get('users').loadNonNull(userId)
])
return Promise.all(
auths.map(async (auth) => {
const provider = await dataLoader.get('integrationProviders').loadNonNull(auth.providerId)
return MSTeamsNotificationHelper({
...(provider as IntegrationProviderMSTeams),
userId,
email: user.email

const providers = (
await Promise.all(
auths.map(async (auth) => {
const {providerId} = auth
const [provider, events] = await Promise.all([
dataLoader
.get('integrationProviders')
.loadNonNull(providerId) as Promise<IntegrationProviderMSTeams>,
dataLoader.get('notificationSettingsByProviderIdAndTeamId').load({providerId, teamId})
])
if (events.includes(event)) {
return provider
}
return null
})
)
).filter(isValid)

return providers.map((provider) =>
MSTeamsNotificationHelper({
...(provider as IntegrationProviderMSTeams),
userId,
email: user.email
})
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import {IntegrationProviderMattermost} from '../../../../postgres/queries/getInt
import {SlackNotification, Team} from '../../../../postgres/types'
import IUser from '../../../../postgres/types/IUser'
import {AnyMeeting, MeetingTypeEnum} from '../../../../postgres/types/Meeting'
import {NotificationSettings} from '../../../../postgres/types/pg'
import MattermostServerManager from '../../../../utils/MattermostServerManager'
import {analytics} from '../../../../utils/analytics/analytics'
import {toEpochSeconds} from '../../../../utils/epochTime'
import sendToSentry from '../../../../utils/sendToSentry'
import {DataLoaderWorker} from '../../../graphql'
import isValid from '../../../isValid'
import {SlackNotificationEventEnum} from '../../../public/resolverTypes'
import {NotificationIntegrationHelper} from './NotificationIntegrationHelper'
import {createNotifier} from './Notifier'
import getSummaryText from './getSummaryText'
Expand Down Expand Up @@ -359,7 +360,7 @@ async function getMattermost(
dataLoader: DataLoaderWorker,
teamId: string,
userId: string,
event: NotificationSettings['event']
event: SlackNotificationEventEnum
) {
if (MATTERMOST_SECRET && MATTERMOST_URL) {
return [
Expand All @@ -374,17 +375,28 @@ async function getMattermost(
}

const auths = await dataLoader
.get('teamMemberIntegrationAuthsByTeamIdAndEvent')
.load({service: 'mattermost', teamId, event})
.get('teamMemberIntegrationAuthsByTeamIdAndService')
.load({service: 'mattermost', teamId})

return Promise.all(
auths.map(async (auth) => {
const provider = (await dataLoader
.get('integrationProviders')
.loadNonNull(auth.providerId)) as IntegrationProviderMattermost
return MattermostNotificationHelper({...provider, teamId, userId})
})
)
const providers = (
await Promise.all(
auths.map(async (auth) => {
const {providerId} = auth
const [provider, events] = await Promise.all([
dataLoader
.get('integrationProviders')
.loadNonNull(providerId) as Promise<IntegrationProviderMattermost>,
dataLoader.get('notificationSettingsByProviderIdAndTeamId').load({providerId, teamId})
])
if (events.includes(event)) {
return provider
}
return null
})
)
).filter(isValid)

return providers.map((provider) => MattermostNotificationHelper({...provider, teamId, userId}))
}

export const MattermostNotifier = createNotifier(getMattermost)
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,18 @@ const addTeamMemberIntegrationAuth: MutationResolvers['addTeamMemberIntegrationA
})
}

await pg
.insertInto('NotificationSettings')
.columns(['authId', 'event'])
.values(() => ({
authId,
event: sql`unnest(enum_range(NULL::"SlackNotificationEventEnum"))`
}))
.onConflict((oc) => oc.doNothing())
.execute()
if (service === 'msTeams' || service === 'mattermost') {
await pg
.insertInto('TeamNotificationSettings')
.columns(['providerId', 'teamId', 'events'])
.values(() => ({
providerId: providerDbId,
teamId,
events: sql`enum_range(NULL::"SlackNotificationEventEnum")`
}))
.onConflict((oc) => oc.doNothing())
.execute()
}

updateRepoIntegrationsCacheByPerms(dataLoader, viewerId, teamId, true)

Expand Down
22 changes: 14 additions & 8 deletions packages/server/graphql/public/mutations/setNotificationSetting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const setNotificationSetting: MutationResolvers['setNotificationSetting'] = asyn
if (!auth) {
return standardError(new Error('Integration auth not found'), {userId: viewerId})
}
const {teamId, service} = auth
const {providerId, teamId, service} = auth
if (!isTeamMember(authToken, teamId)) {
return standardError(new Error('Attempted teamId spoof'), {userId: viewerId})
}
Expand All @@ -35,18 +35,24 @@ const setNotificationSetting: MutationResolvers['setNotificationSetting'] = asyn
// RESOLUTION
if (isEnabled) {
await pg
.insertInto('NotificationSettings')
.values({authId, event})
.onConflict((oc) => oc.doNothing())
.updateTable('TeamNotificationSettings')
.set(({fn, val}) => ({
events: fn('arr_append_uniq', ['events', val(event)])
}))
.where('providerId', '=', providerId)
.where('teamId', '=', teamId)
.execute()
} else {
await pg
.deleteFrom('NotificationSettings')
.where('authId', '=', authId)
.where('event', '=', event)
.updateTable('TeamNotificationSettings')
.set(({fn, val}) => ({
events: fn('array_remove', ['events', val(event)])
}))
.where('providerId', '=', providerId)
.where('teamId', '=', teamId)
.execute()
}
const data = {authId}
const data = {authId, providerId, teamId}
publish(SubscriptionChannel.TEAM, teamId, 'SetNotificationSettingSuccess', data, subOptions)
return data
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import {SetNotificationSettingSuccessResolvers} from '../resolverTypes'

export type SetNotificationSettingSuccessSource = {
authId: number
providerId: number
teamId: string
}

const SetNotificationSettingSuccess: SetNotificationSettingSuccessResolvers = {
auth: async ({authId}, _args, {dataLoader}) => {
return dataLoader.get('teamMemberIntegrationAuths').loadNonNull(authId)
},
events: async ({authId}, _args, {dataLoader}) => {
return dataLoader.get('notificationSettingsByAuthId').load(authId)
events: async ({providerId, teamId}, _args, {dataLoader}) => {
return dataLoader.get('notificationSettingsByProviderIdAndTeamId').load({providerId, teamId})
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const TeamMemberIntegrationAuthWebhook: TeamMemberIntegrationAuthWebhookResolver
provider: async ({providerId}, _args, {dataLoader}) => {
return dataLoader.get('integrationProviders').loadNonNull(providerId)
},
events: async ({id}, _args, {dataLoader}) => {
return dataLoader.get('notificationSettingsByAuthId').load(id)
events: async ({teamId, providerId}, _args, {dataLoader}) => {
return dataLoader.get('notificationSettingsByProviderIdAndTeamId').load({providerId, teamId})
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import {sql, type Kysely} from 'kysely'

export async function up(db: Kysely<any>): Promise<void> {
// Schema changes significantly, easier to create a new table
await db.schema
.createTable('TeamNotificationSettings')
.addColumn('id', 'serial', (col) => col.primaryKey())
.addColumn('providerId', 'integer', (col) =>
col.references('IntegrationProvider.id').onDelete('cascade').notNull()
)
.addColumn('teamId', 'varchar(100)', (col) =>
col.references('Team.id').onDelete('cascade').notNull()
)
.addColumn('channelId', 'varchar(255)')
.addColumn('events', sql`"SlackNotificationEventEnum"[]`, (col) =>
col.defaultTo(sql`enum_range(NULL::"SlackNotificationEventEnum")`).notNull()
)
.addUniqueConstraint(
'TeamNotificationSettings_providerId_teamId_channelId_key',
['providerId', 'teamId', 'channelId'],
(uc) => uc.nullsNotDistinct()
)
.execute()

await db
.insertInto('TeamNotificationSettings')
.columns(['providerId', 'teamId', 'events'])
.expression((eb) =>
eb
.selectFrom('NotificationSettings')
.innerJoin('TeamMemberIntegrationAuth as auth', 'authId', 'auth.id')
.select(['providerId', 'auth.teamId', sql`array_agg(event)`])
// There was a bug which might have added settings for other providers like gcal
.where((eb) => eb.or([eb('service', '=', 'mattermost'), eb('service', '=', 'msTeams')]))
.groupBy(['providerId', 'auth.teamId'])
)
.execute()

/* dropping the old table will be done in a later change
await db.schema
.dropTable('NotificationSettings')
.execute()
*/
}

export async function down(db: Kysely<any>): Promise<void> {
/*
await db.schema
.createTable('NotificationSettings')
.addColumn('id', 'serial', (col) => col.primaryKey())
.addColumn('authId', 'integer', (col) =>
col.references('TeamMemberIntegrationAuth.id').onDelete('cascade').notNull()
)
.addColumn('event', sql`"SlackNotificationEventEnum"`, (col) => col.notNull())
.addUniqueConstraint('NotificationSettings_authId_event_key', ['authId', 'event'])
.execute()
await db.schema
.createIndex('NotificationSettings_authId_idx')
.on('NotificationSettings')
.column('authId')
.execute()
await db
.insertInto('NotificationSettings')
.columns(['authId', 'event'])
.expression((eb) =>
eb
.selectFrom('TeamMemberIntegrationAuth as auth')
.innerJoin('TeamNotificationSettings as settings', (join) => join
.onRef('auth.teamId', '=', 'settings.teamId')
.onRef('auth.providerId', '=', 'settings.providerId')
)
.select(['auth.id', sql`unnest(events)`])
)
.execute()
*/

await db.schema.dropTable('TeamNotificationSettings').execute()
}
Loading

0 comments on commit 6c7e914

Please sign in to comment.