diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 31066a24ab..4300147cd5 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -29,6 +29,7 @@ import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.DeliveryStatus import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.chat.TypingState +import com.flipcash.services.models.chat.isDmAddressable import com.flipcash.services.user.UserManager import com.flipcash.shared.amountentry.AmountEntryDelegate import com.flipcash.shared.amountentry.AmountEntryLabel @@ -418,19 +419,21 @@ internal class ChatViewModel @Inject constructor( .launchIn(viewModelScope) // Observe member identity — if the other member loses identity (e.g. unlinked - // their phone), mark the chat as read-only. + // their phone), mark the chat as read-only. Gated by chat type through the same rule the + // feed filters on, so a tip DM — addressed by user id, named by handle — is never + // deactivated for lacking a name or a phone. stateFlow.mapNotNull { it.chatId } .distinctUntilChanged() - .flatMapLatest { chatCoordinator.observeMembers(it) } - .map { members -> + .flatMapLatest { chatId -> + combine( + chatCoordinator.observeMembers(chatId), + stateFlow.map { it.chatType }.distinctUntilChanged(), + ) { members, chatType -> members to chatType } + } + .map { (members, chatType) -> val selfId = userManager.accountId val other = members.firstOrNull { it.userId != selfId } - if (other != null) { - val profile = other.userProfile - val hasIdentity = profile.displayName.isNotBlank() || - !profile.verifiedPhoneNumber.isNullOrBlank() - !hasIdentity - } else false + if (other != null) !isDmAddressable(chatType, other.userProfile) else false } .distinctUntilChanged() .onEach { dispatchEvent(Event.ChatDeactivated(isReadOnly = it)) } 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 8008b7403e..60fe42e866 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 @@ -10,6 +10,7 @@ import com.flipcash.services.models.chat.ChatMember import com.flipcash.services.models.chat.ChatMetadata import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.PointerType +import com.flipcash.services.models.chat.isDmAddressable import com.flipcash.shared.chat.ChatHydrationState import com.flipcash.shared.chat.ChatSummary import com.flipcash.shared.chat.FeedOperations @@ -110,12 +111,7 @@ class FeedSyncDelegate @Inject constructor( // Contact DMs require a resolvable identity (phone / display name). Tip DMs are // identified by user id and have no phone by design, so they are never dropped. - if (chatType == ChatType.CONTACT_DM) { - val profile = otherMember.userProfile - val hasIdentity = profile.displayName.isNotBlank() || - !profile.verifiedPhoneNumber.isNullOrBlank() - if (!hasIdentity) return@mapNotNull null - } + if (!isDmAddressable(chatType, otherMember.userProfile)) return@mapNotNull null val readPointer = metadata.members .firstOrNull { it.userId == selfId } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/DmAddressability.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/DmAddressability.kt new file mode 100644 index 0000000000..09e0281b6e --- /dev/null +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/DmAddressability.kt @@ -0,0 +1,30 @@ +package com.flipcash.services.models.chat + +import com.flipcash.services.models.UserProfile + +/** + * Whether a DM with [counterparty] can still be addressed, and so whether it stays usable. + * + * The two kinds of DM are addressed differently, and the gate follows from that: + * + * - A `CONTACT_DM` is reached through the counterparty's phone number and named from the device + * contact. If they unlink their phone and have no display name left, there is nothing to address + * or name them by, so the chat is dropped from the feed and the open conversation goes read-only. + * - A `TIP_DM` is reached by user id. The counterparty has no phone by design and need never have + * set a display name — a claimed `@handle` names them instead. Gating one would hide + * or deactivate a conversation that is perfectly addressable, so it never is. + * + * Any other [chatType] — including [ChatType.UNKNOWN], which is what a conversation reports until + * its kind resolves — is left alone. That matters for the open conversation: gating on an + * unresolved type would flash the deactivated composer before the type settles. + * + * This is one rule with two callers on purpose. It was previously written out at both, and the two + * copies disagreed: the feed exempted tip DMs and the conversation did not, so a name-less tipper + * appeared in the tips list and then opened read-only. + */ +fun isDmAddressable(chatType: ChatType, counterparty: UserProfile): Boolean = + if (chatType == ChatType.CONTACT_DM) { + counterparty.displayName.isNotBlank() || !counterparty.verifiedPhoneNumber.isNullOrBlank() + } else { + true + } diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DmAddressabilityTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DmAddressabilityTest.kt new file mode 100644 index 0000000000..ca2d072818 --- /dev/null +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DmAddressabilityTest.kt @@ -0,0 +1,83 @@ +package com.flipcash.services.models.chat + +import com.flipcash.services.models.UserProfile +import com.flipcash.services.models.VerifiableContactMethod +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * The rule the feed filters on and the open conversation deactivates on. They used to be two + * copies that disagreed about tip DMs, so the cases below are written from the caller's side: + * what should stay open, and what should not. + */ +class DmAddressabilityTest { + + private fun profile( + displayName: String = "", + phone: String? = null, + phoneVerified: Boolean = true, + username: String? = null, + ) = UserProfile.Empty.copy( + displayName = displayName, + username = username, + phoneNumber = phone?.let { + VerifiableContactMethod(value = it, verified = phoneVerified) + }, + ) + + @Test + fun `a tip DM counterparty with only a handle is addressable`() { + assertTrue(isDmAddressable(ChatType.TIP_DM, profile(username = "sally_streamer"))) + } + + @Test + fun `a tip DM counterparty with nothing at all is still addressable`() { + // Addressed by user id, so there is nothing for a missing name to break. + assertTrue(isDmAddressable(ChatType.TIP_DM, UserProfile.Empty)) + } + + @Test + fun `a contact DM counterparty with a name is addressable`() { + assertTrue(isDmAddressable(ChatType.CONTACT_DM, profile(displayName = "Ada Lovelace"))) + } + + @Test + fun `a contact DM counterparty with only a verified phone is addressable`() { + assertTrue(isDmAddressable(ChatType.CONTACT_DM, profile(phone = "+15551234567"))) + } + + @Test + fun `a contact DM counterparty who unlinked their phone and has no name is not`() { + assertFalse(isDmAddressable(ChatType.CONTACT_DM, UserProfile.Empty)) + } + + @Test + fun `an unverified phone does not count as identity`() { + assertFalse( + isDmAddressable( + ChatType.CONTACT_DM, + profile(phone = "+15551234567", phoneVerified = false), + ) + ) + } + + @Test + fun `a blank name does not count as identity`() { + assertFalse(isDmAddressable(ChatType.CONTACT_DM, profile(displayName = " "))) + } + + @Test + fun `a handle alone does not rescue a contact DM`() { + // Deliberate: a contact DM is reached through the phone number, so a handle is not a + // substitute for one. Only the naming rule (nameOrHandle) treats them as interchangeable. + assertFalse(isDmAddressable(ChatType.CONTACT_DM, profile(username = "ada"))) + } + + @Test + fun `an unresolved chat type is left open`() { + // The open conversation reports UNKNOWN until its kind settles; gating there would flash + // the deactivated composer. + assertTrue(isDmAddressable(ChatType.UNKNOWN, UserProfile.Empty)) + } +}