From aaa8643e50375e3de6128665b0ace0d409479b89 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 29 Sep 2026 16:54:31 -0400 Subject: [PATCH 1/2] fix(chat): keep a member's phone and email when another chat omits them A user's profile row is shared by every chat they're in, and each member-list write replaced it whole. The server includes a member's phone number in CONTACT_DM feeds but not in TIP_DM feeds, so the stored number depended on which chat was written last. A full sync writes TIP_DMs last, which cleared it. Member writes now keep a stored phone or email when the incoming copy has none, and replace every other column as before. --- .../app/persistence/dao/UserProfileDao.kt | 28 ++++++-- .../persistence/UserProfileMigrationTest.kt | 64 +++++++++++++++++++ .../sources/ChatMemberDataSource.kt | 6 +- 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/UserProfileDao.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/UserProfileDao.kt index 71ee0761fd..9f5904f48a 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/UserProfileDao.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/UserProfileDao.kt @@ -4,6 +4,7 @@ import androidx.room.Dao import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query +import androidx.room.Transaction import androidx.room.Update import com.flipcash.app.persistence.entities.UserProfileEntity import com.flipcash.services.models.chat.MediaItem @@ -12,13 +13,32 @@ import kotlinx.coroutines.flow.Flow @Dao interface UserProfileDao { - /** - * Authoritative full-profile write (chat member sync). The caller has the complete - * profile, so a whole-row replace is correct — and it clears any staged migration blob. - */ + /** Whole-row replace, which also clears any staged migration blob. */ @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsertFull(profiles: List) + /** + * Writes the profiles a chat's member list carries. A user's row is shared by every chat + * they are in, but the server sends their phone number and email only in some of them (a + * CONTACT_DM member carries the number, the same user as a TIP_DM member does not). So a + * member without a phone or email keeps the one already stored instead of erasing it; every + * other column is replaced, and the staged migration blob is cleared as [upsertFull] does. + */ + @Transaction + suspend fun upsertMembers(profiles: List) { + upsertFull( + profiles.map { incoming -> + val stored = getByUserId(incoming.userIdHex) ?: return@map incoming + incoming.copy( + phoneValue = incoming.phoneValue ?: stored.phoneValue, + phoneVerified = if (incoming.phoneValue != null) incoming.phoneVerified else stored.phoneVerified, + emailValue = incoming.emailValue ?: stored.emailValue, + emailVerified = if (incoming.emailValue != null) incoming.emailVerified else stored.emailVerified, + ) + } + ) + } + /** * Partial write for callers that only know a user's public identity — name, avatar and handle * (blocklist sync, a single `GetProfile`). Inserts a new row, or updates *only* those three diff --git a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/UserProfileMigrationTest.kt b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/UserProfileMigrationTest.kt index 4188406a5c..60042d32c1 100644 --- a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/UserProfileMigrationTest.kt +++ b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/UserProfileMigrationTest.kt @@ -241,6 +241,70 @@ class UserProfileDaoTest { assertEquals("", dao.getByUserId("u1")?.displayName) } + // A user's row is shared by every chat they're in, but the server sends their phone number + // only in some of them: a CONTACT_DM member carries it, the same user in a TIP_DM does not. + private fun memberWithoutContact(userId: String, displayName: String = "Alice") = + fullProfile(userId).copy( + displayName = displayName, + phoneValue = null, phoneVerified = null, + emailValue = null, emailVerified = null, + ) + + @Test + fun `member write without a phone or email keeps the stored ones`() = runBlocking { + val dao = db.userProfileDao() + dao.upsertMembers(listOf(fullProfile("u1"))) + + dao.upsertMembers(listOf(memberWithoutContact("u1"))) + + val stored = dao.getByUserId("u1") + assertEquals("+15551234567", stored?.phoneValue) + assertEquals(true, stored?.phoneVerified) + assertEquals("a@b.com", stored?.emailValue) + assertEquals(false, stored?.emailVerified) + } + + @Test + fun `member write without a phone still updates the name`() = runBlocking { + val dao = db.userProfileDao() + dao.upsertMembers(listOf(fullProfile("u1"))) + + dao.upsertMembers(listOf(memberWithoutContact("u1", displayName = "Alice B"))) + + assertEquals("Alice B", dao.getByUserId("u1")?.displayName) + } + + @Test + fun `member write with a new phone replaces the stored one`() = runBlocking { + val dao = db.userProfileDao() + dao.upsertMembers(listOf(fullProfile("u1"))) + + dao.upsertMembers(listOf(fullProfile("u1").copy(phoneValue = "+15550000002", phoneVerified = false))) + + val stored = dao.getByUserId("u1") + assertEquals("+15550000002", stored?.phoneValue) + assertEquals(false, stored?.phoneVerified) + } + + @Test + fun `member write inserts a new user as given`() = runBlocking { + val dao = db.userProfileDao() + + dao.upsertMembers(listOf(memberWithoutContact("u1"))) + + assertNull(dao.getByUserId("u1")?.phoneValue) + } + + @Test + fun `member write clears a staged migration blob`() = runBlocking { + val dao = db.userProfileDao() + dao.upsertFull(listOf(fullProfile("u1").copy(pendingMigrationJson = "{}"))) + + dao.upsertMembers(listOf(memberWithoutContact("u1"))) + + assertNull(dao.getByUserId("u1")?.pendingMigrationJson) + } + @Test fun `chat member relation joins the shared normalized profile`() = runBlocking { db.userProfileDao().upsertFull(listOf(fullProfile("u1"))) diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMemberDataSource.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMemberDataSource.kt index 12190537b9..b47bba7cb1 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMemberDataSource.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMemberDataSource.kt @@ -67,9 +67,9 @@ class ChatMemberDataSource @Inject constructor( val hex = mapper.chatIdHex(chatId) // Member rows and their normalized profiles are written in one transaction so a // member never observes a missing profile mid-write. Profiles go first (the - // @Relation reads them), and use the authoritative full-profile upsert. + // @Relation reads them). database.withTransaction { - database.userProfileDao().upsertFull(profileRows(members)) + database.userProfileDao().upsertMembers(profileRows(members)) database.chatMemberDao().upsert(members.map { mapper.toEntity(hex, it) }) } } @@ -105,7 +105,7 @@ class ChatMemberDataSource @Inject constructor( val database = db ?: return val hex = mapper.chatIdHex(chatId) database.withTransaction { - database.userProfileDao().upsertFull(profileRows(members)) + database.userProfileDao().upsertMembers(profileRows(members)) database.chatMemberDao().upsert(members.map { mapper.toEntity(hex, it) }) database.chatMemberDao().deleteMembersNotIn( chatIdHex = hex, From 4aa98682eb916a65864dab3c89e71d36bad709df Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 29 Sep 2026 16:54:32 -0400 Subject: [PATCH 2/2] fix(notifications): prefer a sender's display name over their phone number The sender name came from ContactResolver.resolveName first, which returns a formatted number when the address book has no contact for it. A sender whose profile carried a phone number was shown as that number instead of their display name. Only an address-book name now comes before the profile's display name and handle; the formatted number is the last fallback. It stays in the candidate list so it's still stripped from a message body the server prefixed with it. --- .../flipcash/app/contacts/ContactResolver.kt | 10 +++- .../app/contacts/ContactResolverTest.kt | 12 +++++ .../app/notifications/NotificationService.kt | 11 ++-- .../notifications/SenderAttributionPlanner.kt | 30 +++++++++++ .../SenderAttributionPlannerTest.kt | 52 +++++++++++++++++++ 5 files changed, 108 insertions(+), 7 deletions(-) diff --git a/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/ContactResolver.kt b/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/ContactResolver.kt index 20a18edbcb..275371d85e 100644 --- a/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/ContactResolver.kt +++ b/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/ContactResolver.kt @@ -12,7 +12,11 @@ class ContactResolver @Inject constructor( private val deviceContactLookup: DeviceContactLookup, private val phoneUtils: PhoneUtils, ) { - suspend fun resolveName(e164: String, fallback: String = e164): String { + suspend fun resolveName(e164: String, fallback: String = e164): String = + resolveContactName(e164) ?: formatNumber(e164, fallback) + + /** The address-book name for [e164], or null when no contact has that number. */ + suspend fun resolveContactName(e164: String): String? { val cached = contactDataSource.getDisplayName(e164) val device = deviceContactLookup.lookupDisplayName(e164) @@ -22,9 +26,11 @@ class ContactResolver @Inject constructor( } return cached - ?: runCatching { phoneUtils.formatNumber(e164) }.getOrDefault(fallback) } + fun formatNumber(e164: String, fallback: String = e164): String = + runCatching { phoneUtils.formatNumber(e164) }.getOrDefault(fallback) + suspend fun resolvePhotoUri(e164: String): String? { val cached = contactDataSource.getPhotoUri(e164) if (cached != null) return cached diff --git a/apps/flipcash/shared/contacts/src/test/kotlin/com/flipcash/app/contacts/ContactResolverTest.kt b/apps/flipcash/shared/contacts/src/test/kotlin/com/flipcash/app/contacts/ContactResolverTest.kt index 546f63f5bc..1bf3c24efc 100644 --- a/apps/flipcash/shared/contacts/src/test/kotlin/com/flipcash/app/contacts/ContactResolverTest.kt +++ b/apps/flipcash/shared/contacts/src/test/kotlin/com/flipcash/app/contacts/ContactResolverTest.kt @@ -83,6 +83,18 @@ class ContactResolverTest { assertEquals("Alice Device", result) } + @Test + fun `contact name is null without a contact rather than the formatted number`() = runTest { + val result = resolver(formatted = "+1 (555) 123-4567").resolveContactName(e164) + assertNull(result) + } + + @Test + fun `contact name comes from the device`() = runTest { + val result = resolver(dbName = "Alice DB", deviceName = "Alice Device").resolveContactName(e164) + assertEquals("Alice Device", result) + } + // endregion // region resolvePhotoUri diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index a090fc7ba3..eed8f6646b 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -345,11 +345,12 @@ class NotificationService : FirebaseMessagingService(), // Every name this sender is known by, not just the one rendered: the server composed the // body from one of them and the line is drawn with another, so stripping the sender's name // out of their own message needs all of them. See [planMessageBody]. - val senderNames = listOf( - e164?.let { contactResolver.resolveName(it) }, - sender?.profile?.displayName, - sender?.profile?.socialHandle(), - title, + val senderNames = senderNameCandidates( + contactName = e164?.let { contactResolver.resolveContactName(it) }, + displayName = sender?.profile?.displayName, + handle = sender?.profile?.socialHandle(), + title = title, + number = e164?.let { contactResolver.formatNumber(it) }, ) val senderName = senderNames.firstOrNull { !it.isNullOrBlank() }.orEmpty() diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/SenderAttributionPlanner.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/SenderAttributionPlanner.kt index 47455a11dd..889e6b9b38 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/SenderAttributionPlanner.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/SenderAttributionPlanner.kt @@ -50,3 +50,33 @@ fun planSenderLookup( chatType == ChatType.GROUP -> SenderLookup.None else -> SenderLookup.OtherMember } + +/** + * The name a chat notification shows for its sender, best first. + * + * Only an address-book name beats the sender's own profile, because it's the name the user chose + * for them. A bare phone number comes last: a profile carries one only in some chats, so ranking it + * above the display name would flip the same sender between their name and their number depending + * on which chat's member list was stored last. + * + * @param contactName the address-book name for the sender's number, null when there is no contact + * @param number the sender's number, formatted for display + */ +fun senderNameCandidates( + contactName: String?, + displayName: String?, + handle: String?, + title: String?, + number: String?, +): List = listOf(contactName, displayName, handle, title, number) + +/** The first usable name from [senderNameCandidates], or empty when there is none. */ +fun planSenderName( + contactName: String?, + displayName: String?, + handle: String?, + title: String?, + number: String?, +): String = senderNameCandidates(contactName, displayName, handle, title, number) + .firstOrNull { !it.isNullOrBlank() } + .orEmpty() diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/SenderAttributionPlannerTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/SenderAttributionPlannerTest.kt index 8396cbf109..117104b5b4 100644 --- a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/SenderAttributionPlannerTest.kt +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/SenderAttributionPlannerTest.kt @@ -104,4 +104,56 @@ class SenderAttributionPlannerTest { } // endregion + + // region Sender name + + @Test + fun `a device-contact name wins over the profile`() { + assertEquals( + "Mom", + planSenderName(contactName = "Mom", displayName = "Alice", handle = "alice", title = "Alice", number = "+1 555-000-0001"), + ) + } + + @Test + fun `a display name wins over a phone number`() { + assertEquals( + "Alice", + planSenderName(contactName = null, displayName = "Alice", handle = null, title = null, number = "+1 555-000-0001"), + ) + } + + @Test + fun `a handle is used when there is no display name`() { + assertEquals( + "alice", + planSenderName(contactName = null, displayName = " ", handle = "alice", title = "Server", number = "+1 555-000-0001"), + ) + } + + @Test + fun `the push title is used before a phone number`() { + assertEquals( + "Server", + planSenderName(contactName = null, displayName = null, handle = null, title = "Server", number = "+1 555-000-0001"), + ) + } + + @Test + fun `a phone number is the last resort`() { + assertEquals( + "+1 555-000-0001", + planSenderName(contactName = null, displayName = null, handle = null, title = null, number = "+1 555-000-0001"), + ) + } + + @Test + fun `no name at all is empty`() { + assertEquals( + "", + planSenderName(contactName = null, displayName = null, handle = null, title = null, number = null), + ) + } + + // endregion }