diff --git a/apps/flipcash/core/src/main/res/values/strings.xml b/apps/flipcash/core/src/main/res/values/strings.xml index 92f37fa94..7e29676c6 100644 --- a/apps/flipcash/core/src/main/res/values/strings.xml +++ b/apps/flipcash/core/src/main/res/values/strings.xml @@ -898,7 +898,6 @@ Edited You deleted this message This message was deleted - Message deleted Copy diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt index 4b19f2ca3..4a14fee51 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt @@ -79,10 +79,10 @@ private fun ChatSummary.formatPreview( resources.getString(previewRes, label) } - // The conversation list says a message is gone rather than going blank, which would - // otherwise read as the whole conversation having no messages. - is MessageContent.Deleted -> - resources.getString(R.string.label_chat_preview_deletedMessage) + // The feed carries the newest message that still has content, so a tombstone only + // reaches here when every message in the chat is deleted — and then there is nothing + // to preview. + is MessageContent.Deleted -> null // TODO: is MessageContent.Media -> null diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt index cf5cd9f28..addfab9e8 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt @@ -223,8 +223,12 @@ class FeedSyncDelegate @Inject constructor( ): List { return metadataEntities.map { entity -> val members = membersByChat[entity.chatIdHex] ?: emptyList() + // Deliberately the newest *visible* message, not the newest row: deleting the newest + // message drops the feed back to the one before it, so the preview reads that message + // instead of "Message deleted" and its unread splat clears with it (the fallback sits + // at or below the read pointer whenever the deleted message was the only unread one). val lastMessage = entity.lastMessageId?.let { - messageDataSource.getLatest(entity.chatIdHex) + messageDataSource.getLatestVisible(entity.chatIdHex) } metadataDataSource.toMetadata(entity, members, lastMessage) } diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt index ae9f54946..d82b1d194 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt @@ -318,10 +318,13 @@ class MessagingDelegate @Inject constructor( } override suspend fun markAsRead(chatId: ChatId): Result { - val messageId = stateHolder.current.feed - .firstOrNull { it.chatId == chatId } - ?.lastMessage?.messageId - ?: messageDataSource.getLatestMessageId(chatId) + // The stored newest id, tombstones included — the feed's own `lastMessage` skips them so + // the list can preview the last message with content, and reading it here would park the + // pointer below a deleted message and leave the chat permanently unread. + val messageId = messageDataSource.getLatestMessageId(chatId) + ?: stateHolder.current.feed + .firstOrNull { it.chatId == chatId } + ?.lastMessage?.messageId ?: return Result.success(Unit) return advanceReadPointer(chatId, messageId) .also { dismissNotifications(chatId) } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FeedDeletedMessageTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FeedDeletedMessageTest.kt new file mode 100644 index 000000000..613a83b58 --- /dev/null +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FeedDeletedMessageTest.kt @@ -0,0 +1,173 @@ +package com.flipcash.shared.chat + +import com.flipcash.app.persistence.entities.ChatMetadataEntity +import com.flipcash.app.persistence.sources.ChatMemberDataSource +import com.flipcash.app.persistence.sources.ChatMessageDataSource +import com.flipcash.app.persistence.sources.ChatMetadataDataSource +import com.flipcash.services.controllers.ChatController +import com.flipcash.services.models.UserProfile +import com.flipcash.services.models.VerifiableContactMethod +import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMember +import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatMetadata +import com.flipcash.services.models.chat.ChatType +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.services.models.chat.MessagePointer +import com.flipcash.services.models.chat.PointerType +import com.flipcash.services.user.UserManager +import com.flipcash.shared.chat.internal.ChatStateHolder +import com.flipcash.shared.chat.internal.delegates.FeedSyncDelegate +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.time.Instant + +/** + * Deleting the newest message must not leave the conversation list reading "Message deleted" with + * an unread splat beside it. The feed row is built from the newest message that still has content, + * so the preview falls back to the one before the tombstone and the unread check — which compares + * that same message against the READ pointer — clears with it. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class FeedDeletedMessageTest { + + private val selfId = listOf(1, 2, 3) + private val otherId = listOf(4, 5, 6) + private val chatId = ChatId("aabbccdd") + + private fun message(messageId: Long, content: MessageContent) = ChatMessage( + messageId = messageId, + senderId = otherId, + content = listOf(content), + timestamp = Instant.fromEpochSeconds(messageId), + unreadSeq = messageId, + ) + + /** [readPointer] is the signed-in user's READ watermark; the counterparty is addressable. */ + private class Harness(selfId: List, otherId: List, chatId: ChatId, readPointer: Long) { + private val entity = ChatMetadataEntity( + chatIdHex = "aabbccdd", + chatType = ChatType.CONTACT_DM.name, + lastActivityEpochMs = 2_000, + lastMessageId = 2, + ) + + private val members = listOf( + ChatMember( + userId = selfId, + userProfile = UserProfile.Empty, + pointers = listOf( + MessagePointer( + type = PointerType.READ, + userId = selfId, + value = readPointer, + timestamp = Instant.fromEpochSeconds(1_000), + ) + ), + ), + ChatMember( + userId = otherId, + userProfile = UserProfile.Empty.copy( + displayName = "Ada", + phoneNumber = VerifiableContactMethod("+15551234567", verified = true), + ), + pointers = emptyList(), + ), + ) + + val messageDataSource = mockk(relaxed = true) + + private val metadataDataSource = mockk(relaxed = true).also { source -> + every { source.observeAll() } returns flowOf(listOf(entity)) + every { source.toMetadata(any(), any(), any()) } answers { + ChatMetadata( + chatId = chatId, + type = ChatType.CONTACT_DM, + members = secondArg(), + lastMessage = thirdArg(), + lastActivity = Instant.fromEpochSeconds(2), + ) + } + } + + private val memberDataSource = mockk(relaxed = true).also { source -> + every { source.observeAll() } returns flowOf(mapOf(entity.chatIdHex to members)) + } + + val delegate = FeedSyncDelegate( + chatController = mockk(relaxed = true), + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + stateHolder = ChatStateHolder(), + userManager = mockk(relaxed = true).also { + every { it.accountId } returns selfId + every { it.profile } returns null + }, + ) + + suspend fun summary(scope: TestScope): ChatSummary? { + delegate.initialize(scope.backgroundScope) + delegate.observeFeedFromDb() + scope.runCurrent() + return delegate.feed(ChatType.CONTACT_DM).first().firstOrNull() + } + } + + @Test + fun `the preview falls back to the newest message that still has content`() = runTest { + val harness = Harness(selfId, otherId, chatId, readPointer = 1) + // Message 2 was deleted; the visible newest is message 1. + coEvery { harness.messageDataSource.getLatestVisible(any()) } returns + message(1, MessageContent.Text("still here")) + + val summary = harness.summary(this) + + assertEquals(1L, summary?.metadata?.lastMessage?.messageId) + assertEquals( + MessageContent.Text("still here"), + summary?.metadata?.lastMessage?.content?.first(), + ) + } + + @Test + fun `deleting the only unread message clears the splat`() = runTest { + // The READ pointer sits at 1: message 2 arrived unread, then was deleted. + val harness = Harness(selfId, otherId, chatId, readPointer = 1) + coEvery { harness.messageDataSource.getLatestVisible(any()) } returns + message(1, MessageContent.Text("read already")) + + assertEquals(0, harness.summary(this)?.unreadCount) + } + + @Test + fun `an unread message older than the tombstone keeps the splat`() = runTest { + // Nothing has been read, so the message the preview falls back to is itself unread. + val harness = Harness(selfId, otherId, chatId, readPointer = 0) + coEvery { harness.messageDataSource.getLatestVisible(any()) } returns + message(1, MessageContent.Text("never read")) + + assertEquals(1, harness.summary(this)?.unreadCount) + } + + @Test + fun `a chat with nothing but tombstones has no preview`() = runTest { + val harness = Harness(selfId, otherId, chatId, readPointer = 0) + coEvery { harness.messageDataSource.getLatestVisible(any()) } returns null + + val summary = harness.summary(this) + + assertNull(summary?.metadata?.lastMessage) + assertEquals(0, summary?.unreadCount) + } +} diff --git a/apps/flipcash/shared/persistence/db/schemas/com.flipcash.app.persistence.FlipcashDatabase/32.json b/apps/flipcash/shared/persistence/db/schemas/com.flipcash.app.persistence.FlipcashDatabase/32.json new file mode 100644 index 000000000..2ddbfe15c --- /dev/null +++ b/apps/flipcash/shared/persistence/db/schemas/com.flipcash.app.persistence.FlipcashDatabase/32.json @@ -0,0 +1,775 @@ +{ + "formatVersion": 1, + "database": { + "version": 32, + "identityHash": "caddad298682ce83a51f6ca679a32b89", + "entities": [ + { + "tableName": "messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`idBase58` TEXT NOT NULL, `text` TEXT NOT NULL, `amountUsdc` INTEGER, `amountNative` INTEGER, `nativeCurrency` TEXT, `rate` REAL, `state` TEXT NOT NULL, `timestamp` INTEGER NOT NULL, `metadata` TEXT, `mintBase58` TEXT DEFAULT 'EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v', `textSubstitutions` TEXT, PRIMARY KEY(`idBase58`))", + "fields": [ + { + "fieldPath": "idBase58", + "columnName": "idBase58", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "text", + "columnName": "text", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "amountUsdc", + "columnName": "amountUsdc", + "affinity": "INTEGER" + }, + { + "fieldPath": "amountNative", + "columnName": "amountNative", + "affinity": "INTEGER" + }, + { + "fieldPath": "nativeCurrency", + "columnName": "nativeCurrency", + "affinity": "TEXT" + }, + { + "fieldPath": "rate", + "columnName": "rate", + "affinity": "REAL" + }, + { + "fieldPath": "state", + "columnName": "state", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "timestamp", + "columnName": "timestamp", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "metadata", + "columnName": "metadata", + "affinity": "TEXT" + }, + { + "fieldPath": "mintBase58", + "columnName": "mintBase58", + "affinity": "TEXT", + "defaultValue": "'EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v'" + }, + { + "fieldPath": "textSubstitutions", + "columnName": "textSubstitutions", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "idBase58" + ] + } + }, + { + "tableName": "tokens", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`address` TEXT NOT NULL, `decimals` INTEGER NOT NULL, `name` TEXT NOT NULL, `symbol` TEXT NOT NULL, `created_at` INTEGER, `description` TEXT NOT NULL, `image_url` TEXT NOT NULL, `social_links` TEXT, `bill_customizations` TEXT, `holder_metrics` TEXT, `market_cap_metrics` TEXT, `vm_vm` TEXT NOT NULL, `vm_authority` TEXT NOT NULL, `vm_lock_duration_days` INTEGER NOT NULL, `lp_currency_config` TEXT, `lp_liquidity_pool` TEXT, `lp_seed` TEXT, `lp_authority` TEXT, `lp_mint_vault` TEXT, `lp_core_mint_vault` TEXT, `lp_circulating_supply_quarks` INTEGER, `lp_sell_fee_bps` INTEGER, `lp_price_amount_usd` REAL, `lp_market_cap_amount_usd` REAL, PRIMARY KEY(`address`))", + "fields": [ + { + "fieldPath": "address", + "columnName": "address", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "decimals", + "columnName": "decimals", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "symbol", + "columnName": "symbol", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdAt", + "columnName": "created_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imageUrl", + "columnName": "image_url", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "socialLinks", + "columnName": "social_links", + "affinity": "TEXT" + }, + { + "fieldPath": "billCustomizationsJson", + "columnName": "bill_customizations", + "affinity": "TEXT" + }, + { + "fieldPath": "holderMetricsJson", + "columnName": "holder_metrics", + "affinity": "TEXT" + }, + { + "fieldPath": "marketCapMetricsJson", + "columnName": "market_cap_metrics", + "affinity": "TEXT" + }, + { + "fieldPath": "vmMetadata.vm", + "columnName": "vm_vm", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "vmMetadata.authority", + "columnName": "vm_authority", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "vmMetadata.lockDurationInDays", + "columnName": "vm_lock_duration_days", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "launchpadMetadata.currencyConfig", + "columnName": "lp_currency_config", + "affinity": "TEXT" + }, + { + "fieldPath": "launchpadMetadata.liquidityPool", + "columnName": "lp_liquidity_pool", + "affinity": "TEXT" + }, + { + "fieldPath": "launchpadMetadata.seed", + "columnName": "lp_seed", + "affinity": "TEXT" + }, + { + "fieldPath": "launchpadMetadata.authority", + "columnName": "lp_authority", + "affinity": "TEXT" + }, + { + "fieldPath": "launchpadMetadata.mintVault", + "columnName": "lp_mint_vault", + "affinity": "TEXT" + }, + { + "fieldPath": "launchpadMetadata.coreMintVault", + "columnName": "lp_core_mint_vault", + "affinity": "TEXT" + }, + { + "fieldPath": "launchpadMetadata.currentCirculatingSupplyQuarks", + "columnName": "lp_circulating_supply_quarks", + "affinity": "INTEGER" + }, + { + "fieldPath": "launchpadMetadata.sellFeeBps", + "columnName": "lp_sell_fee_bps", + "affinity": "INTEGER" + }, + { + "fieldPath": "launchpadMetadata.priceAmount", + "columnName": "lp_price_amount_usd", + "affinity": "REAL" + }, + { + "fieldPath": "launchpadMetadata.marketCapAmount", + "columnName": "lp_market_cap_amount_usd", + "affinity": "REAL" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "address" + ] + } + }, + { + "tableName": "token_social_links", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `token_address` TEXT NOT NULL, `type` TEXT NOT NULL, `value` TEXT NOT NULL, FOREIGN KEY(`token_address`) REFERENCES `tokens`(`address`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "tokenAddress", + "columnName": "token_address", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "value", + "columnName": "value", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_token_social_links_token_address", + "unique": false, + "columnNames": [ + "token_address" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_token_social_links_token_address` ON `${TABLE_NAME}` (`token_address`)" + } + ], + "foreignKeys": [ + { + "table": "tokens", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "token_address" + ], + "referencedColumns": [ + "address" + ] + } + ] + }, + { + "tableName": "token_valuation", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`token_address` TEXT NOT NULL, `balance_quarks` INTEGER NOT NULL, `cost_basis` REAL NOT NULL, PRIMARY KEY(`token_address`), FOREIGN KEY(`token_address`) REFERENCES `tokens`(`address`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "tokenAddress", + "columnName": "token_address", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "balanceQuarks", + "columnName": "balance_quarks", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "costBasis", + "columnName": "cost_basis", + "affinity": "REAL", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "token_address" + ] + }, + "indices": [ + { + "name": "index_token_valuation_token_address", + "unique": false, + "columnNames": [ + "token_address" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_token_valuation_token_address` ON `${TABLE_NAME}` (`token_address`)" + } + ], + "foreignKeys": [ + { + "table": "tokens", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "token_address" + ], + "referencedColumns": [ + "address" + ] + } + ] + }, + { + "tableName": "currency_creator_draft", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `name` TEXT NOT NULL, `description` TEXT NOT NULL, `icon_uri` TEXT, `bill_customizations` TEXT, `attestations` TEXT, `current_step` TEXT NOT NULL, `created_mint` TEXT, `saved_at` INTEGER NOT NULL)", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "iconUri", + "columnName": "icon_uri", + "affinity": "TEXT" + }, + { + "fieldPath": "billCustomizations", + "columnName": "bill_customizations", + "affinity": "TEXT" + }, + { + "fieldPath": "attestations", + "columnName": "attestations", + "affinity": "TEXT" + }, + { + "fieldPath": "currentStep", + "columnName": "current_step", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdMint", + "columnName": "created_mint", + "affinity": "TEXT" + }, + { + "fieldPath": "savedAt", + "columnName": "saved_at", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "contact_sync_state", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER NOT NULL, `checksumBytes` BLOB NOT NULL, `lastSyncTimestamp` INTEGER NOT NULL, `needsFullUpload` INTEGER NOT NULL, `hasDiscoveredFlipcashContacts` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "checksumBytes", + "columnName": "checksumBytes", + "affinity": "BLOB", + "notNull": true + }, + { + "fieldPath": "lastSyncTimestamp", + "columnName": "lastSyncTimestamp", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "needsFullUpload", + "columnName": "needsFullUpload", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "hasDiscoveredFlipcashContacts", + "columnName": "hasDiscoveredFlipcashContacts", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "contact_mapping", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`e164` TEXT NOT NULL, `androidContactId` INTEGER NOT NULL, `displayName` TEXT NOT NULL, `photoUri` TEXT, `isOnFlipcash` INTEGER NOT NULL, `displayNumber` TEXT NOT NULL DEFAULT '', `dmChatId` TEXT NOT NULL DEFAULT '', `joinedAtEpochSeconds` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`e164`))", + "fields": [ + { + "fieldPath": "e164", + "columnName": "e164", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "androidContactId", + "columnName": "androidContactId", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "photoUri", + "columnName": "photoUri", + "affinity": "TEXT" + }, + { + "fieldPath": "isOnFlipcash", + "columnName": "isOnFlipcash", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "displayNumber", + "columnName": "displayNumber", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "dmChatId", + "columnName": "dmChatId", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "joinedAtEpochSeconds", + "columnName": "joinedAtEpochSeconds", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "e164" + ] + } + }, + { + "tableName": "chat_metadata", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`chat_id_hex` TEXT NOT NULL, `chat_type` TEXT NOT NULL, `last_activity_epoch_ms` INTEGER NOT NULL, `last_message_id` INTEGER, `latest_event_sequence` INTEGER NOT NULL DEFAULT 0, `is_hidden` INTEGER NOT NULL DEFAULT 0, `analytics_counted_through` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`chat_id_hex`))", + "fields": [ + { + "fieldPath": "chatIdHex", + "columnName": "chat_id_hex", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "chatType", + "columnName": "chat_type", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "lastActivityEpochMs", + "columnName": "last_activity_epoch_ms", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastMessageId", + "columnName": "last_message_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "latestEventSequence", + "columnName": "latest_event_sequence", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "isHidden", + "columnName": "is_hidden", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "analyticsCountedThrough", + "columnName": "analytics_counted_through", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "chat_id_hex" + ] + }, + "indices": [ + { + "name": "index_chat_metadata_last_activity_epoch_ms", + "unique": false, + "columnNames": [ + "last_activity_epoch_ms" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_chat_metadata_last_activity_epoch_ms` ON `${TABLE_NAME}` (`last_activity_epoch_ms`)" + } + ] + }, + { + "tableName": "chat_messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`chat_id_hex` TEXT NOT NULL, `message_id` INTEGER NOT NULL, `sender_id_hex` TEXT, `content_json` TEXT, `timestamp_epoch_ms` INTEGER NOT NULL, `unread_seq` INTEGER NOT NULL, `status` TEXT NOT NULL DEFAULT 'SENT', `pending_client_id_hex` TEXT, `event_sequence` INTEGER NOT NULL DEFAULT 0, `last_edited_ts_epoch_ms` INTEGER, `reactions_json` TEXT, `is_deleted` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`chat_id_hex`, `message_id`))", + "fields": [ + { + "fieldPath": "chatIdHex", + "columnName": "chat_id_hex", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "messageId", + "columnName": "message_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "senderIdHex", + "columnName": "sender_id_hex", + "affinity": "TEXT" + }, + { + "fieldPath": "contentJson", + "columnName": "content_json", + "affinity": "TEXT" + }, + { + "fieldPath": "timestampEpochMs", + "columnName": "timestamp_epoch_ms", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "unreadSeq", + "columnName": "unread_seq", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "status", + "columnName": "status", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'SENT'" + }, + { + "fieldPath": "pendingClientIdHex", + "columnName": "pending_client_id_hex", + "affinity": "TEXT" + }, + { + "fieldPath": "eventSequence", + "columnName": "event_sequence", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "lastEditedTsEpochMs", + "columnName": "last_edited_ts_epoch_ms", + "affinity": "INTEGER" + }, + { + "fieldPath": "reactionsJson", + "columnName": "reactions_json", + "affinity": "TEXT" + }, + { + "fieldPath": "isDeleted", + "columnName": "is_deleted", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "chat_id_hex", + "message_id" + ] + } + }, + { + "tableName": "chat_members", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`chat_id_hex` TEXT NOT NULL, `user_id_hex` TEXT NOT NULL, `pointers_json` TEXT, PRIMARY KEY(`chat_id_hex`, `user_id_hex`))", + "fields": [ + { + "fieldPath": "chatIdHex", + "columnName": "chat_id_hex", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "userIdHex", + "columnName": "user_id_hex", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "pointersJson", + "columnName": "pointers_json", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "chat_id_hex", + "user_id_hex" + ] + } + }, + { + "tableName": "blocked_users", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`user_id_hex` TEXT NOT NULL, `blocked_at_epoch_ms` INTEGER NOT NULL, PRIMARY KEY(`user_id_hex`))", + "fields": [ + { + "fieldPath": "userIdHex", + "columnName": "user_id_hex", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "blockedAtEpochMs", + "columnName": "blocked_at_epoch_ms", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "user_id_hex" + ] + } + }, + { + "tableName": "user_profiles", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`user_id_hex` TEXT NOT NULL, `display_name` TEXT NOT NULL, `phone_value` TEXT, `phone_verified` INTEGER, `email_value` TEXT, `email_verified` INTEGER, `social_accounts_json` TEXT, `profile_picture_json` TEXT, `username` TEXT, `pending_migration_json` TEXT, PRIMARY KEY(`user_id_hex`))", + "fields": [ + { + "fieldPath": "userIdHex", + "columnName": "user_id_hex", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "display_name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "phoneValue", + "columnName": "phone_value", + "affinity": "TEXT" + }, + { + "fieldPath": "phoneVerified", + "columnName": "phone_verified", + "affinity": "INTEGER" + }, + { + "fieldPath": "emailValue", + "columnName": "email_value", + "affinity": "TEXT" + }, + { + "fieldPath": "emailVerified", + "columnName": "email_verified", + "affinity": "INTEGER" + }, + { + "fieldPath": "socialAccounts", + "columnName": "social_accounts_json", + "affinity": "TEXT" + }, + { + "fieldPath": "profilePicture", + "columnName": "profile_picture_json", + "affinity": "TEXT" + }, + { + "fieldPath": "username", + "columnName": "username", + "affinity": "TEXT" + }, + { + "fieldPath": "pendingMigrationJson", + "columnName": "pending_migration_json", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "user_id_hex" + ] + } + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'caddad298682ce83a51f6ca679a32b89')" + ] + } +} \ No newline at end of file diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt index fc4276eac..336a30435 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/FlipcashDatabase.kt @@ -92,8 +92,9 @@ import com.getcode.utils.subByteArray AutoMigration(from = 28, to = 29, spec = FlipcashDatabase.Migration28To29::class), AutoMigration(from = 29, to = 30), // chat_metadata.analytics_counted_through AutoMigration(from = 30, to = 31), // user_profiles.username (nullable) + AutoMigration(from = 31, to = 32, spec = FlipcashDatabase.Migration31To32::class), ], - version = 31, + version = 32, ) @TypeConverters(TokenTypeConverters::class, ChatTypeConverters::class) abstract class FlipcashDatabase : RoomDatabase() { @@ -214,6 +215,26 @@ abstract class FlipcashDatabase : RoomDatabase() { } } + /** + * Adds `chat_messages.is_deleted` and backfills it for rows cached before the column existed. + * + * The flag is written from the domain content at map time, so only pre-existing rows need the + * one-shot repair, and the serialized discriminator is the only evidence they carry. A text + * message quoting that literal string would be misread as a tombstone here — a preview + * falling back one message, and only until the server resends the row. That is the reason the + * flag is a column at all rather than this match being the permanent query. + */ + class Migration31To32 : AutoMigrationSpec { + override fun onPostMigrate(connection: SQLiteConnection) { + connection.execSQL(BACKFILL_TOMBSTONES) + } + + companion object { + const val BACKFILL_TOMBSTONES = + "UPDATE chat_messages SET is_deleted = 1 WHERE content_json LIKE '%\"type\":\"deleted\"%'" + } + } + companion object { /** diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt index cdc81c25e..6238920d9 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt @@ -29,6 +29,18 @@ interface ChatMessageDao { @Query("SELECT * FROM chat_messages WHERE chat_id_hex = :chatIdHex ORDER BY timestamp_epoch_ms DESC LIMIT 1") suspend fun getLatest(chatIdHex: String): ChatMessageEntity? + /** + * The newest message that still has content — tombstones skipped. + * + * This is what the conversation list previews and what its unread check reads, so deleting the + * newest message falls the row back to the one before it instead of reading "Message deleted". + * Deliberately separate from [getLatest]: identity-keyed anchors (mark-read, receive buzz) need + * the newest id including tombstones, or a delete would regress the read pointer and leave the + * chat unread forever. + */ + @Query("SELECT * FROM chat_messages WHERE chat_id_hex = :chatIdHex AND is_deleted = 0 ORDER BY timestamp_epoch_ms DESC LIMIT 1") + suspend fun getLatestVisible(chatIdHex: String): ChatMessageEntity? + @Query( "SELECT * FROM chat_messages " + "WHERE chat_id_hex = :chatIdHex " + diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/entities/ChatMessageEntity.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/entities/ChatMessageEntity.kt index e3d177e05..370697d99 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/entities/ChatMessageEntity.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/entities/ChatMessageEntity.kt @@ -26,4 +26,12 @@ data class ChatMessageEntity( @ColumnInfo(name = "event_sequence", defaultValue = "0") val eventSequence: Long = 0, @ColumnInfo(name = "last_edited_ts_epoch_ms") val lastEditedTsEpochMs: Long? = null, @ColumnInfo(name = "reactions_json") val reactionsJson: String? = null, + /** + * The row is a tombstone — the message was deleted and its content is gone. + * + * Kept as a column rather than derived from [contentJson] so the feed's "newest *visible* + * message" query can filter in SQL. Matching on the serialized discriminator would work until + * someone sends a message whose text happens to contain it. + */ + @ColumnInfo(name = "is_deleted", defaultValue = "0") val isDeleted: Boolean = false, ) diff --git a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageDaoTest.kt b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageDaoTest.kt new file mode 100644 index 000000000..20ac4e4fe --- /dev/null +++ b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageDaoTest.kt @@ -0,0 +1,111 @@ +package com.flipcash.app.persistence.dao + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import com.flipcash.app.persistence.FlipcashDatabase +import com.flipcash.app.persistence.converters.MessageContentSerialized +import com.flipcash.app.persistence.entities.ChatMessageEntity +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * Covers the split between the newest stored row and the newest row that still has content. The + * conversation list previews the latter so a delete falls back to the message before it; mark-read + * and the receive buzz anchor on the former so a delete can't rewind the read pointer. + */ +@RunWith(RobolectricTestRunner::class) +class ChatMessageDaoTest { + + private lateinit var db: FlipcashDatabase + private lateinit var dao: ChatMessageDao + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, FlipcashDatabase::class.java) + .allowMainThreadQueries() + .build() + dao = db.chatMessageDao() + } + + @After + fun tearDown() { + db.close() + } + + private fun text(messageId: Long, body: String) = ChatMessageEntity( + chatIdHex = CHAT_HEX, + messageId = messageId, + senderIdHex = SENDER_HEX, + contentJson = listOf(MessageContentSerialized.Text(body)), + timestampEpochMs = messageId * 1_000, + unreadSeq = messageId, + ) + + private fun tombstone(messageId: Long) = text(messageId, "gone").copy( + contentJson = listOf(MessageContentSerialized.Deleted(deletedAt = 1, deletedBy = SENDER_HEX)), + isDeleted = true, + ) + + @Test + fun `getLatestVisible skips the tombstone the newest row became`() = runTest { + dao.upsert(listOf(text(1, "one"), text(2, "two"))) + dao.upsert(tombstone(2)) + + assertEquals(2L, dao.getLatest(CHAT_HEX)?.messageId) + assertEquals(1L, dao.getLatestVisible(CHAT_HEX)?.messageId) + } + + @Test + fun `getLatestVisible returns the newest row when nothing is deleted`() = runTest { + dao.upsert(listOf(text(1, "one"), text(2, "two"))) + + assertEquals(2L, dao.getLatestVisible(CHAT_HEX)?.messageId) + } + + @Test + fun `getLatestVisible is null once every message is deleted`() = runTest { + dao.upsert(listOf(text(1, "one"), text(2, "two"))) + dao.upsert(listOf(tombstone(1), tombstone(2))) + + assertEquals(2L, dao.getLatest(CHAT_HEX)?.messageId) + assertNull(dao.getLatestVisible(CHAT_HEX)) + } + + /** + * The 31 -> 32 backfill has only the serialized blob to go on, so it matches the discriminator + * kotlinx.serialization writes for a tombstone. This pins that string to what the converters + * actually produce — and to what they don't produce for a message that still has content. + */ + @Test + fun `the migration backfill flags stored tombstones and leaves text alone`() = runTest { + // The pre-32 shape: the flag defaulted to 0 for every cached row. + dao.upsert(listOf(text(1, "one"), tombstone(2).copy(isDeleted = false))) + assertEquals(2L, dao.getLatestVisible(CHAT_HEX)?.messageId) + + db.openHelper.writableDatabase.execSQL(FlipcashDatabase.Migration31To32.BACKFILL_TOMBSTONES) + + assertEquals(1L, dao.getLatestVisible(CHAT_HEX)?.messageId) + } + + @Test + fun `getLatestVisible ignores other chats`() = runTest { + dao.upsert(listOf(text(1, "one"), text(2, "two"))) + dao.upsert(text(3, "elsewhere").copy(chatIdHex = OTHER_HEX)) + + assertEquals(2L, dao.getLatestVisible(CHAT_HEX)?.messageId) + } + + private companion object { + const val CHAT_HEX = "aabb" + const val OTHER_HEX = "ccdd" + const val SENDER_HEX = "1122" + } +} diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt index 215cedf3b..f36569854 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt @@ -118,6 +118,10 @@ class ChatMessageDataSource @Inject constructor( suspend fun getLatest(chatIdHex: String): ChatMessage? = db?.chatMessageDao()?.getLatest(chatIdHex)?.let { toChatMessage(it) } + /** The newest message that isn't a tombstone — what the conversation list previews. */ + suspend fun getLatestVisible(chatIdHex: String): ChatMessage? = + db?.chatMessageDao()?.getLatestVisible(chatIdHex)?.let { toChatMessage(it) } + suspend fun hasMessages(chatId: ChatId): Boolean = db?.chatMessageDao()?.getLatest(mapper.chatIdHex(chatId)) != null diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt index 3f92ad34e..4d69e0904 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt @@ -104,6 +104,7 @@ class ChatEntityMapper @Inject constructor() { reactionsJson = message.reactions?.toSerialized()?.let { kotlinx.serialization.json.Json.encodeToString(it) }, + isDeleted = message.content.any { it is MessageContent.Deleted }, ) } diff --git a/apps/flipcash/shared/persistence/sources/src/test/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapperTest.kt b/apps/flipcash/shared/persistence/sources/src/test/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapperTest.kt index 028590e92..3b0e9b258 100644 --- a/apps/flipcash/shared/persistence/sources/src/test/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapperTest.kt +++ b/apps/flipcash/shared/persistence/sources/src/test/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapperTest.kt @@ -6,6 +6,8 @@ import com.flipcash.app.persistence.entities.ChatMetadataEntity import com.flipcash.services.models.UserProfile import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMember +import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.handle import com.flipcash.services.models.chat.ChatMetadata import com.flipcash.services.models.chat.ChatType @@ -92,6 +94,31 @@ class ChatEntityMapperTest { assertEquals("@sally_streamer", readBack.userProfile.handle) } + /** + * `is_deleted` is what the conversation list's "newest message that still has content" query + * filters on, and the mapper is the only thing that ever writes it. If a tombstone were stored + * with the flag clear, the list would preview "Message deleted" again. + */ + @Test + fun `a tombstone is flagged deleted and a text message is not`() { + fun entity(content: MessageContent) = mapper.toEntity( + CHAT_HEX, + ChatMessage( + messageId = 1, + senderId = listOf(0xAB.toByte()), + content = listOf(content), + timestamp = Instant.fromEpochSeconds(1_000), + unreadSeq = 1, + ), + ) + + assertEquals( + true, + entity(MessageContent.Deleted(deletedTs = Instant.fromEpochSeconds(2_000), deletedBy = listOf(0xAB.toByte()))).isDeleted, + ) + assertEquals(false, entity(MessageContent.Text("still here")).isDeleted) + } + private companion object { const val CHAT_HEX = "aabbccdd" }