modify code based on code review

Author: m0nggh

packages/backend/.env-example

@@ -32,6 +32,7 @@ M365_SG_GOVT_ALLOWED_SENSITIVITY_LABEL_GUIDS_CSV=11111111-1111-1111-1111-1111111
 LAUNCH_DARKLY_SDK_KEY=...
 MAX_JOB_ATTEMPTS=3
 POSTMAN_SMS_QPS_LIMIT_PER_CAMPAIGN=3
+ONBOARDING_EMAIL_WEBHOOK_URL=test-webhook-url
 
 # LOCAL DEV
 SERVE_WEB_APP_SEPARATELY=true

packages/backend/src/graphql/__tests__/mutations/login-with-selected-sgid.test.ts

@@ -83,7 +83,7 @@ describe('Login with selected SGID', () => {
     expect(mocks.getOrCreateUser).toHaveBeenCalledWith(
       '[email protected]',
     )
-    expect(mocks.sendOnboardingEmail).toHaveBeenCalledWith('abc-def')
+    expect(mocks.sendOnboardingEmail).toHaveBeenCalledWith({ id: 'abc-def' })
     expect(mocks.updateLastLogin).toHaveBeenCalledWith('abc-def')
     expect(mocks.setAuthCookie).toHaveBeenCalledWith(expect.anything(), {
       userId: 'abc-def',

packages/backend/src/graphql/__tests__/mutations/login-with-sgid.test.ts

@@ -93,7 +93,7 @@ describe('Login with SGID', () => {
     expect(mocks.getOrCreateUser).toHaveBeenCalledWith(
       '[email protected]',
     )
-    expect(mocks.sendOnboardingEmail).toHaveBeenCalledWith('abc-def')
+    expect(mocks.sendOnboardingEmail).toHaveBeenCalledWith({ id: 'abc-def' })
     expect(mocks.updateLastLogin).toHaveBeenCalledWith('abc-def')
     expect(mocks.setAuthCookie).toHaveBeenCalledWith(expect.anything(), {
       userId: 'abc-def',
@@ -184,7 +184,7 @@ describe('Login with SGID', () => {
     const result = await loginWithSgid(null, STUB_PARAMS, STUB_CONTEXT)
 
     expect(mocks.getOrCreateUser).toHaveBeenCalledWith('[email protected]')
-    expect(mocks.sendOnboardingEmail).toHaveBeenCalledWith('abc-def')
+    expect(mocks.sendOnboardingEmail).toHaveBeenCalledWith({ id: 'abc-def' })
     expect(mocks.updateLastLogin).toHaveBeenCalledWith('abc-def')
     expect(mocks.setAuthCookie).toHaveBeenCalledWith(expect.anything(), {
       userId: 'abc-def',

packages/backend/src/graphql/mutations/login-with-selected-sgid.ts

@@ -49,7 +49,7 @@ const loginWithSelectedSgid: MutationResolvers['loginWithSelectedSgid'] =
     }
 
     const user = await getOrCreateUser(workEmail)
-    await sendOnboardingEmail(user.id)
+    await sendOnboardingEmail(user)
     await updateLastLogin(user.id)
     setAuthCookie(context.res, { userId: user.id })
 

packages/backend/src/graphql/mutations/login-with-sgid.ts

@@ -134,7 +134,7 @@ const loginWithSgid: MutationResolvers['loginWithSgid'] = async (
   // Log user in directly if there is only 1 employment.
   if (publicOfficerEmployments.length === 1) {
     const user = await getOrCreateUser(publicOfficerEmployments[0].workEmail)
-    await sendOnboardingEmail(user.id)
+    await sendOnboardingEmail(user)
     await updateLastLogin(user.id)
     setAuthCookie(context.res, { userId: user.id })
 

packages/backend/src/graphql/mutations/verify-otp.ts

@@ -51,7 +51,7 @@ const verifyOtp: MutationResolvers['verifyOtp'] = async (
   ) {
     throw new BaseError('Invalid OTP')
   }
-  await sendOnboardingEmail(user.id)
+  await sendOnboardingEmail(user)
 
   // reset otp columns
   await user.$query().patch({

packages/backend/src/helpers/__tests__/auth.test.ts

@@ -229,31 +229,17 @@ describe('Auth helpers', () => {
     afterEach(() => {
       vi.restoreAllMocks()
     })
-
-    it('throws error with no user id', async () => {
-      await expect(sendOnboardingEmail('')).rejects.toThrowError(
-        'User id required',
-      )
-    })
-
-    it('throws error with non-existent user id', async () => {
-      mocks.findById.mockResolvedValueOnce(null)
-      await expect(sendOnboardingEmail('non-existent-id')).rejects.toThrowError(
-        'User not found',
-      )
-    })
-
     it('does not send email if user has logged in before', async () => {
       const mockUser = {
         id: 'test-id',
         email: '[email protected]',
         lastLoginAt: new Date(),
         createdAt: new Date(),
-      }
+      } as unknown as User
 
       mocks.findById.mockResolvedValueOnce(mockUser)
 
-      await sendOnboardingEmail(mockUser.id)
+      await sendOnboardingEmail(mockUser)
       expect(axios.post).not.toHaveBeenCalled()
     })
 
@@ -262,11 +248,11 @@ describe('Auth helpers', () => {
         id: 'test-id',
         email: '[email protected]',
         createdAt: new Date('2024-01-01'), // Before release date
-      }
+      } as unknown as User
 
       mocks.findById.mockResolvedValueOnce(mockUser)
 
-      await sendOnboardingEmail(mockUser.id)
+      await sendOnboardingEmail(mockUser)
       expect(axios.post).not.toHaveBeenCalled()
     })
 
@@ -275,11 +261,11 @@ describe('Auth helpers', () => {
         id: 'test-id',
         email: '[email protected]',
         createdAt: new Date('2025-03-01'), // After release date
-      }
+      } as unknown as User
 
       mocks.findById.mockResolvedValueOnce(mockUser)
 
-      await sendOnboardingEmail(mockUser.id)
+      await sendOnboardingEmail(mockUser)
       expect(axios.post).not.toHaveBeenCalled()
     })
 
@@ -296,7 +282,7 @@ describe('Auth helpers', () => {
       mocks.findById.mockResolvedValueOnce(mockUser)
       vi.mocked(axios.post).mockResolvedValueOnce({ data: {} })
 
-      await sendOnboardingEmail(mockUser.id)
+      await sendOnboardingEmail(mockUser)
 
       expect(axios.post).toHaveBeenCalledWith(
         appConfig.onboardingEmailWebhookUrl,
@@ -319,7 +305,7 @@ describe('Auth helpers', () => {
 
       mocks.findById.mockResolvedValueOnce(mockUser)
 
-      await sendOnboardingEmail(mockUser.id)
+      await sendOnboardingEmail(mockUser)
       expect(axios.post).not.toHaveBeenCalled()
     })
   })

packages/backend/src/helpers/auth.ts

@@ -63,16 +63,7 @@ export async function getOrCreateUser(email: string): Promise<User> {
   return user
 }
 
-export async function sendOnboardingEmail(id: string) {
-  if (!id) {
-    throw new Error('User id required!')
-  }
-
-  const user = await User.query().findById(id)
-  if (!user) {
-    throw new Error('User not found!')
-  }
-
+export async function sendOnboardingEmail(user: User) {
   // check if user has logged in before and has been created
   // after the specified date for the release of onboarding email
   if (
@@ -82,7 +73,7 @@ export async function sendOnboardingEmail(id: string) {
     return
   }
   // call plumber webhook to send onboarding email only in prod
-  if (appConfig.isProd && appConfig.onboardingEmailWebhookUrl) {
+  if (!appConfig.isDev && appConfig.onboardingEmailWebhookUrl) {
     await axios.post(appConfig.onboardingEmailWebhookUrl, {
       email: user.email,
     })