Resolve messaging schema conflicts and finalize messaging data model#44
Resolve messaging schema conflicts and finalize messaging data model#44
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a nullable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![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)
backend/convex/messages.ts (1)
119-123:⚠️ Potential issue | 🟡 MinorDebug conversation creation uses
usersdocument IDs instead of auth subject strings.Line 121/122 take
v.id('users'), but conversation participant identity checks compare against authidentity.subject. This can create debug conversations that participants can’t access.Proposed fix
export const debugCreateConversationID = internalMutation({ args: { listingId: v.id('listings'), - buyerId: v.id('users'), - sellerId: v.id('users'), + buyerId: v.string(), + sellerId: v.string(), },Also applies to: 127-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/messages.ts` around lines 119 - 123, The args schema in backend/convex/messages.ts incorrectly uses v.id('users') for buyerId and sellerId (and the duplicate block at the later lines), but conversation participant checks compare against auth identity.subject strings; change those arg validators to accept the auth subject string (e.g., v.string()) and update any debug conversation creation call sites to pass identity.subject for buyerId/sellerId instead of user document IDs so the stored participant identifiers match identity.subject checks.
🧹 Nitpick comments (1)
backend/convex/messages.ts (1)
56-67: Constraintypeto a known set before persisting.Line 67 defaults correctly, but currently any arbitrary string passes through and is stored (Line 110). Add a strict allowlist (or validator union) to prevent data drift.
Proposed tightening
export const sendMessage = action({ args: { conversationId: v.id('conversations'), body: v.string(), type: v.optional(v.string()), }, handler: async (ctx, args): Promise<{ messageId: string }> => { @@ - const type = args.type ?? 'text'; + const type = args.type ?? 'text'; + const allowedTypes = new Set(['text']); + if (!allowedTypes.has(type)) { + throw new ConvexError('Invalid message type'); + } @@ const result = await ctx.runMutation(internal.messages.internalSendMessage, { @@ - type: type, + type, });Also applies to: 110-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/messages.ts` around lines 56 - 67, The handler currently accepts any args.type string and persists it; add a strict allowlist (e.g. const ALLOWED_MESSAGE_TYPES = ['text','image','system',...] ) and validate args.type against that before defaulting to 'text' and before saving, throwing a ConvexError on invalid types; update the type schema/validator (where v.optional(v.string()) is declared) to a union or enforce the same allowlist there so only permitted types are accepted and stored (reference: handler, args.type, ConvexError).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/convex/messages.ts`:
- Around line 285-305: The backfill currently loads entire conversations and
messages collections with ctx.db.query(...).collect() and patches them all at
once (variables: conversations, messages, conversationPatches, messagePatches,
ctx.db.patch), which is unbounded; change it to a bounded, rerunnable batching
loop: query each collection in fixed-size pages (e.g., limit N, use a
cursor/offset or last-id pagination) and process/patch only that page, commit
counts per page, and repeat until no more results; ensure each patch remains
idempotent by checking the same conditions (participantIds missing/invalid or
message.type missing) before patching so the job can be safely retried.
---
Outside diff comments:
In `@backend/convex/messages.ts`:
- Around line 119-123: The args schema in backend/convex/messages.ts incorrectly
uses v.id('users') for buyerId and sellerId (and the duplicate block at the
later lines), but conversation participant checks compare against auth
identity.subject strings; change those arg validators to accept the auth subject
string (e.g., v.string()) and update any debug conversation creation call sites
to pass identity.subject for buyerId/sellerId instead of user document IDs so
the stored participant identifiers match identity.subject checks.
---
Nitpick comments:
In `@backend/convex/messages.ts`:
- Around line 56-67: The handler currently accepts any args.type string and
persists it; add a strict allowlist (e.g. const ALLOWED_MESSAGE_TYPES =
['text','image','system',...] ) and validate args.type against that before
defaulting to 'text' and before saving, throwing a ConvexError on invalid types;
update the type schema/validator (where v.optional(v.string()) is declared) to a
union or enforce the same allowlist there so only permitted types are accepted
and stored (reference: handler, args.type, ConvexError).
|
Created clean companion branch Feedback:
|
jaydonkc
force-pushed
the
feature/POLY-38-resolve-messaging-schema-and-finalize-data-model
branch
from
29155c4 to
792754d
Compare
2 participants
Linked Issues
Closes #38
Linear: POLY-38 (e.g., POLY-123)
Summary
Resolved merge conflicts from cherry-picking
13ec9d7intofeature/POLY-38-resolve-messaging-schema-and-finalize-data-modeland established a single messaging model.Changes
backend/convex/schema.tsbackend/convex/messages.tsconversations.participantIdsconversations.lastMessageIdmessages.typereadAt) instead of boolean.buyerId,sellerId,senderId,recipientId) as auth subject strings.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
N/A
Summary by CodeRabbit