fix: make email notifications support multiple files #38184
Merged
+58
−32
Conversation
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
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 32160cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughRefactors server email assembly to include multiple file attachments and mixed text, adds plural English i18n keys for multi-file messages, introduces optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UploadService as File Upload Service
participant Server
participant Email as Email Service
User->>UploadService: Upload file(s) + message
UploadService->>Server: parseFileIntoMessageAttachments(files) (adds typeGroup)
Server->>Server: persist IMessage with files[]
Server->>Server: build email content (header + contentParts for text + per-file entries)
Server->>Email: send composed email with attachments
Email-->>User: deliver notification (inbox)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38184 +/- ##
===========================================
+ Coverage 70.62% 70.64% +0.01%
===========================================
Files 3136 3136
Lines 108626 108637 +11
Branches 19584 19576 -8
===========================================
+ Hits 76716 76744 +28
+ Misses 29898 29888 -10
+ Partials 2012 2005 -7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "01/17 04:14 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Sat, 17 Jan 2026 04:14:51 GMT |
![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: 2
🤖 Fix all issues with AI agents
In `@apps/meteor/app/lib/server/functions/notifications/email.js`:
- Line 34: The hasText check currently treats undefined/null as text because it
uses message.msg !== ''; change the check to ensure message.msg is a non-empty
string (e.g., const hasText = typeof message.msg === 'string' &&
message.msg.trim() !== ''), and when passing into escapeHTML ensure you only
call it with a string (e.g., escapeHTML(message.msg || '') or guard by hasText)
so "undefined" is never rendered in the email body.
- Around line 79-89: The current block aligns files to attachments by index
which is fragile; change it to match each file to its metadata by name (e.g.,
for each file in files use message.attachments?.find(a => a.title === (file.name
|| '')) or similar) instead of using attachments[index]. Also add a defensive
fallback for missing file.name (use empty string or a safe fallback) and
continue to use escapeHTML for both the file name and the matched
attachment.description when building part; update references in this block
(files, message.attachments, escapeHTML) to use the name-based lookup pattern
used in sendTranscript.ts and OmnichannelTranscript.ts.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/rotten-foxes-marry.mdapps/meteor/app/lib/server/functions/notifications/email.jspackages/i18n/src/locales/en.i18n.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/lib/server/functions/notifications/email.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
.changeset/rotten-foxes-marry.mdpackages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/rotten-foxes-marry.mdpackages/i18n/src/locales/en.i18n.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (1/4)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (3/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (1/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (2/5)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (4/4)
🔇 Additional comments (6)
.changeset/rotten-foxes-marry.md (1)
1-6: LGTM! Well-formed changeset.The changeset correctly specifies patch-level bumps for both affected packages (
@rocket.chat/i18nfor translation key additions and@rocket.chat/meteorfor the email notification logic changes). The description accurately summarizes the fix.packages/i18n/src/locales/en.i18n.json (1)
5643-5644: Looks good; please double-checkcountinterpolation and plural key selection.These new plural keys are consistent with the existing singular variants and the repo’s approach (separate keys vs
{one/other}blocks). Also good that they’re only added toen.i18n.json(no need to touch other locales). Based on learnings, …Only ask: ensure the email notification code always:
- uses these keys only when
count > 1(socount=1doesn’t render “1 files”), and- passes the interpolation param as
count(not e.g.filesCount).apps/meteor/app/lib/server/functions/notifications/email.js (4)
37-55: LGTM!The header selection logic correctly handles all combinations of single/multiple files and group/direct chat scenarios. The
countparameter is properly passed for i18n pluralization support.
57-63: LGTM!The updated e2e check correctly allows file metadata to be displayed even for encrypted messages, while the early return for disabled
Email_notification_show_messageis appropriate.
65-77: LGTM!The text content processing correctly escapes HTML, processes message tokens, and converts newlines to
<br/>tags for email rendering.
91-93: LGTM!The content assembly correctly combines the header with all content parts, maintaining proper HTML formatting with
<br/>separators.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
|
@ricardogarim, conversation resolved as requested. |
KevLehman
previously approved these changes
Jan 14, 2026
pierre-lehnen-rc
approved these changes
Jan 16, 2026
tassoevan
approved these changes
Jan 16, 2026
ricardogarim
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Proposed changes (including videos or screenshots)
As per CORE-1629, this PR adds support for multiple file attachments in email notifications.
Previously, when a user received an offline email notification for a message containing multiple files, only the first file was shown. Now all files are listed in the notification email.
Issue(s)
Steps to test or reproduce
You can get docker-compose of the email local services on #38183.
1. Setup
2. Test Single File (baseline)
<UserA> sent you a filewith the file name3. Test Multiple Files (new feature)
<UserA> sent you X files(with correct count)4. Test in Channel (group chat)
<UserA> uploaded X files on <channel>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.