Skip to content

Commit 48e4733

Browse files
authored
fix(chat): render the gate for a group this device has never synced (#1498)
* fix(chat): render the gate for a group this device has never synced An invite link or a push tap can name a group no sync has brought down, and nothing fetched it: the group feed, a join and the roster stream all carry only chats the viewer is already in, so `observeMetadata` emitted null and the screen had no title, no info card and no gate — with no later read that would supply them. Our own testing missed it because the group was always already cached, from having created it or left it. `hydrateChat` fetches that chat by id, and only that chat: a stored row returns null so the Room observation stays the only source and a synced chat costs no round trip. `GetChat` does not say whether the caller is in the chat, so membership comes back unknown rather than guessed, and nothing is written to the row — a stored guess would either open a group the viewer has not joined or gate a member whose feed has not landed yet. Membership is `Boolean?` from the delegate through to the screen's subject. Unknown withholds what the viewer sees: the blur, the gate, and the invite link, since handing out an invite is an action rather than a view. It does not withhold what the gate's button offers — access still resolves with `isMember == true`, so an unknown viewer gets Join or Buy More off their own balance. The transcript is fetched only for a viewer the gate lets through, so a withheld group never pulls the messages the blur is over; a member on a fresh install gets theirs when the feed sync lands, which already loads a group whose applied cursor is still zero. * fix(chat): use the people glyph on the group profile's invite row `ic_at` belongs to the row inside the sheet this opens, where it stands for the username an invite 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 New Chat's group row already uses `ic_group_3` for.
1 parent 84f56c8 commit 48e4733

10 files changed

Lines changed: 182 additions & 16 deletions

File tree

‎apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatSubject.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,17 @@ sealed interface ChatSubject {
7171
* because a group has none.
7272
*
7373
* [isMember] rides along because the access gate reads it in the same breath as [rules], and
74-
* splitting them would let the screen blur a chat it had just joined for one frame.
74+
* splitting them would let the screen blur a chat it had just joined for one frame. It is null
75+
* for a group hydrated by id, which cannot know it — see `ChatMembership.isMember`. What the
76+
* viewer may see treats null as a non-member; what the gate's button offers does not.
7577
*/
7678
data class Group(
7779
val chatId: ChatId,
7880
val groupTitle: String?,
7981
val picture: MediaItem?,
8082
val memberCount: Long,
8183
val rules: ChatRules?,
82-
val isMember: Boolean,
84+
val isMember: Boolean?,
8385
) : ChatSubject {
8486
override val title: String get() = groupTitle.orEmpty()
8587
override val subtitle: String? get() = null

‎apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,8 @@ internal class ChatViewModel @Inject constructor(
306306
* short.
307307
*/
308308
val isGatedPreview: Boolean
309-
get() = joinProgress.success || (subject as? ChatSubject.Group)?.isMember == false
309+
get() = joinProgress.success ||
310+
(subject is ChatSubject.Group && subject.isMember != true)
310311

311312
/**
312313
* 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(
317318
*/
318319
val groupInviteUrl: String?
319320
get() = (subject as? ChatSubject.Group)
320-
?.takeIf { it.isMember }
321+
?.takeIf { it.isMember == true }
321322
?.let { Linkify.groupChatInvite(it.chatId) }
322323
}
323324

@@ -926,6 +927,37 @@ internal class ChatViewModel @Inject constructor(
926927
}
927928
}
928929

930+
/**
931+
* Gets the chat on screen onto it: its own row if the device has one, otherwise the server's
932+
* copy, and then the transcript if the viewer is entitled to it.
933+
*
934+
* A chat reached by invite link or push tap may never have been synced — the group feed, a join
935+
* and the roster stream all only ever carry chats the viewer is already in — so without the
936+
* hydration the screen has no title, no info card and no gate, and nothing that would later
937+
* supply them. A chat the device already holds costs no round trip: [ChatCoordinator.hydrateChat]
938+
* answers null for it and the Room observation stays the only source, so opening a synced chat
939+
* behaves exactly as it did.
940+
*
941+
* The transcript is fetched only for a viewer the gate lets through. A withheld group is drawn
942+
* as a placeholder rather than fetched, so asking for its messages would pull the very ones the
943+
* blur is over — and an unknown membership is withheld here for the same reason it is on screen.
944+
* A member whose feed has not synced yet is not stranded: the group feed sync sends its own load
945+
* for a group whose cursor is still at zero.
946+
*/
947+
private suspend fun openTranscript(chatId: ChatId) {
948+
val hydrated = chatCoordinator.hydrateChat(chatId) ?: run {
949+
chatCoordinator.loadMessages(chatId)
950+
return
951+
}
952+
953+
if (hydrated.metadata.type != ChatType.GROUP) {
954+
chatCoordinator.loadMessages(chatId)
955+
return
956+
}
957+
958+
dispatchEvent(Event.OnGroupResolved(hydrated))
959+
}
960+
929961
private fun initChatHandlers() {
930962
// Ahead of the transcript, so the first mapping already has the real windows rather than
931963
// the fallbacks the default carries.
@@ -972,7 +1004,7 @@ internal class ChatViewModel @Inject constructor(
9721004
if (chatId != null) {
9731005
dispatchEvent(Event.ChatFound(chatId))
9741006
chatCoordinator.setActiveChatId(chatId)
975-
viewModelScope.launch { chatCoordinator.loadMessages(chatId) }
1007+
viewModelScope.launch { openTranscript(chatId) }
9761008
chatCoordinator.dismissNotifications(chatId)
9771009
} else {
9781010
// No existing chat means no messages yet, so typing stays disabled. The
@@ -1101,7 +1133,7 @@ internal class ChatViewModel @Inject constructor(
11011133
flowOf(null)
11021134
} else {
11031135
tokenCoordinator.groupAccess(
1104-
isMember = group.isMember,
1136+
isMember = group.isMember == true,
11051137
rules = group.rules,
11061138
isStaff = userFlags.resolvedFlags.map { it.isStaff.effectiveValue },
11071139
)

‎apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,15 @@ internal sealed interface GroupProfileAction {
2727
* The same invite the empty transcript offers, for a group that already has messages in it.
2828
*
2929
* Shown only while there is a link to hand out, which is the same condition as being a member.
30+
*
31+
* The people glyph, not the `@` one: `ic_at` belongs to the row *inside* the sheet this opens,
32+
* where it stands for the username the link is sent to (node 10127:118328). On the row that opens
33+
* the sheet it said the wrong thing — what the row does is add people to the group, which is what
34+
* the New Chat group row uses the same icon for.
3035
*/
3136
internal data object InviteToGroup : FullMenuItem<GroupProfileAction>() {
3237
override val icon: Painter
33-
@Composable get() = painterResource(R.drawable.ic_at)
38+
@Composable get() = painterResource(R.drawable.ic_group_3)
3439

3540
override val name: String
3641
@Composable get() = stringResource(R.string.action_invitePeopleToJoin)

‎apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSubjectTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class ChatSubjectTest {
8484
)
8585

8686
assertEquals("", subject.title)
87-
assertFalse(subject.isMember)
87+
assertEquals(false, subject.isMember)
8888
}
8989

9090
/**

‎apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatViewModelStateTest.kt‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class ChatViewModelStateTest {
2929
private val groupChatId = ChatId(groupUuid.bytes)
3030
private val unmet = ChatRuleRequirement.MinimumBalance(Fiat(100.0), emptyList())
3131

32-
private fun group(isMember: Boolean) = ChatSubject.Group(
32+
private fun group(isMember: Boolean?) = ChatSubject.Group(
3333
chatId = groupChatId,
3434
groupTitle = "Ballers",
3535
picture = null,
@@ -127,6 +127,35 @@ class ChatViewModelStateTest {
127127
)
128128
}
129129

130+
@Test
131+
fun `a group whose membership is unknown stays withheld`() {
132+
// A chat hydrated by id — an invite link or a push tap into a group the device has never
133+
// synced — cannot know whether the viewer is in it, because GetChat does not say. Withheld
134+
// is the safe half of that: a member whose feed has not landed yet reads the blur for a
135+
// beat, where the other way round would hand a non-member the transcript outright.
136+
assertTrue(
137+
ChatViewModel.State(
138+
subject = group(isMember = null),
139+
groupAccess = null,
140+
).isGatedPreview
141+
)
142+
// Still withheld once the balance answers: what the button offers is decided by the access,
143+
// not the blur.
144+
assertTrue(
145+
ChatViewModel.State(
146+
subject = group(isMember = null),
147+
groupAccess = GroupAccess.Eligible,
148+
).isGatedPreview
149+
)
150+
}
151+
152+
@Test
153+
fun `an unknown membership has no invite to share`() {
154+
// The link is an action, not a view: offering it would invite people into a group this
155+
// device cannot yet say the viewer belongs to.
156+
assertNull(ChatViewModel.State(subject = group(isMember = null)).groupInviteUrl)
157+
}
158+
130159
@Test
131160
fun `a DM has no gate to render`() {
132161
// The null access is the whole point: a default would blur every contact conversation.

‎apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,21 @@ interface MessagingOperations {
183183
*/
184184
fun observeMetadata(chatId: ChatId): Flow<ChatMembership?>
185185

186+
/**
187+
* Fetches [chatId] from the server for a chat this device holds no row for, so a conversation
188+
* reached by invite link or push tap can render before it has ever been synced.
189+
*
190+
* Returns `null` when the row is already stored — [observeMetadata] is answering in that case,
191+
* and a second copy would only race it — and when the fetch fails.
192+
*
193+
* Deliberately not persisted. `GetChat` returns the chat, not the caller's relationship to it,
194+
* and the row's membership column has to be written with something, so storing this would
195+
* record a guess: `true` opens a group the viewer has not joined, `false` gates a member whose
196+
* feed has not synced yet. The result carries membership as `null` instead, and the join path
197+
* writes the row once the server has confirmed it.
198+
*/
199+
suspend fun hydrateChat(chatId: ChatId): ChatMembership?
200+
186201
/**
187202
* Every profile this device holds, keyed by user-id hex.
188203
*

‎apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatMembership.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,14 @@ import com.flipcash.services.models.chat.ChatMetadata
1212
*/
1313
data class ChatMembership(
1414
val metadata: ChatMetadata,
15-
val isMember: Boolean,
15+
/**
16+
* Whether the viewer is in this chat, or `null` when nothing local answers that yet.
17+
*
18+
* Only [MessagingOperations.hydrateChat] produces `null`: it fetches a chat the device holds no
19+
* row for, and `GetChat` returns the chat without the caller's relationship to it, so an
20+
* unsynced chat has no answer rather than a negative one. Callers deciding what the viewer may
21+
* *do* may read it as "not a member". Callers deciding what the viewer may *see* must withhold,
22+
* because an unknown rendered as a member shows the transcript the gate exists to withhold.
23+
*/
24+
val isMember: Boolean?,
1625
)

‎apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,15 @@ class MessagingDelegate @Inject constructor(
115115
notificationManager.cancel(chatId.hashCode())
116116
}
117117

118+
override suspend fun hydrateChat(chatId: ChatId): ChatMembership? {
119+
if (metadataDataSource.exists(chatId)) return null
120+
val metadata = chatController.getChat(chatId).getOrElse {
121+
trace(tag = TAG, message = "Hydrate failed for $chatId", type = TraceType.Error)
122+
return null
123+
}
124+
return ChatMembership(metadata = metadata, isMember = null)
125+
}
126+
118127
override fun observeMessages(chatId: ChatId): Flow<List<ChatMessage>> {
119128
return messageDataSource.observeMessages(chatId)
120129
}

‎apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingMetadataObservationTest.kt‎

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@ import com.flipcash.app.persistence.entities.ChatMetadataEntity
66
import com.flipcash.app.persistence.sources.ChatMemberDataSource
77
import com.flipcash.app.persistence.sources.ChatMessageDataSource
88
import com.flipcash.app.persistence.sources.ChatMetadataDataSource
9+
import com.flipcash.services.controllers.ChatController
910
import com.flipcash.services.models.chat.ChatId
1011
import com.flipcash.services.models.chat.ChatMetadata
1112
import com.flipcash.services.models.chat.ChatType
1213
import com.flipcash.services.models.chat.RosterSummary
1314
import com.flipcash.shared.chat.internal.delegates.MessagingDelegate
15+
import io.mockk.coEvery
16+
import io.mockk.coVerify
1417
import io.mockk.every
1518
import io.mockk.mockk
1619
import kotlinx.coroutines.flow.MutableStateFlow
@@ -19,8 +22,7 @@ import kotlinx.coroutines.test.TestCoroutineScheduler
1922
import kotlinx.coroutines.test.runTest
2023
import org.junit.Test
2124
import kotlin.test.assertEquals
22-
import kotlin.test.assertFalse
23-
import kotlin.test.assertTrue
25+
import kotlin.test.assertNull
2426
import kotlin.time.Instant
2527

2628
/**
@@ -77,12 +79,12 @@ class MessagingMetadataObservationTest {
7779
subject.observeMetadata(chatId).test {
7880
val first = awaitItem()!!
7981
assertEquals(2L, first.metadata.rosterSummary.memberCount)
80-
assertFalse(first.isMember)
82+
assertEquals(false, first.isMember)
8183

8284
rows.value = entity(memberCount = 3, isMember = true)
8385
val second = awaitItem()!!
8486
assertEquals(3L, second.metadata.rosterSummary.memberCount)
85-
assertTrue(second.isMember)
87+
assertEquals(true, second.isMember)
8688

8789
cancelAndIgnoreRemainingEvents()
8890
}
@@ -106,12 +108,73 @@ class MessagingMetadataObservationTest {
106108
}
107109
}
108110

111+
/**
112+
* A chat the device has no row for is fetched, and carries no membership answer.
113+
*
114+
* This is the cold open: an invite link or a push tap into a group no sync has ever brought
115+
* down. Without the fetch the screen has nothing to draw — the Room observation above is the
116+
* only other source and it emits null — and `GetChat` cannot say whether the viewer is in the
117+
* chat, so the result withholds rather than guesses. Nothing is written: the membership column
118+
* would have to hold one guess or the other.
119+
*/
120+
@Test
121+
fun `hydrateChat fetches an unstored chat and reports membership as unknown`() =
122+
runTest(dispatchers.dispatcher) {
123+
val metadataDataSource = mockk<ChatMetadataDataSource>(relaxed = true) {
124+
coEvery { exists(chatId) } returns false
125+
}
126+
val chatController = mockk<ChatController>(relaxed = true) {
127+
coEvery { getChat(chatId) } returns Result.success(metadata(memberCount = 4))
128+
}
129+
130+
val subject = delegate(
131+
metadataDataSource = metadataDataSource,
132+
memberDataSource = mockk(relaxed = true),
133+
messageDataSource = mockk(relaxed = true),
134+
chatController = chatController,
135+
)
136+
137+
val hydrated = subject.hydrateChat(chatId)
138+
139+
assertEquals(4L, hydrated?.metadata?.rosterSummary?.memberCount)
140+
assertNull(hydrated?.isMember)
141+
coVerify(exactly = 0) { metadataDataSource.upsert(any<ChatMetadata>()) }
142+
coVerify(exactly = 0) { metadataDataSource.upsert(any<List<ChatMetadata>>()) }
143+
coVerify(exactly = 0) {
144+
metadataDataSource.setMembership(any<ChatId>(), any<Boolean>())
145+
}
146+
}
147+
148+
/**
149+
* A stored chat is not refetched: `observeMetadata` is already answering for it, with the
150+
* membership the row holds, and a second copy would only race it.
151+
*/
152+
@Test
153+
fun `hydrateChat leaves a stored chat to the observation`() =
154+
runTest(dispatchers.dispatcher) {
155+
val metadataDataSource = mockk<ChatMetadataDataSource>(relaxed = true) {
156+
coEvery { exists(chatId) } returns true
157+
}
158+
val chatController = mockk<ChatController>(relaxed = true)
159+
160+
val subject = delegate(
161+
metadataDataSource = metadataDataSource,
162+
memberDataSource = mockk(relaxed = true),
163+
messageDataSource = mockk(relaxed = true),
164+
chatController = chatController,
165+
)
166+
167+
assertNull(subject.hydrateChat(chatId))
168+
coVerify(exactly = 0) { chatController.getChat(any()) }
169+
}
170+
109171
private fun delegate(
110172
metadataDataSource: ChatMetadataDataSource,
111173
memberDataSource: ChatMemberDataSource,
112174
messageDataSource: ChatMessageDataSource,
175+
chatController: ChatController = mockk(relaxed = true),
113176
) = MessagingDelegate(
114-
chatController = mockk(relaxed = true),
177+
chatController = chatController,
115178
messagingController = mockk(relaxed = true),
116179
metadataDataSource = metadataDataSource,
117180
messageDataSource = messageDataSource,

‎apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ class ChatEntityMapper @Inject constructor() {
6767
*
6868
* [isMember] is not on [ChatMetadata]: every payload that carries a group today is one
6969
* the caller is in, so it is a property of the write, not of the metadata. The feed and
70-
* a join write `true`; a gated preview fetched by chat id writes `false`.
70+
* a join write `true`, which is why that is the default. A chat fetched by id for a gated
71+
* preview is not written at all — `GetChat` cannot say whether the viewer is in it, and the
72+
* column would have to hold a guess either way.
7173
*/
7274
fun toEntity(metadata: ChatMetadata, isMember: Boolean = true): ChatMetadataEntity {
7375
return ChatMetadataEntity(

0 commit comments

Comments
 (0)