feat: add journeyTransferFromAnonymous mutation to transfer journeys …#8946
Open
mikeallisonJS wants to merge 1 commit intomainfrom
Open
feat: add journeyTransferFromAnonymous mutation to transfer journeys …#8946mikeallisonJS wants to merge 1 commit intomainfrom
mikeallisonJS wants to merge 1 commit intomainfrom
Conversation
…from anonymous users - Introduced a new mutation `journeyTransferFromAnonymous` in the GraphQL schema to allow the transfer of journeys from anonymous users to a specified team. - Updated the schema and generated types to include the new mutation and its arguments. - Added corresponding tests to ensure the functionality works as expected. - Enhanced existing types with `updatedAt` fields for better tracking of changes.
Contributor
WalkthroughA new GraphQL mutation Changes
Sequence DiagramsequenceDiagram
actor User
participant GraphQL
participant Mutation as journeyTransferFromAnonymous<br/>(Resolver)
participant Prisma as Prisma<br/>(Database)
User->>GraphQL: journeyTransferFromAnonymous(journeyId, teamId?)
GraphQL->>Mutation: Execute with auth context
Mutation->>Mutation: Validate authenticated user
Mutation->>Prisma: Load journey + userJourneys + teams
Mutation->>Mutation: Validate journey exists & has owner
Mutation->>Mutation: Check if user already owns journey
alt User does not own journey
Mutation->>Mutation: Verify owner has no email (anonymous)
Mutation->>Mutation: Resolve destination teamId<br/>(provided → manager team → first team)
else User owns journey
Mutation->>Mutation: Use existing team or resolve
end
alt Team changed or ownership needs update
Mutation->>Prisma: Begin transaction
Mutation->>Prisma: Delete old userJourney records
Mutation->>Prisma: Create new owner userJourney<br/>(if user didn't own)
Mutation->>Prisma: Update journey.teamId
Mutation->>Mutation: Check if old team empty
alt Old team has no remaining journeys
Mutation->>Prisma: Delete userTeam rows for old team
Mutation->>Prisma: Delete team record
end
Mutation->>Prisma: Commit transaction
else Already on correct team & owned
Mutation->>Mutation: Return journey (no-op)
end
Mutation->>GraphQL: Return transferred Journey
GraphQL->>User: Journey
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 d695765
☁️ Nx Cloud last updated this comment at |
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.spec.ts (1)
126-326: Reduceanycasts in mocks to keep this spec type-safe.The repeated
as anyandfn: anypatterns can hide contract drift between resolver queries and fixtures.♻️ Suggested typed pattern
+type JourneyFindUniqueResult = Awaited< + ReturnType<typeof prismaMock.journey.findUnique> +> + +type TxCallback = Parameters<typeof prismaMock.$transaction>[0] + - prismaMock.journey.findUnique.mockResolvedValue(mockJourney as any) + prismaMock.journey.findUnique.mockResolvedValue( + mockJourney as JourneyFindUniqueResult + ) - prismaMock.$transaction.mockImplementation(async (fn: any) => fn(txMock)) + prismaMock.$transaction.mockImplementation(async (fn: TxCallback) => fn(txMock))As per coding guidelines:
**/*.{ts,tsx}: Define a type if possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.spec.ts` around lines 126 - 326, Tests use broad any casts (e.g., mockJourney as any, fn: any, txMock returned by makeTxMock), which weakens type safety; replace those with concrete types by typing fixtures and mocks to Prisma/GraphQL types and the transaction helper — e.g., type mockJourney and mockUserTeam as the appropriate Prisma model payloads, have makeTxMock return a typed TxMock interface, and type the $transaction implementation parameter as (tx: TxMock) => Promise<any> instead of any; update calls like prismaMock.journey.findUnique.mockResolvedValue(...) and mockPrismaUsers.user.findFirst.mockResolvedValue(...) to use the properly typed fixtures so the compiler catches contract drift.apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts (3)
140-142: Silent.catch(() => undefined)may mask unexpected errors.While catching errors during team deletion handles FK constraint violations gracefully, catching and ignoring all errors could mask unexpected database issues or misconfigurations.
Consider logging the error or being more selective about which errors to suppress.
💡 Suggested improvement
- await tx.team - .delete({ where: { id: oldTeamId } }) - .catch(() => undefined) + await tx.team + .delete({ where: { id: oldTeamId } }) + .catch((error) => { + // Expected: FK constraints if related records still exist + // Log unexpected errors for observability + if (!error.code || error.code !== 'P2003') { + // Consider logging: logger.warn({ error, oldTeamId }, 'Unexpected error deleting team') + } + })Prisma error code
P2003indicates foreign key constraint violation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts` around lines 140 - 142, The silent .catch on tx.team.delete({ where: { id: oldTeamId } }) hides all errors; change it to handle only foreign-key constraint errors (Prisma error code P2003) by detecting Prisma.PrismaClientKnownRequestError (e.g., instanceof Prisma.PrismaClientKnownRequestError and err.code === 'P2003') and swallow that case, but for any other error log it (use the existing logger such as processLogger or transaction logger) and rethrow so unexpected DB issues aren't masked. Ensure the fix references tx.team.delete and oldTeamId and imports Prisma types if needed.
48-59: Consider explicit handling when owner user record is missing.If
ownerUserisnull(user record not found in users DB), the current logic silently allows the transfer. While this may be intentional (treating missing users as anonymous), the implicit behavior could mask data inconsistencies.The anonymous check correctly uses
email != nullper the canonical definition inapis/api-users/src/schema/builder.ts.💡 Optional: Make missing user handling explicit
if (!alreadyOwns) { const ownerUser = await prismaUsers.user.findFirst({ where: { userId: ownerUserJourney.userId }, select: { email: true } }) - if (ownerUser != null && ownerUser.email != null) { + // If user record exists and has an email, they are not anonymous + if (ownerUser?.email != null) { throw new GraphQLError( 'Journey owner is not an anonymous user; transfer is not permitted', { extensions: { code: 'FORBIDDEN' } } ) } + // ownerUser == null or ownerUser.email == null means anonymous or missing user record }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts` around lines 48 - 59, The current transfer logic silently treats a missing user record as anonymous; update the check around prismaUsers.user.findFirst (used to fetch ownerUser for ownerUserJourney) to explicitly handle ownerUser === null: either reject the transfer with a GraphQLError (e.g., same FORBIDDEN code) or log/throw a distinct error indicating a missing owner record before allowing transfer; modify the block that uses alreadyOwns and ownerUser to explicitly branch on ownerUser === null (instead of implicitly permitting transfer) so the intent is clear and data inconsistencies are not silently ignored.
129-133: Type safety:resolvedTeamIdisstring | nullbut guaranteed non-null here.At line 132,
resolvedTeamIdhas typestring | null, but we've guaranteed it's non-null by the check at lines 99-103. Consider using a non-null assertion or restructuring to help TypeScript understand this invariant.💡 Optional: Use non-null assertion for clarity
const updated = await tx.journey.update({ ...query, where: { id: journeyId }, - data: { teamId: resolvedTeamId } + data: { teamId: resolvedTeamId! } })Alternatively, throw immediately after resolution and use
as string:// After line 97 if (resolvedTeamId == null) { throw new GraphQLError('No team found for the current user', { extensions: { code: 'BAD_REQUEST' } }) } const finalTeamId: string = resolvedTeamId // Use finalTeamId below🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts` around lines 129 - 133, The variable resolvedTeamId is typed as string | null but is guaranteed non-null by your earlier check; update the code so TypeScript knows it cannot be null before calling tx.journey.update. Either assert non-null when passing it (resolvedTeamId!) or assign it to a non-null typed constant (e.g., const finalTeamId: string = resolvedTeamId) after the null-check and use finalTeamId in the tx.journey.update call; reference resolvedTeamId and the tx.journey.update call to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts`:
- Around line 23-29: The Prisma query that fetches the journey via
prisma.journey.findUnique does not respect soft deletes; update the query
referenced (prisma.journey.findUnique used when resolving
journeyTransferFromAnonymous) to filter out deleted journeys by adding
deletedAt: null to the where clause (e.g. change to a composite where/AND or use
findFirst with where: { id: journeyId, deletedAt: null }) so transfers cannot
act on soft-deleted Journey records; keep the existing include (userJourneys,
team) unchanged.
---
Nitpick comments:
In
`@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.spec.ts`:
- Around line 126-326: Tests use broad any casts (e.g., mockJourney as any, fn:
any, txMock returned by makeTxMock), which weakens type safety; replace those
with concrete types by typing fixtures and mocks to Prisma/GraphQL types and the
transaction helper — e.g., type mockJourney and mockUserTeam as the appropriate
Prisma model payloads, have makeTxMock return a typed TxMock interface, and type
the $transaction implementation parameter as (tx: TxMock) => Promise<any>
instead of any; update calls like
prismaMock.journey.findUnique.mockResolvedValue(...) and
mockPrismaUsers.user.findFirst.mockResolvedValue(...) to use the properly typed
fixtures so the compiler catches contract drift.
In
`@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts`:
- Around line 140-142: The silent .catch on tx.team.delete({ where: { id:
oldTeamId } }) hides all errors; change it to handle only foreign-key constraint
errors (Prisma error code P2003) by detecting
Prisma.PrismaClientKnownRequestError (e.g., instanceof
Prisma.PrismaClientKnownRequestError and err.code === 'P2003') and swallow that
case, but for any other error log it (use the existing logger such as
processLogger or transaction logger) and rethrow so unexpected DB issues aren't
masked. Ensure the fix references tx.team.delete and oldTeamId and imports
Prisma types if needed.
- Around line 48-59: The current transfer logic silently treats a missing user
record as anonymous; update the check around prismaUsers.user.findFirst (used to
fetch ownerUser for ownerUserJourney) to explicitly handle ownerUser === null:
either reject the transfer with a GraphQLError (e.g., same FORBIDDEN code) or
log/throw a distinct error indicating a missing owner record before allowing
transfer; modify the block that uses alreadyOwns and ownerUser to explicitly
branch on ownerUser === null (instead of implicitly permitting transfer) so the
intent is clear and data inconsistencies are not silently ignored.
- Around line 129-133: The variable resolvedTeamId is typed as string | null but
is guaranteed non-null by your earlier check; update the code so TypeScript
knows it cannot be null before calling tx.journey.update. Either assert non-null
when passing it (resolvedTeamId!) or assign it to a non-null typed constant
(e.g., const finalTeamId: string = resolvedTeamId) after the null-check and use
finalTeamId in the tx.journey.update call; reference resolvedTeamId and the
tx.journey.update call to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3ef1cf00-3d05-4b11-88d9-eaafd5ea48a4
⛔ Files ignored due to path filters (2)
apis/api-journeys/src/__generated__/graphql.tsis excluded by!**/__generated__/**libs/shared/gql/src/__generated__/graphql-env.d.tsis excluded by!**/__generated__/**
📒 Files selected for processing (5)
apis/api-gateway/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys-modern/src/schema/journey/index.tsapis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.spec.tsapis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts
Comment on lines
+23
to
+29
| const journey = await prisma.journey.findUnique({ | ||
| where: { id: journeyId }, | ||
| include: { | ||
| userJourneys: true, | ||
| team: { include: { userTeams: true } } | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
Add deletedAt: null filter to respect soft deletes.
The journey fetch does not filter out soft-deleted journeys. This could allow transferring ownership of a deleted journey, which is likely unintended.
As per coding guidelines: "Always filter where: { deletedAt: null } in Prisma queries to respect soft deletes on blocks, journeys, and other entities."
🔧 Proposed fix
const journey = await prisma.journey.findUnique({
- where: { id: journeyId },
+ where: { id: journeyId, deletedAt: null },
include: {
userJourneys: true,
team: { include: { userTeams: true } }
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const journey = await prisma.journey.findUnique({ | |
| where: { id: journeyId }, | |
| include: { | |
| userJourneys: true, | |
| team: { include: { userTeams: true } } | |
| } | |
| }) | |
| const journey = await prisma.journey.findFirst({ | |
| where: { id: journeyId, deletedAt: null }, | |
| include: { | |
| userJourneys: true, | |
| team: { include: { userTeams: true } } | |
| } | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apis/api-journeys-modern/src/schema/journey/journeyTransferFromAnonymous.mutation.ts`
around lines 23 - 29, The Prisma query that fetches the journey via
prisma.journey.findUnique does not respect soft deletes; update the query
referenced (prisma.journey.findUnique used when resolving
journeyTransferFromAnonymous) to filter out deleted journeys by adding
deletedAt: null to the where clause (e.g. change to a composite where/AND or use
findFirst with where: { id: journeyId, deletedAt: null }) so transfers cannot
act on soft-deleted Journey records; keep the existing include (userJourneys,
team) unchanged.
Kneesal
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…from anonymous users
journeyTransferFromAnonymousin the GraphQL schema to allow the transfer of journeys from anonymous users to a specified team.updatedAtfields for better tracking of changes.Summary by CodeRabbit