feat(conversations): bulk archive/unarchive, read filter, and status banners#817
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 introduces conversation status management and bulk archive/unarchive operations across the platform. Changes include: removing an icon from the empty state component, adding an optional Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. Important Merge conflicts detected (Beta)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 9
🤖 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 355-359: The hidden-count is hardcoded as totalMessages - 2 but
the preview actually renders up to Math.min(2, lastDateGroupCount) messages from
the final date group; change collapsedHiddenCount to subtract the actual number
of messages rendered in the collapsed preview. Specifically, inspect
displayMessages/grouping logic used in the preview render (the same code that
renders the last date group), compute lastGroupCount (number of messages in the
final date group) and visibleInCollapsed = Math.min(2, lastGroupCount), then set
collapsedHiddenCount = totalMessages - visibleInCollapsed and use that value
wherever the CTA count is shown (replace the hardcoded subtraction near
collapsedHiddenCount and the similar occurrence around lines 400-407).
- Around line 89-90: The collapse state is persistent across conversation
switches; add logic to reinitialize it whenever selectedConversationId changes
by adding an effect that calls setIsThreadCollapsed(true) (or whatever the
default collapsed value should be) whenever selectedConversationId updates.
Locate the state variables isThreadCollapsed and setIsThreadCollapsed in
conversation-panel.tsx and add a useEffect(() => setIsThreadCollapsed(true),
[selectedConversationId]) to reset the UI per selectedConversationId.
In
`@services/platform/app/features/conversations/components/conversations-list.tsx`:
- Around line 261-263: Replace the hardcoded 'Unknown' fallback in the JSX
expression ({conversation.customer?.name || conversation?.title || 'Unknown'})
with the repository's shared i18n unknown label; import and use the existing
translation helper (e.g., t or useTranslation) and substitute the fallback with
the translated key (for example: {conversation.customer?.name ||
conversation?.title || t('shared.unknownLabel')}) inside the ConversationsList
component so the fallback is localized.
- Around line 265-267: The blue unread indicator (span rendered when
conversation.unread_count > 0) is currently purely visual and not exposed to
assistive tech; make the dot decorative by ensuring the span has
aria-hidden="true" (or equivalent) and update the conversation row/button
accessible name to include an "unread" hint when conversation.unread_count > 0
(e.g., append a visually-hidden/Screen Reader only text or include "unread" in
the aria-label for the element that represents the conversation row/button).
Target the span that checks conversation.unread_count > 0 and the parent
button/element that sets the accessible name to implement this change.
In `@services/platform/app/features/conversations/components/conversations.tsx`:
- Around line 277-287: The bulk "mark spam" icon is actionable but has no
onClick hookup; pull the spam handler from useBulkActions (e.g., const {
markSpam, isBulkProcessing } = useBulkActions()) and wire it to the Button's
onClick (onClick={() => markSpam(selectedIds)}), keeping the existing
disabled={isBulkProcessing} and aria-label usage; if the hook does not expose a
spam handler, conditionally hide the Tooltip/Button when markSpam is undefined
to avoid showing a non-functional control.
- Around line 190-215: The dropdown trigger is currently a plain div and is not
keyboard-focusable, preventing keyboard/AT users from opening the read filter;
replace the trigger div with a semantic, focusable element (e.g., a <button
type="button"> or the DropdownMenu-provided Trigger component) keeping the same
className logic (including the readFilter !== 'all' bg class), preserve the
inner stopPropagation handlers around the Checkbox (id "select-all",
selectAllChecked, onCheckedChange handleSelectAll) so checkbox clicks don't open
the menu, and add appropriate accessibility attributes (aria-haspopup="menu" and
aria-expanded bound to the menu state) so the DropdownMenu/trigger behavior
remains keyboard accessible.
In `@services/platform/convex/conversations/bulk_archive_conversations.ts`:
- Around line 8-16: The bulkArchiveConversations handler currently fetches
conversation rows and proceeds to build patches/audit entries without verifying
tenant ownership; update the function signature to accept args.organizationId
and, after loading each conversation from args.conversationIds (the array used
in ctx.db.get calls) validate that conversation.organizationId ===
args.organizationId and reject (throw) on any mismatch before constructing
patches or inserting audit rows (including where
firstValidConversation.organizationId is used); apply the same per-record org
check to the other similar blocks referenced (lines 24-44 and 65-84) so no
cross-org conversation is mutated or logged.
In `@services/platform/convex/conversations/bulk_unarchive_conversations.ts`:
- Around line 8-16: In bulkUnarchiveConversations, validate organization
ownership for each loaded conversation: after fetching conversations (the
conversations array) check that args.organizationId ===
conversation.organizationId for every conversation (and before using
firstValidConversation for logging or before any ctx.db.patch/insert), and
reject or skip any conversation IDs that belong to a different org (return a
failed BulkOperationResult or omit them and report them as errors) so no
cross-org records are modified or misattributed; apply the same per-conversation
org check before any subsequent patches/inserts in the function.
In `@services/platform/convex/conversations/mutations.ts`:
- Around line 122-129: The bulk conversation mutations lack org-scoped
authorization: add an organizationId arg to the mutation signatures (e.g., in
the mutationWithRLS for bulkArchiveConversations and the similar bulkUnarchive
mutation) and, inside the handler flow (and in the corresponding helper
functions ConversationsHelpers.bulkArchiveConversations and
ConversationsHelpers.bulkUnarchiveConversations), after fetching each
conversation via ctx.db.get(id) immediately validate that
conversation.organizationId === args.organizationId and reject (skip/throw) any
that do not match before queuing any patch or insert; ensure this check runs
before any database patch/insert or audit log enqueueing so no cross-org updates
or misattributed audit entries can occur.
🪄 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: 89168f92-9606-48a0-a0fc-1fdae778c8cc
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (12)
services/platform/app/features/conversations/components/activate-conversations-empty-state.tsxservices/platform/app/features/conversations/components/conversation-panel.tsxservices/platform/app/features/conversations/components/conversations-list.tsxservices/platform/app/features/conversations/components/conversations-skeleton.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_archive_conversations.tsservices/platform/convex/conversations/bulk_unarchive_conversations.tsservices/platform/convex/conversations/helpers.tsservices/platform/convex/conversations/mutations.tsservices/platform/messages/en.json
| const [isThreadCollapsed, setIsThreadCollapsed] = useState(true); | ||
|
|
There was a problem hiding this comment.
Reset the collapse state when the selected conversation changes.
isThreadCollapsed lives for the lifetime of the panel, so after a user expands one long thread, the next selected thread will stay expanded too. The new collapse UX should reinitialize per selectedConversationId.
♻️ Suggested fix
const [isThreadCollapsed, setIsThreadCollapsed] = useState(true);
+
+ useEffect(() => {
+ setIsThreadCollapsed(true);
+ }, [selectedConversationId]);📝 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.
| const [isThreadCollapsed, setIsThreadCollapsed] = useState(true); | |
| const [isThreadCollapsed, setIsThreadCollapsed] = useState(true); | |
| useEffect(() => { | |
| setIsThreadCollapsed(true); | |
| }, [selectedConversationId]); |
🤖 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 89 - 90, The collapse state is persistent across conversation
switches; add logic to reinitialize it whenever selectedConversationId changes
by adding an effect that calls setIsThreadCollapsed(true) (or whatever the
default collapsed value should be) whenever selectedConversationId updates.
Locate the state variables isThreadCollapsed and setIsThreadCollapsed in
conversation-panel.tsx and add a useEffect(() => setIsThreadCollapsed(true),
[selectedConversationId]) to reset the UI per selectedConversationId.
| const totalMessages = displayMessages.length; | ||
| const COLLAPSE_THRESHOLD = 4; | ||
| const showCollapse = totalMessages > COLLAPSE_THRESHOLD && isThreadCollapsed; | ||
| const collapsedHiddenCount = totalMessages - 2; | ||
|
|
There was a problem hiding this comment.
Derive the hidden-message count from the preview you actually render.
Line 358 hardcodes totalMessages - 2, but Lines 403-407 only render up to two messages from the last date group. When that final group contains one message, the CTA count is off by one because the UI subtracts two even though only one message remains visible.
Also applies to: 400-407
🤖 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 355 - 359, The hidden-count is hardcoded as totalMessages - 2 but
the preview actually renders up to Math.min(2, lastDateGroupCount) messages from
the final date group; change collapsedHiddenCount to subtract the actual number
of messages rendered in the collapsed preview. Specifically, inspect
displayMessages/grouping logic used in the preview render (the same code that
renders the last date group), compute lastGroupCount (number of messages in the
final date group) and visibleInCollapsed = Math.min(2, lastGroupCount), then set
collapsedHiddenCount = totalMessages - visibleInCollapsed and use that value
wherever the CTA count is shown (replace the hardcoded subtraction near
collapsedHiddenCount and the similar occurrence around lines 400-407).
| {conversation.customer?.name || | ||
| conversation?.title || | ||
| 'Unknown'} |
There was a problem hiding this comment.
Localize the fallback title instead of hardcoding English.
'Unknown' is now user-visible text inside a translated view. Please source this from i18n so non-English locales don't regress here. The repo already has shared unknown-label copy available.
🤖 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-list.tsx`
around lines 261 - 263, Replace the hardcoded 'Unknown' fallback in the JSX
expression ({conversation.customer?.name || conversation?.title || 'Unknown'})
with the repository's shared i18n unknown label; import and use the existing
translation helper (e.g., t or useTranslation) and substitute the fallback with
the translated key (for example: {conversation.customer?.name ||
conversation?.title || t('shared.unknownLabel')}) inside the ConversationsList
component so the fallback is localized.
| {conversation.unread_count > 0 && ( | ||
| <span className="size-1.5 shrink-0 rounded-full bg-blue-500" /> | ||
| )} |
There was a problem hiding this comment.
Expose unread state to assistive tech.
The new blue dot is purely visual, so screen-reader users can no longer tell which conversations are unread. Keep the dot decorative and add unread text to the button’s accessible name.
♿ Suggested fix
{conversation.unread_count > 0 && (
- <span className="size-1.5 shrink-0 rounded-full bg-blue-500" />
+ <>
+ <span
+ aria-hidden="true"
+ className="size-1.5 shrink-0 rounded-full bg-blue-500"
+ />
+ <span className="sr-only">{t('filter.unread')}</span>
+ </>
)}📝 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.
| {conversation.unread_count > 0 && ( | |
| <span className="size-1.5 shrink-0 rounded-full bg-blue-500" /> | |
| )} | |
| {conversation.unread_count > 0 && ( | |
| <> | |
| <span | |
| aria-hidden="true" | |
| className="size-1.5 shrink-0 rounded-full bg-blue-500" | |
| /> | |
| <span className="sr-only">{t('filter.unread')}</span> | |
| </> | |
| )} |
🤖 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-list.tsx`
around lines 265 - 267, The blue unread indicator (span rendered when
conversation.unread_count > 0) is currently purely visual and not exposed to
assistive tech; make the dot decorative by ensuring the span has
aria-hidden="true" (or equivalent) and update the conversation row/button
accessible name to include an "unread" hint when conversation.unread_count > 0
(e.g., append a visually-hidden/Screen Reader only text or include "unread" in
the aria-label for the element that represents the conversation row/button).
Target the span that checks conversation.unread_count > 0 and the parent
button/element that sets the accessible name to implement this change.
| <DropdownMenu | ||
| trigger={ | ||
| <div | ||
| className={cn( | ||
| 'flex shrink-0 items-center gap-0.5 rounded pr-1 py-0.5', | ||
| readFilter !== 'all' && 'bg-blue-100', | ||
| )} | ||
| > | ||
| {/* Prevent checkbox clicks from opening the dropdown */} | ||
| {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} | ||
| <div | ||
| onClick={(e) => e.stopPropagation()} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| onKeyDown={(e) => e.stopPropagation()} | ||
| > | ||
| <Checkbox | ||
| id="select-all" | ||
| checked={selectAllChecked} | ||
| onCheckedChange={handleSelectAll} | ||
| aria-label={tCommon('aria.selectAll')} | ||
| disabled={isLoading} | ||
| /> | ||
| </div> | ||
| <ChevronDownIcon className="text-muted-foreground size-3.5" /> | ||
| </div> | ||
| } |
There was a problem hiding this comment.
Use a keyboard-focusable trigger for the read filter.
Line 192 renders the dropdown trigger as a plain div, so the new filter control cannot be focused or opened from the keyboard. That blocks the read/unread filter for keyboard and assistive-tech users.
🤖 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 190 - 215, The dropdown trigger is currently a plain div and is not
keyboard-focusable, preventing keyboard/AT users from opening the read filter;
replace the trigger div with a semantic, focusable element (e.g., a <button
type="button"> or the DropdownMenu-provided Trigger component) keeping the same
className logic (including the readFilter !== 'all' bg class), preserve the
inner stopPropagation handlers around the Checkbox (id "select-all",
selectAllChecked, onCheckedChange handleSelectAll) so checkbox clicks don't open
the menu, and add appropriate accessibility attributes (aria-haspopup="menu" and
aria-expanded bound to the menu state) so the DropdownMenu/trigger behavior
remains keyboard accessible.
| {status === 'open' && ( | ||
| <Tooltip content={tConversations('bulk.markSpam')}> | ||
| <Button | ||
| size="icon" | ||
| variant="ghost" | ||
| disabled={isBulkProcessing} | ||
| aria-label={tConversations('bulk.markSpam')} | ||
| > | ||
| <ShieldXIcon className="size-4" /> | ||
| </Button> | ||
| </Tooltip> |
There was a problem hiding this comment.
Wire the bulk “mark spam” action or hide it.
Line 279 shows an enabled bulk spam action, but the button never gets an onClick, and this component does not receive any spam handler from useBulkActions. Right now the icon looks actionable but does nothing.
🤖 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 277 - 287, The bulk "mark spam" icon is actionable but has no
onClick hookup; pull the spam handler from useBulkActions (e.g., const {
markSpam, isBulkProcessing } = useBulkActions()) and wire it to the Button's
onClick (onClick={() => markSpam(selectedIds)}), keeping the existing
disabled={isBulkProcessing} and aria-label usage; if the hook does not expose a
spam handler, conditionally hide the Tooltip/Button when markSpam is undefined
to avoid showing a non-functional control.
| export async function bulkArchiveConversations( | ||
| ctx: MutationCtx, | ||
| args: { | ||
| conversationIds: Array<Id<'conversations'>>; | ||
| }, | ||
| ): Promise<BulkOperationResult> { | ||
| const conversations = await Promise.all( | ||
| args.conversationIds.map((id) => ctx.db.get(id)), | ||
| ); |
There was a problem hiding this comment.
Reject cross-org conversation IDs before archiving.
This helper archives every fetched conversation ID without checking that each record belongs to the caller’s organization. If a leaked ID from another org reaches this path, we will mutate it and then write the audit entry under firstValidConversation.organizationId, which is the wrong tenant context. Please pass organizationId through the mutation args and fail any mismatch before building patches.
Based on learnings: Enforce organization isolation for conversation write paths. After loading/fetching the conversation, verify that args.organizationId === conversation.organizationId before performing any inserts or patches.
Also applies to: 24-44, 65-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/conversations/bulk_archive_conversations.ts` around
lines 8 - 16, The bulkArchiveConversations handler currently fetches
conversation rows and proceeds to build patches/audit entries without verifying
tenant ownership; update the function signature to accept args.organizationId
and, after loading each conversation from args.conversationIds (the array used
in ctx.db.get calls) validate that conversation.organizationId ===
args.organizationId and reject (throw) on any mismatch before constructing
patches or inserting audit rows (including where
firstValidConversation.organizationId is used); apply the same per-record org
check to the other similar blocks referenced (lines 24-44 and 65-84) so no
cross-org conversation is mutated or logged.
| export async function bulkUnarchiveConversations( | ||
| ctx: MutationCtx, | ||
| args: { | ||
| conversationIds: Array<Id<'conversations'>>; | ||
| }, | ||
| ): Promise<BulkOperationResult> { | ||
| const conversations = await Promise.all( | ||
| args.conversationIds.map((id) => ctx.db.get(id)), | ||
| ); |
There was a problem hiding this comment.
Reject cross-org conversation IDs before unarchiving.
Like the archive helper, this mutation trusts raw conversation IDs and patches whatever records it can load, but it never verifies org ownership per conversation. A leaked ID from another tenant would be mutated here and then logged under firstValidConversation.organizationId, which misattributes the operation as well.
Based on learnings: Enforce organization isolation for conversation write paths. After loading/fetching the conversation, verify that args.organizationId === conversation.organizationId before performing any inserts or patches.
Also applies to: 24-43, 64-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/conversations/bulk_unarchive_conversations.ts`
around lines 8 - 16, In bulkUnarchiveConversations, validate organization
ownership for each loaded conversation: after fetching conversations (the
conversations array) check that args.organizationId ===
conversation.organizationId for every conversation (and before using
firstValidConversation for logging or before any ctx.db.patch/insert), and
reject or skip any conversation IDs that belong to a different org (return a
failed BulkOperationResult or omit them and report them as errors) so no
cross-org records are modified or misattributed; apply the same per-conversation
org check before any subsequent patches/inserts in the function.
| export const bulkArchiveConversations = mutationWithRLS({ | ||
| args: { | ||
| conversationIds: v.array(v.id('conversations')), | ||
| }, | ||
| returns: bulkOperationResultValidator, | ||
| handler: async (ctx, args) => { | ||
| return await ConversationsHelpers.bulkArchiveConversations(ctx, args); | ||
| }, |
There was a problem hiding this comment.
Restore org-scoped authorization on the new bulk mutations.
Both helpers patch whatever IDs are passed after ctx.db.get(id), but these mutation boundaries don't take organizationId, so they can't enforce the org match that other conversation write paths use. With mutationWithRLS, that means a caller can still archive/unarchive conversations from another organization they belong to, and the audit log will be attributed to whichever conversation is loaded first.
🔐 Suggested boundary fix
export const bulkArchiveConversations = mutationWithRLS({
args: {
+ organizationId: v.string(),
conversationIds: v.array(v.id('conversations')),
},
returns: bulkOperationResultValidator,
handler: async (ctx, args) => {
return await ConversationsHelpers.bulkArchiveConversations(ctx, args);
},
});
export const bulkUnarchiveConversations = mutationWithRLS({
args: {
+ organizationId: v.string(),
conversationIds: v.array(v.id('conversations')),
},
returns: bulkOperationResultValidator,
handler: async (ctx, args) => {
return await ConversationsHelpers.bulkUnarchiveConversations(ctx, args);
},
});Then reject any loaded conversation whose conversation.organizationId !== args.organizationId before queuing a patch in both helper implementations.
Based on learnings: For conversation write paths, validate args.organizationId matches the loaded conversation.organizationId immediately after fetching the conversation and before any inserts/patches.
Also applies to: 153-160
🤖 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 122 - 129,
The bulk conversation mutations lack org-scoped authorization: add an
organizationId arg to the mutation signatures (e.g., in the mutationWithRLS for
bulkArchiveConversations and the similar bulkUnarchive mutation) and, inside the
handler flow (and in the corresponding helper functions
ConversationsHelpers.bulkArchiveConversations and
ConversationsHelpers.bulkUnarchiveConversations), after fetching each
conversation via ctx.db.get(id) immediately validate that
conversation.organizationId === args.organizationId and reject (skip/throw) any
that do not match before queuing any patch or insert; ensure this check runs
before any database patch/insert or audit log enqueueing so no cross-org updates
or misattributed audit entries can occur.
Add bulk archive and unarchive operations for conversations with corresponding backend mutations, frontend hooks, and toast feedback. Redesign the inbox toolbar with icon-button bulk actions, a read/unread radio filter, and selected-count display. Add contextual action banners at the bottom of the conversation panel for closed, archived, and spam states with reopen/unarchive/not-spam buttons. Implement thread collapsing for long conversations and error state with retry. Update conversation list row layout with tighter spacing, unread dot indicator, and subject preview line.
Israeltheminer
force-pushed
the
feat/bulk-archive-status-banners
branch
from
a623649 to
1280dae
Compare
Add aria-label and role to unread dot for screen reader support. Disable bulk mark-spam button since no handler is implemented yet. Make filter dropdown trigger a focusable button for keyboard access.
…tus-banners # Conflicts: # services/platform/messages/en.json
Israeltheminer
force-pushed
the
feat/bulk-archive-status-banners
branch
from
a746898 to
6ec9d1b
Compare
1 participant
Summary
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Improvements