Skip to content

Comments

Staging#715

Merged
MrgSub merged 6 commits intomainfrom
staging
Apr 18, 2025
Merged

Staging#715
MrgSub merged 6 commits intomainfrom
staging

Conversation

@MrgSub
Copy link
Collaborator

@MrgSub MrgSub commented Apr 18, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved parsing and validation of recipient email addresses to ensure only valid email addresses are used when sending mail.
    • Adjusted email read/unread logic to more accurately mark threads and messages as read based on unread status.
  • Style
    • Applied code formatting and consistency improvements across several components, including import order, indentation, and semicolon usage.
  • New Features
    • Added conditional display of the Golden Ticket modal in the navigation based on user session state.
  • Chores
    • Added debug logging in settings and mail components to assist with troubleshooting.
  • Refactor
    • Updated asynchronous data updates to use more robust handling with Promise.allSettled.
  • Bug Fixes
    • Ensured consistent display of reply counts in tooltips during demo mode for accurate user information.

@vercel
Copy link

vercel bot commented Apr 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
0 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2025 3:08am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 18, 2025

Walkthrough

This update introduces targeted enhancements and minor refactoring across several components in the mail application. Debug logging is added to settings and mail iframe components for improved traceability. Email address parsing and validation are refined in the Google mail driver to ensure clean recipient data. The logic for marking threads as read is streamlined, and asynchronous state mutations now use Promise.allSettled for improved reliability. Additional formatting and import order improvements are made in navigation and mail display components, with no changes to the signatures of exported entities.

Changes

File(s) Change Summary
apps/mail/actions/settings.ts Added console.log statements to log the settings object before and after validation in saveUserSettings. No changes to logic or exports.
apps/mail/app/api/driver/google.ts Updated the parseOutgoing function to parse and validate email addresses from the "to" recipients array. Now extracts clean email addresses using a regular expression, logs errors, and throws if invalid. No changes to exports.
apps/mail/components/mail/mail-iframe.tsx Moved import of DOMPurify, added missing semicolons, reformatted and wrapped onTrustSender in useCallback with a debug log, improved arrow function syntax, and performed whitespace cleanup. No functional changes to iframe logic or exports.
apps/mail/components/mail/thread-display.tsx Modified the logic for marking emails as read: now only marks thread and unread emails if unread exist, logs reply/unread counts, and uses Promise.allSettled for async mutations. Similar update in handleMarkAsUnread. No changes to exports.
apps/mail/components/ui/nav-main.tsx Reordered imports, improved formatting and indentation, updated badge rendering to use ternary expressions, and expanded JSX for readability. Conditionally renders GoldenTicketModal based on session state. No changes to exports.
apps/mail/components/mail/mail-list.tsx Changed tooltip rendering logic to use a single stable random count per render for replies count tooltip, ensuring consistent display and tooltip text. No changes to exports.

Sequence Diagram(s)

sequenceDiagram
    participant UI as User Interface
    participant ThreadDisplay as Thread Display Component
    participant API as API/Backend

    UI->>ThreadDisplay: Open email thread
    ThreadDisplay->>ThreadDisplay: Check unread emails
    alt If unread emails exist
        ThreadDisplay->>API: markAsRead(threadId, unreadEmailIds)
        API-->>ThreadDisplay: Response
        ThreadDisplay->>ThreadDisplay: Log reply and unread counts
        ThreadDisplay->>API: mutateThread(), mutateStats() (allSettled)
    else No unread emails
        ThreadDisplay->>ThreadDisplay: Log reply and unread counts
    end
Loading

Possibly related PRs

  • auto update #662: Updates the same effect hook for marking emails as read and adds a mutateStats() call, closely related to the changes in thread state management.
  • thread action buttons #597: Refactors handleMarkAsUnread and thread state logic in the same component, directly overlapping with the logic changes in this PR.
  • fix: random reply count in mail list #714: Modifies tooltip rendering logic in mail-list.tsx to ensure consistent counts, related to the changes in the replies count tooltip here.

Suggested reviewers

  • ahmetskilinc

Poem

In the warren of code, a rabbit peeks,
Debug logs sparkle, new logic tweaks.
Emails parsed with careful eyes,
Promises settled, no surprise.
Navigation neat and settings bright—
A hop, a fix, and all feels right! 🐇✨

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
apps/mail/actions/settings.ts (1)

25-29: Added debug logging for settings validation

These debug logs provide visibility into the state of the settings object before and after validation, which can help with troubleshooting validation issues.

While this is useful for development, consider making these logs conditional based on environment to avoid cluttering production logs.

- console.log(settings, 'before');
+ if (process.env.NODE_ENV !== 'production') console.log(settings, 'before');

- console.log(settings, 'after');
+ if (process.env.NODE_ENV !== 'production') console.log(settings, 'after');
apps/mail/components/mail/thread-display.tsx (1)

170-174: Added debug logging for email thread metrics

This log provides visibility into the number of total replies and unread emails, which helps with debugging thread rendering issues.

Consider making this log conditional based on environment or a debug flag to avoid cluttering production logs.

- console.log({
-   totalReplies: emailData.totalReplies,
-   unreadEmails: unreadEmails.length,
- });
+ if (process.env.NODE_ENV !== 'production') {
+   console.log({
+     totalReplies: emailData.totalReplies,
+     unreadEmails: unreadEmails.length,
+   });
+ }
apps/mail/components/mail/mail-iframe.tsx (1)

24-49: Consider removing debug console.log before production

The useCallback optimization is great, but there's a console.log statement that should be removed before deploying to production.

  const onTrustSender = useCallback(
    async (senderEmail: string) => {
      setImagesEnabled(true);

-     console.log(settings);
+
      const existingSettings = settings ?? {
        ...defaultUserSettings,
        timezone: getBrowserTimezone(),
      };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f701da7 and b1880cf.

📒 Files selected for processing (5)
  • apps/mail/actions/settings.ts (1 hunks)
  • apps/mail/app/api/driver/google.ts (1 hunks)
  • apps/mail/components/mail/mail-iframe.tsx (8 hunks)
  • apps/mail/components/mail/thread-display.tsx (2 hunks)
  • apps/mail/components/ui/nav-main.tsx (6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/mail/components/mail/thread-display.tsx (1)
apps/mail/actions/mail.ts (1)
  • markAsRead (56-66)
🔇 Additional comments (11)
apps/mail/app/api/driver/google.ts (1)

271-284: Improved email address parsing logic

The changes improve the logic for parsing email addresses from the "to" recipients by properly handling email addresses that may include display names in angle brackets (e.g., "Display Name email@example.com").

This enhancement ensures that only the actual email address is extracted and validated before use, which helps prevent issues with malformed recipient data.

apps/mail/components/mail/thread-display.tsx (2)

174-182: Simplified thread marking logic and improved promise handling

The code now only marks threads as read when there are unread emails, which is more efficient. Additionally, using Promise.allSettled instead of Promise.all improves reliability by ensuring both mutations complete regardless of errors.

This change prevents partial updates of the UI state, which could lead to inconsistent displays.


230-230: Improved reliability in unread marking with Promise.allSettled

Changing from Promise.all to Promise.allSettled ensures that both mutation operations (stats and thread) complete even if one fails, improving the reliability of UI state updates.

apps/mail/components/ui/nav-main.tsx (3)

14-17: Improved import organization

The imports have been reordered for better grouping and readability.


182-227: Improved formatting and code structure in NavItem component

The code formatting changes improve readability and consistency in the NavItem component, particularly in how refs are defined and how badge counts are displayed based on stats.

The conditional rendering with the stats check is now more clearly structured.


237-239: Improved formatting of Link component

The Link component is now formatted with each attribute on its own line, which improves readability.

apps/mail/components/mail/mail-iframe.tsx (5)

9-9: Improved import organization

Moving the DOMPurify import earlier with other library imports improves code organization and readability.


16-16: Style consistency improvement

Added semicolon after useState for consistent code style.


103-103: Style consistency improvement

Added semicolon after setCspViolation for consistent code style.


262-263: Formatting improvements for better readability

Consistent spacing and formatting throughout the component enhances code readability and maintainability.

Also applies to: 273-274, 279-280, 282-283, 287-288, 297-298, 302-303, 312-313, 324-325


340-341: Style consistency improvement

Added a proper newline at the end of the file follows best practices.

fix: random reply count in mail list
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/mail/components/mail/mail-list.tsx (1)

223-238: Improved tooltip display logic with consistent count values.

The updated code replaces multiple random number calls with a single calculation that's used consistently throughout the component. This prevents the confusing scenario where the displayed count and tooltip text might show different values.

For better readability, consider extracting this tooltip component logic to a separate function outside the render path:

- {Math.random() > 0.5 &&
-   (() => {
-     const count = Math.floor(Math.random() * 10) + 1;
-     return (
-       <Tooltip>
-         <TooltipTrigger asChild>
-           <span className="rounded-md border border-dotted px-[5px] py-[1px] text-xs opacity-70">
-             {count}
-           </span>
-         </TooltipTrigger>
-         <TooltipContent className="px-1 py-0 text-xs">
-           {t('common.mail.replies', { count })}
-         </TooltipContent>
-       </Tooltip>
-     );
-   })()}
+ {Math.random() > 0.5 && renderReplyCountTooltip()}

Then add this function to the component:

const renderReplyCountTooltip = () => {
  const count = Math.floor(Math.random() * 10) + 1;
  return (
    <Tooltip>
      <TooltipTrigger asChild>
        <span className="rounded-md border border-dotted px-[5px] py-[1px] text-xs opacity-70">
          {count}
        </span>
      </TooltipTrigger>
      <TooltipContent className="px-1 py-0 text-xs">
        {t('common.mail.replies', { count })}
      </TooltipContent>
    </Tooltip>
  );
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b1880cf and b3e5e62.

📒 Files selected for processing (1)
  • apps/mail/components/mail/mail-list.tsx (1 hunks)

@MrgSub MrgSub merged commit 618300a into main Apr 18, 2025
4 checks passed
This was referenced Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants