Feature/poly 12 inbox query - #25
Taye-Staats wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthroughAdds a Convex-backed messaging feature (schema, mutations/queries, handler), tests and a Convex server mock, and updates Jest/Babel/TypeScript configs to enable backend testing with ESM/TypeScript support. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Convex as ConvexHandler
participant DB as ConvexDB
Client->>Convex: listUserConversations(limit, cursor)
Convex->>DB: query conversationParticipants by_user_lastActivityAt (userId, cursor, limit)
DB-->>Convex: participantRows[]
loop per participantRow
Convex->>DB: query conversations by id
DB-->>Convex: conversation
Convex->>DB: query messages by_conversation (limit=1 desc)
DB-->>Convex: lastMessage
Convex->>DB: read participantRow.unreadCount or compute unread messages
DB-->>Convex: unreadCount
Convex-->>Client: conversation preview (otherUserId, lastMessagePreview, unreadCount, lastMessageAt, ...)
end
alt more results
Convex-->>Client: nextCursor
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/convex/__mocks__/server.js`:
- Around line 4-27: The mock wrappers for query, mutation, and action drop
additional handler parameters by only accepting and forwarding a single "args"
argument; update each factory (query, mutation, action) to accept variadic
arguments (e.g., use (...args) in the returned async function) and call
config.handler with those same arguments (pass through all received args) when
config.handler exists, keeping the existing fallback return of config otherwise;
reference the factory names query/mutation/action and the config.handler check
to locate the changes.
In `@backend/convex/messages.ts`:
- Around line 84-90: The API declares a cursor in listUserConversations but the
handler listUserConversationsHandler ignores it and always returns nextCursor:
null; either implement cursor-based pagination or remove the param to avoid a
misleading API. Fix option A (implement): update listUserConversationsHandler to
accept args.cursor, use it to filter/seek results (e.g., fetch conversations
with lastMessageAt < cursor or > depending on sort), apply args.limit (+1 to
detect more), slice to limit, and set nextCursor to the last returned
conversation's lastMessageAt (or null if no more). Fix option B (remove): remove
the cursor arg from the listUserConversations args and from the handler
signature and ensure nextCursor remains omitted or always null. Ensure
references to cursor and nextCursor are consistent across listUserConversations
and listUserConversationsHandler.
- Around line 4-21: The sendMessage mutation currently trusts the
client-supplied senderId and doesn't update conversation metadata; change it to
use ctx.auth.getUserIdentity() to obtain the true sender, remove or ignore the
args.senderId value, fetch the conversation by args.conversationId and verify
that the authenticated user's id is in the conversation.participantIds (throw an
error if not), then insert the message using the authenticated id and after
insertion update the conversation row's lastMessageAt, lastMessageId and
updatedAt fields with the new message's timestamp and id; reference the
sendMessage handler, ctx.auth.getUserIdentity(), participantIds, and the
conversations' lastMessageAt/lastMessageId/updatedAt when making these changes.
In `@jest.config.js`:
- Around line 9-22: The PR clears Jest's default node_modules ignore by setting
testPathIgnorePatterns: [], which removes the safety boundary; restore the
default behavior by either removing the testPathIgnorePatterns line or setting
testPathIgnorePatterns to include '/node_modules/' so Jest ignores node_modules
again (aligning with coveragePathIgnorePatterns and existing testMatch). Locate
the testPathIgnorePatterns entry in jest.config.js and update or delete it
accordingly.
🧹 Nitpick comments (3)
backend/convex/schema.ts (1)
35-46: Add participant lookup indexes for conversation queries.Lines 35–46 define
buyerId/sellerIdbut there’s no index to query by either, which forces full scans inlistUserConversations. Consider adding indexes likeby_buyerIdandby_sellerId(or a participant index if supported).♻️ Suggested index additions
conversations: defineTable({ listingId: v.id('listings'), buyerId: v.string(), sellerId: v.string(), participantIds: v.array(v.string()), lastMessageAt: v.optional(v.number()), lastMessageId: v.optional(v.id('messages')), createdAt: v.number(), updatedAt: v.number(), }) .index('by_listing_buyer_seller', ['listingId', 'buyerId', 'sellerId']) + .index('by_buyerId', ['buyerId']) + .index('by_sellerId', ['sellerId']) .index('by_updatedAt', ['updatedAt']),backend/convex/messages.ts (1)
33-61: Avoid full scans and per‑conversation message collects.Line 34 collects all conversations, then each conversation performs two message queries, including a full
collect()for unread counting. This is O(N) + N×M and won’t scale. Prefer indexed queries by buyerId/sellerId (after adding indexes) and consider denormalizinglastMessagePreview/unreadCountat write time, or at least filtering unread messages server‑side before collecting.backend/jest.config.js (1)
5-8: Escape the leading dot in the moduleNameMapper regex.
^./_generated/server$matches any single character before the slash; escaping keeps it specific to./_generated/serverand avoids accidental matches.♻️ Suggested change
- '^./_generated/server$': '<rootDir>/convex/__mocks__/server.js', + '^\\./_generated/server$': '<rootDir>/convex/__mocks__/server.js',
jaydonkc
left a comment
jaydonkc
left a comment
There was a problem hiding this comment.
Please resolve the CodeRabbit warnings
SamanSP1386
left a comment
SamanSP1386
left a comment
There was a problem hiding this comment.
testPathIgnorePatterns: [] removes Jest’s default /node_modules/ ignore — probably best to drop that line or keep the default for consistency.
Also, we’re trusting senderId from the client when sending messages. We should derive it from ctx.auth.getUserIdentity(), verify the user is in participantIds, and update conversation metadata so inbox ordering stays correct.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/convex/messages.ts`:
- Around line 80-99: The loop over displayedConversations causes an N+1 query:
for each conversation you call ctx.db.query('messages') twice (producing lastMsg
and allMessages/unreadCount); fix by batching: collect all conversation IDs from
displayedConversations, run a single messages query to fetch relevant messages
(or at least the latest message per conversation and unread messages filtered by
senderId/read) and then compute lastMsg and unreadCount in-memory for each conv
(use the existing variables lastMsg/unreadCount names and the same
withIndex('by_conversation') pattern but with a single batched query), or
alternatively denormalize unreadCount onto the conversation document and update
it on send/read so the loop can read conv.unreadCount directly; update the code
around displayedConversations, lastMsg, allMessages, and unreadCount
accordingly.
- Around line 60-63: The current code does a full table scan via
ctx.db.query('conversations').collect() (allConversations -> conversations)
which loads every conversation into memory; instead create and use a Convex
index or a join table so filtering happens server-side: add an index on
participantIds (or create a conversation_participants table mapping
userId->conversationId), then query that index/table to fetch only conversations
for userId and perform the sort (by lastMessageAt || createdAt) in the DB query;
replace the use of ctx.db.query('conversations').collect() and subsequent
client-side .filter/.sort with a server-side indexed query that returns only the
needed conversation rows.
- Around line 66-71: The cursor parsing currently uses parseInt(args.cursor, 10)
without validating the result, so a non-numeric cursor yields NaN and breaks the
filter; update the code around parseInt(args.cursor, 10) (the cursorValue
variable) to validate the parsed value (e.g., const cursorValue =
Number.parseInt(args.cursor, 10); if (Number.isNaN(cursorValue)) { /* either
ignore the cursor filter or throw a clear validation error */ }) and only apply
the conversations = conversations.filter(...) when cursorValue is a valid
number, ensuring comparisons like (conv.lastMessageAt || conv.createdAt) <
cursorValue run with a numeric RHS.
In `@backend/jest.config.js`:
- Around line 6-9: The moduleNameMapper entry for the '^.+/_generated/server$'
pattern points to an incorrect path containing a duplicated backend/ prefix;
update the mapped path in backend/jest.config.js (the moduleNameMapper object)
so that '^.+/_generated/server$' resolves to the actual mock file at
'<rootDir>/convex/__mocks__/server.js' (remove the extra "backend/" segment) to
ensure Jest finds the mock.
🧹 Nitpick comments (3)
backend/convex/__mocks__/server.js (1)
28-31: Internal handler mocks don't wrap the handler like external ones do.
internalQuery,internalMutation,internalAction, andhttpActionreturnconfigas-is rather than wrappingconfig.handlerin an async function. This is fine if no tests currently exercise internal handlers, but it's worth noting the inconsistency for when they're needed later.backend/convex/messages.ts (2)
50-50: Handler usesanyfor bothctxandargs, losing all type safety.Since this handler is exported and shared between the query definition and tests, consider typing it with Convex's
QueryCtxand a proper args interface. This would catch misuse at compile time rather than at runtime.Sketch
-export const listUserConversationsHandler = async (ctx: any, args: any) => { +import { QueryCtx } from './_generated/server'; + +interface ListUserConversationsArgs { + limit?: number; + cursor?: string; +} + +export const listUserConversationsHandler = async ( + ctx: QueryCtx, + args: ListUserConversationsArgs +) => {
107-107:||fallback treats empty-string message body as "No messages yet".If
lastMsg.bodyis'', the||operator falls through to the default. Use??(nullish coalescing) if you want to preserve empty strings.- lastMessagePreview: lastMsg?.body || 'No messages yet', + lastMessagePreview: lastMsg?.body ?? 'No messages yet',
SamanSP1386
left a comment
SamanSP1386
left a comment
There was a problem hiding this comment.
There is some potential issues, take a look at them.
3 participants
Linked Issues
Closes # Sub Issue 6
Linear: (e.g., POLY-12)
Summary
Briefly explain the change and why.
Add listUserConversations() in backend/convex/messages.ts
Added Security to make sure the users can only access there messages
Calculates unread count
Shows last user message
How to Test
Steps to verify locally:
npm run lintnpm run typechecknpm testnpm run dev:backend(in terminal A)npm run dev(in terminal B)Checklist
npm run lint)devScreenshots / Demos
(if UI or visible behavior - attach images, videos, or GIFs)
Summary by CodeRabbit
New Features
Tests
Chores