fix: empty conversations #423
Conversation
- Added support for initiating new conversations using recipient ID. - Updated ChatConversation component to handle both existing and new conversations. - Modified routing logic to differentiate between existing and new conversations. - Enhanced message sending logic to accommodate new conversation creation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds dual-mode chat creation/loading: routes can represent new conversations (new-{recipientId}) or existing ones; ChatConversation accepts optional conversationId and recipientId, fetches recipient profile for new chats, and sends the first message to create or reuse a conversation. Server sendMessage supports recipientId, creates/looks up conversation, and returns message with conversationId. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as pages/chat/index.vue
participant Router
participant CC as components/.../ChatConversation.vue
participant API as server/trpc/routers/profiles.ts
participant Chat as server/trpc/routers/chat.ts
participant SSE as SSE Broker
User->>UI: Select "Message" on a profile
UI->>Router: push /chat/new-{recipientId}
Router-->>CC: Mount with recipientId, conversationId = null
CC->>API: profiles.getById(recipientId)
API-->>CC: Minimal profile (id, name, photo, username)
User->>CC: Type + Send
CC->>Chat: sendMessage({ recipientId, content })
alt Existing conversation for pair
Chat->>Chat: Find by pairKey
else No existing conversation
Chat->>Chat: Create conversation (aId/bId order)
end
Chat->>Chat: Create message, update lastSeen/lastMessage
Chat-->>CC: message + conversationId
Chat-->>SSE: Publish conversation:{id} events
CC->>Router: push /chat/{conversationId}
sequenceDiagram
autonumber
participant CC as components/.../ChatConversation.vue
participant Chat as server/trpc/routers/chat.ts
participant SSE as SSE Broker
Note over CC: Existing conversation mode (conversationId present)
CC->>SSE: Subscribe conversation:{conversationId}
CC->>Chat: sendMessage({ conversationId, content })
Chat->>Chat: Validate membership, persist message, update lastSeen/lastMessage
Chat-->>CC: message + conversationId
Chat-->>SSE: Publish conversation:{conversationId} events
CC->>SSE: Reconnect only if conversationId exists
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
egemengunel
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat/ChatConversation.vue (1)
111-122: Close SSE when switching from an existing conversation to a new one.If
conversationIdbecomes null (navigating to/chat/new-…), the current watch doesn’t close the existing EventSource, leaking a live connection.Apply this diff to handle the null transition and clear timers:
watch( () => props.conversationId, (newId, oldId) => { - if (newId && newId !== oldId) { + if (newId && newId !== oldId) { isLoading.value = true conversation.value = null error.value = null subscribeToConversation(newId) markAsRead(newId) + } else if (!newId && oldId) { + // Transitioning to new conversation (no id): tear down SSE & timers + eventSource?.close() + eventSource = null + if (markAsReadTimer) { + clearTimeout(markAsReadTimer) + markAsReadTimer = null + } + if (reconnectTimer) { + clearTimeout(reconnectTimer) + reconnectTimer = null + } + conversation.value = null + error.value = null + isLoading.value = false } } )
🧹 Nitpick comments (11)
server/trpc/routers/profiles.ts (1)
172-183: Tighten input validation for getById to avoid empty/invalid IDs.Allowing an empty string or malformed ID through
.input(z.string())is easy to hit (e.g.,/chat/new-route edge). Enforce non-empty (or your canonical ID format) at the boundary.Apply this diff:
- getById: publicProcedure.input(z.string()).query(async ({ input }) => { + getById: publicProcedure.input(z.string().min(1)).query(async ({ input }) => {pages/chat/index.vue (1)
70-75: Close modal after successful navigation and URL‑encode the ID.If
router.pushfails, the modal currently closes, losing user context. Also encode the ID defensively.- // Navigate to new conversation URL instead of creating empty conversation - showNewConversationModal.value = false - await router.push(`/chat/new-${selectedUser.value.id}`) + // Navigate to new conversation URL instead of creating empty conversation + await router.push(`/chat/new-${encodeURIComponent(selectedUser.value.id)}`) + showNewConversationModal.value = falsecomposables/useContact.ts (1)
33-33: Encode the profile ID in the navigation URL.Defensive against unexpected characters; safe no‑op for cuid/uuid.
- await navigateTo(`/chat/new-${profile.id}`) + await navigateTo(`/chat/new-${encodeURIComponent(profile.id)}`)pages/chat/[id].vue (1)
30-37: Guard against “new-” with missing recipient ID.
substring(4)can yield an empty string; that flows to the API. Returnnullwhen recipientId is empty.-const recipientId = computed(() => { - const id = route.params.id as string - // Extract recipient ID from "new-{recipientId}" format - if (id.startsWith('new-')) { - return id.substring(4) // Remove "new-" prefix - } - return null -}) +const recipientId = computed(() => { + const id = route.params.id as string + if (!id.startsWith('new-')) return null + const rec = id.slice(4) // Remove "new-" prefix + return rec.length > 0 ? rec : null +})schemas/chat.ts (1)
63-72: Reject empty IDs and surface path‑specific validation errors (better DX).Adds
.min(1)and usessuperRefineto annotate both fields when XOR fails.-export const sendMessageSchema = z - .object({ - conversationId: z.string().optional(), - recipientId: z.string().optional(), - content: contentSchema, - }) - .refine( - (data) => !!data.conversationId !== !!data.recipientId, - 'Either conversationId or recipientId must be provided, but not both.' - ) +export const sendMessageSchema = z + .object({ + conversationId: z.string().min(1).optional(), + recipientId: z.string().min(1).optional(), + content: contentSchema, + }) + .superRefine((data, ctx) => { + const hasConversation = + typeof data.conversationId === 'string' && data.conversationId.length > 0 + const hasRecipient = + typeof data.recipientId === 'string' && data.recipientId.length > 0 + if (hasConversation === hasRecipient) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'Either conversationId or recipientId must be provided, but not both.', + path: ['conversationId'], + }) + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'Either conversationId or recipientId must be provided, but not both.', + path: ['recipientId'], + }) + } + })components/chat/ChatConversation.vue (2)
41-51: Prefer username fallback when name is null for new conversations.Avoids showing generic “User” when we already have a handle.
- () => { - if (!props.recipientId) return Promise.resolve(null) - return $client.profiles.getById.query(props.recipientId) - }, + () => { + if (!props.recipientId) return Promise.resolve(null) + return $client.profiles.getById.query(props.recipientId) + },And below in
otherParticipantobject, use username as a display fallback:- return { - id: recipientProfile.value.id, - name: recipientProfile.value.name, - photo: recipientProfile.value.photo, - } + return { + id: recipientProfile.value.id, + name: + recipientProfile.value.name ?? + (recipientProfile.value.username + ? `@${recipientProfile.value.username}` + : 'User'), + photo: recipientProfile.value.photo, + }
179-187: Don’t forcibly resubscribe a healthy SSE connection every 8s.Closing and reopening on
onopencan churn connections and duplicate events. Let the connection persist; rely ononerrorbackoff for recovery.eventSource.onopen = () => { connectionAttempts.value = 0 reconnectDelay.value = 2000 error.value = null - if (reconnectTimer) clearTimeout(reconnectTimer) - - reconnectTimer = setTimeout(() => { - if (eventSource && eventSource.readyState === 1 && props.conversationId) { - eventSource.close() - subscribeToConversation(props.conversationId) - } - }, 8000) + if (reconnectTimer) { + clearTimeout(reconnectTimer) + reconnectTimer = null + } }server/trpc/routers/chat.ts (4)
187-209: LGTM on using conversation.id; minor: reuse a single timestamp.The transactional message create + conversation update looks good and uses conversation.id consistently. For consistency and fewer clock reads, compute once.
- const createdMessage = await tx.message.create({ + const now = new Date() + const createdMessage = await tx.message.create({ data: { conversationId: conversation.id, senderId: me, content: input.content, }, include: { sender: { select: { id: true, name: true, photo: true } }, }, }) await tx.conversation.update({ where: { id: conversation.id }, data: { lastMessageId: createdMessage.id, - ...(conversation.aId === me - ? { aLastSeenAt: new Date() } - : { bLastSeenAt: new Date() }), + ...(conversation.aId === me + ? { aLastSeenAt: now } + : { bLastSeenAt: now }), }, })
218-235: Avoid shadowing “other”; optional: rename for clarity.There’s an “other” in the new‑conversation branch and another here. Renaming improves readability.
- const other = - conversation.aId === me ? conversation.bId : conversation.aId - publish(`inbox:${other}`, { + const recipientIdForInbox = + conversation.aId === me ? conversation.bId : conversation.aId + publish(`inbox:${recipientIdForInbox}`, { type: 'conversation.updated', conversationId: conversation.id, })
270-286: pairKey is unique in schema — findUnique is safe; optional: add rate limitingschema.prisma's Conversation model declares pairKey String @unique (and also @@unique([aId, bId])), so prisma.conversation.findUnique({ where: { pairKey } }) is appropriate.
- Optional: add a lightweight rate‑limit on server/trpc/routers/chat.ts → findConversationByUserId to protect against frequent UI calls.
23-26: Add a stable secondary sort for conversations (use updatedAt); indexes already presentSorting only by lastMessage.createdAt leaves threads with null lastMessage unordered — add updatedAt as a tie‑breaker.
Location: server/trpc/routers/chat.ts (orderBy clause)
- orderBy: { lastMessage: { createdAt: 'desc' } }, + orderBy: [ + { lastMessage: { createdAt: 'desc' } }, + { updatedAt: 'desc' } + ],Verified: schema.prisma Conversation model already has @@index([aId, updatedAt]) and @@index([bId, updatedAt]) — no index changes required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/chat/ChatConversation.vue(7 hunks)composables/useContact.ts(1 hunks)pages/chat/[id].vue(2 hunks)pages/chat/index.vue(1 hunks)schemas/chat.ts(1 hunks)server/trpc/routers/chat.ts(5 hunks)server/trpc/routers/profiles.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T06:33:48.474Z
Learnt from: egemengunel
PR: we-dance/v4#414
File: server/trpc/routers/chat.ts:24-35
Timestamp: 2025-09-09T06:33:48.474Z
Learning: In the WeDance chat system, lastMessageId and lastMessage fields were removed from the Conversation schema and should not be reintroduced. The team prefers to avoid schema changes for chat ordering improvements.
Applied to files:
schemas/chat.ts
🧬 Code graph analysis (2)
server/trpc/routers/profiles.ts (2)
server/trpc/init.ts (1)
publicProcedure(31-31)server/prisma.ts (1)
prisma(5-8)
server/trpc/routers/chat.ts (3)
server/prisma.ts (1)
prisma(5-8)server/utils/sse.ts (1)
publish(32-49)server/trpc/init.ts (1)
publicProcedure(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (1)
components/chat/ChatConversation.vue (1)
224-239: Type‑safety: ensure mutate return includes conversationId.If tRPC infers the return type, great. Otherwise, assert/derive the type to avoid runtime undefined access in
/chat/${result.conversationId}.If needed, narrow the result:
const result = await $client.chat.sendMessage.mutate({ recipientId: props.recipientId, content, }) // result as { conversationId: string }Can you confirm
sendMessage’s output shape is typed to includeconversationIdon the client?
| let conversation: { id: string; aId: string; bId: string } | ||
| if (input.conversationId) { | ||
| // Existing conversation | ||
| const conv = await prisma.conversation.findFirst({ | ||
| where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, | ||
| select: { id: true, aId: true, bId: true }, | ||
| }) | ||
| if (!conv) throw new TRPCError({ code: 'NOT_FOUND' }) | ||
| conversation = conv | ||
| } else { | ||
| // New conversation | ||
| const other = input.recipientId! | ||
| if (other === me) throw new TRPCError({ code: 'BAD_REQUEST' }) | ||
|
|
||
| // Verify recipient exists | ||
| const otherProfile = await prisma.profile.findUnique({ | ||
| where: { id: other }, | ||
| select: { id: true }, | ||
| }) | ||
| if (!otherProfile) { | ||
| throw new TRPCError({ code: 'NOT_FOUND' }) | ||
| } | ||
| // Check if conversation already exists | ||
| const key = pairKeyFor(me, other) | ||
| const existing = await prisma.conversation.findUnique({ | ||
| where: { pairKey: key }, | ||
| select: { id: true, aId: true, bId: true }, | ||
| }) | ||
|
|
||
| if (existing) { | ||
| conversation = existing | ||
| } else { | ||
| // Create new conversation | ||
| const [aId, bId] = me < other ? [me, other] : [other, me] | ||
| conversation = await prisma.conversation.create({ | ||
| data: { | ||
| pairKey: key, | ||
| aId, | ||
| bId, | ||
| ...(aId === me | ||
| ? { aLastSeenAt: new Date() } | ||
| : { bLastSeenAt: new Date() }), | ||
| }, | ||
| select: { id: true, aId: true, bId: true }, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Race on conversation creation can throw P2002; also risks empty conversations if message creation fails.
Two concurrent new-message requests to the same recipient can both miss the precheck and one will hit a unique constraint on pairKey. Additionally, creating the conversation outside the message transaction can leave an empty conversation if the message write fails—contradicting the “no empty chats” goal.
Minimal fix: catch P2002 and refetch.
- } else {
+ } else {
// Create new conversation
const [aId, bId] = me < other ? [me, other] : [other, me]
- conversation = await prisma.conversation.create({
- data: {
- pairKey: key,
- aId,
- bId,
- ...(aId === me
- ? { aLastSeenAt: new Date() }
- : { bLastSeenAt: new Date() }),
- },
- select: { id: true, aId: true, bId: true },
- })
+ try {
+ conversation = await prisma.conversation.create({
+ data: {
+ pairKey: key,
+ aId,
+ bId,
+ ...(aId === me
+ ? { aLastSeenAt: new Date() }
+ : { bLastSeenAt: new Date() }),
+ },
+ select: { id: true, aId: true, bId: true },
+ })
+ } catch (error) {
+ if (
+ error instanceof Prisma.PrismaClientKnownRequestError &&
+ error.code === 'P2002'
+ ) {
+ const winner = await prisma.conversation.findUnique({
+ where: { pairKey: key },
+ select: { id: true, aId: true, bId: true },
+ })
+ if (winner) {
+ conversation = winner
+ } else {
+ throw error
+ }
+ } else {
+ throw error
+ }
+ }
}Preferred follow‑up: make the whole “resolve conversation + create message + update lastSeen/lastMessageId” a single prisma.$transaction to guarantee atomicity and prevent empty threads. Also consider deprecating the public createConversation route if the UI no longer uses it.
Verify assumptions:
🏁 Script executed:
#!/bin/bash
# 1) Ensure pairKey is unique (required for P2002 behavior)
fd -a -i schema.prisma | xargs -I{} rg -n -C2 $'model Conversation\\b|pairKey|@unique|@@unique' {}
# 2) Check if any client still calls createConversation
rg -nP -C2 '\bcreateConversation\('
# 3) Confirm sendMessageSchema enforces XOR between conversationId and recipientId
rg -nP -C2 'sendMessageSchema|conversationId|recipientId' server schemasLength of output: 13755
Fix race on conversation creation (P2002) and prevent empty conversations
- Evidence: schema.prisma defines Conversation.pairKey as @unique (model Conversation, ~lines 656–663) -> concurrent creates can raise Prisma P2002. server/trpc/routers/chat.ts creates the conversation before the message transaction (conversation resolve/create at lines ~139–185; tx.message.create at ~188) — a failed message write can leave an empty conversation. pages/chat/index.vue (line ~64) still calls createConversation so the route can't be removed yet.
- Minimal fix: catch P2002 on create and refetch the winner. Apply the diff below.
- } else {
+ } else {
// Create new conversation
const [aId, bId] = me < other ? [me, other] : [other, me]
- conversation = await prisma.conversation.create({
- data: {
- pairKey: key,
- aId,
- bId,
- ...(aId === me
- ? { aLastSeenAt: new Date() }
- : { bLastSeenAt: new Date() }),
- },
- select: { id: true, aId: true, bId: true },
- })
+ try {
+ conversation = await prisma.conversation.create({
+ data: {
+ pairKey: key,
+ aId,
+ bId,
+ ...(aId === me
+ ? { aLastSeenAt: new Date() }
+ : { bLastSeenAt: new Date() }),
+ },
+ select: { id: true, aId: true, bId: true },
+ })
+ } catch (error) {
+ if (
+ error instanceof Prisma.PrismaClientKnownRequestError &&
+ error.code === 'P2002'
+ ) {
+ const winner = await prisma.conversation.findUnique({
+ where: { pairKey: key },
+ select: { id: true, aId: true, bId: true },
+ })
+ if (winner) {
+ conversation = winner
+ } else {
+ throw error
+ }
+ } else {
+ throw error
+ }
+ }
}- Preferred follow‑up (required): make "resolve conversation + create message + update lastSeen/lastMessageId" a single prisma.$transaction so conversation and message creation are atomic (change in server/trpc/routers/chat.ts sendMessage mutation).
📝 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.
| let conversation: { id: string; aId: string; bId: string } | |
| if (input.conversationId) { | |
| // Existing conversation | |
| const conv = await prisma.conversation.findFirst({ | |
| where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| if (!conv) throw new TRPCError({ code: 'NOT_FOUND' }) | |
| conversation = conv | |
| } else { | |
| // New conversation | |
| const other = input.recipientId! | |
| if (other === me) throw new TRPCError({ code: 'BAD_REQUEST' }) | |
| // Verify recipient exists | |
| const otherProfile = await prisma.profile.findUnique({ | |
| where: { id: other }, | |
| select: { id: true }, | |
| }) | |
| if (!otherProfile) { | |
| throw new TRPCError({ code: 'NOT_FOUND' }) | |
| } | |
| // Check if conversation already exists | |
| const key = pairKeyFor(me, other) | |
| const existing = await prisma.conversation.findUnique({ | |
| where: { pairKey: key }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| if (existing) { | |
| conversation = existing | |
| } else { | |
| // Create new conversation | |
| const [aId, bId] = me < other ? [me, other] : [other, me] | |
| conversation = await prisma.conversation.create({ | |
| data: { | |
| pairKey: key, | |
| aId, | |
| bId, | |
| ...(aId === me | |
| ? { aLastSeenAt: new Date() } | |
| : { bLastSeenAt: new Date() }), | |
| }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| } | |
| } | |
| let conversation: { id: string; aId: string; bId: string } | |
| if (input.conversationId) { | |
| // Existing conversation | |
| const conv = await prisma.conversation.findFirst({ | |
| where: { id: input.conversationId, OR: [{ aId: me }, { bId: me }] }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| if (!conv) throw new TRPCError({ code: 'NOT_FOUND' }) | |
| conversation = conv | |
| } else { | |
| // New conversation | |
| const other = input.recipientId! | |
| if (other === me) throw new TRPCError({ code: 'BAD_REQUEST' }) | |
| // Verify recipient exists | |
| const otherProfile = await prisma.profile.findUnique({ | |
| where: { id: other }, | |
| select: { id: true }, | |
| }) | |
| if (!otherProfile) { | |
| throw new TRPCError({ code: 'NOT_FOUND' }) | |
| } | |
| // Check if conversation already exists | |
| const key = pairKeyFor(me, other) | |
| const existing = await prisma.conversation.findUnique({ | |
| where: { pairKey: key }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| if (existing) { | |
| conversation = existing | |
| } else { | |
| // Create new conversation | |
| const [aId, bId] = me < other ? [me, other] : [other, me] | |
| try { | |
| conversation = await prisma.conversation.create({ | |
| data: { | |
| pairKey: key, | |
| aId, | |
| bId, | |
| ...(aId === me | |
| ? { aLastSeenAt: new Date() } | |
| : { bLastSeenAt: new Date() }), | |
| }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| } catch (error) { | |
| if ( | |
| error instanceof Prisma.PrismaClientKnownRequestError && | |
| error.code === 'P2002' | |
| ) { | |
| const winner = await prisma.conversation.findUnique({ | |
| where: { pairKey: key }, | |
| select: { id: true, aId: true, bId: true }, | |
| }) | |
| if (winner) { | |
| conversation = winner | |
| } else { | |
| throw error | |
| } | |
| } else { | |
| throw error | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In server/trpc/routers/chat.ts around lines 139–185, the code can race creating
a Conversation (Prisma P2002) and may leave empty conversations if a concurrent
create wins; update the conversation creation block to catch a Prisma unique
constraint error (P2002) when prisma.conversation.create fails, and in that case
refetch the conversation via findUnique({ where: { pairKey: key } }) and assign
that result to conversation; rethrow other errors. Keep the existing logic for
the non-race path unchanged. As a follow-up, wrap conversation resolve + message
create + lastSeen/lastMessageId updates in a single prisma.$transaction to make
the operation atomic.
The chat functionality doesn't create empty chats with this new implementation
Summary
This PR refactors the chat feature to fix an issue where empty conversations were being created in the database and displayed in the conversation list before any messages were sent.
Problem
Previously, a new
Conversationrecord was created in the database the moment a user clicked the "Message" button on a profile or in the "New Message" modal. If the user then navigated away without sending a message, this empty conversation would remain, cluttering both the database and the user's conversation list.Solution
The conversation creation logic has been moved from the initial "start conversation" action to the point where the first message is actually sent.
Here's how the new flow works:
Temporary Chat State: When a user initiates a new chat, they are now navigated to a temporary, client-side-only chat view (e.g.,
/chat/new-{recipientId}). No database entry is created at this stage.sendMessageMutation: ThesendMessagetRPC mutation has been enhanced to handle two scenarios:conversationIdis provided, it adds the message to the existing chat.recipientIdis provided (for a new chat), it performs the following operations:Conversationrecord.Messagerecord.Conversationwith thelastMessageId.Summary by CodeRabbit