fix: Bug - Video attachment on android gets sent in the message bubble above with no preview image
This commit is contained in:
@@ -97,6 +97,10 @@ sequenceDiagram
|
||||
User->>DC: broadcastMessage(delete-message)
|
||||
```
|
||||
|
||||
### Attachment binding (pre-allocated message id)
|
||||
|
||||
`ChatMessagesComponent.handleMessageSubmitted` pre-allocates the message id via `planChatMessageSend` (`domain/rules/chat-message-send.rules.ts`), passes it to `MessagesActions.sendMessage({ id, ... })`, and binds any pending files to that **same** id with `AttachmentFacade.publishAttachments(id, ...)`. The send effect honours a provided `id` (falling back to a fresh UUID). Never re-discover the just-sent message by matching its `content` to attach files — caption-less media all share an empty content string, so a content match is ambiguous and races the async create-effect, grouping attachments onto a sibling bubble and leaving an empty message behind.
|
||||
|
||||
## Message integrity
|
||||
|
||||
Outgoing creates/edits/deletes also emit signed `message-revision` events and persist revision audit rows locally. Sync inventories include `revision` and `headHash`; merge prefers a verified higher revision over legacy timestamp comparison. See `agents-docs/features/message-integrity.md` and `MessageRevisionService`.
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
import {
|
||||
describe,
|
||||
expect,
|
||||
it
|
||||
} from 'vitest';
|
||||
|
||||
import { planChatMessageSend } from './chat-message-send.rules';
|
||||
|
||||
function makeFile(name: string): File {
|
||||
return new File([
|
||||
new Uint8Array([
|
||||
1,
|
||||
2,
|
||||
3
|
||||
])
|
||||
], name, { type: 'video/mp4' });
|
||||
}
|
||||
|
||||
describe('planChatMessageSend', () => {
|
||||
it('binds attachments to the exact id carried by the dispatched action', () => {
|
||||
const video = makeFile('clip.mp4');
|
||||
const plan = planChatMessageSend({
|
||||
generateId: () => 'msg-123',
|
||||
content: '',
|
||||
pendingFiles: [video]
|
||||
});
|
||||
|
||||
expect(plan.action.id).toBe('msg-123');
|
||||
expect(plan.attachmentBinding).not.toBeNull();
|
||||
// The whole point of the fix: same id for the action and the attachment binding.
|
||||
expect(plan.attachmentBinding?.messageId).toBe(plan.action.id);
|
||||
expect(plan.attachmentBinding?.files).toEqual([video]);
|
||||
});
|
||||
|
||||
it('gives each caption-less media send a distinct id so attachments never group onto a sibling', () => {
|
||||
const ids = ['msg-1', 'msg-2'];
|
||||
|
||||
let call = 0;
|
||||
|
||||
const generateId = () => ids[call++];
|
||||
const first = planChatMessageSend({ generateId, content: '', pendingFiles: [makeFile('first.mp4')] });
|
||||
const second = planChatMessageSend({ generateId, content: '', pendingFiles: [makeFile('second.mp4')] });
|
||||
|
||||
expect(first.action.id).toBe('msg-1');
|
||||
expect(second.action.id).toBe('msg-2');
|
||||
expect(first.action.id).not.toBe(second.action.id);
|
||||
expect(first.attachmentBinding?.messageId).toBe('msg-1');
|
||||
expect(second.attachmentBinding?.messageId).toBe('msg-2');
|
||||
});
|
||||
|
||||
it('produces no attachment binding when there are no pending files', () => {
|
||||
const plan = planChatMessageSend({
|
||||
generateId: () => 'msg-text-only',
|
||||
content: 'hello',
|
||||
pendingFiles: [],
|
||||
replyToId: 'reply-1',
|
||||
channelId: 'general'
|
||||
});
|
||||
|
||||
expect(plan.attachmentBinding).toBeNull();
|
||||
expect(plan.action).toEqual({
|
||||
id: 'msg-text-only',
|
||||
content: 'hello',
|
||||
replyToId: 'reply-1',
|
||||
channelId: 'general'
|
||||
});
|
||||
});
|
||||
|
||||
it('normalizes a null channel id to undefined', () => {
|
||||
const plan = planChatMessageSend({
|
||||
generateId: () => 'msg-x',
|
||||
content: 'hi',
|
||||
pendingFiles: [],
|
||||
channelId: null
|
||||
});
|
||||
|
||||
expect(plan.action.channelId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,52 @@
|
||||
/**
|
||||
* Pure planning for an outgoing chat message and its attachments.
|
||||
*
|
||||
* The single invariant this module guarantees is that the id carried by the
|
||||
* dispatched `Send Message` action is the *same* id used to bind any pending
|
||||
* attachments. Earlier code dispatched the message without an id and then
|
||||
* tried to re-discover the just-created message by matching its `content`.
|
||||
* For caption-less media (content === '') that match is ambiguous and races
|
||||
* the asynchronous message-create effect, so an attachment could bind to a
|
||||
* sibling message - grouping a video onto the bubble above and leaving an
|
||||
* empty message behind (Android-visible, but a latent bug on every platform).
|
||||
*/
|
||||
export interface ChatMessageSendAction {
|
||||
id: string;
|
||||
content: string;
|
||||
replyToId?: string;
|
||||
channelId?: string;
|
||||
}
|
||||
|
||||
export interface ChatMessageAttachmentBinding {
|
||||
messageId: string;
|
||||
files: File[];
|
||||
}
|
||||
|
||||
export interface ChatMessageSendPlan {
|
||||
action: ChatMessageSendAction;
|
||||
attachmentBinding: ChatMessageAttachmentBinding | null;
|
||||
}
|
||||
|
||||
export interface ChatMessageSendInput {
|
||||
generateId: () => string;
|
||||
content: string;
|
||||
pendingFiles: File[];
|
||||
replyToId?: string;
|
||||
channelId?: string | null;
|
||||
}
|
||||
|
||||
export function planChatMessageSend(input: ChatMessageSendInput): ChatMessageSendPlan {
|
||||
const id = input.generateId();
|
||||
|
||||
return {
|
||||
action: {
|
||||
id,
|
||||
content: input.content,
|
||||
replyToId: input.replyToId,
|
||||
channelId: input.channelId ?? undefined
|
||||
},
|
||||
attachmentBinding: input.pendingFiles.length > 0
|
||||
? { messageId: id, files: input.pendingFiles }
|
||||
: null
|
||||
};
|
||||
}
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
import { toObservable, toSignal } from '@angular/core/rxjs-interop';
|
||||
import { switchMap } from 'rxjs/operators';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
import { ElectronBridgeService } from '../../../../core/platform/electron/electron-bridge.service';
|
||||
import { ViewportService } from '../../../../core/platform';
|
||||
import { BottomSheetComponent } from '../../../../shared';
|
||||
@@ -36,6 +37,7 @@ import { KlipyGifPickerComponent } from '../klipy-gif-picker/klipy-gif-picker.co
|
||||
import { ChatMessageListComponent } from './components/message-list/chat-message-list.component';
|
||||
import { ChatMessageOverlaysComponent } from './components/message-overlays/chat-message-overlays.component';
|
||||
import { stepLightboxIndex } from '../../domain/rules/chat-message-lightbox.rules';
|
||||
import { planChatMessageSend } from '../../domain/rules/chat-message-send.rules';
|
||||
import {
|
||||
ChatLightboxState,
|
||||
ChatMessageComposerSubmitEvent,
|
||||
@@ -117,18 +119,20 @@ export class ChatMessagesComponent {
|
||||
handleMessageSubmitted(event: ChatMessageComposerSubmitEvent): void {
|
||||
this.messageList?.scrollToBottomAfterLocalSend();
|
||||
|
||||
this.store.dispatch(
|
||||
MessagesActions.sendMessage({
|
||||
content: event.content,
|
||||
replyToId: this.replyTo()?.id,
|
||||
channelId: this.activeChannelId()
|
||||
})
|
||||
);
|
||||
const plan = planChatMessageSend({
|
||||
generateId: uuidv4,
|
||||
content: event.content,
|
||||
pendingFiles: event.pendingFiles,
|
||||
replyToId: this.replyTo()?.id,
|
||||
channelId: this.activeChannelId()
|
||||
});
|
||||
|
||||
this.store.dispatch(MessagesActions.sendMessage(plan.action));
|
||||
|
||||
this.clearReply();
|
||||
|
||||
if (event.pendingFiles.length > 0) {
|
||||
setTimeout(() => this.attachFilesToLastOwnMessage(event.content, event.pendingFiles), 100);
|
||||
if (plan.attachmentBinding) {
|
||||
this.attachFilesToMessage(plan.attachmentBinding.messageId, plan.attachmentBinding.files);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -493,21 +497,14 @@ export class ChatMessagesComponent {
|
||||
});
|
||||
}
|
||||
|
||||
private attachFilesToLastOwnMessage(content: string, pendingFiles: File[]): void {
|
||||
private attachFilesToMessage(messageId: string, pendingFiles: File[]): void {
|
||||
const currentUserId = this.currentUser()?.id;
|
||||
const roomId = this.currentRoom()?.id;
|
||||
|
||||
if (!currentUserId)
|
||||
return;
|
||||
|
||||
const message = [...this.channelMessages()]
|
||||
.reverse()
|
||||
.find((entry) => entry.senderId === currentUserId && entry.content === content && !entry.isDeleted);
|
||||
|
||||
if (!message) {
|
||||
setTimeout(() => this.attachFilesToLastOwnMessage(content, pendingFiles), 150);
|
||||
return;
|
||||
if (roomId) {
|
||||
this.attachmentsSvc.rememberMessageRoom(messageId, roomId);
|
||||
}
|
||||
|
||||
this.attachmentsSvc.publishAttachments(message.id, pendingFiles, currentUserId || undefined);
|
||||
this.attachmentsSvc.publishAttachments(messageId, pendingFiles, currentUserId || undefined);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user