-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: support multiple file exports in room message export #38292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 73f4907 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38292 +/- ##
===========================================
+ Coverage 70.74% 70.75% +0.01%
===========================================
Files 3142 3142
Lines 108926 108927 +1
Branches 19602 19590 -12
===========================================
+ Hits 77059 77074 +15
+ Misses 29863 29854 -9
+ Partials 2004 1999 -5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
WalkthroughThis PR fixes room message export functionality to handle messages with multiple file attachments. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 3 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts`:
- Around line 234-239: The current fallback logic for files drops a legacy
msg.file when msg.files exists but is empty; change the selection so that if
msg.files is absent or an empty array you fall back to msg.file (e.g., use
msg.files only when it is a non-empty array, otherwise use [msg.file] if
present) before applying the thumbnail filter and then push to result.uploads
and call exportMessageObject; update the files assignment near the existing
files const and keep using result.uploads.push(...files) and
result.messages.push(exportMessageObject(exportType, messageObject, files)).
| // handle both new format (msg.files array) and old format (msg.file) for backward compatibility | ||
| // and filter out thumbnails (typeGroup === 'thumb') to only include actual files | ||
| const files = (msg.files || (msg.file ? [msg.file] : [])).filter((file) => file && file.typeGroup !== 'thumb'); | ||
|
|
||
| result.messages.push(exportMessageObject(exportType, messageObject, msg.file)); | ||
| result.uploads.push(...files); | ||
| result.messages.push(exportMessageObject(exportType, messageObject, files)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid dropping legacy msg.file when msg.files is empty.
If msg.files is an empty array but msg.file exists, the current fallback skips the legacy file. Consider falling back when the array is empty to prevent missing attachments.
🛠️ Suggested fix
- const files = (msg.files || (msg.file ? [msg.file] : [])).filter((file) => file && file.typeGroup !== 'thumb');
+ const filesSource = msg.files && msg.files.length ? msg.files : msg.file ? [msg.file] : [];
+ const files = filesSource.filter((file) => file && file.typeGroup !== 'thumb');🤖 Prompt for AI Agents
In `@apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts` around lines
234 - 239, The current fallback logic for files drops a legacy msg.file when
msg.files exists but is empty; change the selection so that if msg.files is
absent or an empty array you fall back to msg.file (e.g., use msg.files only
when it is a non-empty array, otherwise use [msg.file] if present) before
applying the thumbnail filter and then push to result.uploads and call
exportMessageObject; update the files assignment near the existing files const
and keep using result.uploads.push(...files) and
result.messages.push(exportMessageObject(exportType, messageObject, files)).
Proposed changes (including videos or screenshots)
Messages with multiple file attachments were only exporting the first file when using the "Export Room Messages" feature.
The
exportRoomMessagesfunction only handledmsg.file(deprecated single file property) and ignoredmsg.files(new array for multiple files).Issue(s)
Steps to test or reproduce
*you'll have to enable email locally - more infos can be found here: #38183
Before the change
After the change
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.