From e3b35e82695925b2acab9de269c601ad8344bdeb Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 27 Aug 2026 18:00:46 -0400 Subject: [PATCH 1/2] feat(user-profile): gate Save on a real edit and discard it on back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name and username steps seeded their field from the stored profile and then enabled the confirm button on any non-blank text, so opening either one armed Save before the user had changed anything — a tap that submitted the value that was already there. Both now keep the loaded value as a baseline in state and compare the field against it, so one character's difference is what arms the button and typing the original back in disarms it. Nodes 9553:113166 and 9553:113168. The seeding flows also lacked distinctUntilChanged, so any unrelated emission from the profile re-seeded the field and overwrote in-progress typing. Leaving a step throws the edit away. The back icon dispatches DiscardChanges, which puts the baseline back in the field; a gesture back reaches the same outcome because each step's ViewModel is scoped to its nav entry and goes with the pop. The photo step needed more than that: an abandoned pick left a re-encoded file in the cache with nothing to delete it, so onCleared drops it, covering the gesture back and the successful upload alike. --- .../internal/name/NameEntryScreen.kt | 6 +- .../internal/name/NameEntryViewModel.kt | 38 ++++++-- .../internal/photo/PhotoSelectionScreen.kt | 3 + .../internal/photo/PhotoSelectionViewModel.kt | 28 +++++- .../internal/username/UsernameEntryScreen.kt | 6 +- .../username/UsernameEntryViewModel.kt | 40 +++++++-- .../internal/name/NameEntryStateTest.kt | 88 +++++++++++++++++++ .../username/UsernameEntryStateTest.kt | 80 +++++++++++++++++ 8 files changed, 275 insertions(+), 14 deletions(-) create mode 100644 apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt create mode 100644 apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryScreen.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryScreen.kt index 8e3abd7cfd..168031b3d2 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryScreen.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryScreen.kt @@ -62,6 +62,10 @@ internal fun NameEntryScreen( AppBarWithTitle( onBackIconClicked = { keyboard.hideIfVisible { + // Leaving throws the edit away rather than carrying it back in. A gesture + // back lands in the same place by a different route: the step's ViewModel + // is scoped to its nav entry, so popping the entry drops the field with it. + viewModel.dispatchEvent(NameEntryViewModel.Event.DiscardChanges) flowNavigator.back() } }, @@ -113,7 +117,7 @@ private fun NameEntryScreenContent( bottom = CodeTheme.dimens.grid.x3 ).imePadding(), text = stringResource(R.string.action_next), - enabled = state.hasName && state.processingState.isIdle, + enabled = state.hasName && state.isChanged && state.processingState.isIdle, isLoading = state.processingState.loading, isSuccess = state.processingState.success, onClick = { diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt index 1b9844882c..9b4d9e0523 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt @@ -21,6 +21,7 @@ import com.getcode.view.BaseViewModel import com.getcode.view.LoadingSuccessState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map @@ -43,11 +44,20 @@ class NameEntryViewModel @Inject constructor( ) { data class State( val nameFieldState: TextFieldState = TextFieldState(), + /** + * The stored display name: what the field is seeded with, what an edit is measured + * against, and what a discarded edit reverts to. + */ + val savedName: String = "", val attestation: ModerationResult.Attestation = ModerationResult.Attestation.Empty, val processingState: LoadingSuccessState = LoadingSuccessState(), ) { val hasName: Boolean get() = nameFieldState.text.isNotBlank() + + /** Node 9553:113166 — one character's difference is enough to arm the confirm button. */ + val isChanged: Boolean + get() = nameFieldState.text.toString() != savedName } sealed interface Event { @@ -57,16 +67,32 @@ class NameEntryViewModel @Inject constructor( val success: Boolean = false ) : Event + /** The stored name arrived (or changed) — the field and the baseline follow it. */ + data class OnSavedNameLoaded(val name: String) : Event + + /** Back was pressed with an uncommitted edit: put [State.savedName] back in the field. */ + data object DiscardChanges : Event + data object OnNameApproved : Event } init { userManager.state .mapNotNull { it.userProfile } - .map { profile -> profile.displayName } + .map { profile -> profile.displayName.orEmpty() } + // Without this, any unrelated emission from the profile re-seeds the field and + // overwrites whatever the user is part-way through typing. + .distinctUntilChanged() .onEach { name -> - val inputState = stateFlow.value.nameFieldState - inputState.setTextAndPlaceCursorAtEnd(name.orEmpty()) + dispatchEvent(Event.OnSavedNameLoaded(name)) + stateFlow.value.nameFieldState.setTextAndPlaceCursorAtEnd(name) + }.launchIn(viewModelScope) + + eventFlow + .filterIsInstance() + .onEach { + val state = stateFlow.value + state.nameFieldState.setTextAndPlaceCursorAtEnd(state.savedName) }.launchIn(viewModelScope) eventFlow @@ -157,10 +183,12 @@ class NameEntryViewModel @Inject constructor( } } } - companion object { - private val updateStateForEvent: (Event) -> (State.() -> State) = { event -> + internal companion object { + val updateStateForEvent: (Event) -> (State.() -> State) = { event -> when (event) { is Event.CheckName -> { state -> state } + is Event.OnSavedNameLoaded -> { state -> state.copy(savedName = event.name) } + Event.DiscardChanges -> { state -> state } is Event.UpdateProcessingState -> { state -> val current = state.processingState state.copy( diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt index 665d4b68e1..846c41e9d3 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt @@ -70,6 +70,9 @@ internal fun PhotoSelectionScreen() { titleAlignment = Alignment.CenterHorizontally, onBackIconClicked = { keyboard.hideIfVisible { + // Leaving throws the pick away rather than carrying it back in; the stored + // picture is whatever it was before the step opened. + viewModel.dispatchEvent(PhotoSelectionViewModel.Event.DiscardChanges) flowNavigator.back() } }, diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt index 0d8a2cb881..0443d81418 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt @@ -76,6 +76,9 @@ class PhotoSelectionViewModel @Inject constructor( ) : Event data object OnImageApproved : Event + + /** Back was pressed with an unsaved pick: drop it, leaving the stored picture as it was. */ + data object DiscardChanges : Event data class OnImageSelected(val image: Uri) : Event data class OnImageCached(val image: Uri, val mimeType: String) : Event data object OnImageCleared : Event @@ -158,12 +161,32 @@ class PhotoSelectionViewModel @Inject constructor( }, onError = { cause -> dispatchEvent(Event.UpdateProcessingState()) - stateFlow.value.image.dataOrNull?.let { contentReader.removeFromCache(it) } - dispatchEvent(Event.OnImageCleared) + discardPendingImage() handleUploadFailure(cause) } ) .launchIn(viewModelScope) + + eventFlow + .filterIsInstance() + .onEach { discardPendingImage() } + .launchIn(viewModelScope) + } + + /** + * A pick that never reached the server is only a re-encoded file in the cache, so leaving the + * step has to delete it — nothing else ever will. Covers the paths the back button doesn't: + * a gesture back, and the successful upload that makes the local copy redundant. + */ + override fun onCleared() { + discardPendingImage() + super.onCleared() + } + + private fun discardPendingImage() { + val pending = stateFlow.value.image.dataOrNull ?: return + contentReader.removeFromCache(pending) + dispatchEvent(Event.OnImageCleared) } /** Clears the pending selection and surfaces [title]/[message] to the user. */ @@ -364,6 +387,7 @@ class PhotoSelectionViewModel @Inject constructor( } Event.OnImageApproved -> { state -> state } + Event.DiscardChanges -> { state -> state } is Event.OnImageCached -> { state -> state.copy(image = Loadable.Loaded(event.image), imageMimeType = event.mimeType) } diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryScreen.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryScreen.kt index 9d2f567b9a..f3ba8de84c 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryScreen.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryScreen.kt @@ -58,6 +58,10 @@ internal fun UsernameEntryScreen() { AppBarWithTitle( onBackIconClicked = { keyboard.hideIfVisible { + // Leaving throws the edit away rather than carrying it back in. A gesture back + // lands in the same place by a different route: the step's ViewModel is scoped + // to its nav entry, so popping the entry drops the field with it. + viewModel.dispatchEvent(UsernameEntryViewModel.Event.DiscardChanges) flowNavigator.back() } }, @@ -105,7 +109,7 @@ private fun UsernameEntryScreenContent( bottom = CodeTheme.dimens.grid.x3 ).imePadding(), text = stringResource(R.string.action_next), - enabled = state.hasUsername && state.processingState.isIdle, + enabled = state.hasUsername && state.isChanged && state.processingState.isIdle, isLoading = state.processingState.loading, isSuccess = state.processingState.success, onClick = { diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt index c3bb936322..bbdec66ada 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt @@ -20,6 +20,7 @@ import com.getcode.view.BaseViewModel import com.getcode.view.LoadingSuccessState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map @@ -54,10 +55,19 @@ class UsernameEntryViewModel @Inject constructor( ) { data class State( val usernameFieldState: TextFieldState = TextFieldState(), + /** + * The claimed handle, or empty before a first claim: what the field is seeded with, what + * an edit is measured against, and what a discarded edit reverts to. + */ + val savedUsername: String = "", val processingState: LoadingSuccessState = LoadingSuccessState(), ) { val hasUsername: Boolean get() = usernameFieldState.text.isNotBlank() + + /** Node 9553:113168 — one character's difference is enough to arm the confirm button. */ + val isChanged: Boolean + get() = usernameFieldState.text.toString() != savedUsername } sealed interface Event { @@ -67,16 +77,32 @@ class UsernameEntryViewModel @Inject constructor( val success: Boolean = false ) : Event + /** The claimed handle arrived (or changed) — the field and the baseline follow it. */ + data class OnSavedUsernameLoaded(val username: String) : Event + + /** Back was pressed with an uncommitted edit: put [State.savedUsername] back in the field. */ + data object DiscardChanges : Event + data object OnUsernameApproved : Event } init { userManager.state .mapNotNull { it.userProfile } - .map { profile -> profile.username } + .map { profile -> profile.username.orEmpty() } + // Without this, any unrelated emission from the profile re-seeds the field and + // overwrites whatever the user is part-way through typing. + .distinctUntilChanged() .onEach { username -> - val inputState = stateFlow.value.usernameFieldState - inputState.setTextAndPlaceCursorAtEnd(username.orEmpty()) + dispatchEvent(Event.OnSavedUsernameLoaded(username)) + stateFlow.value.usernameFieldState.setTextAndPlaceCursorAtEnd(username) + }.launchIn(viewModelScope) + + eventFlow + .filterIsInstance() + .onEach { + val state = stateFlow.value + state.usernameFieldState.setTextAndPlaceCursorAtEnd(state.savedUsername) }.launchIn(viewModelScope) eventFlow @@ -188,10 +214,14 @@ class UsernameEntryViewModel @Inject constructor( R.string.error_description_profileNameNotAllowedFlaggedSpam } - companion object { - private val updateStateForEvent: (Event) -> (State.() -> State) = { event -> + internal companion object { + val updateStateForEvent: (Event) -> (State.() -> State) = { event -> when (event) { Event.CheckUsername -> { state -> state } + is Event.OnSavedUsernameLoaded -> { state -> + state.copy(savedUsername = event.username) + } + Event.DiscardChanges -> { state -> state } is Event.UpdateProcessingState -> { state -> val current = state.processingState state.copy( diff --git a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt new file mode 100644 index 0000000000..938043dfa2 --- /dev/null +++ b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt @@ -0,0 +1,88 @@ +package com.flipcash.app.userprofile.internal.name + +import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * The confirm button reads [NameEntryViewModel.State.isChanged], so these cover what arms it: + * a name that differs from the stored one, and nothing else. + */ +class NameEntryStateTest { + + private val reduce = NameEntryViewModel.updateStateForEvent + + private fun stateWith(saved: String, typed: String = saved): NameEntryViewModel.State { + val state = reduce(NameEntryViewModel.Event.OnSavedNameLoaded(saved))( + NameEntryViewModel.State() + ) + state.nameFieldState.setTextAndPlaceCursorAtEnd(typed) + return state + } + + @Test + fun `an empty field on a fresh account is unchanged`() { + val state = NameEntryViewModel.State() + + assertEquals("", state.savedName) + assertFalse(state.isChanged) + assertFalse(state.hasName) + } + + @Test + fun `the seeded name arrives unchanged`() { + val state = stateWith(saved = "Ada") + + assertTrue(state.hasName) + assertFalse(state.isChanged) + } + + @Test + fun `one character is enough to count as a change`() { + val state = stateWith(saved = "Ada", typed = "Adam") + + assertTrue(state.isChanged) + } + + @Test + fun `typing the stored name back in is not a change`() { + val state = stateWith(saved = "Ada", typed = "Adam") + state.nameFieldState.setTextAndPlaceCursorAtEnd("Ada") + + assertFalse(state.isChanged) + } + + @Test + fun `a first name on an account without one counts as a change`() { + val state = stateWith(saved = "", typed = "Ada") + + assertTrue(state.isChanged) + assertTrue(state.hasName) + } + + @Test + fun `clearing the field is a change the confirm button still refuses`() { + val state = stateWith(saved = "Ada", typed = "") + + assertTrue(state.isChanged) + assertFalse(state.hasName) + } + + @Test + fun `saving a new name moves the baseline`() { + val state = stateWith(saved = "Ada", typed = "Adam") + val saved = reduce(NameEntryViewModel.Event.OnSavedNameLoaded("Adam"))(state) + + assertEquals("Adam", saved.savedName) + assertFalse(saved.isChanged) + } + + @Test + fun `discarding leaves the state alone - the field reset is the view model's own`() { + val state = stateWith(saved = "Ada", typed = "Adam") + + assertEquals(state, reduce(NameEntryViewModel.Event.DiscardChanges)(state)) + } +} diff --git a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt new file mode 100644 index 0000000000..52e89ca2e8 --- /dev/null +++ b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt @@ -0,0 +1,80 @@ +package com.flipcash.app.userprofile.internal.username + +import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Same baseline rule as the name step, over the two cases this screen serves: a first claim, where + * the stored handle is empty, and a change, where it is not. + */ +class UsernameEntryStateTest { + + private val reduce = UsernameEntryViewModel.updateStateForEvent + + private fun stateWith(saved: String, typed: String = saved): UsernameEntryViewModel.State { + val state = reduce(UsernameEntryViewModel.Event.OnSavedUsernameLoaded(saved))( + UsernameEntryViewModel.State() + ) + state.usernameFieldState.setTextAndPlaceCursorAtEnd(typed) + return state + } + + @Test + fun `an unclaimed handle starts unchanged`() { + val state = UsernameEntryViewModel.State() + + assertEquals("", state.savedUsername) + assertFalse(state.isChanged) + assertFalse(state.hasUsername) + } + + @Test + fun `a first claim counts as a change`() { + val state = stateWith(saved = "", typed = "ada") + + assertTrue(state.isChanged) + assertTrue(state.hasUsername) + } + + @Test + fun `the claimed handle arrives unchanged`() { + val state = stateWith(saved = "ada") + + assertTrue(state.hasUsername) + assertFalse(state.isChanged) + } + + @Test + fun `one character is enough to count as a change`() { + val state = stateWith(saved = "ada", typed = "adam") + + assertTrue(state.isChanged) + } + + @Test + fun `typing the claimed handle back in is not a change`() { + val state = stateWith(saved = "ada", typed = "adam") + state.usernameFieldState.setTextAndPlaceCursorAtEnd("ada") + + assertFalse(state.isChanged) + } + + @Test + fun `claiming a new handle moves the baseline`() { + val state = stateWith(saved = "ada", typed = "adam") + val saved = reduce(UsernameEntryViewModel.Event.OnSavedUsernameLoaded("adam"))(state) + + assertEquals("adam", saved.savedUsername) + assertFalse(saved.isChanged) + } + + @Test + fun `discarding leaves the state alone - the field reset is the view model's own`() { + val state = stateWith(saved = "ada", typed = "adam") + + assertEquals(state, reduce(UsernameEntryViewModel.Event.DiscardChanges)(state)) + } +} From 528506d54434c11e8d98cf08ac84a0a163f80126 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 27 Aug 2026 18:42:57 -0400 Subject: [PATCH 2/2] feat(user-profile): open the photo step on the current avatar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Set Profile Picture step opened on an empty well with a "+", so an account that already has a picture looked like it had none. It now seeds from the stored profile picture and falls back to it when a pick is discarded. The seeded value is a server-side MediaItem, never a local Uri, so it cannot be mistaken for a pick: Save stays disabled until one is made. That gate moves into State.isChanged, matching the name and username steps. Also stops the name and username fields being clobbered mid-edit. Both seeded the field on every profile emission, and the 60s poll publishes UserProfile.Empty when it cannot find the server profile — a different value, so distinctUntilChanged let it through and it overwrote whatever was being typed. The field now follows the store only while it is untouched; the baseline still moves either way, so the confirm button stays honest. --- .../features/user-profile/build.gradle.kts | 1 + .../internal/name/NameEntryViewModel.kt | 10 +- .../internal/photo/PhotoSelectionScreen.kt | 14 ++- .../internal/photo/PhotoSelectionViewModel.kt | 40 ++++++- .../username/UsernameEntryViewModel.kt | 10 +- .../internal/name/NameEntryStateTest.kt | 14 +++ .../internal/photo/PhotoSelectionStateTest.kt | 101 ++++++++++++++++++ .../username/UsernameEntryStateTest.kt | 14 +++ 8 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionStateTest.kt diff --git a/apps/flipcash/features/user-profile/build.gradle.kts b/apps/flipcash/features/user-profile/build.gradle.kts index cd2e26821a..22a363ef7e 100644 --- a/apps/flipcash/features/user-profile/build.gradle.kts +++ b/apps/flipcash/features/user-profile/build.gradle.kts @@ -16,6 +16,7 @@ dependencies { implementation(project(":apps:flipcash:shared:analytics")) implementation(project(":apps:flipcash:shared:blob")) + implementation(project(":apps:flipcash:shared:common-ui")) implementation(project(":apps:flipcash:shared:featureflags")) implementation(project(":apps:flipcash:shared:userflags")) diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt index 9b4d9e0523..aafcfd9896 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModel.kt @@ -84,8 +84,16 @@ class NameEntryViewModel @Inject constructor( // overwrites whatever the user is part-way through typing. .distinctUntilChanged() .onEach { name -> + // distinctUntilChanged only stops an identical value from landing again; the + // profile is polled every 60s and a refresh that can't find the server profile + // publishes UserProfile.Empty, so a *different* name can still arrive mid-edit. + // The field follows the store only while it is untouched — an edit owns it, and + // the baseline moves under it so the confirm button stays honest either way. + val pristine = !stateFlow.value.isChanged dispatchEvent(Event.OnSavedNameLoaded(name)) - stateFlow.value.nameFieldState.setTextAndPlaceCursorAtEnd(name) + if (pristine) { + stateFlow.value.nameFieldState.setTextAndPlaceCursorAtEnd(name) + } }.launchIn(viewModelScope) eventFlow diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt index 846c41e9d3..be47713f6d 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt @@ -41,6 +41,7 @@ import com.flipcash.app.core.ui.transitions.sharedBoundsTransition import com.flipcash.app.core.userprofile.UpdateProfileResult import com.flipcash.app.core.userprofile.UpdateProfileStep import com.flipcash.core.R +import com.flipcash.shared.common.ui.ContactAvatar import com.getcode.navigation.flow.rememberFlowNavigator import com.getcode.theme.CodeTheme import com.getcode.theme.White50 @@ -119,7 +120,7 @@ private fun PhotoSelectionScreenContent( .navigationBarsPadding() .padding(bottom = CodeTheme.dimens.grid.x3), text = stringResource(R.string.action_save), - enabled = state.image.isLoaded() && state.processingState.isIdle, + enabled = state.isChanged && state.processingState.isIdle, isLoading = state.processingState.loading, isSuccess = state.processingState.success, onClick = { @@ -174,6 +175,17 @@ private fun PhotoSelectionScreenContent( CodeCircularProgressIndicator() } } + // No pick pending: show whatever picture is already stored, so the + // step opens on the current avatar rather than an empty well. It is a + // server-side MediaItem, so it can't be mistaken for a pick — Save + // stays disabled until one is made. + state.savedPicture != null -> { + ContactAvatar( + image = state.savedPicture, + displayName = state.name, + modifier = Modifier.fillMaxSize(), + ) + } else -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Icon( diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt index 0443d81418..640f326f6f 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionViewModel.kt @@ -5,6 +5,7 @@ import androidx.annotation.StringRes import androidx.lifecycle.viewModelScope import com.flipcash.app.blob.BlobStorageCoordinator import com.flipcash.app.core.data.Loadable +import com.flipcash.app.core.data.isLoaded import com.flipcash.app.core.extensions.flatMapResult import com.flipcash.app.core.extensions.onResult import com.flipcash.services.models.blob.ImageConstraints @@ -17,6 +18,7 @@ import com.flipcash.services.models.BlobRejectedException import com.flipcash.services.models.ImageModerationError import com.flipcash.services.models.ModerationResult import com.flipcash.services.models.TextModerationError +import com.flipcash.services.models.chat.MediaItem import com.flipcash.services.models.chat.RejectionReason import com.flipcash.services.user.UserManager import com.getcode.manager.BottomBarManager @@ -28,6 +30,7 @@ import com.getcode.view.BaseViewModel import com.getcode.view.LoadingSuccessState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest @@ -52,12 +55,21 @@ class PhotoSelectionViewModel @Inject constructor( private val resources: ResourceHelper, val contentReader: ContentReader, ) : BaseViewModel( - initialState = State(name = userManager.profile?.displayName.orEmpty()), + initialState = State( + name = userManager.profile?.displayName.orEmpty(), + savedPicture = userManager.profile?.profilePicture, + ), updateStateForEvent = updateStateForEvent, defaultDispatcher = dispatchers.Default, ) { data class State( val name: String, + /** + * The stored profile picture, shown until a pick replaces it and again if that pick is + * discarded. Display only: it is a server-side [MediaItem], never a local [Uri], so it + * can't arm Save — [image] holding a pick is still the only thing that counts as a change. + */ + val savedPicture: MediaItem? = null, val image: Loadable = Loadable.Loading(), val attestation: ModerationResult.Attestation = ModerationResult.Attestation.Empty, val processingState: LoadingSuccessState = LoadingSuccessState(), @@ -65,10 +77,21 @@ class PhotoSelectionViewModel @Inject constructor( val uploadPolicy: UploadPolicy? = null, // MIME type of the re-encoded image bytes to upload; resolved from the selected image. val imageMimeType: String = uploadMimeFor(null), - ) + ) { + /** + * The image case of nodes 9553:113166 / 9553:113168: a pick is the only thing that arms + * Save. [savedPicture] is display only, so opening the step on the stored avatar leaves + * this false. + */ + val isChanged: Boolean + get() = image.isLoaded() + } sealed interface Event { data object CheckImage : Event + + /** The stored picture arrived, or changed — including to null when it is unset. */ + data class OnSavedPictureLoaded(val picture: MediaItem?) : Event data class UploadPolicyLoaded(val policy: UploadPolicy) : Event data class UpdateProcessingState( val loading: Boolean = false, @@ -85,6 +108,14 @@ class PhotoSelectionViewModel @Inject constructor( } init { + // Keeps the seeded avatar current: a save merges the server's renditions back into the + // profile, so this is also what swaps the stored picture in once an upload lands. + userManager.state + .map { it.userProfile?.profilePicture } + .distinctUntilChanged() + .onEach { dispatchEvent(Event.OnSavedPictureLoaded(it)) } + .launchIn(viewModelScope) + // Observe the policy — the coordinator serves the launch-preloaded cache and self-refreshes // it if it has aged past its ttl, re-emitting the fresh value here. blobStorage.policy @@ -372,9 +403,12 @@ class PhotoSelectionViewModel @Inject constructor( // Under-shoot the estimated fitting edge so re-encode overhead doesn't push us back over. private const val RESIZE_SAFETY = 0.9 - private val updateStateForEvent: (Event) -> (State.() -> State) = { event -> + internal val updateStateForEvent: (Event) -> (State.() -> State) = { event -> when (event) { Event.CheckImage -> { state -> state } + is Event.OnSavedPictureLoaded -> { state -> + state.copy(savedPicture = event.picture) + } is Event.UploadPolicyLoaded -> { state -> state.copy(uploadPolicy = event.policy) } is Event.UpdateProcessingState -> { state -> val current = state.processingState diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt index bbdec66ada..84fa7f78da 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryViewModel.kt @@ -94,8 +94,16 @@ class UsernameEntryViewModel @Inject constructor( // overwrites whatever the user is part-way through typing. .distinctUntilChanged() .onEach { username -> + // distinctUntilChanged only stops an identical value from landing again; the + // profile is polled every 60s and a refresh that can't find the server profile + // publishes UserProfile.Empty, so a *different* handle can still arrive mid-edit. + // The field follows the store only while it is untouched — an edit owns it, and + // the baseline moves under it so the confirm button stays honest either way. + val pristine = !stateFlow.value.isChanged dispatchEvent(Event.OnSavedUsernameLoaded(username)) - stateFlow.value.usernameFieldState.setTextAndPlaceCursorAtEnd(username) + if (pristine) { + stateFlow.value.usernameFieldState.setTextAndPlaceCursorAtEnd(username) + } }.launchIn(viewModelScope) eventFlow diff --git a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt index 938043dfa2..b9e91c9105 100644 --- a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt +++ b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryStateTest.kt @@ -85,4 +85,18 @@ class NameEntryStateTest { assertEquals(state, reduce(NameEntryViewModel.Event.DiscardChanges)(state)) } + + @Test + fun `a refresh that moves the baseline leaves the edit in the field`() { + val editing = stateWith(saved = "Brandon McAnsh", typed = "Brandon McAnshx") + + // The 60s profile poll can publish a different name — reducing it must not touch + // the field, only the baseline the field is measured against. + val refreshed = reduce( + NameEntryViewModel.Event.OnSavedNameLoaded("") + )(editing) + + assertEquals("Brandon McAnshx", refreshed.nameFieldState.text.toString()) + assertTrue(refreshed.isChanged) + } } diff --git a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionStateTest.kt b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionStateTest.kt new file mode 100644 index 0000000000..3f7b95884b --- /dev/null +++ b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionStateTest.kt @@ -0,0 +1,101 @@ +package com.flipcash.app.userprofile.internal.photo + +import android.net.Uri +import com.flipcash.services.models.chat.MediaItem +import org.mockito.kotlin.mock +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class PhotoSelectionStateTest { + + private val reduce = PhotoSelectionViewModel.updateStateForEvent + + private val storedPicture = MediaItem(renditions = emptyList()) + private val pick: Uri = mock() + + // imageMimeType is supplied so State's default doesn't reach for Android's MimeTypeMap, + // which has no implementation under plain JVM unit tests. + private fun state(name: String = "Ada") = + PhotoSelectionViewModel.State(name = name, imageMimeType = "image/jpeg") + + private fun PhotoSelectionViewModel.State.reduced(event: PhotoSelectionViewModel.Event) = + reduce(event)(this) + + @Test + fun `an account with no picture opens on nothing to save`() { + val state = state() + assertNull(state.savedPicture) + assertFalse(state.isChanged) + } + + @Test + fun `the stored picture is seeded without arming save`() { + val seeded = state().reduced( + PhotoSelectionViewModel.Event.OnSavedPictureLoaded(storedPicture) + ) + + assertEquals(storedPicture, seeded.savedPicture) + assertFalse(seeded.isChanged) + } + + @Test + fun `a cached pick is what counts as a change`() { + val picked = state() + .reduced(PhotoSelectionViewModel.Event.OnSavedPictureLoaded(storedPicture)) + .reduced(PhotoSelectionViewModel.Event.OnImageCached(pick, "image/png")) + + assertTrue(picked.isChanged) + assertEquals(pick, picked.image.dataOrNull) + assertEquals("image/png", picked.imageMimeType) + // The stored picture stays put — it's what a discard falls back to. + assertEquals(storedPicture, picked.savedPicture) + } + + @Test + fun `a pick still being re-encoded has nothing to save yet`() { + val selected = state().reduced(PhotoSelectionViewModel.Event.OnImageSelected(pick)) + + assertFalse(selected.isChanged) + assertEquals(pick, selected.image.dataOrNull) + } + + @Test + fun `clearing the pick leaves the stored picture showing`() { + val cleared = state() + .reduced(PhotoSelectionViewModel.Event.OnSavedPictureLoaded(storedPicture)) + .reduced(PhotoSelectionViewModel.Event.OnImageCached(pick, "image/png")) + .reduced(PhotoSelectionViewModel.Event.OnImageCleared) + + assertFalse(cleared.isChanged) + assertNull(cleared.image.dataOrNull) + assertEquals(storedPicture, cleared.savedPicture) + } + + @Test + fun `a saved upload replaces the stored picture`() { + val uploaded = MediaItem(renditions = emptyList()) + val state = state() + .reduced(PhotoSelectionViewModel.Event.OnSavedPictureLoaded(storedPicture)) + .reduced(PhotoSelectionViewModel.Event.OnSavedPictureLoaded(uploaded)) + + assertEquals(uploaded, state.savedPicture) + } + + @Test + fun `no-op events return state unchanged`() { + val state = state().reduced( + PhotoSelectionViewModel.Event.OnSavedPictureLoaded(storedPicture) + ) + val noOpEvents = listOf( + PhotoSelectionViewModel.Event.CheckImage, + PhotoSelectionViewModel.Event.OnImageApproved, + PhotoSelectionViewModel.Event.DiscardChanges, + ) + noOpEvents.forEach { event -> + assertEquals(state, state.reduced(event), "Event $event should be no-op") + } + } +} diff --git a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt index 52e89ca2e8..cf10597071 100644 --- a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt +++ b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/username/UsernameEntryStateTest.kt @@ -77,4 +77,18 @@ class UsernameEntryStateTest { assertEquals(state, reduce(UsernameEntryViewModel.Event.DiscardChanges)(state)) } + + @Test + fun `a refresh that moves the baseline leaves the edit in the field`() { + val editing = stateWith(saved = "mcansh", typed = "mcanshzz") + + // The 60s profile poll can publish a different handle — reducing it must not touch + // the field, only the baseline the field is measured against. + val refreshed = reduce( + UsernameEntryViewModel.Event.OnSavedUsernameLoaded("") + )(editing) + + assertEquals("mcanshzz", refreshed.usernameFieldState.text.toString()) + assertTrue(refreshed.isChanged) + } }