fix #1654 added counter for remaining labels.#1660
fix #1654 added counter for remaining labels.#1660anuragnegi000 wants to merge 19 commits intoMail-0:stagingfrom
Conversation
|
""" WalkthroughThe changes update badge rendering logic in the mail navigation UI, revise backend label count calculation to use total messages and include an archive count, add a debug method for Gmail labels, and enhance logging in the mail count API endpoint. No exported/public API signatures are altered except for the new debug method. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as NavMain (UI)
participant API as mailRouter (API)
participant Agent as GoogleMailManager
UI->>API: GET /mail/count
API->>API: Log "mail.count endpoint called"
API->>Agent: agent.count()
Agent->>Agent: Calculate total messages per label
Agent->>Agent: Query archive count
Agent->>API: Return label counts including archive
API->>UI: Return label counts
UI->>UI: Render badges where count > 0 using getItemCount
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Fixed folder message count display in the sidebar navigation by implementing proper label counting for draft, sent, and archive folders across mail providers.
- Enhanced Gmail label counting in
apps/server/src/lib/driver/google.tsto use messagesTotal instead of threadsUnread - Improved folder mapping in
apps/mail/components/ui/nav-main.tsxwith case-insensitive label matching - Added debug logging in
apps/server/src/trpc/routes/mail.tsfor count endpoint troubleshooting - Removed unused 'arch' import from mail.ts that should be cleaned up
3 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/server/src/lib/driver/google.ts (2)
208-220: Remove debug logging and consider performance implications.The console.log statement should be removed for production code. Additionally, the archive search with
in:archivequery might be slow for users with large archives since it processes all archived messages.- console.log("Archive count from search:", archiveCount);Consider caching this count or making it optional if performance becomes an issue.
1287-1328: LGTM! Consider adding documentation.The debug method is well-implemented and follows existing patterns. Consider adding JSDoc comments to clarify this is for debugging purposes only.
+ /** + * Debug method to retrieve detailed information about all Gmail labels. + * This method is intended for debugging purposes only. + */ public async debugLabels() {apps/server/src/trpc/routes/mail.ts (1)
59-65: Consider using proper logging levels instead of console.log.While the debug logging is helpful for development, consider using a proper logging framework with appropriate log levels for production code.
- console.log('🎯 TRPC mail.count endpoint called!'); + // Consider using logger.debug() instead - console.log('🤖 Agent created, calling count()...'); + // Consider using logger.debug() instead - console.log('📊 Count result received:', result?.length, 'labels'); + // Consider using logger.debug() instead
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/mail/components/ui/nav-main.tsx(1 hunks)apps/server/src/lib/driver/google.ts(2 hunks)apps/server/src/trpc/routes/mail.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/server/src/trpc/routes/mail.ts (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.731Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.376Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
apps/mail/components/ui/nav-main.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/server/src/lib/driver/google.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1622
File: apps/server/src/lib/email-verification.ts:189-189
Timestamp: 2025-07-05T05:27:24.592Z
Learning: During testing phases, debug logging should be kept active in apps/server/src/lib/email-verification.ts for BIMI validation and email verification debugging, even if it's verbose.
🪛 Biome (1.9.4)
apps/mail/components/ui/nav-main.tsx
[error] 328-328: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
|
@MrgSub PTAL |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/mail/components/ui/nav-main.tsx (1)
64-91: Excellent optimization with proper memoization.The
statsMapimplementation effectively addresses the previous feedback to move logic outside the render function. The memoization correctly:
- Maps both stat labels and folder IDs to their counts
- Filters for positive counts only
- Handles case-insensitive matching appropriately
However, consider adding a trailing comma to the
idToLabelMapobject for better diff readability:const idToLabelMap: Record<string, string> = { 'inbox': 'INBOX', 'drafts': 'DRAFT', 'sent': 'SENT', 'spam': 'SPAM', 'trash': 'TRASH', - 'archive': 'ARCHIVE' + 'archive': 'ARCHIVE', };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/ui/nav-main.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/ui/nav-main.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🪛 Biome (1.9.4)
apps/mail/components/ui/nav-main.tsx
[error] 310-310: Shouldn't redeclare 'NavItem'. Consider to delete it or rename it.
'NavItem' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (5)
apps/mail/components/ui/nav-main.tsx (5)
19-19: Import additions look good.The added imports for
useCallback,useRef, anduseMemoare appropriately used in the optimization changes.
93-98: Clean and efficient callback implementation.The
getItemCountcallback properly handles edge cases and provides a clean interface for retrieving item counts. The memoization dependency onstatsMapensures optimal performance.
268-268: Proper callback passing to NavItem.The
getItemCountcallback is correctly passed to the NavItem component, enabling the optimized badge rendering.
310-310: Address the static analysis warning about function redeclaration.The static analysis tool is incorrectly flagging this as a redeclaration. This is actually a function definition, not a redeclaration. The
NavIteminterface is defined at line 32, and this is the function implementation. The static analysis tool is producing a false positive.The function signature correctly includes the new
getItemCountparameter.
352-359: Simplified and efficient badge rendering logic.The updated badge rendering logic is much cleaner and more efficient than the previous implementation. It:
- Uses the memoized
getItemCountcallback- Eliminates the need for complex conditional checks
- Properly handles zero counts by returning
null- Maintains proper localization formatting
This change successfully addresses the PR objective of showing message counts for all folders including archive.
|
@MrgSub shifted logic to the top of the file, PTAL |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/mail/components/ui/nav-main.tsx (1)
64-91: Excellent optimization that addresses previous feedback.The
statsMapmemoization effectively moves the label mapping logic outside of render functions, addressing the concern raised in previous reviews. The implementation correctly:
- Maps both uppercase labels and corresponding folder IDs to their counts
- Only includes positive counts to avoid displaying zero badges
- Uses proper memoization with
statsas the dependencyHowever, consider adding the trailing comma as suggested in previous reviews:
'trash': 'TRASH', - 'archive': 'ARCHIVE' + 'archive': 'ARCHIVE',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/ui/nav-main.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/ui/nav-main.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🧬 Code Graph Analysis (1)
apps/mail/components/ui/nav-main.tsx (4)
apps/mail/config/navigation.ts (1)
NavItem(24-35)apps/server/src/lib/driver/google.ts (1)
count(181-225)apps/server/src/lib/driver/microsoft.ts (1)
count(170-227)apps/mail/components/ui/badge.tsx (1)
Badge(38-38)
🪛 Biome (1.9.4)
apps/mail/components/ui/nav-main.tsx
[error] 308-308: Shouldn't redeclare 'NavItem'. Consider to delete it or rename it.
'NavItem' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (5)
apps/mail/components/ui/nav-main.tsx (5)
19-19: Good addition of required React hooks.The addition of
useCallbackanduseMemoimports is necessary for the performance optimizations implemented in this component.
93-98: Well-implemented callback function.The
getItemCountcallback is properly memoized and provides a clean API for retrieving item counts. The implementation correctly handles undefined itemId values and returns 0 when no count is available.
268-268: Proper prop passing to NavItem.The
getItemCountfunction is correctly passed as a prop to enable the badge rendering functionality in the NavItem component.
308-308: Function signature update correctly implements the feature.The updated
NavItemfunction signature properly includes the optionalgetItemCountparameter, allowing the component to display message counts when available.Note: The static analysis warning about redeclaring
NavItemappears to be a false positive - this is defining a function component, not redeclaring a variable.
350-357: Simplified and improved badge rendering logic.The new badge rendering implementation is much cleaner and addresses the previous optional chaining concerns. The logic now:
- Uses the memoized
getItemCountfunction instead of inline calculations- Properly handles undefined values without optional chaining issues
- Only renders badges for positive counts
- Maintains the same visual styling and formatting
This successfully implements the PR objective of showing message counts for all navigation items including draft, sent, and archive folders.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/mail/components/ui/nav-main.tsx (2)
64-77: Consider optimizing the statsMap to avoid duplicate entries.The current implementation stores each count twice (lowercase and uppercase). Consider storing normalized keys only:
const statsMap = useMemo(() => { if (!stats) return new Map<string, number>(); const map = new Map<string, number>(); stats.forEach(stat => { if (stat.label && stat.count && stat.count > 0) { - map.set(stat.label.toLowerCase(), stat.count); - map.set(stat.label.toUpperCase(), stat.count); + // Store with normalized key (lowercase) + map.set(stat.label.toLowerCase(), stat.count); } }); return map; }, [stats]);
79-89: Simplify getItemCount to match normalized statsMap.If you optimize statsMap to store only lowercase keys, update this function accordingly:
const getItemCount = useCallback((itemId: string | undefined) => { if (!itemId || !statsMap.size) return 0; - const lowerItemId = itemId.toLowerCase(); - if (statsMap.has(lowerItemId)) { - return statsMap.get(lowerItemId) || 0; - } - - const upperItemId = itemId.toUpperCase(); - return statsMap.get(upperItemId) || 0; + // Always check with normalized key (lowercase) + return statsMap.get(itemId.toLowerCase()) || 0; }, [statsMap]);apps/server/src/lib/driver/google.ts (2)
228-249: Optimize the enhancedCounts construction to avoid redundant loops.The current implementation loops through
labelCountstwice. Consider combining the logic:const enhancedCounts: Array<{ label: string; count: number }> = []; - labelCounts.forEach(stat => { - if (stat.count > 0) { - enhancedCounts.push(stat); - } - }); - - labelCounts.forEach(stat => { - if (stat.label && stat.count > 0) { - Object.entries(idToLabelMap).forEach(([id, gmailLabel]) => { - if (stat.label.toUpperCase() === gmailLabel) { - enhancedCounts.push({ - label: id, - count: stat.count - }); - } - }); - } - }); + labelCounts.forEach(stat => { + if (stat.label && stat.count > 0) { + // Add original label + enhancedCounts.push(stat); + + // Add mapped label ID if exists + const mappedEntry = Object.entries(idToLabelMap).find( + ([_, gmailLabel]) => stat.label.toUpperCase() === gmailLabel + ); + if (mappedEntry) { + enhancedCounts.push({ + label: mappedEntry[0], + count: stat.count + }); + } + } + });
1318-1358: Consider restricting debugLabels access in production.The
debugLabelsmethod exposes detailed label information and generates verbose logs. Consider making it private or adding a debug flag guard to prevent accidental usage in production.- public async debugLabels() { + private async debugLabels() { return this.withErrorHandler( 'debugLabels', async () => { + // Only run in development or when debug flag is set + if (process.env.NODE_ENV === 'production' && !env.DEBUG_LABELS) { + return []; + } const userLabels = await this.gmail.users.labels.list({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/ui/nav-main.tsx(5 hunks)apps/server/src/lib/driver/google.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/server/src/lib/driver/google.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1622
File: apps/server/src/lib/email-verification.ts:189-189
Timestamp: 2025-07-05T05:27:24.592Z
Learning: During testing phases, debug logging should be kept active in apps/server/src/lib/email-verification.ts for BIMI validation and email verification debugging, even if it's verbose.
apps/mail/components/ui/nav-main.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🧬 Code Graph Analysis (1)
apps/mail/components/ui/nav-main.tsx (4)
apps/mail/config/navigation.ts (1)
NavItem(24-35)apps/server/src/lib/driver/google.ts (1)
count(181-253)apps/server/src/lib/driver/microsoft.ts (1)
count(170-227)apps/mail/components/ui/badge.tsx (1)
Badge(38-38)
🪛 Biome (1.9.4)
apps/mail/components/ui/nav-main.tsx
[error] 299-299: Shouldn't redeclare 'NavItem'. Consider to delete it or rename it.
'NavItem' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (2)
apps/mail/components/ui/nav-main.tsx (1)
341-348: LGTM! Clean implementation of badge rendering.The simplified badge rendering logic successfully addresses the reviewer's feedback about moving logic outside the render function. The implementation correctly displays counts when available.
apps/server/src/lib/driver/google.ts (1)
217-226: Good implementation of archive count retrieval.The archive search correctly uses
resultSizeEstimatefor efficient count retrieval without fetching actual messages.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/mail/components/ui/nav-main.tsx(5 hunks)apps/server/src/lib/driver/google.ts(2 hunks)apps/server/src/trpc/routes/mail.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/server/src/trpc/routes/mail.ts
- apps/server/src/lib/driver/google.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/ui/nav-main.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🪛 Biome (1.9.4)
apps/mail/components/ui/nav-main.tsx
[error] 299-299: Shouldn't redeclare 'NavItem'. Consider to delete it or rename it.
'NavItem' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (4)
apps/mail/components/ui/nav-main.tsx (4)
19-19: Import statement looks good.The added imports are properly used in the new memoization and callback logic.
79-89: Efficient count lookup implementation.The
getItemCountcallback provides a clean abstraction for retrieving item counts with proper memoization. The logic correctly handles both lowercase and uppercase label matching.
259-259: Correct prop passing.The
getItemCountprop is properly passed to theNavItemcomponent.
341-348: Excellent refactoring of badge logic.The new implementation is much cleaner and more efficient than the previous complex inline logic. Moving the logic outside the render function addresses the previous review feedback perfectly. The count check and badge rendering is now straightforward and readable.
|
please rebase @anuragnegi000 |
2 participants
Description
Please provide a clear description of your changes.
Fixed the draft, send, archive count display in the sidebar navigation. The issue was that the draft, sent, archive folder was showing no message count.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
Before:-
After:-
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Bug Fixes
Chores