diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatSubject.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatSubject.kt index 9219f0a761..d1a055e96e 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatSubject.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatSubject.kt @@ -71,7 +71,9 @@ sealed interface ChatSubject { * because a group has none. * * [isMember] rides along because the access gate reads it in the same breath as [rules], and - * splitting them would let the screen blur a chat it had just joined for one frame. + * splitting them would let the screen blur a chat it had just joined for one frame. It is null + * for a group hydrated by id, which cannot know it — see `ChatMembership.isMember`. What the + * viewer may see treats null as a non-member; what the gate's button offers does not. */ data class Group( val chatId: ChatId, @@ -79,7 +81,7 @@ sealed interface ChatSubject { val picture: MediaItem?, val memberCount: Long, val rules: ChatRules?, - val isMember: Boolean, + val isMember: Boolean?, ) : ChatSubject { override val title: String get() = groupTitle.orEmpty() override val subtitle: String? get() = null 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 ad9e17c91c..dc043ef159 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 @@ -306,7 +306,8 @@ internal class ChatViewModel @Inject constructor( * short. */ val isGatedPreview: Boolean - get() = joinProgress.success || (subject as? ChatSubject.Group)?.isMember == false + get() = joinProgress.success || + (subject is ChatSubject.Group && subject.isMember != true) /** * The link that invites someone into this group, or `null` when there is nobody to invite: @@ -317,7 +318,7 @@ internal class ChatViewModel @Inject constructor( */ val groupInviteUrl: String? get() = (subject as? ChatSubject.Group) - ?.takeIf { it.isMember } + ?.takeIf { it.isMember == true } ?.let { Linkify.groupChatInvite(it.chatId) } } @@ -926,6 +927,37 @@ internal class ChatViewModel @Inject constructor( } } + /** + * Gets the chat on screen onto it: its own row if the device has one, otherwise the server's + * copy, and then the transcript if the viewer is entitled to it. + * + * A chat reached by invite link or push tap may never have been synced — the group feed, a join + * and the roster stream all only ever carry chats the viewer is already in — so without the + * hydration the screen has no title, no info card and no gate, and nothing that would later + * supply them. A chat the device already holds costs no round trip: [ChatCoordinator.hydrateChat] + * answers null for it and the Room observation stays the only source, so opening a synced chat + * behaves exactly as it did. + * + * The transcript is fetched only for a viewer the gate lets through. A withheld group is drawn + * as a placeholder rather than fetched, so asking for its messages would pull the very ones the + * blur is over — and an unknown membership is withheld here for the same reason it is on screen. + * A member whose feed has not synced yet is not stranded: the group feed sync sends its own load + * for a group whose cursor is still at zero. + */ + private suspend fun openTranscript(chatId: ChatId) { + val hydrated = chatCoordinator.hydrateChat(chatId) ?: run { + chatCoordinator.loadMessages(chatId) + return + } + + if (hydrated.metadata.type != ChatType.GROUP) { + chatCoordinator.loadMessages(chatId) + return + } + + dispatchEvent(Event.OnGroupResolved(hydrated)) + } + private fun initChatHandlers() { // Ahead of the transcript, so the first mapping already has the real windows rather than // the fallbacks the default carries. @@ -972,7 +1004,7 @@ internal class ChatViewModel @Inject constructor( if (chatId != null) { dispatchEvent(Event.ChatFound(chatId)) chatCoordinator.setActiveChatId(chatId) - viewModelScope.launch { chatCoordinator.loadMessages(chatId) } + viewModelScope.launch { openTranscript(chatId) } chatCoordinator.dismissNotifications(chatId) } else { // No existing chat means no messages yet, so typing stays disabled. The @@ -1101,7 +1133,7 @@ internal class ChatViewModel @Inject constructor( flowOf(null) } else { tokenCoordinator.groupAccess( - isMember = group.isMember, + isMember = group.isMember == true, rules = group.rules, isStaff = userFlags.resolvedFlags.map { it.isStaff.effectiveValue }, ) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt index 6aadfbf62e..c85dd45b2c 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt @@ -27,10 +27,15 @@ internal sealed interface GroupProfileAction { * The same invite the empty transcript offers, for a group that already has messages in it. * * Shown only while there is a link to hand out, which is the same condition as being a member. + * + * The people glyph, not the `@` one: `ic_at` belongs to the row *inside* the sheet this opens, + * where it stands for the username the link is sent to (node 10127:118328). On the row that opens + * the sheet it said the wrong thing — what the row does is add people to the group, which is what + * the New Chat group row uses the same icon for. */ internal data object InviteToGroup : FullMenuItem() { override val icon: Painter - @Composable get() = painterResource(R.drawable.ic_at) + @Composable get() = painterResource(R.drawable.ic_group_3) override val name: String @Composable get() = stringResource(R.string.action_invitePeopleToJoin) diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSubjectTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSubjectTest.kt index aed53e1e35..849ff28011 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSubjectTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSubjectTest.kt @@ -84,7 +84,7 @@ class ChatSubjectTest { ) assertEquals("", subject.title) - assertFalse(subject.isMember) + assertEquals(false, subject.isMember) } /** diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatViewModelStateTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatViewModelStateTest.kt index 8bc763fb2d..9f433096b8 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatViewModelStateTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatViewModelStateTest.kt @@ -29,7 +29,7 @@ class ChatViewModelStateTest { private val groupChatId = ChatId(groupUuid.bytes) private val unmet = ChatRuleRequirement.MinimumBalance(Fiat(100.0), emptyList()) - private fun group(isMember: Boolean) = ChatSubject.Group( + private fun group(isMember: Boolean?) = ChatSubject.Group( chatId = groupChatId, groupTitle = "Ballers", picture = null, @@ -127,6 +127,35 @@ class ChatViewModelStateTest { ) } + @Test + fun `a group whose membership is unknown stays withheld`() { + // A chat hydrated by id — an invite link or a push tap into a group the device has never + // synced — cannot know whether the viewer is in it, because GetChat does not say. Withheld + // is the safe half of that: a member whose feed has not landed yet reads the blur for a + // beat, where the other way round would hand a non-member the transcript outright. + assertTrue( + ChatViewModel.State( + subject = group(isMember = null), + groupAccess = null, + ).isGatedPreview + ) + // Still withheld once the balance answers: what the button offers is decided by the access, + // not the blur. + assertTrue( + ChatViewModel.State( + subject = group(isMember = null), + groupAccess = GroupAccess.Eligible, + ).isGatedPreview + ) + } + + @Test + fun `an unknown membership has no invite to share`() { + // The link is an action, not a view: offering it would invite people into a group this + // device cannot yet say the viewer belongs to. + assertNull(ChatViewModel.State(subject = group(isMember = null)).groupInviteUrl) + } + @Test fun `a DM has no gate to render`() { // The null access is the whole point: a default would blur every contact conversation. diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt index cf378a408d..d769191afc 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt @@ -183,6 +183,21 @@ interface MessagingOperations { */ fun observeMetadata(chatId: ChatId): Flow + /** + * Fetches [chatId] from the server for a chat this device holds no row for, so a conversation + * reached by invite link or push tap can render before it has ever been synced. + * + * Returns `null` when the row is already stored — [observeMetadata] is answering in that case, + * and a second copy would only race it — and when the fetch fails. + * + * Deliberately not persisted. `GetChat` returns the chat, not the caller's relationship to it, + * and the row's membership column has to be written with something, so storing this would + * record a guess: `true` opens a group the viewer has not joined, `false` gates a member whose + * feed has not synced yet. The result carries membership as `null` instead, and the join path + * writes the row once the server has confirmed it. + */ + suspend fun hydrateChat(chatId: ChatId): ChatMembership? + /** * Every profile this device holds, keyed by user-id hex. * diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatMembership.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatMembership.kt index 2df27aa7b7..ff5ac2788f 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatMembership.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatMembership.kt @@ -12,5 +12,14 @@ import com.flipcash.services.models.chat.ChatMetadata */ data class ChatMembership( val metadata: ChatMetadata, - val isMember: Boolean, + /** + * Whether the viewer is in this chat, or `null` when nothing local answers that yet. + * + * Only [MessagingOperations.hydrateChat] produces `null`: it fetches a chat the device holds no + * row for, and `GetChat` returns the chat without the caller's relationship to it, so an + * unsynced chat has no answer rather than a negative one. Callers deciding what the viewer may + * *do* may read it as "not a member". Callers deciding what the viewer may *see* must withhold, + * because an unknown rendered as a member shows the transcript the gate exists to withhold. + */ + val isMember: Boolean?, ) 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 6680931158..6e3cffc0bd 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 @@ -115,6 +115,15 @@ class MessagingDelegate @Inject constructor( notificationManager.cancel(chatId.hashCode()) } + override suspend fun hydrateChat(chatId: ChatId): ChatMembership? { + if (metadataDataSource.exists(chatId)) return null + val metadata = chatController.getChat(chatId).getOrElse { + trace(tag = TAG, message = "Hydrate failed for $chatId", type = TraceType.Error) + return null + } + return ChatMembership(metadata = metadata, isMember = null) + } + override fun observeMessages(chatId: ChatId): Flow> { return messageDataSource.observeMessages(chatId) } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingMetadataObservationTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingMetadataObservationTest.kt index 6244ae7225..6881cd8c6f 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingMetadataObservationTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingMetadataObservationTest.kt @@ -6,11 +6,14 @@ 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.chat.ChatId import com.flipcash.services.models.chat.ChatMetadata import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.RosterSummary import com.flipcash.shared.chat.internal.delegates.MessagingDelegate +import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.flow.MutableStateFlow @@ -19,8 +22,7 @@ import kotlinx.coroutines.test.TestCoroutineScheduler import kotlinx.coroutines.test.runTest import org.junit.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertTrue +import kotlin.test.assertNull import kotlin.time.Instant /** @@ -77,12 +79,12 @@ class MessagingMetadataObservationTest { subject.observeMetadata(chatId).test { val first = awaitItem()!! assertEquals(2L, first.metadata.rosterSummary.memberCount) - assertFalse(first.isMember) + assertEquals(false, first.isMember) rows.value = entity(memberCount = 3, isMember = true) val second = awaitItem()!! assertEquals(3L, second.metadata.rosterSummary.memberCount) - assertTrue(second.isMember) + assertEquals(true, second.isMember) cancelAndIgnoreRemainingEvents() } @@ -106,12 +108,73 @@ class MessagingMetadataObservationTest { } } + /** + * A chat the device has no row for is fetched, and carries no membership answer. + * + * This is the cold open: an invite link or a push tap into a group no sync has ever brought + * down. Without the fetch the screen has nothing to draw — the Room observation above is the + * only other source and it emits null — and `GetChat` cannot say whether the viewer is in the + * chat, so the result withholds rather than guesses. Nothing is written: the membership column + * would have to hold one guess or the other. + */ + @Test + fun `hydrateChat fetches an unstored chat and reports membership as unknown`() = + runTest(dispatchers.dispatcher) { + val metadataDataSource = mockk(relaxed = true) { + coEvery { exists(chatId) } returns false + } + val chatController = mockk(relaxed = true) { + coEvery { getChat(chatId) } returns Result.success(metadata(memberCount = 4)) + } + + val subject = delegate( + metadataDataSource = metadataDataSource, + memberDataSource = mockk(relaxed = true), + messageDataSource = mockk(relaxed = true), + chatController = chatController, + ) + + val hydrated = subject.hydrateChat(chatId) + + assertEquals(4L, hydrated?.metadata?.rosterSummary?.memberCount) + assertNull(hydrated?.isMember) + coVerify(exactly = 0) { metadataDataSource.upsert(any()) } + coVerify(exactly = 0) { metadataDataSource.upsert(any>()) } + coVerify(exactly = 0) { + metadataDataSource.setMembership(any(), any()) + } + } + + /** + * A stored chat is not refetched: `observeMetadata` is already answering for it, with the + * membership the row holds, and a second copy would only race it. + */ + @Test + fun `hydrateChat leaves a stored chat to the observation`() = + runTest(dispatchers.dispatcher) { + val metadataDataSource = mockk(relaxed = true) { + coEvery { exists(chatId) } returns true + } + val chatController = mockk(relaxed = true) + + val subject = delegate( + metadataDataSource = metadataDataSource, + memberDataSource = mockk(relaxed = true), + messageDataSource = mockk(relaxed = true), + chatController = chatController, + ) + + assertNull(subject.hydrateChat(chatId)) + coVerify(exactly = 0) { chatController.getChat(any()) } + } + private fun delegate( metadataDataSource: ChatMetadataDataSource, memberDataSource: ChatMemberDataSource, messageDataSource: ChatMessageDataSource, + chatController: ChatController = mockk(relaxed = true), ) = MessagingDelegate( - chatController = mockk(relaxed = true), + chatController = chatController, messagingController = mockk(relaxed = true), metadataDataSource = metadataDataSource, messageDataSource = messageDataSource, 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 ea034d00e5..a27cb3790c 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 @@ -67,7 +67,9 @@ class ChatEntityMapper @Inject constructor() { * * [isMember] is not on [ChatMetadata]: every payload that carries a group today is one * the caller is in, so it is a property of the write, not of the metadata. The feed and - * a join write `true`; a gated preview fetched by chat id writes `false`. + * a join write `true`, which is why that is the default. A chat fetched by id for a gated + * preview is not written at all — `GetChat` cannot say whether the viewer is in it, and the + * column would have to hold a guess either way. */ fun toEntity(metadata: ChatMetadata, isMember: Boolean = true): ChatMetadataEntity { return ChatMetadataEntity(