feat: Support multipart message replies [WPB-15705] #19953
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for multipart message replies, extending the existing quote/reply functionality to handle multipart messages (messages with text and attachments). The implementation mirrors the existing single-part message reply handling by adding parallel code paths for multipart messages throughout the quote processing pipeline.
- Adds quote support to MultipartMessageAddEvent type definition
- Extends reply/quote middleware to handle multipart messages alongside regular messages
- Updates event service filters to find replies in both message types
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| RepliesUpdaterMiddleware.ts | Adds multipart message edit event handling and updates reply invalidation/update logic to support both message types |
| QuoteDecoderMiddleware.ts | Implements quote decoding for multipart messages with new handlers for add and edit events |
| EventService.ts | Updates event filters to search for quotes in both regular and multipart message structures |
| EventBuilder.ts | Extends MultipartMessageAddEvent type to include quote property and associated metadata fields |
| MessageHasher.ts | Adds hash generation support for multipart message content |
src/script/repositories/event/preprocessor/RepliesUpdaterMiddleware.ts
Outdated
Show resolved
Hide resolved
src/script/repositories/event/preprocessor/RepliesUpdaterMiddleware.ts
Outdated
Show resolved
Hide resolved
src/script/repositories/event/preprocessor/RepliesUpdaterMiddleware.ts
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #19953 +/- ##
==========================================
- Coverage 44.62% 44.54% -0.08%
==========================================
Files 1311 1311
Lines 33029 33086 +57
Branches 7315 7334 +19
==========================================
- Hits 14739 14738 -1
- Misses 16539 16591 +52
- Partials 1751 1757 +6 🚀 New features to boost your workflow:
|
|
🔗 Download Full Report Artifact 🧪 Playwright Test Summary
specs/Accessibility/Accessibility.spec.ts (❌ 1 failed,
|
|
| /** | ||
| * will update the message ID of all the replies to an edited multipart message | ||
| */ | ||
| private async handleMultipartEditEvent(event: MultipartMessageAddEvent, originalMessageId: string) { | ||
| const {originalEvent, replies} = await this.findRepliesToMessage(event.conversation, originalMessageId, event.id); | ||
| if (!originalEvent || !event.id) { | ||
| return event; | ||
| } | ||
|
|
||
| this.logger.info(`Updating '${replies.length}' replies to updated multipart message '${originalMessageId}'`); | ||
| await this.updateRepliesToEditedMessage(replies, event.id); | ||
| return event; |
There was a problem hiding this comment.
The handleEditEvent and handleMultipartEditEvent methods have identical implementations. Consider refactoring to eliminate this duplication by making a single method that accepts MessageAddEvent | MultipartMessageAddEvent as its parameter type.
| ? originalEvent.data.text.quote | ||
| : originalEvent.data.quote; | ||
|
|
||
| const decoratedData = {...event.data, text: {...event.data.text, quote: originalQuote} as any}; |
There was a problem hiding this comment.
The 'as any' type assertion circumvents TypeScript's type safety. The issue appears to be that MultiPartContent['text'] doesn't natively include a 'quote' property. Consider updating the type definition of MultiPartContent in the wireapp/core library to properly include quote support, or create a local type that extends MultiPartContent['text'] with the quote property to maintain type safety.
| }, | ||
| }; | ||
|
|
||
| const decoratedData = {...event.data, text: {...event.data.text, quote: quoteData} as any}; |
There was a problem hiding this comment.
The 'as any' type assertion circumvents TypeScript's type safety. Consider creating a properly typed interface that extends MultiPartContent['text'] to include the quote property, maintaining type safety throughout the codebase.
| hash: quote.quotedMessageSha256, | ||
| }; | ||
|
|
||
| const decoratedData = {...event.data, text: {...event.data.text, quote: quoteData} as any}; |
There was a problem hiding this comment.
The 'as any' type assertion circumvents TypeScript's type safety. Consider creating a properly typed interface that extends MultiPartContent['text'] to include the quote property, maintaining type safety throughout the codebase.
| if (reply.type === ClientEvent.CONVERSATION.MESSAGE_ADD) { | ||
| reply.data.quote = {error: {type: QuoteEntity.ERROR.MESSAGE_NOT_FOUND}}; | ||
| } else if (reply.type === ClientEvent.CONVERSATION.MULTIPART_MESSAGE_ADD && reply.data.text) { | ||
| reply.data.text.quote = {error: {type: QuoteEntity.ERROR.MESSAGE_NOT_FOUND}} as any; |
There was a problem hiding this comment.
The 'as any' type assertion circumvents TypeScript's type safety. Consider creating a properly typed interface that extends the text quote property to maintain type safety.
| reply.data.text.quote = {error: {type: QuoteEntity.ERROR.MESSAGE_NOT_FOUND}} as any; | |
| type TextQuote = NonNullable<NonNullable<typeof reply.data.text>['quote']>; | |
| const messageNotFoundQuote: TextQuote = {error: {type: QuoteEntity.ERROR.MESSAGE_NOT_FOUND}}; | |
| reply.data.text.quote = messageNotFoundQuote; |
Pull Request
Summary
What did I change and why?
This PR adds support for multipart message replies, extending the existing quote/reply functionality to handle multipart messages (messages with text and attachments). The implementation mirrors the existing single-part message reply handling by adding parallel code paths for multipart messages throughout the quote processing pipeline.
Adds quote support to MultipartMessageAddEvent type definition
Extends reply/quote middleware to handle multipart messages alongside regular messages
Updates event service filters to find replies in both message types
Added e2e test for replying to multipart message TC-8787
Risks and how to roll out / roll back (e.g. feature flags):
none.
Security Checklist (required)
Accessibility (required)
Standards Acknowledgement (required)