chore: Disable message read receipts in federated rooms #37534
+112
−4
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 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 |
🦋 Changeset detectedLatest commit: 01c2166 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
WalkthroughThis pull request implements read receipt suppression in federated rooms by extracting the MessageListContext default value into a named constant, adding unit tests for read receipt features, and modifying the MessageListProvider to conditionally disable read receipts when a room is federated. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Suggested labels
Suggested reviewers
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 |
aleksandernsilva
changed the title
aleksandernsilva
changed the title
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/15 22:28", "11/16 01:28", "11/17 23:50", "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 16:42", "12/08 18: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]
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]
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]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 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]
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]
line "queue-worker-service" [0.14, 0.14, 0.14, 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]
line "rocketchat" [0.36, 0.36, 0.35, 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.35]
line "stream-hub-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]
Statistics (last 15 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 08 Dec 2025 18:03:02 GMT |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37534 +/- ##
===========================================
+ Coverage 67.78% 67.80% +0.02%
===========================================
Files 3449 3449
Lines 113987 113972 -15
Branches 20956 20956
===========================================
+ Hits 77262 77279 +17
+ Misses 34610 34576 -34
- Partials 2115 2117 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
aleksandernsilva
force-pushed
the
chore/disable-read-receipts-federation
branch
from
567d1ac to
58e3a16
Compare
aleksandernsilva
force-pushed
the
chore/disable-read-receipts-federation
branch
from
71892f1 to
7e7aa19
Compare
aleksandernsilva
force-pushed
the
chore/disable-read-receipts-federation
branch
from
7e7aa19 to
01c2166
Compare
![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: 1
📜 Review details
Configuration used: CodeRabbit 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 (5)
.changeset/cold-chefs-rhyme.md(1 hunks)apps/meteor/client/components/message/list/MessageListContext.tsx(2 hunks)apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx(1 hunks)apps/meteor/client/components/message/variants/RoomMessage.spec.tsx(2 hunks)apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx(2 hunks)
🧰 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/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsxapps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsxapps/meteor/client/components/message/variants/RoomMessage.spec.tsxapps/meteor/client/components/message/list/MessageListContext.tsx
🧠 Learnings (11)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
📚 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:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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 : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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 : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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 : Group related tests in the same file
Applied to files:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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 : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx
📚 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:
apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx.changeset/cold-chefs-rhyme.md
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx.changeset/cold-chefs-rhyme.md
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx.changeset/cold-chefs-rhyme.md
🧬 Code graph analysis (3)
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx (3)
apps/meteor/client/components/message/list/MessageListContext.tsx (1)
useMessageListReadReceipts(107-107)apps/meteor/tests/mocks/data.ts (1)
createFakeMessage(93-108)apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.tsx (1)
useReadReceiptsDetailsAction(8-36)
apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx (1)
packages/core-typings/src/IRoom.ts (1)
isRoomFederated(122-122)
apps/meteor/client/components/message/variants/RoomMessage.spec.tsx (2)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/client/components/message/list/MessageListContext.tsx (2)
MessageListContext(79-79)messageListContextDefaultValue(48-77)
🔇 Additional comments (4)
.changeset/cold-chefs-rhyme.md (1)
1-5: LGTM!The changeset clearly documents the change and provides context about re-enabling the feature when federation compatibility is achieved.
apps/meteor/client/components/message/list/MessageListContext.tsx (1)
48-79: LGTM! Clean refactor that improves testability.Extracting the default value into a named constant makes it reusable in test files, enabling cleaner test setup and assertions. The refactor maintains type safety and doesn't alter runtime behavior.
apps/meteor/client/components/message/variants/RoomMessage.spec.tsx (1)
111-159: LGTM! Comprehensive test coverage for read receipts.The tests properly verify that read receipt indicators are shown when enabled and hidden when disabled, using clean context provider wrapping to control feature flags.
apps/meteor/client/components/message/toolbar/useReadReceiptsDetailsAction.spec.tsx (1)
1-50: LGTM! Thorough unit test coverage.The tests comprehensively cover all branches of
useReadReceiptsDetailsAction: returning null when read receipts are disabled, returning null when store users is disabled, and returning the expected action config when both are enabled. Clean mocking and proper test isolation.
|
|
||
| const autoLinkDomains = useSetting('Message_CustomDomain_AutoLink', ''); | ||
| const readReceiptsEnabled = useSetting('Message_Read_Receipt_Enabled', false); | ||
| const readReceiptsEnabled = useSetting('Message_Read_Receipt_Enabled', false) && !isRoomFederated(room); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for read receipt processing in server-side code
rg -n "read.*receipt|receipt.*read" --type=ts --glob='**/server/**' -A 5 -B 2 | head -150Repository: RocketChat/Rocket.Chat
Length of output: 9652
🏁 Script executed:
# Search for federated room checks in read receipt context
rg -n "readReceipt|read_receipt" --type=ts --glob='**/server/**' | head -100Repository: RocketChat/Rocket.Chat
Length of output: 1056
🏁 Script executed:
# Search for isRoomFederated usage in server code
rg -n "isRoomFederated" --type=ts --glob='**/server/**' | head -50Repository: RocketChat/Rocket.Chat
Length of output: 3075
🏁 Script executed:
# Look for read receipt hooks or handlers
rg -n "beforeSetUserStatus|beforeUpdateUserStatusAndRoom|Meteor.methods.*receipt" --type=ts --glob='**/server/**' -A 3 | head -100Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
cat -n apps/meteor/ee/app/message-read-receipt/server/hooks/afterReadMessages.tsRepository: RocketChat/Rocket.Chat
Length of output: 1056
🏁 Script executed:
cat -n apps/meteor/ee/app/message-read-receipt/server/hooks/afterSaveMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 804
🏁 Script executed:
cat -n apps/meteor/ee/server/lib/message-read-receipt/ReadReceipt.ts | head -200Repository: RocketChat/Rocket.Chat
Length of output: 6406
Backend read receipt processing must also be blocked for federated rooms.
The UI change correctly disables read receipt indicators in federated rooms, but backend processing continues unaddressed. The read receipt hooks (afterReadMessages and afterSaveMessage) and the ReadReceipt class methods (markMessagesAsRead, markMessageAsReadBySender, storeThreadMessagesReadReceipts) lack any federated room checks. This causes read receipt data to be stored in the database for federated rooms despite being hidden in the UI, creating data inconsistencies.
Add isRoomFederated checks to these backend methods to prevent unnecessary read receipt storage and processing for federated rooms.
🤖 Prompt for AI Agents
In apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx
around line 43 and in the backend read-receipt code (the afterReadMessages and
afterSaveMessage hooks and the ReadReceipt class methods markMessagesAsRead,
markMessageAsReadBySender, and storeThreadMessagesReadReceipts), the backend
still processes and stores read receipts for federated rooms; update each
backend entrypoint to call isRoomFederated(room) early and skip processing
(return/do nothing) when it returns true, ensuring you import or reference the
same isRoomFederated utility and pass the proper room object so no read receipt
DB writes or further processing occur for federated rooms.
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)
This PR's disables read receipts for federated rooms. This feature will be re-introduced to federated rooms when the capability becomes fully compatible with Federation.
Issue(s)
FB-45
FB-75
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.