Conversation
closes https://linear.app/ghost/issue/NY-1069 When a user is logged into multiple Gmail accounts, we want to send them to the right inbox. To do that, we shouldn't be escaping the `@` sign.
WalkthroughA new helper function 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/core/server/lib/get-inbox-links.ts (1)
64-66: Consider adding an inline comment explaining why@is deliberately unescaped.Gmail's multi-account URL routing (
/mail/u/<account>/) requires the literal@in the path segment – encoding it as%40sends users to the wrong inbox. A brief doc comment would make this non-obvious invariant clear to future readers.📝 Suggested docstring
+/** + * Encodes a recipient email address for use in a Gmail URL path segment. + * The `@` sign is intentionally left unencoded because Gmail uses it as a + * literal character when routing to the correct account when multiple Google + * accounts are signed in (e.g. `/mail/u/user@gmail.com/`). + */ const encodeRecipientForGmailUrl = (recipient: string) => ( encodeURIComponent(recipient).replaceAll('%40', '@') );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/core/server/lib/get-inbox-links.ts` around lines 64 - 66, The function encodeRecipientForGmailUrl deliberately decodes the %40 back to '@' to preserve Gmail's multi-account routing; add an inline doc comment above encodeRecipientForGmailUrl explaining that Gmail requires a literal '@' in the URL path (encoding it as %40 breaks inbox selection) so future maintainers understand why replaceAll('%40', '@') is intentional; reference the function name encodeRecipientForGmailUrl in the comment and briefly mention the Gmail /mail/u/<account>/ routing invariant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ghost/core/core/server/lib/get-inbox-links.ts`:
- Around line 64-66: The function encodeRecipientForGmailUrl deliberately
decodes the %40 back to '@' to preserve Gmail's multi-account routing; add an
inline doc comment above encodeRecipientForGmailUrl explaining that Gmail
requires a literal '@' in the URL path (encoding it as %40 breaks inbox
selection) so future maintainers understand why replaceAll('%40', '@') is
intentional; reference the function name encodeRecipientForGmailUrl in the
comment and briefly mention the Gmail /mail/u/<account>/ routing invariant.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ghost/core/core/server/lib/get-inbox-links.tsghost/core/test/unit/server/lib/get-inbox-links.test.ts
closes https://linear.app/ghost/issue/NY-1069
When a user is logged into multiple Gmail accounts, we want to send them to the right inbox.
To do that, we shouldn't be escaping the
@sign.