From f6bf090d38f2d4698d12659dbfe43a172443ef0c Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 6 Nov 2026 14:38:39 -0500 Subject: [PATCH 1/3] refactor(messenger): move RestrictedMembersViewModel onto BaseViewModel Load, Unsilence and Unban become Events handled in one eventFlow collector, so a second tap on a row still sees the first one's pending entry. Every state change now goes through updateStateForEvent; the row-lifting rules move from lambdas passed to run() into the Unsilenced and Unbanned reducer branches. The screen dispatches the events instead of calling load, unsilence and unban, and reads stateFlow instead of state. --- .../internal/RestrictedMembersViewModel.kt | 146 ++++++++++++------ .../profile/RestrictedMembersScreen.kt | 14 +- .../internal/RestrictedMembersStateTest.kt | 89 +++++++++++ .../RestrictedMembersViewModelTest.kt | 53 ++++--- 4 files changed, 226 insertions(+), 76 deletions(-) create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt index dd4e3fa110..95165ae9a2 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt @@ -1,6 +1,5 @@ package com.flipcash.app.messenger.internal -import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.flipcash.app.core.toast.SystemToastController import com.flipcash.features.messenger.R @@ -13,11 +12,11 @@ import com.getcode.manager.BottomBarManager import com.getcode.opencode.model.core.ID import com.getcode.util.resources.ResourceHelper import com.getcode.utils.trace +import com.getcode.view.BaseViewModel +import com.flipcash.libs.coroutines.DispatcherProvider import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.update +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import javax.inject.Inject @@ -37,7 +36,12 @@ internal class RestrictedMembersViewModel @Inject constructor( private val resources: ResourceHelper, private val toastController: SystemToastController, private val restrictionWrites: GroupRestrictionWrites, -) : ViewModel() { + dispatchers: DispatcherProvider, +) : BaseViewModel( + initialState = State(), + updateStateForEvent = updateStateForEvent, + defaultDispatcher = dispatchers.Default, +) { data class State( val loading: Boolean = true, @@ -52,16 +56,63 @@ internal class RestrictedMembersViewModel @Inject constructor( } } - private val _state = MutableStateFlow(State()) - val state: StateFlow = _state.asStateFlow() + sealed interface Event { + /** Read every page for [chatId], once per view model; the flow's back stack reopens this one. */ + data class Load(val chatId: ChatId) : Event + + /** Lift [userId]'s silence. [name] goes in the success toast. */ + data class Unsilence(val chatId: ChatId, val userId: ID, val name: String) : Event + + /** Lift [userId]'s ban. */ + data class Unban(val chatId: ChatId, val userId: ID) : Event + + /** A new read started; drops whatever the last one showed. */ + data object LoadStarted : Event + data class Loaded(val users: List) : Event + data object LoadFailed : Event + + data class RestrictionPending(val userId: ID) : Event + data class Unsilenced(val userId: ID) : Event + data class Unbanned(val userId: ID) : Event + data class RestrictionFailed(val userId: ID) : Event + } private var loadedFor: ChatId? = null - /** Reads every page for [chatId] once per view model; the flow's back stack reopens this one. */ - fun load(chatId: ChatId) { + init { + // One collector, so a second tap on a row sees the first one's pending entry. + eventFlow + .onEach { event -> + when (event) { + is Event.Load -> load(event.chatId) + is Event.Unsilence -> run( + userId = event.userId, + call = { chatController.unsilenceUser(event.chatId, event.userId) }, + done = Event.Unsilenced(event.userId), + onSuccess = { + restrictionWrites.lifted(event.chatId, event.userId) + toastController.showToast(R.string.toast_unsilenced, event.name, replacePrevious = true) + }, + titleRes = R.string.error_title_failedToUnsilence, + messageRes = R.string.error_description_failedToUnsilence, + ) + is Event.Unban -> run( + userId = event.userId, + call = { chatController.unbanUser(event.chatId, event.userId) }, + done = Event.Unbanned(event.userId), + titleRes = R.string.error_title_failedToUnban, + messageRes = R.string.error_description_failedToUnban, + ) + else -> Unit + } + } + .launchIn(viewModelScope) + } + + private fun load(chatId: ChatId) { if (loadedFor == chatId) return loadedFor = chatId - _state.update { State() } + dispatchEvent(Event.LoadStarted) viewModelScope.launch { val all = mutableListOf() var options = QueryOptions() @@ -69,7 +120,7 @@ internal class RestrictedMembersViewModel @Inject constructor( val page = chatController.getRestrictedUsers(chatId, options).getOrElse { trace("failed to load restricted users - ${it.localizedMessage}") loadedFor = null - _state.update { s -> s.copy(loading = false, failed = true) } + dispatchEvent(Event.LoadFailed) BottomBarManager.showError( title = resources.getString(R.string.error_title_failedToLoadRestricted), message = resources.getString(R.string.error_description_failedToLoadRestricted), @@ -80,58 +131,29 @@ internal class RestrictedMembersViewModel @Inject constructor( if (!page.hasMore || page.pagingToken == null) break options = options.copy(token = page.pagingToken) } - _state.update { it.copy(loading = false, users = all) } + dispatchEvent(Event.Loaded(all)) } } - fun unsilence(chatId: ChatId, userId: ID, name: String) = run( - userId = userId, - call = { chatController.unsilenceUser(chatId, userId) }, - // A removed member who is also silenced stays in Removed, now without the silence. - apply = { u -> if (u.ban != null) u.copy(silence = null) else null }, - onSuccess = { - restrictionWrites.lifted(chatId, userId) - toastController.showToast(R.string.toast_unsilenced, name, replacePrevious = true) - }, - titleRes = R.string.error_title_failedToUnsilence, - messageRes = R.string.error_description_failedToUnsilence, - ) - - fun unban(chatId: ChatId, userId: ID) = run( - userId = userId, - call = { chatController.unbanUser(chatId, userId) }, - // Unbanning leaves any silence in place, so they move back to Silenced if they had one. - apply = { u -> u.copy(ban = null).takeIf { it.silence.isActiveAt() } }, - titleRes = R.string.error_title_failedToUnban, - messageRes = R.string.error_description_failedToUnban, - ) - private fun run( userId: ID, call: suspend () -> Result, - apply: (RestrictedUser) -> RestrictedUser?, + done: Event, onSuccess: () -> Unit = {}, titleRes: Int, messageRes: Int, ) { - if (userId in _state.value.pending) return - _state.update { it.copy(pending = it.pending + setOf(userId)) } + if (userId in stateFlow.value.pending) return + dispatchEvent(Event.RestrictionPending(userId)) viewModelScope.launch { call() .onSuccess { - _state.update { s -> - s.copy( - pending = s.pending - setOf(userId), - users = s.users.mapNotNull { u -> - if (u.userProfile.userId == userId) apply(u) else u - }, - ) - } + dispatchEvent(done) onSuccess() } .onFailure { trace("failed to update restriction - ${it.localizedMessage}") - _state.update { s -> s.copy(pending = s.pending - setOf(userId)) } + dispatchEvent(Event.RestrictionFailed(userId)) BottomBarManager.showError( title = resources.getString(titleRes), message = resources.getString(messageRes), @@ -139,4 +161,34 @@ internal class RestrictedMembersViewModel @Inject constructor( } } } + + companion object { + val updateStateForEvent: (Event) -> ((State) -> State) = { event -> + when (event) { + is Event.Load, + is Event.Unsilence, + is Event.Unban -> { state -> state } + + Event.LoadStarted -> { _ -> State() } + is Event.Loaded -> { state -> state.copy(loading = false, users = event.users) } + Event.LoadFailed -> { state -> state.copy(loading = false, failed = true) } + + is Event.RestrictionPending -> { state -> state.copy(pending = state.pending + setOf(event.userId)) } + is Event.RestrictionFailed -> { state -> state.copy(pending = state.pending - setOf(event.userId)) } + // A removed member who is also silenced stays in Removed, now without the silence. + is Event.Unsilenced -> { state -> + state.lifted(event.userId) { u -> if (u.ban != null) u.copy(silence = null) else null } + } + // Unbanning leaves any silence in place, so they move back to Silenced if they had one. + is Event.Unbanned -> { state -> + state.lifted(event.userId) { u -> u.copy(ban = null).takeIf { it.silence.isActiveAt() } } + } + } + } + + private fun State.lifted(userId: ID, apply: (RestrictedUser) -> RestrictedUser?): State = copy( + pending = pending - setOf(userId), + users = users.mapNotNull { u -> if (u.userProfile.userId == userId) apply(u) else u }, + ) + } } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/RestrictedMembersScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/RestrictedMembersScreen.kt index bdbededcc5..019ad71e24 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/RestrictedMembersScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/RestrictedMembersScreen.kt @@ -59,11 +59,13 @@ import com.getcode.util.formatLocalized internal fun RestrictedMembersScreen(kind: RestrictedKind, chatViewModel: ChatViewModel) { val viewModel = hiltViewModel() val chatState by chatViewModel.stateFlow.collectAsStateWithLifecycle() - val state by viewModel.state.collectAsStateWithLifecycle() + val state by viewModel.stateFlow.collectAsStateWithLifecycle() val flowNavigator = rememberFlowNavigator() val chatId = chatState.chatId - LaunchedEffect(chatId) { chatId?.let(viewModel::load) } + LaunchedEffect(chatId) { + chatId?.let { viewModel.dispatchEvent(RestrictedMembersViewModel.Event.Load(it)) } + } val rows = state.rows(kind) val unknownName = stringResource(R.string.title_unnamedUser) @@ -122,8 +124,12 @@ internal fun RestrictedMembersScreen(kind: RestrictedKind, chatViewModel: ChatVi userId ?: return@RestrictedRow val id = chatId ?: return@RestrictedRow when (kind) { - RestrictedKind.Silenced -> viewModel.unsilence(id, userId, name) - RestrictedKind.Removed -> viewModel.unban(id, userId) + RestrictedKind.Silenced -> viewModel.dispatchEvent( + RestrictedMembersViewModel.Event.Unsilence(id, userId, name) + ) + RestrictedKind.Removed -> viewModel.dispatchEvent( + RestrictedMembersViewModel.Event.Unban(id, userId) + ) } }, ) diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt new file mode 100644 index 0000000000..f4c2d863b4 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt @@ -0,0 +1,89 @@ +package com.flipcash.app.messenger.internal + +import com.flipcash.app.messenger.internal.RestrictedMembersViewModel.Event +import com.flipcash.app.messenger.internal.RestrictedMembersViewModel.State +import com.flipcash.services.models.UserProfile +import com.flipcash.services.models.chat.BanState +import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.RestrictedUser +import com.flipcash.services.models.chat.SilenceState +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlin.time.Clock + +class RestrictedMembersStateTest { + + private fun reduce(state: State, event: Event) = RestrictedMembersViewModel.updateStateForEvent(event)(state) + + private val chatId = ChatId(ByteArray(32) { 3 }.toList()) + private val banned = BanState(Clock.System.now()) + + private fun user(id: Int, silence: SilenceState? = null, ban: BanState? = null) = RestrictedUser( + userProfile = UserProfile( + displayName = "User $id", socialAccounts = emptyList(), phoneNumber = null, email = null, + userId = listOf(id.toByte()), + ), + silence = silence, + ban = ban, + ) + + private val silenced = user(1, silence = SilenceState.Forever) + private val removed = user(2, ban = banned) + private val both = user(3, silence = SilenceState.Forever, ban = banned) + private val loaded = reduce(State(), Event.Loaded(listOf(silenced, removed, both))) + + @Test + fun `public actions leave state alone until their work reports back`() { + val id = silenced.userProfile.userId!! + assertEquals(loaded, reduce(loaded, Event.Load(chatId))) + assertEquals(loaded, reduce(loaded, Event.Unsilence(chatId, id, "Ada"))) + assertEquals(loaded, reduce(loaded, Event.Unban(chatId, id))) + } + + @Test + fun `a new read starts from an empty loading state`() { + val failed = reduce(loaded, Event.LoadFailed) + assertTrue(failed.failed) + assertFalse(failed.loading) + + assertEquals(State(), reduce(failed, Event.LoadStarted)) + } + + @Test + fun `a failed read keeps the users it had`() { + val failed = reduce(loaded, Event.LoadFailed) + assertEquals(loaded.users, failed.users) + } + + @Test + fun `pending marks a row until it succeeds or fails`() { + val id = silenced.userProfile.userId!! + val pending = reduce(loaded, Event.RestrictionPending(id)) + assertEquals(setOf(id), pending.pending) + + val failed = reduce(pending, Event.RestrictionFailed(id)) + assertTrue(failed.pending.isEmpty()) + assertEquals(loaded.users, failed.users) + } + + @Test + fun `unsilenced drops a silenced row and strips the silence from a removed one`() { + val afterSilenced = reduce(loaded, Event.Unsilenced(silenced.userProfile.userId!!)) + assertEquals(listOf(removed, both), afterSilenced.users) + + val afterBoth = reduce(afterSilenced, Event.Unsilenced(both.userProfile.userId!!)) + assertEquals(listOf(removed, both.copy(silence = null)), afterBoth.users) + } + + @Test + fun `unbanned drops a removed row and moves a silenced one back to silenced`() { + val afterRemoved = reduce(loaded, Event.Unbanned(removed.userProfile.userId!!)) + assertEquals(listOf(silenced, both), afterRemoved.users) + + val afterBoth = reduce(afterRemoved, Event.Unbanned(both.userProfile.userId!!)) + assertEquals(listOf(silenced, both.copy(ban = null)), afterBoth.rows(RestrictedKind.Silenced)) + assertTrue(afterBoth.rows(RestrictedKind.Removed).isEmpty()) + } +} diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModelTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModelTest.kt index 91f9cf0a1a..f973718cc2 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModelTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModelTest.kt @@ -18,6 +18,7 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import com.flipcash.libs.coroutines.TestDispatcherProvider import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain @@ -42,6 +43,8 @@ class RestrictedMembersViewModelTest { private val toasts = mockk(relaxed = true) private val restrictionWrites = GroupRestrictionWrites() private val chatId = ChatId(ByteArray(32) { 3 }.toList()) + private val main = UnconfinedTestDispatcher() + private val dispatchers = TestDispatcherProvider(main) private val soon = SilenceState.Until(Clock.System.now() + 1.hours) private val lapsed = SilenceState.Until(Clock.System.now() - 1.hours) @@ -64,7 +67,7 @@ class RestrictedMembersViewModelTest { @Before fun setUp() { - Dispatchers.setMain(UnconfinedTestDispatcher()) + Dispatchers.setMain(main) BottomBarManager.clear() } @@ -77,12 +80,12 @@ class RestrictedMembersViewModelTest { private fun loaded(vararg users: RestrictedUser): RestrictedMembersViewModel { coEvery { chatController.getRestrictedUsers(chatId, any()) } returns Result.success(RestrictedUsersPage(users.toList(), null, false)) - return RestrictedMembersViewModel(chatController, resources, toasts, restrictionWrites).also { it.load(chatId) } + return RestrictedMembersViewModel(chatController, resources, toasts, restrictionWrites, dispatchers).also { it.dispatchEvent(RestrictedMembersViewModel.Event.Load(chatId)) } } @Test fun `silenced holds active silences with no ban, removed holds every ban`() { - val state = loaded(silencedOnly, silencedForever, lapsedOnly, removed, removedAndSilenced).state.value + val state = loaded(silencedOnly, silencedForever, lapsedOnly, removed, removedAndSilenced).stateFlow.value assertEquals(listOf(silencedOnly, silencedForever), state.rows(RestrictedKind.Silenced)) assertEquals(listOf(removed, removedAndSilenced), state.rows(RestrictedKind.Removed)) @@ -96,15 +99,15 @@ class RestrictedMembersViewModelTest { coEvery { chatController.getRestrictedUsers(chatId, QueryOptions(token = listOf(9))) } returns Result.success(RestrictedUsersPage(listOf(removed), null, false)) - val vm = RestrictedMembersViewModel(chatController, resources, toasts, restrictionWrites).also { it.load(chatId) } + val vm = RestrictedMembersViewModel(chatController, resources, toasts, restrictionWrites, dispatchers).also { it.dispatchEvent(RestrictedMembersViewModel.Event.Load(chatId)) } - assertEquals(listOf(silencedOnly, removed), vm.state.value.users) + assertEquals(listOf(silencedOnly, removed), vm.stateFlow.value.users) } @Test fun `loading the same chat again does not read twice`() { val vm = loaded(silencedOnly) - vm.load(chatId) + vm.dispatchEvent(RestrictedMembersViewModel.Event.Load(chatId)) coVerify(exactly = 1) { chatController.getRestrictedUsers(chatId, any()) } } @@ -112,15 +115,15 @@ class RestrictedMembersViewModelTest { @Test fun `a failed read is flagged and can be retried`() { coEvery { chatController.getRestrictedUsers(chatId, any()) } returns Result.failure(Throwable("down")) - val vm = RestrictedMembersViewModel(chatController, resources, toasts, restrictionWrites).also { it.load(chatId) } + val vm = RestrictedMembersViewModel(chatController, resources, toasts, restrictionWrites, dispatchers).also { it.dispatchEvent(RestrictedMembersViewModel.Event.Load(chatId)) } - assertTrue(vm.state.value.failed) - assertFalse(vm.state.value.loading) + assertTrue(vm.stateFlow.value.failed) + assertFalse(vm.stateFlow.value.loading) coEvery { chatController.getRestrictedUsers(chatId, any()) } returns Result.success(RestrictedUsersPage(listOf(removed), null, false)) - vm.load(chatId) - assertEquals(listOf(removed), vm.state.value.users) + vm.dispatchEvent(RestrictedMembersViewModel.Event.Load(chatId)) + assertEquals(listOf(removed), vm.stateFlow.value.users) } @Test @@ -128,10 +131,10 @@ class RestrictedMembersViewModelTest { coEvery { chatController.unsilenceUser(chatId, any()) } returns Result.success(Unit) val vm = loaded(silencedOnly, silencedForever) - vm.unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada") + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada")) - assertEquals(listOf(silencedForever), vm.state.value.rows(RestrictedKind.Silenced)) - assertTrue(vm.state.value.pending.isEmpty()) + assertEquals(listOf(silencedForever), vm.stateFlow.value.rows(RestrictedKind.Silenced)) + assertTrue(vm.stateFlow.value.pending.isEmpty()) coVerify(exactly = 1) { chatController.getRestrictedUsers(chatId, any()) } } @@ -140,9 +143,9 @@ class RestrictedMembersViewModelTest { coEvery { chatController.unsilenceUser(chatId, any()) } returns Result.success(Unit) val vm = loaded(removedAndSilenced) - vm.unsilence(chatId, removedAndSilenced.userProfile.userId!!, "Ada") + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unsilence(chatId, removedAndSilenced.userProfile.userId!!, "Ada")) - assertEquals(listOf(removedAndSilenced.copy(silence = null)), vm.state.value.rows(RestrictedKind.Removed)) + assertEquals(listOf(removedAndSilenced.copy(silence = null)), vm.stateFlow.value.rows(RestrictedKind.Removed)) } @Test @@ -150,11 +153,11 @@ class RestrictedMembersViewModelTest { coEvery { chatController.unbanUser(chatId, any()) } returns Result.success(Unit) val vm = loaded(removed, removedAndSilenced) - vm.unban(chatId, removed.userProfile.userId!!) - vm.unban(chatId, removedAndSilenced.userProfile.userId!!) + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unban(chatId, removed.userProfile.userId!!)) + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unban(chatId, removedAndSilenced.userProfile.userId!!)) - assertTrue(vm.state.value.rows(RestrictedKind.Removed).isEmpty()) - assertEquals(listOf(removedAndSilenced.copy(ban = null)), vm.state.value.rows(RestrictedKind.Silenced)) + assertTrue(vm.stateFlow.value.rows(RestrictedKind.Removed).isEmpty()) + assertEquals(listOf(removedAndSilenced.copy(ban = null)), vm.stateFlow.value.rows(RestrictedKind.Silenced)) } @Test @@ -162,10 +165,10 @@ class RestrictedMembersViewModelTest { coEvery { chatController.unsilenceUser(chatId, any()) } returns Result.failure(Throwable("denied")) val vm = loaded(silencedOnly) - vm.unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada") + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada")) - assertEquals(listOf(silencedOnly), vm.state.value.rows(RestrictedKind.Silenced)) - assertTrue(vm.state.value.pending.isEmpty()) + assertEquals(listOf(silencedOnly), vm.stateFlow.value.rows(RestrictedKind.Silenced)) + assertTrue(vm.stateFlow.value.pending.isEmpty()) } @Test @@ -173,7 +176,7 @@ class RestrictedMembersViewModelTest { coEvery { chatController.unsilenceUser(chatId, any()) } returns Result.success(Unit) val vm = loaded(silencedOnly) - vm.unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada") + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada")) verify(exactly = 1) { toasts.showToast(R.string.toast_unsilenced, "Ada", replacePrevious = true) } } @@ -183,7 +186,7 @@ class RestrictedMembersViewModelTest { coEvery { chatController.unsilenceUser(chatId, any()) } returns Result.failure(Throwable("denied")) val vm = loaded(silencedOnly) - vm.unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada") + vm.dispatchEvent(RestrictedMembersViewModel.Event.Unsilence(chatId, silencedOnly.userProfile.userId!!, "Ada")) verify(exactly = 0) { toasts.showToast(any(), *anyVararg(), replacePrevious = any()) } } From 5088aa8cd3a49cda6d0f28f229c6e6828576cf4c Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 6 Nov 2026 14:43:30 -0500 Subject: [PATCH 2/3] refactor(messenger): move ReportViewModel onto BaseViewModel Submit becomes an Event; Sending, Sent and SendFailed carry the button state through updateStateForEvent. Sending is dispatched before the send launches, so a second Submit still sees a non-idle button and is dropped, as before. confirmed stays a buffered Channel: it fires from a bottom bar onDismiss that can outlive the collecting composition, and eventFlow has no buffer. --- .../app/messenger/ReportFlowScreen.kt | 8 +-- .../app/messenger/internal/ReportViewModel.kt | 67 +++++++++++++------ .../app/messenger/internal/ReportStateTest.kt | 35 ++++++++++ .../messenger/internal/ReportViewModelTest.kt | 49 +++++++------- 4 files changed, 112 insertions(+), 47 deletions(-) create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportStateTest.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt index d505631b32..0dbe9b1801 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt @@ -98,7 +98,7 @@ private fun reportEntryProvider( private fun FlowReasonSelection(subject: ReportSubject) { val viewModel = flowSharedViewModel() val flowNavigator = rememberFlowNavigator() - val progress by viewModel.state.collectAsStateWithLifecycle() + val progress by viewModel.stateFlow.collectAsStateWithLifecycle() ExitOnceConfirmed(viewModel, flowNavigator) @@ -110,7 +110,7 @@ private fun FlowReasonSelection(subject: ReportSubject) { } else { // Every other reason is the whole report, so the pick is the submit. Asking for // more after someone has already said what is wrong would be asking twice. - viewModel.submit(subject, reason, details = null) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, reason, details = null)) } }, // `back` on its own: at the flow's root it exits the flow itself, so pairing it with an @@ -127,14 +127,14 @@ private fun FlowReportDetails(subject: ReportSubject, reason: ReportReason) { // typed — leaving by the back arrow is usually a second thought about the reason, not about // the words. val details = rememberTextFieldState() - val progress by viewModel.state.collectAsStateWithLifecycle() + val progress by viewModel.stateFlow.collectAsStateWithLifecycle() ExitOnceConfirmed(viewModel, flowNavigator) ReportDetailsContent( state = details, progress = progress, - onSubmit = { typed -> viewModel.submit(subject, reason, typed) }, + onSubmit = { typed -> viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, reason, typed)) }, // Not the flow's root, so this steps back to the reasons rather than leaving. onNavigateUp = { flowNavigator.back() }, ) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt index fc707f98b6..515e93355a 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt @@ -1,16 +1,17 @@ package com.flipcash.app.messenger.internal -import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.flipcash.app.core.chat.ReportSubject import com.flipcash.features.messenger.R import com.flipcash.reporting.ReportDescription import com.flipcash.reporting.ReportReason import com.flipcash.services.controllers.ReportingController +import com.flipcash.libs.coroutines.DispatcherProvider import com.flipcash.services.models.ReportTarget import com.getcode.manager.BottomBarManager import com.getcode.util.resources.ResourceHelper import com.getcode.utils.trace +import com.getcode.view.BaseViewModel import com.getcode.view.LoadingSuccessState import com.getcode.view.MinimumLoadingDuration import com.getcode.view.SuccessHoldDuration @@ -19,9 +20,9 @@ import kotlinx.coroutines.async import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.filterIsInstance +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import javax.inject.Inject @@ -43,18 +44,26 @@ import javax.inject.Inject internal class ReportViewModel @Inject constructor( private val reporting: ReportingController, private val resources: ResourceHelper, -) : ViewModel() { + dispatchers: DispatcherProvider, +) : BaseViewModel( + // What the submit button is showing: the send in flight, then that it landed. + // + // It stays on [LoadingSuccessState.success] rather than returning to idle, because the flow is + // still on screen behind the confirmation and a button back to "Submit Report" would be + // offering to file the same report again. + initialState = LoadingSuccessState(), + updateStateForEvent = updateStateForEvent, + defaultDispatcher = dispatchers.Default, +) { - private val _state = MutableStateFlow(LoadingSuccessState()) + sealed interface Event { + /** [details] is the reporter's own words, and only [ReportReason.Other] collects them. */ + data class Submit(val subject: ReportSubject, val reason: ReportReason, val details: String?) : Event - /** - * What the submit button is showing: the send in flight, then that it landed. - * - * It stays on [LoadingSuccessState.success] rather than returning to idle, because the flow is - * still on screen behind the confirmation and a button back to "Submit Report" would be - * offering to file the same report again. - */ - val state: StateFlow = _state.asStateFlow() + data object Sending : Event + data object Sent : Event + data object SendFailed : Event + } // Buffered rather than a shared flow: this fires once, from a bottom bar callback that can // outlive the composition collecting it, and losing it would strand the flow open. @@ -68,11 +77,17 @@ internal class ReportViewModel @Inject constructor( */ val confirmed: Flow = _confirmed.receiveAsFlow() - /** [details] is the reporter's own words, and only [ReportReason.Other] collects them. */ - fun submit(subject: ReportSubject, reason: ReportReason, details: String?) { + init { + eventFlow + .filterIsInstance() + .onEach { submit(it.subject, it.reason, it.details) } + .launchIn(viewModelScope) + } + + private fun submit(subject: ReportSubject, reason: ReportReason, details: String?) { // Covers a second press both while the first is in flight and after it has succeeded — // either would file the same report twice and answer with two confirmations. - if (!_state.value.isIdle) return + if (!stateFlow.value.isIdle) return val target = when (subject) { is ReportSubject.User -> ReportTarget.User(subject.userId) @@ -83,8 +98,9 @@ internal class ReportViewModel @Inject constructor( ) } + // Set before the launch, so a second Submit queued behind this one sees it. + dispatchEvent(Event.Sending) viewModelScope.launch { - _state.value = LoadingSuccessState(loading = true) // A report is a single unary call and usually answers inside a frame, so the button // crossfaded from its label straight to a checkmark and the send read as never having @@ -98,7 +114,7 @@ internal class ReportViewModel @Inject constructor( sent.await() .onSuccess { floor.join() - _state.value = LoadingSuccessState(success = true) + dispatchEvent(Event.Sent) // The checkmark is drawn on the frame after this is set, so the confirmation // waits out the same hold every other confirm button in the app does. Raising // it immediately would put a scrim over a checkmark nobody had seen yet. @@ -122,7 +138,7 @@ internal class ReportViewModel @Inject constructor( // Back to idle, not to an error state: the flow stays where it is with the // reason still picked, so the retry is one press on a button that has to be // pressable to take it. - _state.value = LoadingSuccessState() + dispatchEvent(Event.SendFailed) BottomBarManager.showError( title = resources.getString(R.string.error_title_failedToReport), message = resources.getString(R.string.error_description_failedToReport), @@ -130,4 +146,15 @@ internal class ReportViewModel @Inject constructor( } } } + + companion object { + val updateStateForEvent: (Event) -> ((LoadingSuccessState) -> LoadingSuccessState) = { event -> + when (event) { + is Event.Submit -> { state -> state } + Event.Sending -> { _ -> LoadingSuccessState(loading = true) } + Event.Sent -> { _ -> LoadingSuccessState(success = true) } + Event.SendFailed -> { _ -> LoadingSuccessState() } + } + } + } } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportStateTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportStateTest.kt new file mode 100644 index 0000000000..2038a6d883 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportStateTest.kt @@ -0,0 +1,35 @@ +package com.flipcash.app.messenger.internal + +import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.app.messenger.internal.ReportViewModel.Event +import com.flipcash.reporting.ReportReason +import com.getcode.view.LoadingSuccessState +import kotlin.test.Test +import kotlin.test.assertEquals + +class ReportStateTest { + + private fun reduce(state: LoadingSuccessState, event: Event) = ReportViewModel.updateStateForEvent(event)(state) + + private val submit = Event.Submit(ReportSubject.User(listOf(1)), ReportReason.Spam, details = null) + + @Test + fun `submit leaves the button alone until the send starts`() { + val sent = LoadingSuccessState(success = true) + assertEquals(LoadingSuccessState(), reduce(LoadingSuccessState(), submit)) + assertEquals(sent, reduce(sent, submit)) + } + + @Test + fun `a send moves the button through loading to success`() { + val sending = reduce(LoadingSuccessState(), Event.Sending) + assertEquals(LoadingSuccessState(loading = true), sending) + assertEquals(LoadingSuccessState(success = true), reduce(sending, Event.Sent)) + } + + @Test + fun `a failed send returns the button to idle so it can be pressed again`() { + val sending = reduce(LoadingSuccessState(), Event.Sending) + assertEquals(LoadingSuccessState(), reduce(sending, Event.SendFailed)) + } +} diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt index 172387fc88..9d2fe88f74 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt @@ -2,6 +2,7 @@ package com.flipcash.app.messenger.internal import com.flipcash.app.core.chat.ReportSubject import com.flipcash.reporting.ReportReason +import com.flipcash.libs.coroutines.TestDispatcherProvider import com.flipcash.services.controllers.ReportingController import com.getcode.manager.BottomBarManager import com.getcode.manager.SelectedBottomBarAction @@ -47,9 +48,11 @@ class ReportViewModelTest { // the test advances. A dispatcher on its own scheduler would make that delay real time. private val scheduler = TestCoroutineScheduler() + private val main = UnconfinedTestDispatcher(scheduler) + @Before fun setUp() { - Dispatchers.setMain(UnconfinedTestDispatcher(scheduler)) + Dispatchers.setMain(main) BottomBarManager.clear() } @@ -59,13 +62,13 @@ class ReportViewModelTest { BottomBarManager.clear() } - private fun viewModel() = ReportViewModel(reporting, resources) + private fun viewModel() = ReportViewModel(reporting, resources, TestDispatcherProvider(main)) @Test fun `a sent report is confirmed, not mentioned in passing`() = runTest(scheduler) { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) - viewModel().submit(subject, ReportReason.Spam, details = null) + viewModel().dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) advanceUntilIdle() val shown = BottomBarManager.messages.value.last() @@ -79,13 +82,13 @@ class ReportViewModelTest { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) val model = viewModel() - model.submit(subject, ReportReason.Spam, details = null) + model.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) - assertTrue(model.state.value.loading) + assertTrue(model.stateFlow.value.loading) advanceTimeBy(MinimumLoadingDuration.inWholeMilliseconds - 1) - assertTrue(model.state.value.loading) + assertTrue(model.stateFlow.value.loading) advanceTimeBy(2) - assertTrue(model.state.value.success) + assertTrue(model.stateFlow.value.success) } @Test @@ -96,14 +99,14 @@ class ReportViewModelTest { coEvery { reporting.report(any(), any()) } coAnswers { answer.await() } val model = viewModel() - model.submit(subject, ReportReason.Spam, details = null) + model.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) advanceTimeBy(MinimumLoadingDuration.inWholeMilliseconds * 2) - assertTrue(model.state.value.loading) + assertTrue(model.stateFlow.value.loading) answer.complete(Result.success(Unit)) advanceTimeBy(1) - assertTrue(model.state.value.success) + assertTrue(model.stateFlow.value.success) } @Test @@ -111,12 +114,12 @@ class ReportViewModelTest { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) val viewModel = viewModel() - viewModel.submit(subject, ReportReason.Spam, details = null) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) // Past the spinner's floor, which is where the checkmark's own hold starts. // `advanceTimeBy` stops short of the endpoint, so the floor's own instant needs the +1. advanceTimeBy(MinimumLoadingDuration.inWholeMilliseconds + 1) - assertTrue("the button never showed the send succeeded", viewModel.state.value.success) + assertTrue("the button never showed the send succeeded", viewModel.stateFlow.value.success) assertTrue( "the confirmation covered the checkmark before it could be read", BottomBarManager.messages.value.isEmpty(), @@ -135,12 +138,12 @@ class ReportViewModelTest { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) val viewModel = viewModel() - viewModel.submit(subject, ReportReason.Spam, details = null) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) advanceUntilIdle() assertTrue( "the button went back to inviting a second report of the same thing", - viewModel.state.value.success, + viewModel.stateFlow.value.success, ) } @@ -154,7 +157,7 @@ class ReportViewModelTest { viewModel.confirmed.collect { closed = true } } - viewModel.submit(subject, ReportReason.Spam, details = null) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) advanceUntilIdle() assertFalse("the flow closed before anyone saw the confirmation", closed) @@ -174,7 +177,7 @@ class ReportViewModelTest { viewModel.confirmed.collect { closed = true } } - viewModel.submit(subject, ReportReason.Spam, details = null) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) advanceUntilIdle() assertEquals( @@ -182,7 +185,7 @@ class ReportViewModelTest { BottomBarManager.messages.value.last().type, ) assertFalse("a failed report closed the flow", closed) - assertTrue("the retry was left disabled", viewModel.state.value.isIdle) + assertTrue("the retry was left disabled", viewModel.stateFlow.value.isIdle) collector.cancel() } @@ -193,14 +196,14 @@ class ReportViewModelTest { coEvery { reporting.report(any(), any()) } coAnswers { inFlight.await() } val viewModel = viewModel() - assertTrue(viewModel.state.value.isIdle) + assertTrue(viewModel.stateFlow.value.isIdle) - viewModel.submit(subject, ReportReason.Spam, details = null) - assertTrue("the button gives no sign the report is being sent", viewModel.state.value.loading) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) + assertTrue("the button gives no sign the report is being sent", viewModel.stateFlow.value.loading) inFlight.complete(Result.success(Unit)) advanceUntilIdle() - assertFalse("the button was still spinning after the report landed", viewModel.state.value.loading) + assertFalse("the button was still spinning after the report landed", viewModel.stateFlow.value.loading) } @Test @@ -209,8 +212,8 @@ class ReportViewModelTest { coEvery { reporting.report(any(), any()) } coAnswers { inFlight.await() } val viewModel = viewModel() - viewModel.submit(subject, ReportReason.Spam, details = null) - viewModel.submit(subject, ReportReason.Spam, details = null) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) + viewModel.dispatchEvent(ReportViewModel.Event.Submit(subject, ReportReason.Spam, details = null)) inFlight.complete(Result.success(Unit)) advanceUntilIdle() From 104ae274d79b915588200d220c5b4ecf601e3f56 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 6 Nov 2026 15:14:27 -0500 Subject: [PATCH 3/3] refactor(messenger): keep RestrictedMembers' loaded chat in State loadedFor was the last private var on the view model. It is now State.loadedFor, set by LoadStarted(chatId) and cleared by LoadFailed, so the repeat-open guard reads stateFlow and the reducer owns every change to it. --- .../internal/RestrictedMembersViewModel.kt | 21 ++++++++++--------- .../internal/RestrictedMembersStateTest.kt | 11 ++++++++-- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt index 95165ae9a2..df58a788ed 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersViewModel.kt @@ -49,6 +49,11 @@ internal class RestrictedMembersViewModel @Inject constructor( val users: List = emptyList(), /** Users whose Unsilence or Unban is in flight. */ val pending: Set = emptySet(), + /** + * The chat the last read was for. A repeat [Event.Load] for it is dropped, since the flow's + * back stack reopens this screen; a failed read clears it so the next open retries. + */ + val loadedFor: ChatId? = null, ) { fun rows(kind: RestrictedKind): List = when (kind) { RestrictedKind.Silenced -> users.filter { it.ban == null && it.silence.isActiveAt() } @@ -66,8 +71,8 @@ internal class RestrictedMembersViewModel @Inject constructor( /** Lift [userId]'s ban. */ data class Unban(val chatId: ChatId, val userId: ID) : Event - /** A new read started; drops whatever the last one showed. */ - data object LoadStarted : Event + /** A new read for [chatId] started; drops whatever the last one showed. */ + data class LoadStarted(val chatId: ChatId) : Event data class Loaded(val users: List) : Event data object LoadFailed : Event @@ -77,8 +82,6 @@ internal class RestrictedMembersViewModel @Inject constructor( data class RestrictionFailed(val userId: ID) : Event } - private var loadedFor: ChatId? = null - init { // One collector, so a second tap on a row sees the first one's pending entry. eventFlow @@ -110,16 +113,14 @@ internal class RestrictedMembersViewModel @Inject constructor( } private fun load(chatId: ChatId) { - if (loadedFor == chatId) return - loadedFor = chatId - dispatchEvent(Event.LoadStarted) + if (stateFlow.value.loadedFor == chatId) return + dispatchEvent(Event.LoadStarted(chatId)) viewModelScope.launch { val all = mutableListOf() var options = QueryOptions() while (true) { val page = chatController.getRestrictedUsers(chatId, options).getOrElse { trace("failed to load restricted users - ${it.localizedMessage}") - loadedFor = null dispatchEvent(Event.LoadFailed) BottomBarManager.showError( title = resources.getString(R.string.error_title_failedToLoadRestricted), @@ -169,9 +170,9 @@ internal class RestrictedMembersViewModel @Inject constructor( is Event.Unsilence, is Event.Unban -> { state -> state } - Event.LoadStarted -> { _ -> State() } + is Event.LoadStarted -> { _ -> State(loadedFor = event.chatId) } is Event.Loaded -> { state -> state.copy(loading = false, users = event.users) } - Event.LoadFailed -> { state -> state.copy(loading = false, failed = true) } + Event.LoadFailed -> { state -> state.copy(loading = false, failed = true, loadedFor = null) } is Event.RestrictionPending -> { state -> state.copy(pending = state.pending + setOf(event.userId)) } is Event.RestrictionFailed -> { state -> state.copy(pending = state.pending - setOf(event.userId)) } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt index f4c2d863b4..effed297c2 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RestrictedMembersStateTest.kt @@ -43,12 +43,19 @@ class RestrictedMembersStateTest { } @Test - fun `a new read starts from an empty loading state`() { + fun `a new read starts from an empty loading state for its chat`() { val failed = reduce(loaded, Event.LoadFailed) assertTrue(failed.failed) assertFalse(failed.loading) - assertEquals(State(), reduce(failed, Event.LoadStarted)) + assertEquals(State(loadedFor = chatId), reduce(failed, Event.LoadStarted(chatId))) + } + + @Test + fun `a failed read forgets its chat so the next open retries`() { + val started = reduce(State(), Event.LoadStarted(chatId)) + assertEquals(chatId, started.loadedFor) + assertEquals(null, reduce(started, Event.LoadFailed).loadedFor) } @Test