fix: broken transcript with multiple files per message #38064
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 7c234e2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
WalkthroughThis PR fixes PDF chat transcript formatting when messages contain multiple files. It updates test fixtures to include multiple files per message, adjusts component styling for proper spacing and wrapping, and modifies the Message component's wrap logic to account for file presence. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?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: 0
🧹 Nitpick comments (2)
ee/packages/pdf-worker/src/templates/ChatTranscript/ChatTranscript.fixtures.ts (1)
157-164: Consider adding variety to the multi-file test fixture.All six files are identical (same name, same buffer). While this tests the layout for multiple files, adding variety could catch additional edge cases:
- Different file types (
.jpg,.txt)- Varying file name lengths
- Mix of valid and invalid buffers
- Different image dimensions
Example with varied test data
files: [ { name: 'screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, - { name: 'screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, - { name: 'screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, - { name: 'screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, - { name: 'screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, - { name: 'screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, + { name: 'document-with-a-very-long-filename.pdf', buffer: Buffer.from(base64Image, 'base64') }, + { name: 'photo.jpg', buffer: Buffer.from(base64Image, 'base64') }, + { name: 'img.png', buffer: Buffer.from(base64Image, 'base64') }, + { name: 'invalid-file.txt', buffer: null }, + { name: 'another-screenshot.png', buffer: Buffer.from(base64Image, 'base64') }, ],ee/packages/pdf-worker/src/templates/ChatTranscript/components/Files.tsx (1)
42-45: Consider a more stable key for file items.Using array
indexas the key (line 44) can cause issues if the files array is modified during rendering, though this is less likely in PDF generation contexts. Since file names might not be unique (as seen in the test fixtures), consider using a combination:-<View style={styles.file} key={index} wrap> +<View style={styles.file} key={`${file.name}-${index}`} wrap>This provides better stability while handling potential duplicate file names.
📜 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 (4)
.changeset/flat-berries-sell.mdee/packages/pdf-worker/src/templates/ChatTranscript/ChatTranscript.fixtures.tsee/packages/pdf-worker/src/templates/ChatTranscript/components/Files.tsxee/packages/pdf-worker/src/templates/ChatTranscript/components/Message.tsx
🧰 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:
ee/packages/pdf-worker/src/templates/ChatTranscript/ChatTranscript.fixtures.tsee/packages/pdf-worker/src/templates/ChatTranscript/components/Message.tsxee/packages/pdf-worker/src/templates/ChatTranscript/components/Files.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
ee/packages/pdf-worker/src/templates/ChatTranscript/ChatTranscript.fixtures.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
ee/packages/pdf-worker/src/templates/ChatTranscript/ChatTranscript.fixtures.ts
⏰ 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). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
.changeset/flat-berries-sell.md (1)
1-5: LGTM! Clear changeset description.The changeset properly documents this patch-level fix for handling multiple files per message in PDF transcripts.
ee/packages/pdf-worker/src/templates/ChatTranscript/components/Message.tsx (1)
55-55: LGTM! Proper wrapping condition for messages with files.The addition of
(message.files && message.files.length > 0)to the wrap condition correctly enables page breaking when files are present. The defensive check for both existence and length is good practice.ee/packages/pdf-worker/src/templates/ChatTranscript/components/Files.tsx (2)
10-29: LGTM! Improved responsive layout for file attachments.The styling updates enhance the layout for multiple files:
- Container wrapper with proper spacing
- File items now use
marginBottomfor consistent spacing- Image width set to
100%for responsiveness while preserving aspect ratio- Individual file name styling improves visual hierarchy
These changes align well with the PR objective of fixing formatting for multiple files per message.
47-50: The bug workaround comment is justified.While the coding guidelines prefer avoiding comments, this one documents a workaround for a known react-pdf bug with proper issue reference. This type of explanatory comment for non-obvious behavior is valuable and should remain.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38064 +/- ##
===========================================
- Coverage 70.66% 70.63% -0.03%
===========================================
Files 3143 3143
Lines 108695 108695
Branches 19514 19503 -11
===========================================
- Hits 76804 76773 -31
- Misses 29895 29915 +20
- Partials 1996 2007 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 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/07 13:37 (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: Wed, 07 Jan 2026 13:37:34 GMT |
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
CORE-1196
Proposed changes (including videos or screenshots)
before:
after:
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.