feat: Support edit for multipart messages [WPB-20940] #19950
Closed
+930
−264
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
There was a problem hiding this comment.
Pull request overview
This pull request adds support for editing multipart messages (messages that contain both text and attachments like cells/tables). The implementation extends the existing message edit functionality to handle the more complex multipart message structure.
Key changes:
- Extended event handling to support editing multipart messages in addition to regular text messages
- Added logic to preserve attachments when editing multipart messages
- Updated type definitions to include multipart message events in editable event types
- Modified the cryptography mapper to handle edited multipart messages
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Dependency updates for @wireapp packages and AWS SDK (3.940.0 → 3.958.0) - routine maintenance |
| package.json | Core library update (@wireapp/core: 46.46.11 → 46.47.0) |
| getCommonMessageUpdates.ts | Added multipart message handling logic to preserve attachments and common properties during edits |
| editedEventHandler.ts | Extended edit validation to accept MULTIPART_MESSAGE_ADD events alongside MESSAGE_ADD |
| Message.ts | Added helper methods to identify and retrieve multipart assets from messages |
| CryptographyMapper.ts | Added multipart message mapping for edited messages |
| MessageRepository.ts | Implemented sendEditMultiPart method and routing logic to choose between text/multipart edit paths |
| EventMapper.ts | Enhanced updateMessageEvent to handle multipart message updates and preserve attachments |
| EventBuilder.ts | Extended MultipartMessageAddEvent type definition to include edit-related fields |
Comments suppressed due to low confidence (1)
src/script/repositories/cryptography/CryptographyMapper.ts:640
- The multipart message mapping doesn't apply the same mention limit validation that regular text messages receive. In
_mapText, there's a check limiting mentions toMAX_MENTIONS_PER_MESSAGE, but when_mapMultipartcalls_mapText, this validation only applies to the text portion. However, since multipart messages can have attachments that might reference user data, consider whether additional validation is needed for the attachments array to prevent potential resource exhaustion or abuse.
private _mapMultipart(text: Text, attachments: MultiPartContent['attachments']): MappedMultipart {
const mappedText = this._mapText(text);
return {
data: {
text: mappedText.data,
attachments,
},
type: ClientEvent.CONVERSATION.MULTIPART_MESSAGE_ADD,
};
}
...t/repositories/event/preprocessor/EventStorageMiddleware/eventHandlers/editedEventHandler.ts
Show resolved
Hide resolved
...ositories/event/preprocessor/EventStorageMiddleware/eventHandlers/getCommonMessageUpdates.ts
Outdated
Show resolved
Hide resolved
...ositories/event/preprocessor/EventStorageMiddleware/eventHandlers/getCommonMessageUpdates.ts
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #19950 +/- ##
==========================================
+ Coverage 44.62% 44.64% +0.01%
==========================================
Files 1311 1311
Lines 33029 33064 +35
Branches 7315 7330 +15
==========================================
+ Hits 14739 14761 +22
- Misses 16539 16547 +8
- Partials 1751 1756 +5 🚀 New features to boost your workflow:
|
Contributor
|
🔗 Download Full Report Artifact 🧪 Playwright Test Summary
specs/Accessibility/Accessibility.spec.ts (❌ 1 failed,
|
zskhan
reviewed
Dec 29, 2025
...t/repositories/event/preprocessor/EventStorageMiddleware/eventHandlers/editedEventHandler.ts
Outdated
Show resolved
Hide resolved
…are/eventHandlers/editedEventHandler.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
||
| hasMultipartAsset(): boolean { | ||
| hasMultipartAsset(): this is ContentMessage { | ||
| return this.isContent() ? this.assets().some(assetEntity => assetEntity.type === AssetType.MULTIPART) : false; |
There was a problem hiding this comment.
There's an inconsistency in how multipart assets are checked. The hasMultipartAsset() method uses assetEntity.type === AssetType.MULTIPART while getMultipartAssets() uses assetEntity.isMultipart(). While both work, it's better to use the type guard method consistently for clarity and maintainability.
Consider updating line 201 to:
return this.isContent() ? this.assets().some(assetEntity => assetEntity.isMultipart()) : false;
Suggested change
| return this.isContent() ? this.assets().some(assetEntity => assetEntity.type === AssetType.MULTIPART) : false; | |
| return this.isContent() ? this.assets().some(assetEntity => assetEntity.isMultipart()) : false; |
test/e2e_tests/specs/CriticalFlow/Cells/editMultipartMessage-TC-8786.spec.ts
Outdated
Show resolved
Hide resolved
test/e2e_tests/specs/CriticalFlow/Cells/editMultipartMessage-TC-8786.spec.ts
Outdated
Show resolved
Hide resolved
…C-8786.spec.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…C-8786.spec.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (3)
src/script/repositories/event/preprocessor/EventStorageMiddleware/eventHandlers/editedEventHandler.ts:90
- The
editedEventHandler.tsfile has been modified to support multipart message editing, including new validation logic that prevents cross-type edits (line 51-53). However, there are no test files for this handler. Given that other event handlers in this directory have test coverage (e.g., reactionEventHandler.test.ts), tests should be added to verify:
- Multipart messages can be edited correctly
- Cross-type edits are prevented (editing text message as multipart or vice versa)
- Validation properly rejects edits from different users
- The handler correctly identifies and processes multipart edit events
export type EditableEvent = MessageAddEvent | MultipartMessageAddEvent;
function validateEditEvent(
originalEvent: HandledEvents | undefined,
editEvent: EditableEvent,
): originalEvent is StoredEvent<EditableEvent> {
if (!originalEvent) {
throwValidationError('Edit event without original event');
}
if (
originalEvent.type !== ClientEvent.CONVERSATION.MESSAGE_ADD &&
originalEvent.type !== ClientEvent.CONVERSATION.MULTIPART_MESSAGE_ADD
) {
throwValidationError('Edit event for non-text message');
}
// do not allow invalid cross-type edits
if (originalEvent.type !== editEvent.type) {
throwValidationError('Edit event type does not match original event type');
}
if (originalEvent.from !== editEvent.from) {
throwValidationError('ID reused by other user');
}
return true;
}
function getUpdatesForEditMessage(originalEvent: StoredEvent<EditableEvent>, newEvent: EditableEvent): EditableEvent {
// Remove reactions, so that likes (hearts) don't stay when a message's text gets edited
const commonUpdates = getCommonMessageUpdates(originalEvent, newEvent);
return {...newEvent, ...commonUpdates, edited_time: newEvent.time, reactions: {}};
}
function computeEventUpdates(originalEvent: StoredEvent<EditableEvent>, newEvent: EditableEvent) {
const primaryKeyUpdate = {primary_key: originalEvent.primary_key};
const updates = getUpdatesForEditMessage(originalEvent, newEvent);
return {...primaryKeyUpdate, ...updates};
}
export const handleEditEvent: EventHandler = async (event, {findEvent}) => {
if (event.type !== CONVERSATION.MESSAGE_ADD && event.type !== CONVERSATION.MULTIPART_MESSAGE_ADD) {
return undefined;
}
const editedEventId = event.data.replacing_message_id;
if (!editedEventId) {
return undefined;
}
const originalEvent = await findEvent(editedEventId);
if (validateEditEvent(originalEvent, event)) {
const updatedEvent = computeEventUpdates(originalEvent, event);
return {type: 'update', event: updatedEvent, updates: updatedEvent};
}
return undefined;
};
src/script/repositories/conversation/EventMapper.ts:690
- The EventMapper has been modified to handle editing of multipart messages in the
updateMessageEventmethod (lines 171-174) and_mapEventMultipartAdd(lines 680-682). However, the test file EventMapper.test.ts does not include tests for multipart message editing scenarios. Tests should be added to verify:
- Multipart messages can be updated when edited (ID changes case at line 171-174)
- The quote data is correctly extracted from multipart messages (line 164)
- Edited multipart messages preserve their attachments
- The edited_time is correctly set for edited multipart messages
updateMessageEvent(originalEntity: ContentMessage, event: LegacyEventRecord): ContentMessage {
const {id, data: eventData, edited_time: editedTime, qualified_conversation} = event;
// Handle quote for both regular text messages and multipart messages
const quoteData = eventData.quote || eventData.text?.quote;
if (quoteData) {
const {hash, message_id: messageId, user_id: userId, error} = quoteData;
originalEntity.quote(new QuoteEntity({error, hash, messageId, userId}));
}
// Handle multipart messages when ID changes (edit case)
if (id !== originalEntity.id && originalEntity.hasMultipartAsset()) {
originalEntity.assets.removeAll();
const multipartAsset = this._mapAssetMultipart(eventData as MultipartMessageAddEvent['data']);
originalEntity.assets.push(multipartAsset);
} else if (id !== originalEntity.id && originalEntity.hasAssetText()) {
// Handle regular text messages when ID changes (edit case)
originalEntity.assets.removeAll();
const textAsset = this._mapAssetText(eventData);
originalEntity.assets.push(textAsset);
} else if (originalEntity.getFirstAsset) {
const asset = originalEntity.getFirstAsset();
if (eventData.status && (asset as FileAsset).status) {
const assetEntity = this._mapAsset(event);
originalEntity.assets([assetEntity]);
}
if (eventData.previews) {
if ((asset as TextAsset).previews().length !== eventData.previews.length) {
const previews = this._mapAssetLinkPreviews(eventData.previews);
(asset as TextAsset).previews(previews as LinkPreviewEntity[]);
}
}
const {
preview_key,
preview_domain = qualified_conversation?.domain || this.fallbackDomain,
preview_otr_key,
preview_sha256,
preview_token,
} = eventData as AssetData;
if (preview_otr_key && preview_key && preview_domain) {
const assetRemoteData = new AssetRemoteData({
assetKey: preview_key,
assetDomain: preview_domain,
otrKey: preview_otr_key,
sha256: preview_sha256,
assetToken: preview_token,
forceCaching: true,
});
(asset as FileAsset).preview_resource(assetRemoteData);
}
}
if (event.reactions) {
originalEntity.reactions(userReactionMapToReactionMap(event.reactions));
originalEntity.version = event.version;
}
if (event.failedToSend) {
originalEntity.failedToSend(event.failedToSend);
}
if (event.fileData) {
originalEntity.fileData(event.fileData);
}
if (event.selected_button_id) {
originalEntity.version = event.version;
}
originalEntity.id = id;
if (originalEntity.isContent() || (originalEntity as Message).isPing()) {
originalEntity.status(event.status ?? StatusType.SENT);
}
originalEntity.replacing_message_id = eventData.replacing_message_id;
if (editedTime || eventData.edited_time) {
originalEntity.edited_timestamp(new Date(editedTime || eventData.edited_time).getTime());
}
return addMetadata(originalEntity, event);
}
/**
* Convert JSON event into a message entity.
*
* @param event Event data
* @param conversationEntity Conversation entity the event belong to
* @returns Mapped message entity
*/
_mapJsonEvent(event: ConversationEvent | ClientConversationEvent, conversationEntity: Conversation) {
let messageEntity;
switch (event.type) {
case CONVERSATION_EVENT.MEMBER_JOIN: {
/* FIXME: the 'as any' is needed here because we need data that comes from the ServiceMiddleware.
* We would need to create a super type that represents an event that has been decorated by middlewares...
*/
messageEntity = this._mapEventMemberJoin(event as any, conversationEntity);
break;
}
case CONVERSATION_EVENT.MEMBER_LEAVE: {
messageEntity = this._mapEventMemberLeave(event);
break;
}
case CONVERSATION_EVENT.RECEIPT_MODE_UPDATE: {
messageEntity = this._mapEventReceiptModeUpdate(event);
break;
}
case CONVERSATION_EVENT.MESSAGE_TIMER_UPDATE: {
messageEntity = this._mapEventMessageTimerUpdate(event);
break;
}
case CONVERSATION_EVENT.RENAME: {
messageEntity = this._mapEventRename(event);
break;
}
case CONVERSATION_EVENT.PROTOCOL_UPDATE: {
messageEntity = this._mapEventProtocolUpdate(event);
break;
}
case ClientEvent.CONVERSATION.ASSET_ADD: {
messageEntity = addMetadata(this._mapEventAssetAdd(event), event);
break;
}
case ClientEvent.CONVERSATION.COMPOSITE_MESSAGE_ADD: {
const addMessage = this._mapEventCompositeMessageAdd(event);
messageEntity = addMetadata(addMessage, event);
break;
}
case ClientEvent.CONVERSATION.DELETE_EVERYWHERE: {
messageEntity = this._mapEventDeleteEverywhere(event);
break;
}
case ClientEvent.CONVERSATION.GROUP_CREATION: {
messageEntity = this._mapEventGroupCreation(event);
break;
}
case ClientEvent.CONVERSATION.INCOMING_MESSAGE_TOO_BIG:
case ClientEvent.CONVERSATION.UNABLE_TO_DECRYPT: {
messageEntity = this._mapEventUnableToDecrypt(event as ErrorEvent);
break;
}
case ClientEvent.CONVERSATION.KNOCK: {
messageEntity = addMetadata(this._mapEventPing(), event);
break;
}
case ClientEvent.CONVERSATION.CALL_TIME_OUT: {
messageEntity = this._mapEventCallingTimeout(event);
break;
}
case ClientEvent.CONVERSATION.FAILED_TO_ADD_USERS: {
messageEntity = this._mapEventFailedToAddUsers(event);
break;
}
case ClientEvent.CONVERSATION.FEDERATION_STOP: {
messageEntity = this._mapEventFederationStop(event);
break;
}
case ClientEvent.CONVERSATION.LEGAL_HOLD_UPDATE: {
messageEntity = this._mapEventLegalHoldUpdate(event);
break;
}
case ClientEvent.CONVERSATION.LOCATION: {
messageEntity = addMetadata(this._mapEventLocation(event), event);
break;
}
case ClientEvent.CONVERSATION.MESSAGE_ADD: {
const addMessage = this._mapEventMessageAdd(event);
messageEntity = addMetadata(addMessage, event);
break;
}
case ClientEvent.CONVERSATION.MULTIPART_MESSAGE_ADD: {
const addMessage = this._mapEventMultipartAdd(event);
messageEntity = addMetadata(addMessage, event);
break;
}
case ClientEvent.CONVERSATION.MISSED_MESSAGES: {
messageEntity = this._mapEventMissedMessages();
break;
}
case ClientEvent.CONVERSATION.JOINED_AFTER_MLS_MIGRATION: {
messageEntity = this._mapEventJoinedAfterMLSMigrationFinalisation();
break;
}
case ClientEvent.CONVERSATION.MLS_MIGRATION_ONGOING_CALL: {
messageEntity = this._mapEventMLSMigrationFinalisationOngoingCall();
break;
}
case ClientEvent.CONVERSATION.MLS_CONVERSATION_RECOVERED: {
messageEntity = this._mapEventMLSConversationRecovered();
break;
}
case ClientEvent.CONVERSATION.ONE2ONE_CREATION: {
messageEntity = this._mapEvent1to1Creation(event);
break;
}
case ClientEvent.CONVERSATION.ONE2ONE_MIGRATED_TO_MLS: {
messageEntity = this._mapEventOneToOneMigratedToMls();
break;
}
case ClientEvent.CONVERSATION.TEAM_MEMBER_LEAVE: {
messageEntity = this._mapEventTeamMemberLeave(event);
break;
}
case ClientEvent.CONVERSATION.VERIFICATION: {
messageEntity = this._mapEventVerification(event);
break;
}
case ClientEvent.CONVERSATION.E2EI_VERIFICATION: {
messageEntity = this._mapEventE2EIVerificationMessage(event);
break;
}
case ClientEvent.CONVERSATION.VOICE_CHANNEL_ACTIVATE: {
messageEntity = this._mapEventVoiceChannelActivate();
break;
}
case ClientEvent.CONVERSATION.VOICE_CHANNEL_DEACTIVATE: {
messageEntity = this._mapEventVoiceChannelDeactivate(event);
break;
}
case ClientEvent.CONVERSATION.FILE_TYPE_RESTRICTED: {
messageEntity = this._mapFileTypeRestricted(event);
break;
}
default: {
const {type, id} = event as LegacyEventRecord;
this.logger.warn(`Ignored unhandled '${type}' event ${id ? `'${id}' ` : ''}`);
throw new ConversationError(
ConversationError.TYPE.MESSAGE_NOT_FOUND,
ConversationError.MESSAGE.MESSAGE_NOT_FOUND,
);
}
}
const {
category,
data,
from,
qualified_from,
id,
primary_key,
time,
type,
version,
from_client_id,
ephemeral_expires,
ephemeral_started,
} = event as LegacyEventRecord;
messageEntity.category = category;
messageEntity.conversation_id = conversationEntity.id;
messageEntity.from = from;
messageEntity.fromDomain = qualified_from?.domain;
messageEntity.fromClientId = from_client_id;
messageEntity.id = id;
messageEntity.primary_key = primary_key;
messageEntity.timestamp(new Date(time).getTime());
messageEntity.type = type;
messageEntity.version = version || 1;
if (data) {
messageEntity.legalHoldStatus = data.legal_hold_status;
}
if (messageEntity.isContent() || messageEntity.isPing()) {
messageEntity.status((event as EventRecord).status ?? StatusType.SENT);
}
if (messageEntity.isComposite()) {
const {selected_button_id, waiting_button_id} = event as LegacyEventRecord;
messageEntity.selectedButtonId(selected_button_id);
messageEntity.waitingButtonId(waiting_button_id);
}
if (messageEntity.isReactable()) {
(messageEntity as ContentMessage).reactions(
userReactionMapToReactionMap((event as LegacyEventRecord).reactions ?? {}),
);
}
if (ephemeral_expires) {
messageEntity.ephemeral_expires(ephemeral_expires);
messageEntity.ephemeral_started(Number(ephemeral_started) || 0);
}
if (isNaN(messageEntity.timestamp())) {
this.logger.warn(`Could not get timestamp for message '${messageEntity.id}'. Skipping it.`);
messageEntity = undefined;
}
return isContentMessage(messageEntity)
? this.updateMessageEvent(messageEntity, event as EventRecord)
: messageEntity;
}
//##############################################################################
// Event mappers
//##############################################################################
/**
* Maps JSON data of conversation.one2one-creation message into message entity.
*
* @param eventData Message data
* @returns Member message entity
*/
private _mapEvent1to1Creation({data: eventData}: LegacyEventRecord) {
const {has_service: hasService, userIds} = eventData;
const messageEntity = new MemberMessage();
messageEntity.memberMessageType = SystemMessageType.CONNECTION_ACCEPTED;
messageEntity.userIds(userIds);
if (hasService) {
messageEntity.showServicesWarning = true;
}
return messageEntity;
}
/**
* Maps JSON data of conversation.asset_add message into message entity.
*
* @param event Message data
* @returns Content message entity
*/
private _mapEventAssetAdd(event: LegacyEventRecord) {
const messageEntity = new ContentMessage();
const assetEntity = this._mapAsset(event);
messageEntity.assets.push(assetEntity);
return messageEntity;
}
/**
* Maps JSON data of delete everywhere event to message entity.
*
* @param eventData Message data
* @returns Delete message entity
*/
private _mapEventDeleteEverywhere({data: eventData}: LegacyEventRecord) {
const messageEntity = new DeleteMessage();
messageEntity.deleted_timestamp = new Date(eventData.deleted_time).getTime();
return messageEntity;
}
/**
* Map JSON ata of group creation event to message entity.
*
* @param eventData Message data
* @returns Member message entity
*/
private _mapEventGroupCreation({data: eventData}: LegacyEventRecord) {
const messageEntity = new MemberMessage();
messageEntity.memberMessageType = SystemMessageType.CONVERSATION_CREATE;
messageEntity.name(eventData.name || '');
messageEntity.userIds(eventData.userIds);
messageEntity.allTeamMembers = eventData.allTeamMembers;
return messageEntity;
}
_mapEventCallingTimeout({data, time}: LegacyEventRecord) {
return new CallingTimeoutMessage(data.reason, parseInt(time, 10));
}
_mapEventFailedToAddUsers({data, time}: FailedToAddUsersMessageEvent) {
return new FailedToAddUsersMessage(data, parseInt(time, 10));
}
_mapEventFederationStop({data, time}: FederationStopEvent) {
return new FederationStopMessage(data.domains, parseInt(time, 10));
}
_mapEventLegalHoldUpdate({data, timestamp}: LegacyEventRecord) {
return new LegalHoldMessage(data.legal_hold_status, timestamp);
}
/**
* Maps JSON data of conversation.location message into message entity.
*
* @param eventData Message data
* @returns Location message entity
*/
private _mapEventLocation({data: eventData}: LegacyEventRecord) {
const location = eventData.location;
const messageEntity = new ContentMessage();
const assetEntity = new Location();
assetEntity.longitude = location.longitude;
assetEntity.latitude = location.latitude;
assetEntity.name = location.name;
assetEntity.zoom = location.zoom;
messageEntity.assets.push(assetEntity);
return messageEntity;
}
/**
* Maps JSON data of conversation.member_join message into message entity.
*
* @param event Message data
* @param conversationEntity Conversation entity the event belong to
* @returns Member message entity
*/
private _mapEventMemberJoin(
event: MemberJoinEvent & {data?: {has_service?: boolean}},
conversationEntity: Conversation,
) {
const {data: eventData, from: sender} = event;
const {has_service: hasService} = eventData;
const userIds = eventData.qualified_user_ids || eventData.user_ids.map(id => ({domain: '', id}));
const messageEntity = new MemberMessage();
const isSingleModeConversation = conversationEntity.is1to1() || conversationEntity.isRequest();
messageEntity.visible(!isSingleModeConversation);
if (conversationEntity.isGroupOrChannel()) {
const messageFromCreator = sender === conversationEntity.creator;
const creatorIndex = userIds.findIndex(user => user.id === sender);
const creatorIsJoiningMember = messageFromCreator && creatorIndex !== -1;
if (creatorIsJoiningMember) {
userIds.splice(creatorIndex, 1);
messageEntity.memberMessageType = SystemMessageType.CONVERSATION_CREATE;
}
if (hasService) {
messageEntity.showServicesWarning = true;
}
messageEntity.userIds(userIds);
}
return messageEntity;
}
/**
* Maps JSON data of conversation.member_leave message into message entity.
*
* @param eventData Message data
* @returns Member message entity
*/
private _mapEventMemberLeave({data: eventData}: MemberLeaveEvent | TeamMemberLeaveEvent) {
const messageEntity = new MemberMessage();
const userIds = eventData.qualified_user_ids || eventData.user_ids.map(id => ({domain: '', id}));
messageEntity.userIds(userIds);
messageEntity.reason = eventData.reason;
return messageEntity;
}
/**
* Maps JSON data of conversation.message_add message into message entity.
*
* @param event Message data
* @returns Content message entity
*/
private _mapEventMessageAdd(event: MessageAddEvent) {
const {data: eventData, edited_time: editedTime} = event;
const messageEntity = new ContentMessage();
const assets = this._mapAssetText(eventData);
messageEntity.assets.push(assets);
messageEntity.replacing_message_id = eventData.replacing_message_id;
if (editedTime) {
messageEntity.edited_timestamp(new Date(editedTime).getTime());
}
if (eventData.quote) {
const {message_id: messageId, user_id: userId, error} = eventData.quote as any;
messageEntity.quote(new QuoteEntity({error, messageId, userId}));
}
return messageEntity;
}
/**
* Maps JSON data of conversation.message_add message into message entity.
*
* @param event Message data
* @returns Content message entity
*/
private _mapEventMultipartAdd(event: MultipartMessageAddEvent) {
const {data: eventData, edited_time: editedTime} = event;
const messageEntity = new ContentMessage();
const assets = this._mapAssetMultipart(eventData);
messageEntity.assets.push(assets);
messageEntity.replacing_message_id = eventData.replacing_message_id;
if (editedTime) {
messageEntity.edited_timestamp(new Date(editedTime).getTime());
}
if (eventData.text?.quote) {
const {message_id: messageId, user_id: userId, error} = eventData.text.quote as any;
messageEntity.quote(new QuoteEntity({error, messageId, userId}));
}
return messageEntity;
src/script/repositories/cryptography/CryptographyMapper.ts:513
- The
_mapEditedmethod in CryptographyMapper has been modified to handle multipart message edits (lines 500-508), including validation that edited multipart messages must have text content. However, the test file CryptographyMapper.test.ts does not include tests for the edited message mapping functionality. Tests should be added to verify:
- Multipart message edits are correctly mapped
- The validation error is thrown when multipart.text is missing
- The replacing_message_id is correctly set for edited multipart messages
- Both text and multipart edits work correctly
private _mapEdited(edited: MessageEdit) {
if (edited.multipart) {
if (!edited.multipart.text) {
const message = 'Edited multipart message is missing required text content.';
throw new CryptographyError(CryptographyError.TYPE.UNHANDLED_TYPE, message);
}
const mappedMultipart = this._mapMultipart(edited.multipart.text as Text, edited.multipart.attachments);
mappedMultipart.data.replacing_message_id = edited.replacingMessageId;
return mappedMultipart;
}
const mappedMessage = this._mapText(edited.text as Text);
mappedMessage.data.replacing_message_id = edited.replacingMessageId;
return mappedMessage;
}
Comment on lines
+36
to
+39
| await textLocator.waitFor({state: 'visible', timeout: 5000}); | ||
| await imageLocator.waitFor({state: 'visible', timeout: 5000}); | ||
|
|
||
| return (await textLocator.isVisible()) && (await imageLocator.isVisible()); |
There was a problem hiding this comment.
The isMultipartMessageVisible method performs separate waitFor operations on the text and image locators, but then checks their visibility again. If either element becomes hidden between the waitFor and the final isVisible checks, this could lead to flaky test behavior. Consider checking visibility immediately after waiting, or combine the checks into a single operation that verifies both elements are visible.
Suggested change
| await textLocator.waitFor({state: 'visible', timeout: 5000}); | |
| await imageLocator.waitFor({state: 'visible', timeout: 5000}); | |
| return (await textLocator.isVisible()) && (await imageLocator.isVisible()); | |
| await Promise.all([ | |
| textLocator.waitFor({state: 'visible', timeout: 5000}), | |
| imageLocator.waitFor({state: 'visible', timeout: 5000}), | |
| ]); | |
| return true; |
| @@ -0,0 +1,240 @@ | |||
| /* | |||
| * Wire | |||
| * Copyright (C) 2018 Wire Swiss GmbH | |||
There was a problem hiding this comment.
The copyright year in the header is 2018, but this is a new file created in 2025. The copyright year should be updated to 2025 to accurately reflect when this file was created.
| }; | ||
| } | ||
|
|
||
| throw new Error('Incompatible event types for computing common message updates'); |
There was a problem hiding this comment.
The error thrown at line 69 may occur if the original and new event types are mismatched (e.g., trying to edit a text message as a multipart message or vice versa). While the validation in editedEventHandler.ts should prevent this, it would be helpful to include the event types in the error message for better debugging. Consider changing the error message to include information about what types were encountered.
Suggested change
| throw new Error('Incompatible event types for computing common message updates'); | |
| const originalType = (originalEvent as any)?.type ?? 'unknown'; | |
| const newType = (newEvent as any)?.type ?? 'unknown'; | |
| throw new Error( | |
| `Incompatible event types for computing common message updates (original: ${originalType}, new: ${newType})`, | |
| ); |
Comment on lines
+94
to
+95
| // Wait for some time before uploading the file to make sure the cell is ready | ||
| await new Promise(resolve => setTimeout(resolve, 5000)); |
There was a problem hiding this comment.
The hardcoded sleep of 5000ms (5 seconds) is a brittle test practice. Instead of using an arbitrary timeout, consider waiting for a specific condition or UI element to be ready. This makes the test more reliable and potentially faster, as it won't wait longer than necessary.
Suggested change
| // Wait for some time before uploading the file to make sure the cell is ready | |
| await new Promise(resolve => setTimeout(resolve, 5000)); | |
| // Wait until the cells conversation is actually ready before continuing | |
| await userBPages.cellsConversation().waitForConversationToBeReady(); |
Comment on lines
+23
to
70
| import {EditableEvent} from './editedEventHandler'; | ||
|
|
||
| function isMultipartEvent(event: EditableEvent): event is MultipartMessageAddEvent { | ||
| return 'attachments' in event.data && Array.isArray(event.data.attachments); | ||
| } | ||
|
|
||
| function isMessageAddEvent(event: EditableEvent): event is MessageAddEvent { | ||
| return !isMultipartEvent(event); | ||
| } | ||
|
|
||
| export function getCommonMessageUpdates( | ||
| originalEvent: StoredEvent<EditableEvent>, | ||
| newEvent: EditableEvent, | ||
| ): EditableEvent { | ||
| const commonProps = { | ||
| edited_time: originalEvent.edited_time, | ||
| read_receipts: !newEvent.read_receipts ? originalEvent.read_receipts : newEvent.read_receipts, | ||
| status: !newEvent.status || newEvent.status < originalEvent.status ? originalEvent.status : newEvent.status, | ||
| time: originalEvent.time, | ||
| }; | ||
|
|
||
| // Handle multipart messages | ||
| if (isMultipartEvent(newEvent) && isMultipartEvent(originalEvent)) { | ||
| return { | ||
| ...newEvent, | ||
| ...commonProps, | ||
| data: { | ||
| ...newEvent.data, | ||
| attachments: newEvent.data.attachments ?? originalEvent.data.attachments, | ||
| expects_read_confirmation: originalEvent.data.expects_read_confirmation, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| // Handle regular text messages | ||
| if (isMessageAddEvent(newEvent) && isMessageAddEvent(originalEvent)) { | ||
| return { | ||
| ...newEvent, | ||
| ...commonProps, | ||
| data: { | ||
| ...newEvent.data, | ||
| expects_read_confirmation: originalEvent.data.expects_read_confirmation, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| throw new Error('Incompatible event types for computing common message updates'); | ||
| } |
There was a problem hiding this comment.
The test file for getCommonMessageUpdates does not include test cases for the new multipart message editing functionality. Since the function now handles both MessageAddEvent and MultipartMessageAddEvent with different logic paths, tests should be added to ensure the multipart case preserves attachments and handles the expects_read_confirmation field correctly. Consider adding test cases for:
- Editing a multipart message preserves its attachments
- The function throws an error when trying to edit a text message as multipart or vice versa
- Common properties are correctly merged for multipart messages
| const userBPage = await userBContext.newPage(); | ||
| const userBPageManager = new PageManager(userBPage); | ||
|
|
||
| const {pages: userBPages, modals: userBModals} = userBPageManager.webapp; |
There was a problem hiding this comment.
Unused variable userBComponents.
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.
Pull Request
Summary
Added support for editing multipart messages.
Added e2e test for editing multipart message.
none.
Security Checklist (required)
Accessibility (required)
Standards Acknowledgement (required)