From 22b8392fd556ca8c880b53d64ecfb1aaba46385e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oriol=20G=C3=B3mez?= Date: Mon, 25 Aug 2025 09:05:05 +0200 Subject: [PATCH] fix: fix delete message logic --- .../src/components/chat/MessageItem.vue | 44 +++++++++- frontend-vue/src/services/sync.ts | 86 ++++++++----------- frontend-vue/src/stores/app.ts | 21 ++++- 3 files changed, 93 insertions(+), 58 deletions(-) diff --git a/frontend-vue/src/components/chat/MessageItem.vue b/frontend-vue/src/components/chat/MessageItem.vue index d036570..d01775d 100644 --- a/frontend-vue/src/components/chat/MessageItem.vue +++ b/frontend-vue/src/components/chat/MessageItem.vue @@ -39,6 +39,7 @@ import { useAudio } from '@/composables/useAudio' import { useToastStore } from '@/stores/toast' import { useAppStore } from '@/stores/app' import { apiService } from '@/services/api' +import { syncService } from '@/services/sync' import { formatSmartTimestamp, formatTimestampForScreenReader } from '@/utils/time' import FileAttachment from './FileAttachment.vue' import type { ExtendedMessage, UnsentMessage, FileAttachment as FileAttachmentType } from '@/types' @@ -205,11 +206,48 @@ const handleDelete = async () => { return } - // Sent message: call API then update store + // Sent message: optimistic removal, then server delete const msg = props.message as ExtendedMessage - await apiService.deleteMessage(msg.channel_id, msg.id) + + // Capture original position for potential rollback + const channelMessages = appStore.messages[msg.channel_id] || [] + const originalIndex = channelMessages.findIndex(m => m.id === msg.id) + + // Optimistically remove from local state for snappy UI appStore.removeMessage(msg.id) - toastStore.success('Message deleted') + + // Focus the closest message immediately after local removal + await nextTick() + if (targetToFocus && document.contains(targetToFocus)) { + if (!targetToFocus.hasAttribute('tabindex')) targetToFocus.setAttribute('tabindex', '-1') + targetToFocus.focus() + } else { + focusFallbackToInput() + } + + try { + await apiService.deleteMessage(msg.channel_id, msg.id) + // Attempt to sync the channel to reconcile with server state + try { + await syncService.syncChannelMessages(msg.channel_id) + } catch (syncError) { + console.warn('Post-delete sync failed; continuing with local state.', syncError) + } + toastStore.success('Message deleted') + } catch (error) { + // Rollback local removal on failure + if (originalIndex !== -1) { + const list = appStore.messages[msg.channel_id] || [] + list.splice(Math.min(originalIndex, list.length), 0, msg) + } + await nextTick() + const restoredEl = document.querySelector(`[data-message-id="${msg.id}"]`) as HTMLElement | null + if (restoredEl) { + if (!restoredEl.hasAttribute('tabindex')) restoredEl.setAttribute('tabindex', '-1') + restoredEl.focus() + } + throw error + } // focus the closest message await nextTick() if (targetToFocus && document.contains(targetToFocus)) { diff --git a/frontend-vue/src/services/sync.ts b/frontend-vue/src/services/sync.ts index 524aa98..b74475c 100644 --- a/frontend-vue/src/services/sync.ts +++ b/frontend-vue/src/services/sync.ts @@ -8,7 +8,11 @@ export class SyncService { } /** - * Sync messages for a channel: merge server data with local data + * Sync messages for a channel: replace local data with server data + * + * Prunes any local messages that are no longer present on the server + * instead of keeping them around. We still keep unsent messages in the + * separate unsent queue handled elsewhere. */ async syncChannelMessages(channelId: number): Promise { try { @@ -19,55 +23,35 @@ export class SyncService { // Get server messages const serverResponse = await apiService.getMessages(channelId) const serverMessages = serverResponse.messages - - // Get local messages - const localMessages = appStore.messages[channelId] || [] - - console.log(`Server has ${serverMessages.length} messages, local has ${localMessages.length} messages`) - - // Merge messages using a simple strategy: - // 1. Create a map of all messages by ID - // 2. Server messages take precedence (they may have been updated) - // 3. Keep local messages that don't exist on server (may be unsent) - - const messageMap = new Map() - - // Add local messages first - localMessages.forEach(msg => { - if (typeof msg.id === 'number') { - messageMap.set(msg.id, msg) - } - }) - - // Add/update with server messages (server wins for conflicts) - serverMessages.forEach((msg: any) => { - // Transform server message format to match our types - const transformedMsg: ExtendedMessage = { - id: msg.id, - channel_id: msg.channelId || msg.channel_id, - content: msg.content, - created_at: msg.createdAt || msg.created_at, - file_id: msg.fileId || msg.file_id, - // Map the flattened file fields from backend - fileId: msg.fileId, - filePath: msg.filePath, - fileType: msg.fileType, - fileSize: msg.fileSize, - originalName: msg.originalName, - fileCreatedAt: msg.fileCreatedAt - } - console.log(`Sync: Processing message ${msg.id}, has file:`, !!msg.fileId, `(${msg.originalName})`) - messageMap.set(msg.id, transformedMsg) - }) - - // Convert back to array, sorted by creation time - const mergedMessages = Array.from(messageMap.values()) - .sort((a, b) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime()) - - console.log(`Merged result: ${mergedMessages.length} messages`) - - // Update local storage - appStore.setMessages(channelId, mergedMessages) + + console.log(`Server has ${serverMessages.length} messages, replacing local set for channel ${channelId}`) + + // Transform and sort server messages only (pruning locals not on server) + const normalizedServerMessages: ExtendedMessage[] = serverMessages + .map((msg: any) => { + const transformedMsg: ExtendedMessage = { + id: msg.id, + channel_id: msg.channelId || msg.channel_id, + content: msg.content, + created_at: msg.createdAt || msg.created_at, + file_id: msg.fileId || msg.file_id, + // Map the flattened file fields from backend + fileId: msg.fileId, + filePath: msg.filePath, + fileType: msg.fileType, + fileSize: msg.fileSize, + originalName: msg.originalName, + fileCreatedAt: msg.fileCreatedAt + } + console.log(`Sync: Processing message ${msg.id}, has file:`, !!msg.fileId, `(${msg.originalName})`) + return transformedMsg + }) + .sort((a: ExtendedMessage, b: ExtendedMessage) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime()) + + console.log(`Pruned + normalized result: ${normalizedServerMessages.length} messages`) + + // Update local storage with server truth + appStore.setMessages(channelId, normalizedServerMessages) await appStore.saveState() } catch (error) { @@ -302,4 +286,4 @@ export class SyncService { } } -export const syncService = new SyncService() \ No newline at end of file +export const syncService = new SyncService() diff --git a/frontend-vue/src/stores/app.ts b/frontend-vue/src/stores/app.ts index 37e6a43..a4e6db9 100644 --- a/frontend-vue/src/stores/app.ts +++ b/frontend-vue/src/stores/app.ts @@ -71,9 +71,22 @@ export const useAppStore = defineStore('app', () => { if (!messages.value[message.channel_id]) { messages.value[message.channel_id] = [] } - messages.value[message.channel_id].push(message) - console.log('Store: Messages for channel', message.channel_id, 'now has', messages.value[message.channel_id].length, 'messages') - + + const channelMessages = messages.value[message.channel_id] + const existingIndex = channelMessages.findIndex(m => m.id === message.id) + + if (existingIndex !== -1) { + // Upsert: update existing to avoid duplicates from WebSocket vs sync + channelMessages[existingIndex] = { ...channelMessages[existingIndex], ...message } + } else { + channelMessages.push(message) + } + + // Keep chronological order by created_at + channelMessages.sort((a, b) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime()) + + console.log('Store: Messages for channel', message.channel_id, 'now has', channelMessages.length, 'messages') + // Note: Auto-save is now handled by the sync service to avoid excessive I/O } @@ -175,4 +188,4 @@ export const useAppStore = defineStore('app', () => { loadState, saveState } -}) \ No newline at end of file +})