feat(profile): confirm a profile change before it is written - #1363
Merged
Conversation
Saving a display name, username, profile picture, or minimum tip wrote straight through on the first tap. Each of those four is an overwrite with no undo, and the username case is worse than the rest: changing a handle releases the old one for anyone to claim. Each screen's Save button now dispatches a confirm event rather than the save event. The ViewModel gate shows a destructive alert whose single action dispatches the original save; Cancel leaves the entry as it is. The save pipelines themselves are unchanged. The prompt is only shown when there is a stored value to replace. A first claim overwrites nothing, and the name step is mandatory during onboarding, where a confirmation would sit on a screen that cannot be backed out of. Minimum tip asks after the below-minimum check, so an amount the server would reject still gets the minimum-tip error instead of a confirm that then fails. Its write moved to a CommitRequested event so it runs on viewModelScope rather than from the dialog callback.
A one-character handle got the "Change Username?" confirmation and was only then told it was too short. The length check ran inside the CheckUsername pipeline, downstream of the gate. It now runs in the gate, ahead of both the confirmation and the first-claim path, and CheckUsername is left as the commit. This matches iOS, whose submit() returns on UsernameValidator.failure(for:) before it reaches setUsername, and matches the minimum-tip gate, which already validated first. Only the local rules move. Taken, moderated, reserved, and insufficient balance all come back from setUsername itself, which is the claim as well as the check, so there is nothing to pre-check without claiming the handle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Saving a display name, username, profile picture, or minimum tip wrote straight through on the first tap. Each of those four is an overwrite with no undo, and the username case is worse than the rest: changing a handle releases the old one, so someone else can claim it.
Each of the four screens now dispatches a confirm event from its Save button instead of the save event. The ViewModel gate shows a destructive (red)
BottomBarManager.showAlertwith one filled action andshowCancel = true; only that action dispatches the original save event. The save pipelines are otherwise untouched.Copy follows one template — "Change X?" / "Are you sure you want to permanently change your X?" / "Change X" / "Cancel" — with username carrying an extra sentence: "You might not be able to get your old username back".
Two judgment calls worth a second opinion:
The prompt only fires on a change. "Permanently change your X" doesn't fit a first claim, and the display-name step is mandatory during onboarding, where a confirmation would sit on a screen you can't back out of. Each gate checks the stored value first (
savedUsername.isBlank(),savedName.isBlank(),savedPicture == null,saved == null) and goes straight to the save when there isn't one."Profile Picture", not "Profile Photo". That matches the My Account row and the rest of that surface (
title_profilePicture,title_setProfilePicture).Both gates that have something local to check run the check first, so an entry that can never be saved says why instead of asking the user to confirm saving it:
CheckUsernamepipeline and into the gate, ahead of both the confirmation and the first-claim path. This matches iOS, whosesubmit()returns onUsernameValidator.failure(for:)before it callssetUsername. Only the local rules move: taken, moderated, reserved, and insufficient balance all come back fromsetUsernameitself, which is the claim as well as the check, so there is nothing to pre-check without claiming the handle.CommitRequestedevent so it runs onviewModelScopeinstead of from the dialog's callback.Display name and profile picture have no local rules to hoist. The name step's only client-side gate is non-blank, which already disables Save, and a picture's size and type are checked when it is picked.
No new tests. The gates live in the ViewModels' init flows, which need mocked Hilt dependencies; the existing
*StateTestfiles only exercise the pure reducer, where the new events are pass-throughs.The same change on iOS is code-payments/code-ios-app#688. That one already validated the username locally before submitting, which is where the second commit here comes from.