feat(conversations): bulk spam/unspam and query optimizations#824
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR adds bulk spam marking and individual conversation deletion capabilities to the conversations feature. The changes include: new Convex mutations ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/conversations/components/conversations.tsx (1)
304-317:⚠️ Potential issue | 🟡 MinorUse spam-specific copy for the spam-tab bulk action.
When
status === 'spam', this control still announces itself asbulk.reopen, while the detail pane uses “Not spam” for the same transition. Reusing “Reopen” here makes the spam tab ambiguous and doesn't match the new bulk spam/unspam flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/conversations/components/conversations.tsx` around lines 304 - 317, The bulk action button uses the generic "bulk.reopen" text/aria-label/tooltip even when status === 'spam', causing ambiguity; update the Tooltip content, Button children text/icon label and aria-label to use a spam-specific translation key when status === 'spam' (e.g., use tConversations('bulk.not_spam') or similar) while keeping the existing tConversations('bulk.reopen') for other statuses; change the conditional logic around Tooltip content, aria-label and the rendered label/icon in the Button that wraps handleBulkReopen/isBulkProcessing so the UI and accessibility copy matches the spam tab flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/conversations/components/conversation-panel.tsx`:
- Around line 633-665: The delete button currently calls deleteConversation
immediately; change it to open the existing confirm-delete dialog pattern
instead and only call deleteConversation from that dialog’s onConfirm handler.
Replace the onClick in this Button (the destructive Button rendering
deleteConversation for conversationId via
toId<'conversations'>(conversation.id)) with code that launches the shared
confirm-delete dialog, and move the deleteConversation invocation (including the
toId conversion, the onSuccess toast + onSelectedConversationChange(null), and
the onError toast/console.error) into the dialog’s confirm callback; keep the
disabled state (isDeleting || isReopening) and button text logic unchanged so
the UI reflects in-progress status.
In `@services/platform/convex/conversations/mutations.ts`:
- Around line 153-161: The bulk mutation handlers (bulkSpamConversations and the
bulk-delete counterpart that calls ConversationsHelpers) accept only
conversationIds so they can't enforce org isolation; add an organizationId arg
to the mutation signatures (e.g. args.organizationId: v.id('organizations')) and
update calls into
ConversationsHelpers.bulkSpamConversations/bulkDeleteConversations to accept it,
then inside the helper(s) verify for each fetched conversation that
conversation.organizationId === args.organizationId and skip/reject any that do
not match before performing any patch/delete or building audit context; ensure
you return an error or omit mutated IDs for non-matching orgs to prevent
cross-organization writes.
In `@services/platform/convex/conversations/query_conversations.ts`:
- Around line 29-47: The current buildOrderedQuery falls back to an org-wide
lastMessageAt index when args.status is undefined, forcing non-status filters
(direction, channel, priority, customerId) to be applied in-memory and causing
pagination to truncate due to the 500-row scan limit; update buildOrderedQuery
to choose an indexed query path for those filters (e.g., use
withIndex('by_org_channel_lastMessageAt'),
withIndex('by_org_priority_lastMessageAt'), etc.) when the corresponding arg is
present and add .eq clauses for organizationId plus the filter field so the DB
can paginate efficiently, or if such *_lastMessageAt indexes do not exist, add
them to the schema before relying on the in-memory fallback.
---
Outside diff comments:
In `@services/platform/app/features/conversations/components/conversations.tsx`:
- Around line 304-317: The bulk action button uses the generic "bulk.reopen"
text/aria-label/tooltip even when status === 'spam', causing ambiguity; update
the Tooltip content, Button children text/icon label and aria-label to use a
spam-specific translation key when status === 'spam' (e.g., use
tConversations('bulk.not_spam') or similar) while keeping the existing
tConversations('bulk.reopen') for other statuses; change the conditional logic
around Tooltip content, aria-label and the rendered label/icon in the Button
that wraps handleBulkReopen/isBulkProcessing so the UI and accessibility copy
matches the spam tab flow.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 772f16d9-aa48-459c-9c82-12feb61eb9ba
⛔ Files ignored due to path filters (2)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**services/platform/convex/betterAuth/_generated/component.tsis excluded by!**/_generated/**
📒 Files selected for processing (15)
services/platform/app/features/conversations/components/conversation-list-panel.tsxservices/platform/app/features/conversations/components/conversation-panel.tsxservices/platform/app/features/conversations/components/conversations.tsxservices/platform/app/features/conversations/hooks/mutations.tsservices/platform/app/features/conversations/hooks/use-bulk-actions.tsservices/platform/convex/conversations/bulk_spam_conversations.test.tsservices/platform/convex/conversations/bulk_spam_conversations.tsservices/platform/convex/conversations/helpers.tsservices/platform/convex/conversations/list_conversations_paginated.test.tsservices/platform/convex/conversations/list_conversations_paginated.tsservices/platform/convex/conversations/mutations.tsservices/platform/convex/conversations/queries.tsservices/platform/convex/conversations/query_conversations.tsservices/platform/convex/conversations/schema.tsservices/platform/messages/en.json
| <Button | ||
| variant="destructive" | ||
| size="sm" | ||
| disabled={isDeleting || isReopening} | ||
| className="h-auto px-3 py-1 text-[13px]" | ||
| onClick={() => { | ||
| deleteConversation( | ||
| { | ||
| conversationId: toId<'conversations'>(conversation.id), | ||
| }, | ||
| { | ||
| onSuccess: () => { | ||
| toast({ | ||
| title: tConversations('panel.deleteSuccess'), | ||
| variant: 'success', | ||
| }); | ||
| onSelectedConversationChange(null); | ||
| }, | ||
| onError: (error) => { | ||
| console.error('Error deleting conversation:', error); | ||
| toast({ | ||
| title: tConversations('panel.deleteFailed'), | ||
| variant: 'destructive', | ||
| }); | ||
| }, | ||
| }, | ||
| ); | ||
| }} | ||
| > | ||
| {isDeleting | ||
| ? tConversations('panel.deleting') | ||
| : tConversations('panel.delete')} | ||
| </Button> |
There was a problem hiding this comment.
Add a confirmation step before deleting a conversation.
This button calls deleteConversation immediately from a single press. For a permanent delete, that makes accidental data loss too easy. Please route this through the existing confirm-delete dialog pattern before firing the mutation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/conversations/components/conversation-panel.tsx`
around lines 633 - 665, The delete button currently calls deleteConversation
immediately; change it to open the existing confirm-delete dialog pattern
instead and only call deleteConversation from that dialog’s onConfirm handler.
Replace the onClick in this Button (the destructive Button rendering
deleteConversation for conversationId via
toId<'conversations'>(conversation.id)) with code that launches the shared
confirm-delete dialog, and move the deleteConversation invocation (including the
toId conversion, the onSuccess toast + onSelectedConversationChange(null), and
the onError toast/console.error) into the dialog’s confirm callback; keep the
disabled state (isDeleting || isReopening) and button text logic unchanged so
the UI reflects in-progress status.
| export const bulkSpamConversations = mutationWithRLS({ | ||
| args: { | ||
| conversationIds: v.array(v.id('conversations')), | ||
| }, | ||
| returns: bulkOperationResultValidator, | ||
| handler: async (ctx, args) => { | ||
| return await ConversationsHelpers.bulkSpamConversations(ctx, args); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Scope bulk spam and delete to the active organization.
Both new write mutations only accept conversation IDs, so the implementation has no way to verify that every fetched record belongs to the current workspace before patch/delete. A user who belongs to multiple orgs can therefore submit IDs from another workspace, and bulk_spam_conversations.ts will even build its audit context from whichever conversation is found first. Add organizationId to both args and reject any fetched conversation whose organizationId does not match before mutating it.
Based on learnings: Enforce organization isolation for conversation write paths by verifying args.organizationId === conversation.organizationId before performing inserts or patches.
Also applies to: 173-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/conversations/mutations.ts` around lines 153 - 161,
The bulk mutation handlers (bulkSpamConversations and the bulk-delete
counterpart that calls ConversationsHelpers) accept only conversationIds so they
can't enforce org isolation; add an organizationId arg to the mutation
signatures (e.g. args.organizationId: v.id('organizations')) and update calls
into ConversationsHelpers.bulkSpamConversations/bulkDeleteConversations to
accept it, then inside the helper(s) verify for each fetched conversation that
conversation.organizationId === args.organizationId and skip/reject any that do
not match before performing any patch/delete or building audit context; ensure
you return an error or omit mutated IDs for non-matching orgs to prevent
cross-organization writes.
…ry optimizations - Add bulk spam/unspam mutations with organization isolation verification - Add delete confirmation dialog for spam conversations - Optimize conversation query with consolidated filtering - Use proper ConversationStatus type instead of string - Add spam status filter to conversation list
Israeltheminer
force-pushed
the
feat/bulk-spam-conversations
branch
from
611b345 to
304344c
Compare
1 participant
Summary
spamstatus to conversation schemalist_conversations_paginatedandquery_conversationsqueriesTest plan
bulk_spam_conversationslist_conversations_paginated🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores