From 1d5b75e19b8123af588b1ea6e80ed8801952a13e Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 3 Sep 2026 17:59:42 -0400 Subject: [PATCH] fix(chat): keep a preview on the row after a delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting the newest message left the conversation list blank until the chat was opened and closed again. Server metadata reports the newest message whatever its state, so the delete arrives as a tombstone `last_message`, and both seats for it — the feed load and a metadata refresh — wrote it to the row verbatim. Nothing draws for a tombstone, so the row went blank with an unread splat beside it. The store now refuses a tombstone as a preview. It keeps the visible message the row already carries, unless that is the message the tombstone replaces, and the feed load fills the gap from the newest visible message in the database — the path that already repaired the row on open, now run where the blank was seated. That is the fallback code-payments/code-android-app#1396 brought Android to; iOS only did it on the paths that already went through `refreshFeedPreview`. The two on-demand hydrates persist the server's copy rather than the store's, so the database still gets the tombstone row the repair reads to tell that the newest message was deleted. --- .../Controllers/ConversationController.swift | 15 ++++- .../Conversation/ConversationStore.swift | 23 ++++++- .../ConversationStoreTests.swift | 60 +++++++++++++++++++ .../ConversationControllerTests.swift | 25 ++++++++ 4 files changed, 120 insertions(+), 3 deletions(-) diff --git a/Flipcash/Core/Controllers/ConversationController.swift b/Flipcash/Core/Controllers/ConversationController.swift index 0ead2db85..2a13af16b 100644 --- a/Flipcash/Core/Controllers/ConversationController.swift +++ b/Flipcash/Core/Controllers/ConversationController.swift @@ -79,7 +79,10 @@ final class ConversationController { do { let conversation = try await fetching.getChat(owner: owner, conversationID: conversationID) store.apply(.metadataRefresh(conversation)) - persistConversation(conversationID) + // The server copy, not the store's: the store drops a tombstone preview, and the + // database wants the row so the repair below can tell the newest message was deleted. + persistConversation(conversation) + refreshFeedPreview(for: conversationID) return conversation } catch { logger.error("Failed to hydrate conversation on demand", metadata: [ @@ -465,7 +468,8 @@ final class ConversationController { do { let conversation = try await fetching.getChat(owner: owner, conversationID: conversationID) store.apply(.metadataRefresh(conversation)) - persistConversation(conversationID) + persistConversation(conversation) + refreshFeedPreview(for: conversationID) } catch { logger.error("Failed to hydrate conversation referenced by the event stream", metadata: [ "conversationID": "\(conversationID)", @@ -504,6 +508,13 @@ final class ConversationController { store.setFeed(conversations, type: type) reconcileHidden() persist(operation: "replace-feed") { try database.replaceConversationFeed(conversations, type: type) } + // The store refuses a tombstone as a preview, so a chat whose newest message is deleted + // seats blank here. Fill it from the newest visible message already cached — the feed + // reloads on every launch and foreground, so without this the row stays blank until the + // transcript is opened. + for conversation in conversations where conversation.lastMessage?.isDeleted == true { + refreshFeedPreview(for: conversation.id) + } return conversations } catch { logger.error("Failed to load conversation feed", metadata: [ diff --git a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift index ac276feb2..64b24cc10 100644 --- a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift +++ b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift @@ -44,7 +44,9 @@ public struct ConversationStore: Sendable { /// Replace the feed from a paged load, sorted most-recent-activity first. public mutating func setFeed(_ conversations: [Conversation]) { - self.conversations = conversations.sorted { $0.lastActivity > $1.lastActivity } + self.conversations = conversations + .map { seated($0) } + .sorted { $0.lastActivity > $1.lastActivity } } /// Replace one type's conversations from that type's paged feed load, @@ -380,6 +382,7 @@ public struct ConversationStore: Sendable { } private mutating func upsert(_ conversation: Conversation) { + let conversation = seated(conversation) if let index = conversations.firstIndex(where: { $0.id == conversation.id }) { conversations[index] = conversation } else { @@ -388,6 +391,24 @@ public struct ConversationStore: Sendable { sort() } + /// `conversation` with a feed preview the row can actually draw: the newest message that still + /// has content. Server metadata reports the newest message whatever its state, so a delete + /// arrives here as a tombstone — which renders as a blank row with an unread splat beside it. + /// It falls back to the visible preview the row already carries, and to nothing when there is + /// none; ``setFeedPreview(_:in:force:)`` then fills it from the database. + /// + /// The fallback is skipped when the tombstone replaces the very message the row was previewing, + /// which would leave deleted content on screen. + private func seated(_ conversation: Conversation) -> Conversation { + guard let tombstone = conversation.lastMessage, tombstone.isDeleted else { return conversation } + var conversation = conversation + let current = conversations.first { $0.id == conversation.id }?.lastMessage + conversation.lastMessage = current.flatMap { current in + !current.isDeleted && current.id != tombstone.id ? current : nil + } + return conversation + } + private mutating func sort() { conversations.sort { $0.lastActivity > $1.lastActivity } } diff --git a/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreTests.swift b/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreTests.swift index aa2da6faf..717d8a0d3 100644 --- a/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreTests.swift +++ b/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreTests.swift @@ -33,6 +33,14 @@ struct ConversationStoreTests { ConversationMessage(id: MessageID(value: id), senderID: nil, content: .text(text), date: Date(timeIntervalSince1970: at), unreadSeq: 0, eventSequence: eventSequence) } + private func tombstone(_ id: UInt64, at: TimeInterval = 0, eventSequence: UInt64 = 0) -> ConversationMessage { + ConversationMessage( + id: MessageID(value: id), senderID: nil, + content: .deleted(.init(deletedBy: nil, deletedAt: Date(timeIntervalSince1970: at))), + date: Date(timeIntervalSince1970: at), unreadSeq: 0, eventSequence: eventSequence + ) + } + private func pending(_ clientID: UUID, _ text: String, sender: UserID? = nil, at: TimeInterval = 0, status: SendStatus = .sending) -> ConversationMessage { ConversationMessage(id: MessageID(value: .max), senderID: sender, content: .text(text), date: Date(timeIntervalSince1970: at), unreadSeq: 0, status: status, clientMessageID: clientID) } @@ -110,6 +118,58 @@ struct ConversationStoreTests { #expect(store.conversations.first?.lastMessage?.id.value == 9) } + // MARK: - Deleted-message previews + + @Test("A metadata refresh whose newest message was deleted keeps the visible preview the row shows") + func metadataRefreshDoesNotBlankTheRowWithATombstone() { + var store = ConversationStore() + store.setFeed([conversation(1, lastActivity: 100)]) + store.setFeedPreview(message(9, "still here"), in: conversationID(1)) + + // The server reports the newest message whatever its state, so a delete arrives as a + // tombstone at a *higher* id than the visible message the row falls back to. + store.apply(.metadataRefresh(Conversation( + id: conversationID(1), members: [], lastMessage: tombstone(10), + lastActivity: Date(timeIntervalSince1970: 100) + ))) + + #expect(store.conversations.first?.lastMessage?.id.value == 9) + } + + @Test("A feed load previewing a tombstone seats no preview, so no unread splat sits beside a blank row") + func feedLoadDropsATombstonePreview() { + let me = UUID() + var store = ConversationStore() + // Nothing cached to fall back to: the only message in the chat was deleted. + store.setFeed([Conversation( + id: conversationID(1), + members: [ConversationMember(userID: me, displayName: "", readPointer: MessageID(value: 1))], + lastMessage: tombstone(10), + lastActivity: Date(timeIntervalSince1970: 100) + )]) + + #expect(store.conversations.first?.lastMessage == nil) + #expect(store.conversations.first?.hasUnread(for: me) == false) + } + + @Test("A tombstone for the previewed message drops it rather than leaving deleted content on the row") + func deletingThePreviewedMessageClearsIt() { + var store = ConversationStore() + store.setFeed([conversation(1, lastActivity: 100)]) + store.setFeedPreview(message(10, "about to go"), in: conversationID(1)) + + store.apply(.metadataRefresh(Conversation( + id: conversationID(1), members: [], lastMessage: tombstone(10), + lastActivity: Date(timeIntervalSince1970: 100) + ))) + #expect(store.conversations.first?.lastMessage == nil) + + // The database repair then supplies the message before it — reachable now that the guard has + // no newer preview to compare against. + store.setFeedPreview(message(9, "still here"), in: conversationID(1)) + #expect(store.conversations.first?.lastMessage?.id.value == 9) + } + @Test("setFeedPreview does not re-sort the feed — activity stays feed-owned") func previewDoesNotResort() { var store = ConversationStore() diff --git a/FlipcashTests/ConversationControllerTests.swift b/FlipcashTests/ConversationControllerTests.swift index 772e3ed43..4d0792c74 100644 --- a/FlipcashTests/ConversationControllerTests.swift +++ b/FlipcashTests/ConversationControllerTests.swift @@ -731,6 +731,31 @@ struct ConversationControllerTests { #expect(controller.messages(for: ConversationID.test(1)).map(\.id.value) == [3, 4]) } + @Test("a feed reporting a deleted newest message previews the message before it, not a blank row") + func feedLoadFallsBackPastATombstone() async throws { + let mock = MockConversations() + let visible = ConversationMessage(id: MessageID(value: 9), senderID: nil, content: .text("hi"), date: Date(timeIntervalSince1970: 90), unreadSeq: 9, eventSequence: 9) + mock.feed = [Conversation(id: ConversationID.test(1), members: [], lastMessage: visible, lastActivity: Date(timeIntervalSince1970: 100))] + mock.messages = [visible] + let controller = makeController(mock) + await controller.loadFeed() + + // The counterpart's newest message is deleted while the chat is closed, so the client never + // sees the event — the delete reaches it only as feed metadata, whose `lastMessage` is the + // tombstone. Seating that verbatim leaves the row blank until the transcript is opened. + let tombstone = ConversationMessage( + id: MessageID(value: 10), senderID: nil, + content: .deleted(.init(deletedBy: nil, deletedAt: Date(timeIntervalSince1970: 100))), + date: Date(timeIntervalSince1970: 100), unreadSeq: 10, eventSequence: 10 + ) + mock.feed = [Conversation(id: ConversationID.test(1), members: [], lastMessage: tombstone, lastActivity: Date(timeIntervalSince1970: 100))] + await controller.loadFeed() + + let conversation = try #require(controller.conversation(withID: .test(1))) + #expect(conversation.lastMessage?.id.value == 9) + #expect(controller.lastMessagePreview(for: conversation) { _ in nil } == "hi") + } + @Test("a second feed load leaves an already-cached transcript alone") func loadFeedSkipsCachedConversation() async { let mock = MockConversations()