diff --git a/src/app/domain/github/githubPush.ts b/src/app/domain/github/githubPush.ts index c12d0a0..90ff1a9 100644 --- a/src/app/domain/github/githubPush.ts +++ b/src/app/domain/github/githubPush.ts @@ -6,6 +6,8 @@ import { GithubPush } from '../types/GithubPush' import { executeAndCatchConditionalCheckFailed } from '../entityStore/entityStoreOperationSupport' import { sendToEventBridge } from '../../outboundInterfaces/eventBridgeBus' import { saveLatestPushes } from './githubLatestPushesPerRef' +import { GithubUserId } from '../types/GithubUserId' +import { getUserIdsForAccount } from './githubMembership' export async function processPushes(appState: AppState, pushes: GithubPush[], publishNotifications: boolean) { if (pushes.length > 0) { @@ -38,3 +40,10 @@ async function savePushes(appState: AppState, pushes: GithubPush[]) { export function latestCommitInPush(push: Pick) { return push.commits[push.commits.length - 1] } + +export async function getRelatedMemberIdsForPush( + appState: AppState, + push: GithubPush +): Promise { + return getUserIdsForAccount(appState, push.accountId) +} diff --git a/src/app/domain/user/userNotifyable.ts b/src/app/domain/user/userNotifyable.ts index 2c89f0e..5d335a4 100644 --- a/src/app/domain/user/userNotifyable.ts +++ b/src/app/domain/user/userNotifyable.ts @@ -6,6 +6,7 @@ import { loadCalculatedUserSettingsOrUseDefaults } from './calculatedUserSetting import { UserScopeReferenceData } from '../types/UserScopeReferenceData' import { GithubWorkflowRunEvent } from '../types/GithubWorkflowRunEvent' import { GithubWorkflowSummary } from '../types/GithubSummaries' +import { GithubRepoKey } from '../types/GithubKeys' export async function filterWorkflowNotifyEnabled( appState: AppState, @@ -45,3 +46,40 @@ async function getWorkflowNotifyEnabledForUser( } return yesNotify } + +export async function filterRepoNotifyEnabled( + appState: AppState, + refData: UserScopeReferenceData, + userIds: GithubUserId[], + repo: GithubRepoKey +): Promise { + const enabledUserIds: GithubUserId[] = [] + for (const userId of userIds) { + if ( + // TODO - don't do this! This is a hack while all users have same visibility to + // avoid a huge amount of queries for large accounts + // What we want instead is some caching for underlying ref data objects + await getRepoNotifyEnabledForUser(appState, repo, { + ...refData, + userId + }) + ) { + enabledUserIds.push(userId) + } + } + return enabledUserIds +} + +async function getRepoNotifyEnabledForUser( + appState: AppState, + repo: GithubRepoKey, + refData: UserScopeReferenceData +) { + const userSettings = await loadCalculatedUserSettingsOrUseDefaults(appState, refData) + const yesNotify = userSettings.github.accounts[repo.accountId]?.repos[repo.repoId].notify + if (yesNotify === undefined) { + logger.warn(`No calculated user notify setting for repo`, { repo, userSettings }) + return false + } + return yesNotify +} diff --git a/src/app/domain/webPush/cicadaEventWebPushPublisher.ts b/src/app/domain/webPush/cicadaEventWebPushPublisher.ts index 7d92c24..5911e7e 100644 --- a/src/app/domain/webPush/cicadaEventWebPushPublisher.ts +++ b/src/app/domain/webPush/cicadaEventWebPushPublisher.ts @@ -10,9 +10,11 @@ import { } from '../github/githubWorkflowRunEvent' import { isCicadaEventBridgeDetail } from '../../outboundInterfaces/eventBridgeBus' import { CicadaWebNotification } from '../../outboundInterfaces/webPushWrapper' -import { filterWorkflowNotifyEnabled } from '../user/userNotifyable' +import { filterRepoNotifyEnabled, filterWorkflowNotifyEnabled } from '../user/userNotifyable' import { loadUserScopeReferenceData } from '../github/userScopeReferenceData' +import { isGithubPush } from '../types/GithubPush' +import { getRelatedMemberIdsForPush } from '../github/githubPush' // TOEventually - these are going to create a lot of queries for subscription lookup for large organizations // May be better to have one table / index for this. @@ -54,20 +56,25 @@ export function generateRunEventNotification( } } -// TOEventually - add this back now that notification preferences added, maybe -// export async function handleNewPush(appState: AppState, eventDetail: unknown) { -// if (!isCicadaEventBridgeDetail(eventDetail) || !isGithubPush(eventDetail.data)) { -// logger.error( -// `Event detail for detail-type ${EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_PUSH} was not of expected format`, -// { commit: eventDetail } -// ) -// return -// } -// -// const push = eventDetail.data -// -// await publishToSubscriptionsForUsers(appState, await getRelatedMemberIdsForPush(appState, push), { -// title: 'New Push', -// body: `New push to ${push.repoName}:${push.ref} by ${push.actor.login}` -// }) -// } +export async function handleNewPush(appState: AppState, eventDetail: unknown) { + if (!isCicadaEventBridgeDetail(eventDetail) || !isGithubPush(eventDetail.data)) { + logger.error( + `Event detail for detail-type ${EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_PUSH} was not of expected format`, + { commit: eventDetail } + ) + return + } + + const push = eventDetail.data + const userIds = await getRelatedMemberIdsForPush(appState, push) + if (userIds.length === 0) return + // TODO - this is a hack to avoid terrible performance - see todo in filterWorkflowNotifyEnabled + const refData = await loadUserScopeReferenceData(appState, userIds[0]) + const notifyEnabledUserIds = await filterRepoNotifyEnabled(appState, refData, userIds, push) + + await publishToSubscriptionsForUsers(appState, notifyEnabledUserIds, { + title: 'New Push', + body: `New push to ${push.repoName}:${push.ref} by ${push.actor.userName}`, + data: { url: push.repoUrl ?? '' } + }) +} diff --git a/src/app/domain/webPush/webPushEventBridgeEventProcessor.ts b/src/app/domain/webPush/webPushEventBridgeEventProcessor.ts index 152ab42..8f07640 100644 --- a/src/app/domain/webPush/webPushEventBridgeEventProcessor.ts +++ b/src/app/domain/webPush/webPushEventBridgeEventProcessor.ts @@ -1,8 +1,12 @@ import { AppState } from '../../environment/AppState' import { EventBridgeEvent } from 'aws-lambda' -import { EVENTBRIDGE_DETAIL_TYPES, EventBridgeDetailType } from '../../../multipleContexts/eventBridge' +import { + EVENTBRIDGE_DETAIL_TYPES, + isWebPushEventBridgeDetailType, + WebPushEventBridgeDetailType +} from '../../../multipleContexts/eventBridge' import { logger } from '../../util/logging' -import { handleNewWorkflowRunEvent } from './cicadaEventWebPushPublisher' +import { handleNewPush, handleNewWorkflowRunEvent } from './cicadaEventWebPushPublisher' import { handleWebPushTest } from './webPushUserTest' export async function processEventBridgeWebPushEvent( @@ -18,18 +22,9 @@ export async function processEventBridgeWebPushEvent( await webPushEventProcessors[detailType](appState, event.detail) } -// We don't necessarily listen for all event bridge events -type WebPushEventBridgeDetailType = Extract< - EventBridgeDetailType, - 'GithubNewWorkflowRunEvent' | 'WebPushTest' -> - -function isWebPushEventBridgeDetailType(x: unknown): x is WebPushEventBridgeDetailType { - return typeof x === 'string' && (x === 'GithubNewWorkflowRunEvent' || x === 'WebPushTest') -} - const webPushEventProcessors: Record = { [EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_WORKFLOW_RUN_EVENT]: handleNewWorkflowRunEvent, + [EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_PUSH]: handleNewPush, [EVENTBRIDGE_DETAIL_TYPES.WEB_PUSH_TEST]: handleWebPushTest } diff --git a/src/cdk/stacks/main/userFacingWeb.ts b/src/cdk/stacks/main/userFacingWeb.ts index bac73b6..40a9846 100644 --- a/src/cdk/stacks/main/userFacingWeb.ts +++ b/src/cdk/stacks/main/userFacingWeb.ts @@ -4,7 +4,7 @@ import { Duration } from 'aws-cdk-lib' import { grantLambdaFunctionPermissionToPutEvents } from '../../support/eventbridge' import { Rule } from 'aws-cdk-lib/aws-events' import * as targets from 'aws-cdk-lib/aws-events-targets' -import { EVENTBRIDGE_DETAIL_TYPES } from '../../../multipleContexts/eventBridge' +import { WEBPUSH_EVENTBRIDGE_DETAIL_TYPES } from '../../../multipleContexts/eventBridge' import { IdentitySource, LambdaIntegration, RequestAuthorizer, RestApi } from 'aws-cdk-lib/aws-apigateway' import { HttpMethod } from 'aws-cdk-lib/aws-apigatewayv2' import { MainStackProps } from './mainStackProps' @@ -102,11 +102,8 @@ function defineWebPushPublisher(scope: Construct, props: UserFacingWebEndpointsP description: `Send Web Push notifications`, eventPattern: { source: [props.appName], - detailType: [ - EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_WORKFLOW_RUN_EVENT, - EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_PUSH, - EVENTBRIDGE_DETAIL_TYPES.WEB_PUSH_TEST - ] + // Need Array.from here otherwise a type error + detailType: Array.from(WEBPUSH_EVENTBRIDGE_DETAIL_TYPES) }, targets: [new targets.LambdaFunction(lambdaFunction)] }) diff --git a/src/multipleContexts/eventBridge.ts b/src/multipleContexts/eventBridge.ts index 5fff782..c77622d 100644 --- a/src/multipleContexts/eventBridge.ts +++ b/src/multipleContexts/eventBridge.ts @@ -1,4 +1,5 @@ // See comment at WebPushEventBridgeDetailTypes if adding more types here + export const EVENTBRIDGE_DETAIL_TYPES = { GITHUB_NEW_PUSH: 'GithubNewPush', GITHUB_NEW_WORKFLOW_RUN_EVENT: 'GithubNewWorkflowRunEvent', @@ -13,3 +14,15 @@ export type EventBridgeDetailType = (typeof EVENTBRIDGE_DETAIL_TYPES)[keyof type export function isEventBridgeDetailType(x: unknown): x is EventBridgeDetailType { return typeof x === 'string' && Object.values(EVENTBRIDGE_DETAIL_TYPES).includes(x as EventBridgeDetailType) } + +export const WEBPUSH_EVENTBRIDGE_DETAIL_TYPES = [ + EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_PUSH, + EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_WORKFLOW_RUN_EVENT, + EVENTBRIDGE_DETAIL_TYPES.WEB_PUSH_TEST +] as const + +export type WebPushEventBridgeDetailType = (typeof WEBPUSH_EVENTBRIDGE_DETAIL_TYPES)[number] + +export function isWebPushEventBridgeDetailType(x: unknown): x is WebPushEventBridgeDetailType { + return typeof x === 'string' && WEBPUSH_EVENTBRIDGE_DETAIL_TYPES.includes(x as WebPushEventBridgeDetailType) +}