calculate message sidebar counts and show in the sidebar UI, issue #361 - #377
Open
vanboom wants to merge 1 commit into
Open
calculate message sidebar counts and show in the sidebar UI, issue #361#377vanboom wants to merge 1 commit into
vanboom wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements conversation count badges in the sidebar by adding state management to track Open-conversation totals across four categories (unassigned, assigned, all, mentioned), fetching those counts on component mount and after status/assignee mutations, and rendering count badges next to the corresponding Inbox navigation items. ChangesSidebar conversation count badges
Sequence DiagramsequenceDiagram
participant Sidebar as Sidebar Component
participant Store as Conversation Store
participant API as Conversation API
Sidebar->>Store: fetchSidebarCounts()
Store->>API: GET /conversations?status=Open&filters=unassigned
API-->>Store: { data: { total: N } }
Store->>API: GET /conversations?status=Open&filters=assigned
API-->>Store: { data: { total: M } }
Store->>API: GET /conversations?status=Open (all)
API-->>Store: { data: { total: K } }
Store->>API: GET /conversations?status=Open&filters=mentioned
API-->>Store: { data: { total: L } }
Store->>Store: sidebarCounts = { unassigned, assigned, all, mentioned }
Store-->>Sidebar: counts updated
Sidebar->>Sidebar: render badges with counts
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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}
Contributor
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)
frontend/apps/main/src/components/sidebar/Sidebar.vue (1)
482-488:⚠️ Potential issue | 🟡 MinorAdd/justify sidebar count for the “All” inbox
frontend/apps/main/src/components/sidebar/Sidebar.vueshows count badges for “assigned/mentioned/unassigned”, but the “All” inbox item renders no badge.frontend/apps/main/src/stores/conversation.jsstill fetches and storessidebarCounts.allfromgetAllConversations, but that value isn’t used in the UI. Either display the “All” badge for consistency or remove thesidebarCounts.alltracking/fetch if it’s intentionally omitted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/main/src/components/sidebar/Sidebar.vue` around lines 482 - 488, The "All" inbox item in Sidebar.vue is missing the badge even though the store (conversation.js) populates sidebarCounts.all from getAllConversations; update the UI to display the same count badge used by other inbox items by wiring sidebarCounts.all into the SidebarMenuButton for the "All" item (the template around SidebarMenuButton / List / navigateToInbox('all')), ensuring it uses the same badge component/prop pattern as the "assigned/mentioned/unassigned" items; alternatively, if "All" should not show a count, remove sidebarCounts.all and the getAllConversations fetch from the conversation store (functions: getAllConversations, sidebarCounts) to keep state consistent.
🧹 Nitpick comments (3)
frontend/apps/main/src/stores/conversation.js (2)
1114-1114: ⚡ Quick winRemove debug logging from production code.
This console.debug statement logs the entire API response object, which can be verbose and is unnecessary in production. Debug logs should be removed before merging.
♻️ Proposed fix to remove debug logging
- console.debug(unassignedRes) // Extract the raw database total row counts from the Go API response🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/main/src/stores/conversation.js` at line 1114, Remove the development debug log that prints the full API response: delete the console.debug(unassignedRes) call in conversation.js (the unassignedRes debug statement). If you need non-verbose tracing in production, replace it with a conditional debug guard or use the app's logger (e.g., only log via store/Logger when NODE_ENV !== 'production') and ensure any remaining logs do not serialize the whole response object; otherwise simply remove the line.
1120-1120: ⚡ Quick winRemove debug logging from production code.
This console.debug statement is unnecessary in production and should be removed before merging.
♻️ Proposed fix to remove debug logging
sidebarCounts.mentioned = mentionedRes?.data?.data?.total || 0 - console.debug(sidebarCounts) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/main/src/stores/conversation.js` at line 1120, Remove the stray production debug log by deleting the console.debug(sidebarCounts) call in the conversation store; locate the usage of the sidebarCounts variable in the file (e.g., where sidebarCounts is computed or used) and remove the console.debug invocation so no console.debug(sidebarCounts) remains in the module (do not replace with console logs — simply remove).frontend/apps/main/src/components/sidebar/Sidebar.vue (1)
450-452: ⚡ Quick winAdd ARIA labels to count badges for screen reader accessibility.
The count badges display numbers without context for screen reader users. Add
aria-labelattributes to make these accessible, e.g.,aria-label="${count} open assigned conversations".♿ Proposed fix to add ARIA labels
For the assigned badge (lines 450-452):
- <span class="ml-auto bg-gray-200 text-gray-700 px-2 py-0.5 rounded-full text-xs font-semibold"> + <span + class="ml-auto bg-gray-200 text-gray-700 px-2 py-0.5 rounded-full text-xs font-semibold" + :aria-label="`${conversationStore.sidebarCounts.assigned} open assigned conversations`" + > {{ conversationStore.sidebarCounts.assigned }} </span>Apply similar changes to the mentioned (lines 462-464) and unassigned (lines 475-477) badges.
Also applies to: 462-464, 475-477
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/main/src/components/sidebar/Sidebar.vue` around lines 450 - 452, The count badges (span elements rendering conversationStore.sidebarCounts.assigned, conversationStore.sidebarCounts.mentioned, and conversationStore.sidebarCounts.unassigned) lack ARIA context; update each badge span to include an aria-label that embeds the numeric value and context (e.g., "{count} assigned conversations" or similar) so screen readers announce meaningful text for assigned, mentioned, and unassigned counts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/apps/main/src/stores/conversation.js`:
- Around line 1099-1121: fetchSidebarCounts currently awaits Promise.all([...])
of api.getUnassignedConversations, api.getAssignedConversations,
api.getAllConversations and api.getMentionedConversations with no error
handling; wrap the concurrent calls in error handling (prefer using
Promise.allSettled or a try/catch around Promise.all) so one failing request
doesn't reject the whole function, log any errors, and set
sidebarCounts.unassigned/assigned/all/mentioned to 0 (or retain previous values)
when their corresponding promise fails; update fetchSidebarCounts and any
callers (e.g., the onMounted in Sidebar.vue) to avoid unhandled rejections and
ensure the UI continues to render even if some API calls fail.
- Line 685: The call to fetchSidebarCounts() should be wrapped in its own
try-catch so failures to refresh the sidebar do not run the surrounding catch
that reports the status-update as failed; after the status update succeeds (the
await on the status update at line ~684), replace the direct await
fetchSidebarCounts() with: try { await fetchSidebarCounts() } catch (err) {
console.error('Failed to refresh sidebar counts', err) } (or use the existing
logger) so that sidebar refresh errors are logged/handled locally and do not
trigger the status-update failure path.
- Line 747: The call to fetchSidebarCounts() should be wrapped in its own
try-catch so failures don't get attributed to the assignee update in
updateStatus; modify the code after the successful assignee update to run await
fetchSidebarCounts() inside a new try block and catch any error separately,
logging a distinct message like "Failed to refresh sidebar counts" (or using the
same error reporting utility used elsewhere) and not rethrowing so the assignee
update remains considered successful.
---
Outside diff comments:
In `@frontend/apps/main/src/components/sidebar/Sidebar.vue`:
- Around line 482-488: The "All" inbox item in Sidebar.vue is missing the badge
even though the store (conversation.js) populates sidebarCounts.all from
getAllConversations; update the UI to display the same count badge used by other
inbox items by wiring sidebarCounts.all into the SidebarMenuButton for the "All"
item (the template around SidebarMenuButton / List / navigateToInbox('all')),
ensuring it uses the same badge component/prop pattern as the
"assigned/mentioned/unassigned" items; alternatively, if "All" should not show a
count, remove sidebarCounts.all and the getAllConversations fetch from the
conversation store (functions: getAllConversations, sidebarCounts) to keep state
consistent.
---
Nitpick comments:
In `@frontend/apps/main/src/components/sidebar/Sidebar.vue`:
- Around line 450-452: The count badges (span elements rendering
conversationStore.sidebarCounts.assigned,
conversationStore.sidebarCounts.mentioned, and
conversationStore.sidebarCounts.unassigned) lack ARIA context; update each badge
span to include an aria-label that embeds the numeric value and context (e.g.,
"{count} assigned conversations" or similar) so screen readers announce
meaningful text for assigned, mentioned, and unassigned counts.
In `@frontend/apps/main/src/stores/conversation.js`:
- Line 1114: Remove the development debug log that prints the full API response:
delete the console.debug(unassignedRes) call in conversation.js (the
unassignedRes debug statement). If you need non-verbose tracing in production,
replace it with a conditional debug guard or use the app's logger (e.g., only
log via store/Logger when NODE_ENV !== 'production') and ensure any remaining
logs do not serialize the whole response object; otherwise simply remove the
line.
- Line 1120: Remove the stray production debug log by deleting the
console.debug(sidebarCounts) call in the conversation store; locate the usage of
the sidebarCounts variable in the file (e.g., where sidebarCounts is computed or
used) and remove the console.debug invocation so no console.debug(sidebarCounts)
remains in the module (do not replace with console logs — simply remove).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b9fd9e3-c55f-4b2a-8d23-bb8867e52edc
📒 Files selected for processing (2)
frontend/apps/main/src/components/sidebar/Sidebar.vuefrontend/apps/main/src/stores/conversation.js
| conversation.data.status = v | ||
| try { | ||
| await api.updateConversationStatus(conversation.data.uuid, { status: v }) | ||
| await fetchSidebarCounts() |
Contributor
There was a problem hiding this comment.
Wrap sidebar count refresh in separate try-catch to avoid misleading error messages.
If fetchSidebarCounts() fails, the existing catch block (lines 686-692) will execute and tell the user that the status update failed. However, the status update actually succeeded (line 684), and only the sidebar count refresh failed. This creates a confusing UX where the conversation status changes but the user sees a failure message.
🛡️ Proposed fix to isolate sidebar count errors
try {
await api.updateConversationStatus(conversation.data.uuid, { status: v })
- await fetchSidebarCounts()
} catch (error) {
if (conversation.data) conversation.data.status = previous
emitter.emit(EMITTER_EVENTS.SHOW_TOAST, {
variant: 'destructive',
description: handleHTTPError(error).message
})
+ return
}
+ // Refresh counts silently; don't block or show errors if this fails
+ fetchSidebarCounts().catch((err) => {
+ console.warn('Failed to refresh sidebar counts after status update:', err)
+ })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/main/src/stores/conversation.js` at line 685, The call to
fetchSidebarCounts() should be wrapped in its own try-catch so failures to
refresh the sidebar do not run the surrounding catch that reports the
status-update as failed; after the status update succeeds (the await on the
status update at line ~684), replace the direct await fetchSidebarCounts() with:
try { await fetchSidebarCounts() } catch (err) { console.error('Failed to
refresh sidebar counts', err) } (or use the existing logger) so that sidebar
refresh errors are logged/handled locally and do not trigger the status-update
failure path.
| async function updateAssignee (type, v) { | ||
| try { | ||
| await api.updateAssignee(conversation.data.uuid, type, v) | ||
| await fetchSidebarCounts() |
Contributor
There was a problem hiding this comment.
Wrap sidebar count refresh in separate try-catch to avoid misleading error messages.
Same issue as in updateStatus (line 685): if fetchSidebarCounts() fails, the existing catch block (lines 748-753) will show an error saying the assignee update failed, but the assignee update at line 746 actually succeeded.
🛡️ Proposed fix to isolate sidebar count errors
try {
await api.updateAssignee(conversation.data.uuid, type, v)
- await fetchSidebarCounts()
} catch (error) {
emitter.emit(EMITTER_EVENTS.SHOW_TOAST, {
variant: 'destructive',
description: handleHTTPError(error).message
})
+ return
}
+ // Refresh counts silently; don't block or show errors if this fails
+ fetchSidebarCounts().catch((err) => {
+ console.warn('Failed to refresh sidebar counts after assignee update:', err)
+ })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/main/src/stores/conversation.js` at line 747, The call to
fetchSidebarCounts() should be wrapped in its own try-catch so failures don't
get attributed to the assignee update in updateStatus; modify the code after the
successful assignee update to run await fetchSidebarCounts() inside a new try
block and catch any error separately, logging a distinct message like "Failed to
refresh sidebar counts" (or using the same error reporting utility used
elsewhere) and not rethrowing so the assignee update remains considered
successful.
Comment on lines
+1099
to
+1121
| async function fetchSidebarCounts() { | ||
| const openFilter = [{ | ||
| model: 'conversation_statuses', | ||
| field: 'name', | ||
| operator: 'equals', | ||
| value: 'Open' | ||
| }] | ||
| const filters = JSON.stringify(openFilter) | ||
| // Fire all requests simultaneously. page_size: 1 keeps payload sizes tiny. | ||
| const [unassignedRes, assignedRes, allRes, mentionedRes] = await Promise.all([ | ||
| api.getUnassignedConversations({ page: 1, page_size: 1, filters: filters }), | ||
| api.getAssignedConversations({ page: 1, page_size: 1, filters: filters }), | ||
| api.getAllConversations({ page: 1, page_size: 1, filters: filters }), | ||
| api.getMentionedConversations({ page: 1, page_size: 1, filters: filters }) | ||
| ]) | ||
| console.debug(unassignedRes) | ||
| // Extract the raw database total row counts from the Go API response | ||
| sidebarCounts.unassigned = unassignedRes?.data?.data?.total || 0 | ||
| sidebarCounts.assigned = assignedRes?.data?.data?.total || 0 | ||
| sidebarCounts.all = allRes?.data?.data?.total || 0 | ||
| sidebarCounts.mentioned = mentionedRes?.data?.data?.total || 0 | ||
| console.debug(sidebarCounts) | ||
| } |
Contributor
There was a problem hiding this comment.
Add error handling to prevent unhandled promise rejections.
fetchSidebarCounts() uses Promise.all() without error handling. If any of the four API calls fail, the entire function rejects. This is called from onMounted in Sidebar.vue (line 238) without a try-catch, which could cause an unhandled promise rejection during component initialization and break the sidebar UI.
🛡️ Proposed fix to add error handling
async function fetchSidebarCounts() {
const openFilter = [{
model: 'conversation_statuses',
field: 'name',
operator: 'equals',
value: 'Open'
}]
const filters = JSON.stringify(openFilter)
- // Fire all requests simultaneously. page_size: 1 keeps payload sizes tiny.
- const [unassignedRes, assignedRes, allRes, mentionedRes] = await Promise.all([
- api.getUnassignedConversations({ page: 1, page_size: 1, filters: filters }),
- api.getAssignedConversations({ page: 1, page_size: 1, filters: filters }),
- api.getAllConversations({ page: 1, page_size: 1, filters: filters }),
- api.getMentionedConversations({ page: 1, page_size: 1, filters: filters })
- ])
- console.debug(unassignedRes)
- // Extract the raw database total row counts from the Go API response
- sidebarCounts.unassigned = unassignedRes?.data?.data?.total || 0
- sidebarCounts.assigned = assignedRes?.data?.data?.total || 0
- sidebarCounts.all = allRes?.data?.data?.total || 0
- sidebarCounts.mentioned = mentionedRes?.data?.data?.total || 0
- console.debug(sidebarCounts)
+ try {
+ // Fire all requests simultaneously. page_size: 1 keeps payload sizes tiny.
+ const [unassignedRes, assignedRes, allRes, mentionedRes] = await Promise.all([
+ api.getUnassignedConversations({ page: 1, page_size: 1, filters: filters }),
+ api.getAssignedConversations({ page: 1, page_size: 1, filters: filters }),
+ api.getAllConversations({ page: 1, page_size: 1, filters: filters }),
+ api.getMentionedConversations({ page: 1, page_size: 1, filters: filters })
+ ])
+ // Extract the raw database total row counts from the Go API response
+ sidebarCounts.unassigned = unassignedRes?.data?.data?.total || 0
+ sidebarCounts.assigned = assignedRes?.data?.data?.total || 0
+ sidebarCounts.all = allRes?.data?.data?.total || 0
+ sidebarCounts.mentioned = mentionedRes?.data?.data?.total || 0
+ } catch (error) {
+ console.warn('Failed to fetch sidebar counts:', error)
+ // Keep existing counts on error rather than resetting to 0
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/main/src/stores/conversation.js` around lines 1099 - 1121,
fetchSidebarCounts currently awaits Promise.all([...]) of
api.getUnassignedConversations, api.getAssignedConversations,
api.getAllConversations and api.getMentionedConversations with no error
handling; wrap the concurrent calls in error handling (prefer using
Promise.allSettled or a try/catch around Promise.all) so one failing request
doesn't reject the whole function, log any errors, and set
sidebarCounts.unassigned/assigned/all/mentioned to 0 (or retain previous values)
when their corresponding promise fails; update fetchSidebarCounts and any
callers (e.g., the onMounted in Sidebar.vue) to avoid unhandled rejections and
ensure the UI continues to render even if some API calls fail.
Owner
|
Two things on the approach: The useful sidebar number is how many open conversations are in each list. That's the actionable count for an agent. And the 4 API calls to get it are wasteful and they get the total count. We can replace them with a single backend query behind a new handler: Something like this SELECT
COUNT(*) FILTER (WHERE assigned_user_id = $1) AS assigned,
COUNT(*) FILTER (WHERE assigned_user_id IS NULL AND assigned_team_id IS NULL) AS unassigned,
COUNT(*) FILTER (WHERE EXISTS (SELECT 1 FROM conversation_mentions cm
WHERE cm.conversation_id = conversations.id
AND (cm.mentioned_user_id = $1
OR EXISTS (SELECT 1 FROM team_members tm
WHERE tm.team_id = cm.mentioned_team_id AND tm.user_id = $1)))) AS mentioned,
COUNT(*) AS "all"
FROM conversations
WHERE status_id IN (SELECT id FROM conversation_statuses WHERE category = 'open'); |
abhinavxd
reviewed
Jun 16, 2026
| {{ t('globals.terms.unassigned') }} | ||
| </span> | ||
|
|
||
| <span class="ml-auto bg-gray-200 text-gray-700 px-2 py-0.5 rounded-full text-xs font-semibold"> |
Owner
There was a problem hiding this comment.
the badge uses hardcoded bg-gray-200 text-gray-700. Please use theme tokens instead (defined in frontend/tailwind.config.cjs, e.g. bg-sidebar-accent text-sidebar-accent-foreground or
bg-muted text-muted-foreground).
Or could use Badge component directly ( not sure )
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #367 - please check closely, not sure if everything is in the correct place for efficiency.
Summary by CodeRabbit
New Features