Re-enable web push for github push events

Author: mikebroberts

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<GithubPush, 'commits'>) {
   return push.commits[push.commits.length - 1]
 }
+
+export async function getRelatedMemberIdsForPush(
+  appState: AppState,
+  push: GithubPush
+): Promise<GithubUserId[]> {
+  return getUserIdsForAccount(appState, push.accountId)
+}

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<GithubUserId[]> {
+  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
+}

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 ?? '' }
+  })
+}

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<WebPushEventBridgeDetailType, WebPushEventProcessor> = {
   [EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_WORKFLOW_RUN_EVENT]: handleNewWorkflowRunEvent,
+  [EVENTBRIDGE_DETAIL_TYPES.GITHUB_NEW_PUSH]: handleNewPush,
   [EVENTBRIDGE_DETAIL_TYPES.WEB_PUSH_TEST]: handleWebPushTest
 }
 

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)]
   })

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)
+}