feat: hide unconfirmed uploads from room's file list #38077
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 1a8346a 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 |
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client (RoomFiles hook)
participant API as Server API (channels/groups/im)
participant DB as Files collection
rect rgb(230,248,255)
Note over Client,API: Client requests confirmed files
Client->>API: GET /v1/.../files?onlyConfirmed=true
end
rect rgb(245,255,230)
Note over API,DB: API augments filter\nwith expiresAt: {$exists: false}
API->>DB: find(filter + expiresAt: {$exists: false}, pagination)
DB-->>API: matched file documents
end
rect rgb(255,243,230)
API-->>Client: 200 OK (filtered files)
Note over Client: map -> UI items, decrypt if needed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![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)
.changeset/ninety-pans-search.md (1)
1-6: Clarify whether the behavioral change is default or conditional.The changeset description states that the file list "only show files that have been successfully attached to a message," but based on the AI summary, the client-side hook passes
onlyConfirmed: trueby default. This suggests the filtering is now the default behavior.However, the description doesn't clarify:
- Whether this filtering is always enabled or conditional on the
onlyConfirmedparameter- That unconfirmed (temporary) uploads are no longer visible to end users
Please refine the description to be explicit about the scope and default behavior. For example: "Changed Room Files list to exclude temporary unconfirmed uploads by default."
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (1)
57-57: HardcodedonlyConfirmed: trueprevents viewing temporary uploads.The client always filters out unconfirmed files. While this aligns with the PR objective, consider whether administrators or power users might need to view temporary/unconfirmed uploads for debugging or moderation purposes.
If flexibility is needed in the future, this could be made configurable through an optional parameter or user permission check.
📜 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 (8)
.changeset/ninety-pans-search.mdapps/meteor/app/api/server/v1/channels.tsapps/meteor/app/api/server/v1/groups.tsapps/meteor/app/api/server/v1/im.tsapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tspackages/rest-typings/src/v1/channels/ChannelsFilesListProps.tspackages/rest-typings/src/v1/dm/DmFileProps.tspackages/rest-typings/src/v1/groups/GroupsFilesProps.ts
🧰 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:
packages/rest-typings/src/v1/channels/ChannelsFilesListProps.tspackages/rest-typings/src/v1/dm/DmFileProps.tspackages/rest-typings/src/v1/groups/GroupsFilesProps.tsapps/meteor/app/api/server/v1/channels.tsapps/meteor/app/api/server/v1/im.tsapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/app/api/server/v1/groups.ts
🧠 Learnings (2)
📚 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/ninety-pans-search.md
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
.changeset/ninety-pans-search.md
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
apps/meteor/app/api/server/v1/groups.ts (1)
395-411: LGTM! Conditional filtering implementation is correct.The implementation correctly filters out unconfirmed uploads by excluding documents with an
expiresAtfield whenonlyConfirmedis truthy. The spread operator pattern cleanly handles the optional filter.apps/meteor/app/api/server/v1/channels.ts (1)
810-832: LGTM! Consistent with other endpoints.The filtering logic matches the implementation in groups.ts and im.ts, providing consistent behavior across all room types.
apps/meteor/app/api/server/v1/im.ts (1)
282-299: LGTM! DM endpoint filtering aligns with channels and groups.The implementation maintains consistency across all room types (channels, groups, and DMs) by using the same filtering approach.
packages/rest-typings/src/v1/channels/ChannelsFilesListProps.ts (3)
14-14: LGTM! Type definition correctly adds optional boolean parameter.The addition of
onlyConfirmed?: booleanproperly extends the type to support the new filtering capability.
53-56: LGTM! Schema validation matches type definition.The JSON schema correctly defines
onlyConfirmedas an optional boolean withnullable: true, ensuring type-safe validation at runtime.
1-63: Type coverage confirmed across all endpoint types.The
onlyConfirmedparameter is consistently defined in all three file list prop types:
- ChannelsFilesListProps:
onlyConfirmed?: boolean(line 14, schema line 53)- GroupsFilesProps:
onlyConfirmed?: boolean(line 13, schema line 51)- DmFileProps:
onlyConfirmed?: boolean(line 12, schema line 51)All type guards are exported with consistent naming conventions.
packages/rest-typings/src/v1/groups/GroupsFilesProps.ts (1)
13-13: LGTM! Type and schema changes are consistent.The addition of the optional
onlyConfirmedparameter to both the TypeScript type and JSON schema is correct. The schema properly usesnullable: trueto allow the field to be omitted, and the boolean type aligns with the TypeScript definition.Also applies to: 51-54
packages/rest-typings/src/v1/dm/DmFileProps.ts (1)
8-13: LGTM! Type and schema updates are well-structured.The addition of the optional
onlyConfirmedparameter is correctly implemented in both the type definition and schema. The type reformatting with intersection types improves clarity, and the schema validation properly usesnullable: trueto make the field optional.Also applies to: 51-54
packages/rest-typings/src/v1/channels/ChannelsFilesListProps.ts
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38077 +/- ##
===========================================
- Coverage 70.65% 70.65% -0.01%
===========================================
Files 3145 3145
Lines 108772 108772
Branches 19547 19565 +18
===========================================
- Hits 76855 76849 -6
- Misses 29916 29920 +4
- Partials 2001 2003 +2
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/08 00:03 (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: Thu, 08 Jan 2026 00:03:47 GMT |
Co-authored-by: Ricardo Garim <rswarovsky@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rest-typings/src/v1/groups/GroupsFilesProps.ts (1)
51-53: Addnullable: trueto match the pattern of other optional properties.All other optional properties in this schema (e.g.,
name,typeGroup,offset,count) includenullable: truein their definitions. TheonlyConfirmedproperty should follow the same pattern for consistency and to prevent potential validation issues ifnullis passed.♻️ Proposed fix
onlyConfirmed: { type: 'boolean', + nullable: true, },
📜 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)
packages/rest-typings/src/v1/channels/ChannelsFilesListProps.tspackages/rest-typings/src/v1/dm/DmFileProps.tspackages/rest-typings/src/v1/groups/GroupsFilesProps.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rest-typings/src/v1/channels/ChannelsFilesListProps.ts
- packages/rest-typings/src/v1/dm/DmFileProps.ts
🧰 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:
packages/rest-typings/src/v1/groups/GroupsFilesProps.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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
packages/rest-typings/src/v1/groups/GroupsFilesProps.ts (1)
13-13: LGTM!The optional
onlyConfirmedboolean property correctly extends the type to support filtering unconfirmed uploads, maintaining backward compatibility.
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Issue(s)
CORE-1604
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.