Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,17 @@ 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,
val groupTitle: String?,
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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) }
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 },
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<GroupProfileAction>() {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class ChatSubjectTest {
)

assertEquals("", subject.title)
assertFalse(subject.isMember)
assertEquals(false, subject.isMember)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,21 @@ interface MessagingOperations {
*/
fun observeMetadata(chatId: ChatId): Flow<ChatMembership?>

/**
* 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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?,
)
Original file line number Diff line number Diff line change
Expand Up @@ -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<List<ChatMessage>> {
return messageDataSource.observeMessages(chatId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

/**
Expand Down Expand Up @@ -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()
}
Expand All @@ -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<ChatMetadataDataSource>(relaxed = true) {
coEvery { exists(chatId) } returns false
}
val chatController = mockk<ChatController>(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<ChatMetadata>()) }
coVerify(exactly = 0) { metadataDataSource.upsert(any<List<ChatMetadata>>()) }
coVerify(exactly = 0) {
metadataDataSource.setMembership(any<ChatId>(), any<Boolean>())
}
}

/**
* 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<ChatMetadataDataSource>(relaxed = true) {
coEvery { exists(chatId) } returns true
}
val chatController = mockk<ChatController>(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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading