-
Notifications
You must be signed in to change notification settings - Fork 14
fix: missing first name in team invite email #8600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a SenderWithId type, relaxes omitted sender fields to include Changes
Sequence Diagram(s)sequenceDiagram
participant JobProcessor as JobProcessor
participant UserService as UserService (GraphQL/Prisma)
participant EmailRenderer as EmailRenderer
participant MailQueue as MailQueue/SMTP
JobProcessor->>UserService: GET_USER(sender.id)
alt user found with firstName
UserService-->>JobProcessor: User { id, firstName, email[, imageUrl] }
JobProcessor->>EmailRenderer: render(template, sender=completeSender)
else user not found or firstName missing
UserService-->>JobProcessor: null or firstName=null
JobProcessor->>JobProcessor: derive firstName from sender.email local-part
JobProcessor->>EmailRenderer: render(template, sender=job-fallback-sender)
end
EmailRenderer-->>MailQueue: enqueue(email payload)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 467d0c7
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apis/api-journeys-modern/src/workers/email/service/service.ts`:
- Around line 95-101: The info log in the ultimate fallback for
completeSender.firstName leaks a full email; update the fallback in the code
that sets completeSender.firstName (the block using emailLocalPart derived from
completeSender.email) to avoid logging the full address—either remove the
${completeSender.email} from the message or log a redacted version (e.g., only
the local part or a masked email) and keep the rest of the behavior unchanged.
🧹 Nitpick comments (1)
apis/api-journeys-modern/src/workers/email/service/service.ts (1)
72-104: Tighten types for sender enrichment and avoidany.
Explicit sender types improve safety and clarify nullable fields. Also clone the sender to avoid mutatingjob.data.sender.♻️ Suggested refactor
+type Sender = { + id?: string | null + firstName?: string | null + lastName?: string | null + email?: string | null + imageUrl?: string | null +} + -async function getCompleteSenderData(jobSender: any): Promise<any> { - let completeSender = jobSender // fallback to job data +async function getCompleteSenderData(jobSender: Sender): Promise<Sender> { + let completeSender: Sender = { ...jobSender } // fallback to job dataAs per coding guidelines, define a type when possible.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apis/api-journeys-modern/src/workers/email/service/prisma.types.tsapis/api-journeys-modern/src/workers/email/service/service.spec.tsapis/api-journeys-modern/src/workers/email/service/service.tsapis/api-journeys/src/app/lib/prisma.types.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.resolver.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.spec.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.resolver.tsapis/api-journeys-modern/src/workers/email/service/prisma.types.tsapis/api-journeys-modern/src/workers/email/service/service.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.tsapis/api-journeys/src/app/lib/prisma.types.tsapis/api-journeys-modern/src/workers/email/service/service.spec.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.resolver.tsapis/api-journeys-modern/src/workers/email/service/prisma.types.tsapis/api-journeys-modern/src/workers/email/service/service.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.tsapis/api-journeys/src/app/lib/prisma.types.tsapis/api-journeys-modern/src/workers/email/service/service.spec.tsapis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.spec.ts
🧠 Learnings (7)
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/watch/test`.
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Enclose SWR-based hooks in `TestSWRConfig` (`apps/watch/test/TestSWRConfig.tsx`) to isolate cache state between assertions in tests.
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Use the shared Jest setup in `apps/watch/setupTests.tsx` which already boots MSW, Next router mock, and has a longer async timeout.
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/resources/test`
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
📚 Learning: 2025-10-07T02:24:21.928Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 7587
File: apis/api-users/src/schema/user/findOrFetchUser.ts:61-69
Timestamp: 2025-10-07T02:24:21.928Z
Learning: In the JesusFilm/core repository, the findOrFetchUser function in apis/api-users/src/schema/user/findOrFetchUser.ts intentionally uses lastName as a fallback for firstName when firstName is not available (firstName: ctxCurrentUser?.firstName ?? ctxCurrentUser?.lastName ?? 'Unknown User'), and leaves the lastName field blank in such cases. This is the expected business logic.
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
📚 Learning: 2025-09-29T23:03:36.840Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7629
File: apis/api-journeys-modern/src/schema/event/utils.ts:43-60
Timestamp: 2025-09-29T23:03:36.840Z
Learning: In the JesusFilm/core repository, do not recommend using Prisma's `upsert` operation for `JourneyVisitor` creation in `apis/api-journeys-modern/src/schema/event/utils.ts` as it is not race condition safe for this use case. The current `findUnique` then `create` pattern is the preferred approach.
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
📚 Learning: 2026-01-09T19:56:38.296Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 8574
File: apis/api-journeys-modern/src/schema/event/radioQuestion/radioQuestionSubmissionEventCreate.mutation.ts:6-6
Timestamp: 2026-01-09T19:56:38.296Z
Learning: In the JesusFilm/core repository, the `radioQuestionSubmissionEventCreate` mutation in `apis/api-journeys-modern/src/schema/event/radioQuestion/radioQuestionSubmissionEventCreate.mutation.ts` intentionally does not call `sendEventsEmail`, unlike `signUpSubmissionEventCreate` and `textResponseSubmissionEventCreate` mutations which do send email notifications.
Applied to files:
apis/api-journeys-modern/src/workers/email/service/service.spec.ts
🧬 Code graph analysis (4)
apis/api-journeys-modern/src/workers/email/service/prisma.types.ts (1)
apis/api-journeys/src/app/lib/prisma.types.ts (3)
TeamInviteAccepted(51-54)TeamWithUserTeam(67-71)TeamInviteJob(56-60)
apis/api-journeys-modern/src/workers/email/service/service.ts (1)
apis/api-journeys-modern/src/emails/templates/TeamInvite/TeamInviteNoAccount/TeamInviteNoAccount.tsx (1)
TeamInviteNoAccountEmail(36-88)
apis/api-journeys/src/app/lib/prisma.types.ts (1)
apis/api-journeys-modern/src/workers/email/service/prisma.types.ts (3)
TeamInviteAccepted(51-54)TeamWithUserTeam(67-71)TeamInviteJob(56-60)
apis/api-journeys-modern/src/workers/email/service/service.spec.ts (2)
apis/api-journeys-modern/src/workers/email/service/prisma.types.ts (1)
TeamInviteJob(56-60)apis/api-journeys/src/app/lib/prisma.types.ts (1)
TeamInviteJob(56-60)
🔇 Additional comments (16)
apis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.spec.ts (2)
38-38: Sender fixture now matches SenderWithId.
Looks aligned with the new sender payload shape used by the email queue.
80-80: Consistent sender id in accepted-invite fixture.
Good update to keep the fixture in sync with the new sender type.apis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.resolver.ts (2)
85-85: Keeping sender.id for invite email payload is appropriate.
This matches the SenderWithId shape needed downstream.
186-186: Keeping user.id for accepted-invite payload is appropriate.
Consistent with sender enrichment requirements.apis/api-journeys/src/app/lib/prisma.types.ts (1)
48-60: SenderWithId usage is consistent for invite payloads.
Good alignment of TeamInvite* sender types with the new shape.apis/api-journeys-modern/src/workers/email/service/prisma.types.ts (1)
48-60: SenderWithId adoption here matches the non-modern types.
Good consistency across the worker boundary.apis/api-journeys/src/app/modules/userTeamInvite/userTeamInvite.service.ts (2)
21-25: Sender type update looks correct for team invite.
Aligns with the updated sender shape.
42-45: Sender type update looks correct for invite-accepted email.
Consistent with downstream SenderWithId usage.apis/api-journeys-modern/src/workers/email/service/service.ts (2)
196-241: Sender enrichment wiring looks solid.
UsingcompleteSenderDatafor both account and no-account templates aligns with the fix.
278-323: Nice improvement to accepted-invite sender data.
Consistent enrichment + subject fallback reads well.apis/api-journeys-modern/src/workers/email/service/service.spec.ts (6)
2-2: No review comment needed here.
57-62: Fixture update is correct.
Addingsender.idaligns with the new sender shape.
87-92: Fixture update is correct.
sender.idaddition matches the updated types.
233-272: Mocks updated appropriately for sender enrichment.
The expanded Apollo responses cover both sender and recipient lookups.
284-338: Good coverage for email-prefix fallback.
This test directly validates the new behavior and guards the regression.
353-397: Invite-accepted tests updated consistently.
First-name additions keep the subject line expectations aligned.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…0-fix-firstname-team-invite' into jacobusbrink/nes-290-fix-firstname-team-invite
…0-fix-firstname-team-invite' into jacobusbrink/nes-290-fix-firstname-team-invite
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
Problem
Partial fix - NES-290
Sending an
invite-to-teamemail to a user that is authenticated with username and password (not gmail auth), results in the user receiving an email that is missing the first name of the inviter.e.g.
and should be
Cause
At the time the job for sending the team invite email is processed, the code extracts the
firstNamefrom the sender (person currently logged in) JWT token.Solution
Instead of extracting the first name from JWT, instead we get it from the database, where it exist already. (stored on user registration).
This results is a reliable first name retrieval.
Additionally, incase the first name was not stored in the database (historic behavior or missing input validation) - I've also introduces some fallback logic.
Fallback - when no first name exists in database, extract a value from the sender email.
e.g. jacobusB77@gmail.com
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.