Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions Flipcash/Core/Screens/Main/Profile/ProfileNameScreen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ struct ProfileNameScreen: View {

@FocusState private var isNameFocused: Bool
@State private var submitTask: Task<Void, Never>?
@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.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -149,15 +167,15 @@ 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"
)

} 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"
)
Expand All @@ -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"
)
Expand Down
25 changes: 18 additions & 7 deletions Flipcash/Core/Screens/Main/Profile/ProfilePhotoScreen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -86,7 +86,7 @@ struct ProfilePhotoScreen: View {

Spacer()

Button(action: state.beginUpload) {
Button(action: submit) {
ButtonStateLabel(completion == .tipcard ? "Next" : "Save", state: buttonState)
}
.buttonStyle(.filled)
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
)
Expand Down Expand Up @@ -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"
)
Expand Down
21 changes: 16 additions & 5 deletions Flipcash/Core/Screens/Main/Username/UsernameEntryScreen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct UsernameEntryScreen: View {
@FocusState private var isFocused: Bool
@State private var input: String = ""
@State private var submitTask: Task<Void, Never>?
@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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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)
}

Expand All @@ -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
}
}
}
Expand Down
62 changes: 62 additions & 0 deletions Flipcash/Core/Screens/Settings/DialogItem+ProfileChange.swift
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
13 changes: 13 additions & 0 deletions Flipcash/Core/Screens/Settings/SetMinimumTipScreen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
82 changes: 82 additions & 0 deletions FlipcashTests/ProfileChangeDialogTests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
4 changes: 2 additions & 2 deletions FlipcashTests/WithdrawViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand All @@ -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")
}
}
Expand Down