feat: migrate sendMessage + getReadReceipts callers to REST#40675
feat: migrate sendMessage + getReadReceipts callers to REST#40675ggazzo wants to merge 5 commits into
Conversation
|
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 |
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40675 +/- ##
===========================================
- Coverage 70.13% 69.82% -0.31%
===========================================
Files 3368 3386 +18
Lines 130022 130393 +371
Branches 22582 22824 +242
===========================================
- Hits 91191 91050 -141
- Misses 35519 36030 +511
- Partials 3312 3313 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
force-pushed
the
chore/ddp-sendmsg-readreceipts-fix
branch
from
ba2cfe9 to
274ea6b
Compare
Two methods that resisted the URL-swap pass in the wider DDP -> REST
migration. This PR contains the deeper refactor each needed.
sendMessage
Replace sdk.call('sendMessage', message, previewUrls) with
sdk.rest.post('/v1/chat.sendMessage', { message, previewUrls }).
In the primary send flow (apps/meteor/client/lib/chats/flows/sendMessage.ts)
the response's server-rendered { message } is fed back into
Messages.state via mapMessageFromApi, replacing the optimistic temp
record in the same tick the REST call resolves. That reproduces the
Minimongo replication the DDP method triggered — composer quote
previews unmount, attachment renderers see attachments[] and urls[],
message _updatedAt advances — all without waiting for the
room-messages stream event to arrive separately.
The seven fire-and-forget callsites do not run the optimistic
reconcile and are straight URL swaps:
- apps/meteor/client/hooks/notification/useNotification.ts
- apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx
- apps/meteor/app/slashcommand-asciiarts/client/{lenny,tableflip,unflip,gimme,shrug}.ts
getReadReceipts
Replace useMethod('getReadReceipts') with
useEndpoint('GET', '/v1/chat.getMessageReadReceipts') in
ReadReceiptsModal. Add rid prop and subscribe to
notify-room/<rid>/messagesRead; on event, invalidate the
['read-receipts', messageId] query so the dialog re-fetches when new
receipts land. mapReadReceiptFromApi revives the Date fields the
REST endpoint serializes as strings.
Server-side: ReadReceipt.markMessagesAsRead /
ReadReceipt.markMessageAsReadBySender /
ReadReceipt.storeThreadMessagesReadReceipts no longer fire-and-forget
the storeReadReceipts insertion — they await it. Closes the
read-after-write race the omnichannel-livechat-read-receipts e2e test
exposed: a fast REST GET could observe a partial set of receipts
because the visitor's self-receipt insert was still in flight when the
test opened the Read-Receipts dialog.
ggazzo
force-pushed
the
chore/ddp-sendmsg-readreceipts-fix
branch
from
274ea6b to
a8c6aa9
Compare
The optimistic record runs through onClientMessageReceived (which the E2EE hook subscribes to) so the ciphertext is rendered as plaintext. The post-REST replacement was bypassing that hook and storing the server-returned ciphertext directly, leaving encrypted rooms showing the raw cipher (or the lastUserMessageBody locator timing out in the e2ee-encrypted-channels e2e suite). Pipe the server response through the same hook before the state replacement. For non-encrypted rooms the hook is a no-op (shouldConvertReceivedMessages returns false), so the change only affects E2EE flows. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The post-REST state.update was replacing the optimistic record with the server's response unconditionally. When the messages stream delivered an update first (read-receipt-driven `unread: false`, async URL/quote attachments arriving via AfterSave hooks, E2EE decrypt results) the REST resolution would overwrite it with the stale snapshot the server captured before those side effects landed — breaking quote rendering in non-encrypted rooms and the 'Message viewed' status icon used by read-receipts e2e tests. Restore the original predicate from the DDP-era code: drop the optimistic `temp` flag only if no other update has touched the record. The stream (Minimongo replication) keeps doing the heavy lifting for async-derived fields. The composer's `dismissAllQuotedMessages()` call already runs from the outer `sendMessage` and doesn't depend on the state.update body, so quote preview unmount keeps working. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
/jira ARCH-2156 |
1 participant
Summary
The two DDP methods that were reverted from #40659 because their client plumbing depends on DDP-specific semantics. This PR contains the deeper refactor each needed.
sendMessagesdk.call('sendMessage', message, previewUrls)→sdk.rest.post('/v1/chat.sendMessage', { message, previewUrls }).apps/meteor/client/lib/chats/flows/sendMessage.ts) feeds the server-rendered{ message }back intoMessages.stateviamapMessageFromApi, replacing the optimistic temp record in the same tick the REST call resolves. Reproduces the Minimongo replication the DDP method triggered.apps/meteor/client/hooks/notification/useNotification.ts(desktop notification reply)apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsxgetReadReceiptsuseMethod('getReadReceipts')→useEndpoint('GET', '/v1/chat.getMessageReadReceipts')inReadReceiptsModal.ridprop and subscribes tonotify-room/<rid>/messagesRead; on event, invalidates the['read-receipts', messageId]react-query so the dialog refetches when new receipts land.mapReadReceiptFromApihelper revives theDatefields the REST endpoint serializes as strings.Server-side race fix
ReadReceipt.markMessagesAsRead,markMessageAsReadBySenderandstoreThreadMessagesReadReceiptsno longer fire-and-forget the innerstoreReadReceipts(...)call — theyawaitit. Closes the read-after-write race that theomnichannel-livechat-read-receiptse2e exposed.Test plan
Task: ARCH-2165