fix: missing reactivity when creating a federated room #37007
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 6dbacd3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 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 a changeset entry for a patch release and modifies the federation branch of createRoom to emit an additional subscription-changed notification immediately after inserting a subscription, addressing composer Join button reactivity when creating federated rooms. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as createRoom (Server)
participant F as Federation
participant DB as DB/Subscriptions
participant N as Notifier
C->>S: createRoom(federated)
S->>F: resolve federated context
S->>DB: insert subscription
DB-->>S: insertedId
rect rgba(230, 246, 255, 0.6)
note right of S: New step
S->>N: notifyOnSubscriptionChangedById(insertedId, "inserted")
end
S->>N: notifyOnRoomChanged(roomId, "changed")
S-->>C: return room/subscription info
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37007 +/- ##
========================================
Coverage 66.21% 66.21%
========================================
Files 3384 3384
Lines 115027 115027
Branches 21064 21069 +5
========================================
+ Hits 76161 76167 +6
+ Misses 36260 36253 -7
- Partials 2606 2607 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![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/pink-pens-march.md (1)
5-5: Polish the release note for clarity.Minor copy edit to improve grammar and flow.
-Fixes reactivity when creating a federated room, that was wrongly showing Join button on composer, requiring a reload or change of route to be able to send a message. +Fixes reactivity when creating a federated room, which incorrectly showed the Join button in the composer and required a reload or route change to send a message.apps/meteor/app/lib/server/functions/createRoom.ts (1)
55-58: Double room “inserted” notifications — confirm intent or make non‑blocking.This block emits
notifyOnRoomChanged(room, 'inserted'), and the top‑level flow emits the same event at Line 252. If the duplicate is unnecessary, consider making this inner call non‑blocking for consistency, or removing it.- await notifyOnSubscriptionChangedById(insertedId, 'inserted'); - await notifyOnRoomChanged(room, 'inserted'); + await notifyOnSubscriptionChangedById(insertedId, 'inserted'); + void notifyOnRoomChanged(room, 'inserted'); // keep event, avoid blocking and align with Line 252
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/pink-pens-march.md(1 hunks)apps/meteor/app/lib/server/functions/createRoom.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/createRoom.ts (1)
apps/meteor/app/lib/server/lib/notifyListener.ts (1)
notifyOnSubscriptionChangedById(535-544)
🔇 Additional comments (2)
.changeset/pink-pens-march.md (1)
1-3: Semver and package scope look correct.Patch bump for @rocket.chat/meteor matches the fix scope.
apps/meteor/app/lib/server/functions/createRoom.ts (1)
55-58: Good: emit subscription change for the federation path to restore reactivity.Calling
notifyOnSubscriptionChangedById(insertedId, 'inserted')aligns the federation path with the non‑federation flow and fixes the Join‑button state.
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
FDR-82
FDR-135
Proposed changes (including videos or screenshots)
Join button was incorrectly being displayed after creating a private federated room, due to
Subscriptionsstore change not being notified, so the created room was not found on the store instantly, requiring a reload or changing routes.Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit