Skip to content

Commit 90e685b

Browse files
authored
fix(chat): rebuild chat history after logout and gate the tip milestone on it (#1331)
* fix(chat): keep chat history across logout so onboarding milestones survive An account that had already sent a tip was shown the new-user tutorial again after signing back in, with "Scan a Tip Card" unticked. The milestone is a read of chat history — ChatMessageDao.hasEverTipped looks for an outgoing message with verb TIPPED — and two things conspired to leave that history empty. Logout wiped the cache. RealSessionController routed AuthState.LoggedOut into ChatCoordinator.reset, which deleted every metadata, message, and member row. Nothing needed it to: FlipcashDatabase names the file from the account entropy, so accounts are already separated by sitting in different databases, and the activity feed has never wiped itself on logout for exactly that reason. Split the coordinator into teardown (connections, jobs, in-memory state) and clearCache (the rows), point logout at teardown, and give deleteAndLogout the explicit wipe — deletion is the one caller that wants the data gone. Account switching goes through logout too, so it stops wiping as a side effect. Login could not rebuild what logout removed. performFeedSync decides which chats need catching up by asking whether a chat has cached messages and whether its stored sequence trails the server's — but it asked after upserting the server's sequence and each chat's lastMessage, so the first read was always true and the second always equal. Both branches were unreachable, leaving each chat holding the single message the feed carried. In the reported case that message was the tip received back, not the tip sent, so hasEverTipped stayed false for good. Snapshot both values before the writes. The delta path had the same shape one level down: performDeltaSync re-read chat_metadata for its starting sequence, which by then held the value the sync had just written, so it asked the server for everything after the server's own latest event. DeltaSyncNeeded now carries the pre-sync sequence. The live gap-fill caller has no write in front of it and keeps reading the row. One gap remains: WalletViewModel.isAwaitingActivity gates on the activity feed's sync state, not the chat cache's, so a cold start can still flash the milestone as incomplete before chat history lands. Closing that needs a "chat history hydrated at least once" signal that doesn't exist yet. * fix(chat): withhold the tip milestone until chat history has hydrated The wallet tab drew "Scan a Tip Card" as outstanding before the chat cache had been reconciled, so an account that had already tipped saw the new-user tutorial for as long as its history took to arrive. `isAwaitingActivity` covered the activity feed's sync state; nothing covered the chat cache. The feed sync cannot serve as that signal. It reports itself `Synced` once the conversation list is written, which is before the per-chat backfill it schedules has run — and that backfill is what surfaces a tip older than a chat's last message. `FeedSyncDelegate` now sends a terminal `Event.CatchUpComplete` after the catch-up loop; because the delegate's events are a FIFO channel drained by one sequential collector in `RealChatCoordinator`, every catch-up item ahead of the marker has finished its suspend call by the time it is routed. Routing it sets `ChatState.historyHydration` to `Hydrated`. A failed sync moves `Unknown` to `Unavailable` instead, so an unreachable server ends the wait rather than extending it; it never downgrades a hydration that already succeeded. `hasEverTipped()` returns `Flow<Boolean?>`, withholding an answer while hydration is `Unknown`. A cached tip still answers immediately — a TIPPED message in the cache is proof whatever the hydration state — so the warm cache path, now the normal one, does not wait on a round-trip. The flow re-subscribes to the Room query on the hydration change rather than combining the two: a combine would emit the pre-backfill answer alongside the "hydrated" flip and reintroduce the flash it is meant to remove. `WalletViewModel` emits nothing while the milestone is null, leaving `onboardingItems` null and the tab on its loading state. A held balance deliberately does not short-circuit this one, unlike the activity-feed clause: it is evidence about money, not about tipping. The cost is that a funded, never-tipped user with a cold cache now waits one chat round-trip before the tab draws.
1 parent 7a4c4d5 commit 90e685b

18 files changed

Lines changed: 994 additions & 50 deletions

File tree

‎apps/flipcash/features/balance/src/main/kotlin/com/flipcash/app/balance/internal/WalletViewModel.kt‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel
2121
import kotlinx.coroutines.flow.combine
2222
import kotlinx.coroutines.flow.filter
2323
import kotlinx.coroutines.flow.filterIsInstance
24+
import kotlinx.coroutines.flow.filterNotNull
2425
import kotlinx.coroutines.flow.flatMapLatest
2526
import kotlinx.coroutines.flow.launchIn
2627
import kotlinx.coroutines.flow.mapNotNull
@@ -77,6 +78,12 @@ internal class WalletViewModel @Inject constructor(
7778
* short-circuit the wait: if there is already activity to draw, there is nothing to
7879
* mistake for a new account — and neither is a held balance, which is a live read of the
7980
* account rather than of the cache.
81+
*
82+
* The two caches are separate and settle separately. The `feedSyncState` clause covers the
83+
* activity feed; the *chat* cache backing the tip milestone is covered by
84+
* [onboardingItems] staying null, because the milestone flow withholds an answer until its
85+
* own history has hydrated. A held balance deliberately does not short-circuit that one:
86+
* it says nothing about whether the account has tipped.
8087
*/
8188
val isAwaitingActivity: Boolean
8289
get() = onboardingItems == null ||
@@ -137,14 +144,21 @@ internal class WalletViewModel @Inject constructor(
137144
chatCoordinator.hasEverTipped(),
138145
tokenCoordinator.hasAnyBalance,
139146
) { hasReceivedMoney, hasTipped, holdsBalance ->
140-
Event.OnOnboardingItemsUpdated(
141-
items = listOf(
142-
TutorialItem.AddMoney(isCompleted = hasReceivedMoney || holdsBalance),
143-
TutorialItem.ScanTipCard(isCompleted = hasTipped),
144-
),
145-
holdsBalance = holdsBalance,
146-
)
147+
// A null tip milestone means the chat cache has not been reconciled yet, and an
148+
// un-hydrated cache reports every account as never having tipped. Emitting nothing
149+
// keeps [State.onboardingItems] null, which holds the whole tab on its spinner rather
150+
// than drawing "Scan a Tip Card" as outstanding to someone who already did it.
151+
hasTipped?.let {
152+
Event.OnOnboardingItemsUpdated(
153+
items = listOf(
154+
TutorialItem.AddMoney(isCompleted = hasReceivedMoney || holdsBalance),
155+
TutorialItem.ScanTipCard(isCompleted = it),
156+
),
157+
holdsBalance = holdsBalance,
158+
)
159+
}
147160
}
161+
.filterNotNull()
148162
.onEach { dispatchEvent(it) }
149163
.launchIn(viewModelScope)
150164
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package com.flipcash.app.balance.internal
2+
3+
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
4+
import com.flipcash.app.analytics.StubFlipcashAnalytics
5+
import com.flipcash.app.balance.internal.components.TutorialItem
6+
import com.flipcash.app.core.MainCoroutineRule
7+
import com.flipcash.app.core.dispatchers.TestDispatchers
8+
import com.flipcash.app.funding.PurchaseMethodController
9+
import com.flipcash.app.tokens.TokenCoordinator
10+
import com.flipcash.app.userflags.UserFlagsCoordinator
11+
import com.flipcash.services.user.UserManager
12+
import com.flipcash.shared.chat.ChatCoordinator
13+
import com.flipcash.shared.transactionhistory.ActivityFeedCoordinator
14+
import io.mockk.every
15+
import io.mockk.mockk
16+
import kotlinx.coroutines.ExperimentalCoroutinesApi
17+
import kotlinx.coroutines.flow.MutableStateFlow
18+
import kotlinx.coroutines.flow.flowOf
19+
import kotlinx.coroutines.test.UnconfinedTestDispatcher
20+
import kotlinx.coroutines.test.advanceUntilIdle
21+
import kotlinx.coroutines.test.runTest
22+
import org.junit.Rule
23+
import org.junit.Test
24+
import kotlin.test.assertEquals
25+
import kotlin.test.assertNotNull
26+
import kotlin.test.assertNull
27+
import kotlin.test.assertTrue
28+
29+
/**
30+
* The tip milestone is read off the chat cache, which reports every account as never having tipped
31+
* until its history has been reconciled. The wallet must not draw that: [WalletViewModel.State.
32+
* onboardingItems] stays null — holding the tab on its loading state — until the chat coordinator
33+
* commits to an answer.
34+
*
35+
* A held balance deliberately does not release the wait here, unlike for the activity feed: it is
36+
* evidence about money, not about tipping.
37+
*/
38+
@OptIn(ExperimentalCoroutinesApi::class)
39+
class WalletMilestoneGatingTest {
40+
41+
@get:Rule
42+
val instantExecutorRule = InstantTaskExecutorRule()
43+
44+
@get:Rule
45+
var mainCoroutineRule = MainCoroutineRule(UnconfinedTestDispatcher())
46+
47+
private val hasEverTipped = MutableStateFlow<Boolean?>(null)
48+
49+
private val chatCoordinator: ChatCoordinator = mockk(relaxed = true) {
50+
every { hasEverTipped() } returns hasEverTipped
51+
}
52+
private val feedCoordinator: ActivityFeedCoordinator = mockk(relaxed = true) {
53+
every { hasEverReceivedMoney() } returns flowOf(true)
54+
}
55+
private val tokenCoordinator: TokenCoordinator = mockk(relaxed = true) {
56+
every { hasAnyBalance } returns flowOf(true)
57+
}
58+
59+
private lateinit var dispatchers: TestDispatchers
60+
61+
private fun createViewModel() = WalletViewModel(
62+
userManager = mockk<UserManager>(relaxed = true),
63+
userFlags = mockk<UserFlagsCoordinator>(relaxed = true),
64+
dispatchers = dispatchers,
65+
purchaseMethodController = mockk<PurchaseMethodController>(relaxed = true),
66+
analytics = StubFlipcashAnalytics(),
67+
chatCoordinator = chatCoordinator,
68+
feedCoordinator = feedCoordinator,
69+
tokenCoordinator = tokenCoordinator,
70+
)
71+
72+
@Test
73+
fun `milestones are withheld while the tip milestone is unknown`() =
74+
runTest(mainCoroutineRule.dispatcher) {
75+
dispatchers = TestDispatchers(testScheduler)
76+
77+
val vm = createViewModel()
78+
advanceUntilIdle()
79+
80+
assertNull(
81+
vm.stateFlow.value.onboardingItems,
82+
"an un-reconciled chat cache reports every account as never having tipped",
83+
)
84+
assertTrue(
85+
vm.stateFlow.value.isAwaitingActivity,
86+
"the tab must keep loading rather than draw a milestone it cannot yet answer",
87+
)
88+
}
89+
90+
@Test
91+
fun `milestones are published once the tip milestone resolves`() =
92+
runTest(mainCoroutineRule.dispatcher) {
93+
dispatchers = TestDispatchers(testScheduler)
94+
95+
val vm = createViewModel()
96+
advanceUntilIdle()
97+
hasEverTipped.value = true
98+
advanceUntilIdle()
99+
100+
val items = vm.stateFlow.value.onboardingItems
101+
assertNotNull(items)
102+
assertEquals(
103+
listOf(true, true),
104+
items.map { it.isCompleted },
105+
"both milestones read complete: the account holds a balance and has tipped",
106+
)
107+
assertTrue(items.any { it is TutorialItem.ScanTipCard })
108+
}
109+
}

‎apps/flipcash/shared/authentication/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ dependencies {
1212
implementation(libs.androidx.datastore)
1313

1414
implementation(project(":apps:flipcash:shared:appsettings"))
15+
implementation(project(":apps:flipcash:shared:chat"))
1516
implementation(project(":apps:flipcash:shared:contacts"))
1617
implementation(project(":apps:flipcash:shared:persistence:provider"))
1718
implementation(project(":apps:flipcash:shared:push"))

‎apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.flipcash.app.auth.internal.credentials.PassphraseCredentialManager
77
import com.flipcash.app.contacts.ContactCoordinator
88
import com.flipcash.app.featureflags.FeatureFlagController
99
import com.flipcash.app.persistence.PersistenceProvider
10+
import com.flipcash.shared.chat.ChatCoordinator
1011
import com.flipcash.app.push.PushTokenProvider
1112
import com.flipcash.app.tokens.TokenCoordinator
1213
import com.flipcash.app.userflags.UserFlagsCoordinator
@@ -54,6 +55,7 @@ class AuthManager @Inject constructor(
5455
private val userFlags: UserFlagsCoordinator,
5556
private val profileCoordinator: ProfileCoordinator,
5657
private val contactCoordinator: ContactCoordinator,
58+
private val chatCoordinator: ChatCoordinator,
5759
private val networkObserver: NetworkConnectivityListener,
5860
private val dispatchers: DispatcherProvider,
5961
// private val analytics: AnalyticsService,
@@ -315,6 +317,11 @@ class AuthManager @Inject constructor(
315317
//todo: add account deletion
316318
// Wipe server contact set before logout while the session can still authenticate.
317319
contactCoordinator.clearServerContactSet()
320+
// Deletion is the only path that erases the local chat cache. Plain logout and account
321+
// switching leave it — the database is per-account, so it never has to be emptied to keep
322+
// accounts apart, and keeping it lets a re-login reconcile rather than refetch. That makes
323+
// this the one caller responsible for the data actually going away.
324+
chatCoordinator.clearCache()
318325
return logout()
319326
}
320327

‎apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import com.flipcash.app.persistence.PersistenceProvider
1111
import com.flipcash.app.push.PushTokenProvider
1212
import com.flipcash.app.tokens.TokenCoordinator
1313
import com.flipcash.app.userflags.UserFlagsCoordinator
14+
import com.flipcash.shared.chat.ChatCoordinator
1415
import com.flipcash.shared.profile.ProfileCoordinator
1516
import com.flipcash.services.controllers.AccountController
1617
import com.flipcash.services.controllers.ProfileController
@@ -65,6 +66,7 @@ class AuthManagerTest {
6566
private val userFlags: UserFlagsCoordinator = mockk(relaxed = true)
6667
private val profileCoordinator: ProfileCoordinator = mockk(relaxed = true)
6768
private val contactCoordinator: ContactCoordinator = mockk(relaxed = true)
69+
private val chatCoordinator: ChatCoordinator = mockk(relaxed = true)
6870
private val userManagerState = MutableStateFlow(UserManager.State())
6971

7072
private val networkConnectivityListener = mockk<NetworkConnectivityListener>(relaxed = true)
@@ -108,6 +110,7 @@ class AuthManagerTest {
108110
userFlags = userFlags,
109111
profileCoordinator = profileCoordinator,
110112
contactCoordinator = contactCoordinator,
113+
chatCoordinator = chatCoordinator,
111114
dispatchers = dispatchers,
112115
networkObserver = networkConnectivityListener,
113116
)
@@ -164,6 +167,27 @@ class AuthManagerTest {
164167
verify { userFlags.clearAll() }
165168
}
166169

170+
@Test
171+
fun `logout leaves the chat cache in place`() = runTest {
172+
coEvery { credentialManager.logout() } returns Result.success(Unit)
173+
174+
authManager.logout()
175+
176+
// The Room file is named from the account entropy, so signing out never has to erase chat
177+
// history to keep accounts apart — and keeping it lets a re-login reconcile what it already
178+
// has instead of rebuilding from one message per chat.
179+
coVerify(exactly = 0) { chatCoordinator.clearCache() }
180+
}
181+
182+
@Test
183+
fun `deleteAndLogout erases the chat cache`() = runTest {
184+
coEvery { credentialManager.logout() } returns Result.success(Unit)
185+
186+
authManager.deleteAndLogout()
187+
188+
coVerify(exactly = 1) { chatCoordinator.clearCache() }
189+
}
190+
167191
@Test
168192
fun `non-soft login sets softLoginDisabled flag`() = runTest {
169193
val entropy = "dGVzdGVudHJvcHkxMjM0NQ=="

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

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,19 @@ interface MessagingOperations {
120120
/** Observes all messages in [chatId] as a flat list. */
121121
fun observeMessages(chatId: ChatId): Flow<List<ChatMessage>>
122122

123-
/** True once the user has ever sent a tip (a Cash message with verb TIPPED) — onboarding milestone. */
124-
fun hasEverTipped(): Flow<Boolean>
123+
/**
124+
* Whether the user has ever sent a tip — a Cash message with verb TIPPED — or `null` while the
125+
* answer is not yet trustworthy.
126+
*
127+
* The read is of a local cache, so an absent TIPPED message means "never tipped" only once that
128+
* cache is known to be complete; before then it is indistinguishable from history that has not
129+
* arrived. Onboarding is the caller, and it draws a tutorial off the answer, so it needs the
130+
* third state rather than a `false` it has to guess about. Resolves to `true`/`false` once
131+
* [ChatHydrationState] leaves [ChatHydrationState.Unknown] — including on
132+
* [ChatHydrationState.Unavailable], so an unreachable server ends the wait instead of extending
133+
* it forever.
134+
*/
135+
fun hasEverTipped(): Flow<Boolean?>
125136

126137
/** Observes messages in [chatId] via Paging 3, with remote-mediated page loads. */
127138
fun observeMessagesPaged(chatId: ChatId): Flow<PagingData<ChatMessage>>
@@ -166,8 +177,27 @@ interface ChatCoordinator : FeedOperations, EventStreamOperations, DmChatResolve
166177
/** Full observable snapshot of chat state (feed, typing, reactions, active chat). */
167178
val state: StateFlow<ChatState>
168179

169-
/** Tears down all connections, clears persisted data, and resets in-memory state. */
170-
suspend fun reset()
180+
/**
181+
* Closes connections, cancels jobs, and drops in-memory state.
182+
*
183+
* Deliberately leaves the persisted cache alone. The Room database is per-account
184+
* (`FlipcashDatabase.init` names the file from the account entropy), so signing out does not
185+
* have to erase anything to keep the next account's data separate — logging in swaps to a
186+
* different file. Keeping the cache lets a re-login reconcile what it already has via the
187+
* feed sync's catch-up instead of rebuilding from nothing, which is what the "send a tip"
188+
* onboarding milestone was silently losing.
189+
*
190+
* @see clearCache for the one caller that does want the data gone.
191+
*/
192+
suspend fun teardown()
193+
194+
/**
195+
* Erases this account's persisted chat history — metadata, messages, and members.
196+
*
197+
* Account deletion only. Ordinary logout and account switching go through [teardown] and
198+
* keep the cache.
199+
*/
200+
suspend fun clearCache()
171201
}
172202

173203
class NoDmChatInitializedException(e164: String) : Exception("No DM chat for $e164")

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ data class ChatState(
1111
val typingIndicators: Map<ChatId, Set<ActiveTypist>> = emptyMap(),
1212
val reactionOverlays: Map<ChatId, Map<Long, ReactionSummary>> = emptyMap(),
1313
val feedSyncState: FeedSyncState = FeedSyncState.Idle,
14+
val historyHydration: ChatHydrationState = ChatHydrationState.Unknown,
1415
val activeChat: ChatId? = null,
1516
)
1617

@@ -30,3 +31,24 @@ enum class FeedSyncState {
3031
Synced,
3132
Error,
3233
}
34+
35+
/**
36+
* How far the *message* cache has got in reconciling itself with the server for the signed-in user.
37+
*
38+
* Distinct from [FeedSyncState], which reports only the conversation-list fetch. That fetch marks
39+
* itself [FeedSyncState.Synced] as soon as the metadata lands, before the per-chat history backfill
40+
* it schedules has run — so it cannot answer "is this account's chat history here yet". Callers that
41+
* read chat history as evidence need that second question: the wallet's "send a tip" milestone looks
42+
* for an outgoing TIPPED message, and an absent one only means the user has never tipped once the
43+
* cache is known to be complete.
44+
*/
45+
enum class ChatHydrationState {
46+
/** No sync has finished a full pass this session — an absent message proves nothing. */
47+
Unknown,
48+
49+
/** A sync succeeded and every catch-up it scheduled has run: an absent message really is absent. */
50+
Hydrated,
51+
52+
/** A sync completed without success. Callers should stop waiting; the next sync will retry. */
53+
Unavailable,
54+
}

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,13 @@ class RealChatCoordinator @Inject constructor(
144144
is FeedSyncDelegate.Event.LoadMessages ->
145145
messagingDelegate.loadMessages(event.chatId)
146146
is FeedSyncDelegate.Event.DeltaSyncNeeded ->
147-
eventStreamDelegate.performDeltaSync(event.chatId)
147+
eventStreamDelegate.performDeltaSync(event.chatId, event.afterSequence)
148+
// Arrives after every catch-up item above it, because this is one sequential
149+
// collector over a FIFO channel. Anything reading chat history as evidence —
150+
// the wallet's "send a tip" milestone — waits for this rather than for the
151+
// feed sync, which reports itself synced before the backfill is scheduled.
152+
FeedSyncDelegate.Event.CatchUpComplete ->
153+
feedDelegate.markHistoryHydrated()
148154
}
149155
}.launchIn(scope)
150156

@@ -197,17 +203,21 @@ class RealChatCoordinator @Inject constructor(
197203

198204
// region ChatCoordinator
199205

200-
override suspend fun reset() {
206+
override suspend fun teardown() {
201207
eventStreamDelegate.stopHeartbeat()
202208
eventStreamDelegate.close()
203209
feedDelegate.cancelJobs()
204210
networkObserverJob?.cancel()
205211
stateHolder.reset()
206212
eventStreamDelegate.clearAll()
207213
cluster.value = null
208-
messagingDelegate.clear()
209214
supervisorJob.cancel()
210-
trace(tag = TAG, message = "reset complete", type = TraceType.Process)
215+
trace(tag = TAG, message = "teardown complete", type = TraceType.Process)
216+
}
217+
218+
override suspend fun clearCache() {
219+
messagingDelegate.clear()
220+
trace(tag = TAG, message = "cache cleared", type = TraceType.Process)
211221
}
212222

213223
// endregion

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,17 @@ class EventStreamDelegate @Inject constructor(
185185
heartbeatJob = null
186186
}
187187

188-
internal suspend fun performDeltaSync(chatId: ChatId) {
189-
val afterSequence = metadataDataSource.getLatestEventSequence(chatId)
188+
/**
189+
* Backfills a chat from [afterSequence] onward.
190+
*
191+
* Callers that have already written the server's sequence to `chat_metadata` before asking for
192+
* a delta must pass the sequence the cache held *beforehand* — reading it here would return
193+
* the value they just wrote and ask the server for everything after its own latest event,
194+
* which is always nothing. The live gap-fill path has no such write in front of it and omits
195+
* the argument.
196+
*/
197+
internal suspend fun performDeltaSync(chatId: ChatId, afterSequence: Long? = null) {
198+
val afterSequence = afterSequence ?: metadataDataSource.getLatestEventSequence(chatId)
190199
trace(tag = TAG, message = "Delta sync for $chatId from sequence $afterSequence", type = TraceType.Process)
191200

192201
try {

0 commit comments

Comments
 (0)