diff --git a/Flipcash/Core/Screens/Main/Profile/ProfileNameScreen.swift b/Flipcash/Core/Screens/Main/Profile/ProfileNameScreen.swift index 120e87973..7bd08ba00 100644 --- a/Flipcash/Core/Screens/Main/Profile/ProfileNameScreen.swift +++ b/Flipcash/Core/Screens/Main/Profile/ProfileNameScreen.swift @@ -28,7 +28,7 @@ struct ProfileNameScreen: View { @FocusState private var isNameFocused: Bool @State private var submitTask: Task? - @State private var errorDialog: DialogItem? + @State private var dialog: DialogItem? /// Drives the Next button: spinner while saving, then the checkmark the /// rest of the app shows on a completed action. @@ -88,7 +88,7 @@ struct ProfileNameScreen: View { .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) } .navigationBarTitleDisplayMode(.inline) - .dialog(item: $errorDialog) + .dialog(item: $dialog) .onAppear { isNameFocused = true } // Leaving the screen abandons the submission: its only continuation is a // push onto a stack this screen no longer sits on. @@ -106,9 +106,27 @@ struct ProfileNameScreen: View { private func submit() { guard let name = state.validatedDisplayName, !isSubmitting else { return } + // Only a replacement is confirmed. Setting a first name gives nothing + // up, and its button says Next because it is a step in setting the + // profile up. + guard hasPreviousName else { + save(name) + return + } + + dialog = .confirmProfileChange(.displayName) { save(name) } + } + + /// True when the profile already carries a name, so Save is replacing one + /// rather than setting the first. + private var hasPreviousName: Bool { + !(sessionContainer.session.profile?.displayName ?? "").isEmpty + } + + private func save(_ name: String) { // Read before the RPC: `updateProfile()` below installs the new name, after // which every submission would look like a replacement. - let hadPreviousName = !(sessionContainer.session.profile?.displayName ?? "").isEmpty + let hadPreviousName = hasPreviousName let source: Analytics.DisplayNameSource = switch completion { case .tipcard: .tipCardSetup case .back: .myAccount @@ -149,7 +167,7 @@ struct ProfileNameScreen: View { } catch ErrorProfile.moderated(let category) { buttonState = .normal logger.info("Display name moderation denied", metadata: ["category": "\(category)"]) - errorDialog = .error( + dialog = .error( title: "This Name is Not Allowed", subtitle: "Try a different name" ) @@ -157,7 +175,7 @@ struct ProfileNameScreen: View { } catch ErrorProfile.invalidDisplayName { buttonState = .normal logger.info("Display name rejected as invalid") - errorDialog = .error( + dialog = .error( title: "This Name Isn't Valid", subtitle: "Try a different name" ) @@ -167,7 +185,7 @@ struct ProfileNameScreen: View { guard !Task.isCancelled else { return } logger.error("Failed to set display name", metadata: ["error": "\(error)"]) ErrorReporting.captureError(error, reason: "Failed to set display name") - errorDialog = .error( + dialog = .error( title: "Couldn't Save Your Name", subtitle: "Try again" ) diff --git a/Flipcash/Core/Screens/Main/Profile/ProfilePhotoScreen.swift b/Flipcash/Core/Screens/Main/Profile/ProfilePhotoScreen.swift index 207bc0018..de3241b25 100644 --- a/Flipcash/Core/Screens/Main/Profile/ProfilePhotoScreen.swift +++ b/Flipcash/Core/Screens/Main/Profile/ProfilePhotoScreen.swift @@ -29,7 +29,7 @@ struct ProfilePhotoScreen: View { @State private var isShowingPhotoPicker = false @State private var isShowingFilePicker = false - @State private var errorDialog: DialogItem? + @State private var dialog: DialogItem? /// Drives the submit button: spinner while uploading, then the checkmark the /// rest of the app shows on a completed action. @@ -86,7 +86,7 @@ struct ProfilePhotoScreen: View { Spacer() - Button(action: state.beginUpload) { + Button(action: submit) { ButtonStateLabel(completion == .tipcard ? "Next" : "Save", state: buttonState) } .buttonStyle(.filled) @@ -101,7 +101,7 @@ struct ProfilePhotoScreen: View { } .navigationTitle(completion == .tipcard ? "" : "Set Profile Picture") .navigationBarTitleDisplayMode(.inline) - .dialog(item: $errorDialog) + .dialog(item: $dialog) .fullScreenCover(isPresented: $isShowingPhotoPicker) { ImagePickerWithEditor( onImagePicked: state.select, @@ -136,6 +136,17 @@ struct ProfilePhotoScreen: View { sessionContainer.session.profile?.profilePicture } + private func submit() { + // Only a replacement is confirmed. A first picture gives nothing up, + // and this screen is how the profile checklist sets one. + guard profilePicture != nil else { + state.beginUpload() + return + } + + dialog = .confirmProfileChange(.profilePicture) { state.beginUpload() } + } + private func upload() async { buttonState = .loading do { @@ -176,20 +187,20 @@ struct ProfilePhotoScreen: View { guard !Task.isCancelled else { return } logger.info("Profile picture upload failed", metadata: ["error": "\(error)"]) ErrorReporting.captureError(error, reason: "Profile picture upload failed") - errorDialog = .profilePictureFailed(error) + dialog = .profilePictureFailed(error) } catch let error as ImageEncoderError { buttonState = .normal logger.error("Failed to encode the profile picture", metadata: ["error": "\(error)"]) ErrorReporting.captureError(error, reason: "Failed to encode the profile picture") - errorDialog = .imageProcessingFailed + dialog = .imageProcessingFailed } catch { buttonState = .normal guard !Task.isCancelled else { return } logger.error("Failed to set profile picture", metadata: ["error": "\(error)"]) ErrorReporting.captureError(error, reason: "Failed to set profile picture") - errorDialog = .error( + dialog = .error( title: "Couldn't Upload Your Photo", subtitle: "Try again" ) @@ -219,7 +230,7 @@ struct ProfilePhotoScreen: View { }.value guard let image else { - errorDialog = .error( + dialog = .error( title: "Couldn't Open That File", subtitle: "Try a different image" ) diff --git a/Flipcash/Core/Screens/Main/Username/UsernameEntryScreen.swift b/Flipcash/Core/Screens/Main/Username/UsernameEntryScreen.swift index 50f92d3be..716e9547c 100644 --- a/Flipcash/Core/Screens/Main/Username/UsernameEntryScreen.swift +++ b/Flipcash/Core/Screens/Main/Username/UsernameEntryScreen.swift @@ -29,7 +29,7 @@ struct UsernameEntryScreen: View { @FocusState private var isFocused: Bool @State private var input: String = "" @State private var submitTask: Task? - @State private var errorDialog: DialogItem? + @State private var dialog: DialogItem? /// Drives the Next button: spinner while claiming, then the checkmark the /// rest of the app shows on a completed action. @@ -89,7 +89,7 @@ struct UsernameEntryScreen: View { .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) } .navigationBarTitleDisplayMode(.inline) - .dialog(item: $errorDialog) + .dialog(item: $dialog) .onAppear { input = currentUsername?.value ?? "" isFocused = true @@ -120,12 +120,23 @@ struct UsernameEntryScreen: View { guard !isSubmitting else { return } if let failure = Self.validator.failure(for: input) { - errorDialog = .usernameValidation(failure) + dialog = .usernameValidation(failure) return } guard let username = Self.validator.validate(input) else { return } + // Only a replacement is confirmed. A first claim gives nothing up, and + // its button says Next because it is a step in setting the profile up. + if currentUsername != nil { + dialog = .confirmProfileChange(.username) { claim(username) } + return + } + + claim(username) + } + + private func claim(_ username: Username) { buttonState = .loading submitTask = Task { defer { submitTask = nil } @@ -156,7 +167,7 @@ struct UsernameEntryScreen: View { // duplicate that classification in a second place. ErrorReporting.captureError(error, reason: "Failed to set username") - errorDialog = .usernameSubmission(error, minimum: minimumBalance) { + dialog = .usernameSubmission(error, minimum: minimumBalance) { router.presentAddMoney(.general, source: .usernameShortfall) } @@ -165,7 +176,7 @@ struct UsernameEntryScreen: View { guard !Task.isCancelled else { return } logger.error("Failed to set username", metadata: ["error": "\(error)"]) ErrorReporting.captureError(error, reason: "Failed to set username") - errorDialog = .usernameGenericFailure + dialog = .usernameGenericFailure } } } diff --git a/Flipcash/Core/Screens/Settings/DialogItem+ProfileChange.swift b/Flipcash/Core/Screens/Settings/DialogItem+ProfileChange.swift new file mode 100644 index 000000000..3fac8d831 --- /dev/null +++ b/Flipcash/Core/Screens/Settings/DialogItem+ProfileChange.swift @@ -0,0 +1,62 @@ +// +// DialogItem+ProfileChange.swift +// Flipcash +// + +import FlipcashUI + +extension DialogItem { + + /// The profile fields My Account can replace. Each screen that edits one + /// also serves as its first-time setup step, so the field is only named + /// here for the replacement case. + enum ProfileField { + + case username + case displayName + case profilePicture + case minimumTip + + /// The label My Account uses for the field, so the dialog names it the + /// same way the row the user tapped did. + var title: String { + switch self { + case .username: "Username" + case .displayName: "Display Name" + case .profilePicture: "Profile Picture" + case .minimumTip: "Minimum Tip" + } + } + + var subtitle: String { + switch self { + case .username: + // A released handle is claimable by anyone, so this change is + // the only one of the four the user may not be able to undo. + "Are you sure you want to permanently change your username? You might not be able to get your old username back" + case .displayName: + "Are you sure you want to permanently change your display name?" + case .profilePicture: + "Are you sure you want to permanently change your profile picture?" + case .minimumTip: + "Are you sure you want to permanently change your minimum tip?" + } + } + } + + /// Confirms replacing a profile field that is already set. Raised on Save, + /// after validation, so the user is never asked to confirm an entry the + /// screen is about to reject anyway. + /// + /// Untracked: the user choosing to change their own display name is not an + /// error worth an analytics event. + static func confirmProfileChange( + _ field: ProfileField, + onConfirm: @escaping () -> Void + ) -> DialogItem { + .alert(title: "Change \(field.title)?", subtitle: field.subtitle) { + DialogAction.destructive("Change \(field.title)", action: onConfirm) + DialogAction.cancel() + } + } +} diff --git a/Flipcash/Core/Screens/Settings/SetMinimumTipScreen.swift b/Flipcash/Core/Screens/Settings/SetMinimumTipScreen.swift index 33ec2da71..9159bece6 100644 --- a/Flipcash/Core/Screens/Settings/SetMinimumTipScreen.swift +++ b/Flipcash/Core/Screens/Settings/SetMinimumTipScreen.swift @@ -100,6 +100,19 @@ struct SetMinimumTipScreen: View { let fee = FiatAmount(value: value, currency: currency) + // Only a replacement is confirmed. A first fee gives nothing up, and + // this screen is how the profile checklist sets one. Read off the + // profile rather than `existingFee`, which is scoped to the currency + // being entered — a fee set in another currency is still being replaced. + guard sessionContainer.session.profile?.minDmChatInitFee != nil else { + save(fee) + return + } + + dialog = .confirmProfileChange(.minimumTip) { save(fee) } + } + + private func save(_ fee: FiatAmount) { actionState = .loading submitTask = Task { defer { submitTask = nil } diff --git a/FlipcashTests/ProfileChangeDialogTests.swift b/FlipcashTests/ProfileChangeDialogTests.swift new file mode 100644 index 000000000..460c08b1c --- /dev/null +++ b/FlipcashTests/ProfileChangeDialogTests.swift @@ -0,0 +1,82 @@ +// +// ProfileChangeDialogTests.swift +// FlipcashTests +// + +import Foundation +import Testing +import FlipcashUI +@testable import Flipcash + +@MainActor +@Suite("Profile change confirmation dialog") +struct ProfileChangeDialogTests { + + @Test("Names the field in the title, the body and the confirming button", arguments: [ + (DialogItem.ProfileField.username, "Username"), + (.displayName, "Display Name"), + (.profilePicture, "Profile Picture"), + (.minimumTip, "Minimum Tip"), + ]) + func namesField(field: DialogItem.ProfileField, label: String) { + let item = DialogItem.confirmProfileChange(field) {} + + #expect(item.title == "Change \(label)?") + #expect(item.subtitle?.contains("permanently change your \(label.lowercased())") == true) + #expect(item.actions.first?.title == "Change \(label)") + } + + @Test("Confirms destructively over Cancel", arguments: [ + DialogItem.ProfileField.username, + .displayName, + .profilePicture, + .minimumTip, + ]) + func actions(field: DialogItem.ProfileField) { + let item = DialogItem.confirmProfileChange(field) {} + + #expect(item.actions.count == 2) + #expect(item.actions[0].kind == .destructive) + #expect(item.actions[1].title == "Cancel") + } + + @Test("Runs the caller's work only once the change is confirmed") + func confirmAction_runsHandler() { + var confirmed = false + let item = DialogItem.confirmProfileChange(.username) { confirmed = true } + + #expect(confirmed == false) + item.actions[0].action() + #expect(confirmed == true) + } + + @Test("Cancel leaves the caller's work unrun") + func cancelAction_doesNotRunHandler() { + var confirmed = false + let item = DialogItem.confirmProfileChange(.username) { confirmed = true } + + item.actions[1].action() + #expect(confirmed == false) + } + + /// The only one of the four that can cost the user something they can't + /// take back, so it is the only one that says so. + @Test("Only the username warns that the old value may be gone for good") + func usernameWarnsAboutLosingTheHandle() { + let username = DialogItem.confirmProfileChange(.username) {} + #expect(username.subtitle?.contains("You might not be able to get your old username back") == true) + + for field in [DialogItem.ProfileField.displayName, .profilePicture, .minimumTip] { + let item = DialogItem.confirmProfileChange(field) {} + #expect(item.subtitle?.contains("get your old") == false) + } + } + + @Test("Red banner, but not an error worth reporting") + func styleIsUntrackedDestructive() { + let item = DialogItem.confirmProfileChange(.minimumTip) {} + + #expect(item.style == .destructive) + #expect(item.tracked == false) + } +} diff --git a/FlipcashTests/WithdrawViewModelTests.swift b/FlipcashTests/WithdrawViewModelTests.swift index a55d1b403..c666a0af3 100644 --- a/FlipcashTests/WithdrawViewModelTests.swift +++ b/FlipcashTests/WithdrawViewModelTests.swift @@ -651,7 +651,7 @@ struct WithdrawViewModelSummaryHelpersTests { switch viewModel.amountSubtitle { case .balanceWithLimit: break - case .singleTransactionLimit, .error: + case .singleTransactionLimit, .error, .hidden: Issue.record("Expected .balanceWithLimit subtitle for valid amount") } } @@ -668,7 +668,7 @@ struct WithdrawViewModelSummaryHelpersTests { case .error(let copy): #expect(copy.contains("Minimum withdrawal")) #expect(copy.contains("0.51")) - case .balanceWithLimit, .singleTransactionLimit: + case .balanceWithLimit, .singleTransactionLimit, .hidden: Issue.record("Expected .error subtitle for amount below fee") } }