Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Woo POS] [Cash & Receipts] Validate cash amount #14811

Open
wants to merge 4 commits into
base: task/14602-handle-cash-payment-success
Choose a base branch
from
Open

[Woo POS] [Cash & Receipts] Validate cash amount #14811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b978f9
compare order total vs merchant input to validate
iamgabrielma Jan 7, 2025
83b7f60
separate change due and error strings when validating
iamgabrielma Jan 7, 2025
fa0bbc3
use textSuccess color for change due
iamgabrielma Jan 7, 2025
1339f75
localized strings
iamgabrielma Jan 7, 2025
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
74 changes: 65 additions & 9 deletions WooCommerce/Classes/POS/Presentation/PointOfSaleCollectCashView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import SwiftUI
import class WooFoundation.CurrencyFormatter

struct PointOfSaleCollectCashView: View {
@Environment(\.colorScheme) var colorScheme
Expand All @@ -8,20 +9,16 @@ struct PointOfSaleCollectCashView: View {
@State private var textFieldAmountInput: String = ""
@State private var isLoading: Bool = false
@State private var errorMessage: String?
@State private var changeDueMessage: String?

private let currencyFormatter: CurrencyFormatter = WooFoundation.CurrencyFormatter(currencySettings: ServiceLocator.currencySettings)

let orderTotal: String

private var formattedOrderTotal: String {
String.localizedStringWithFormat(Localization.backNavigationSubtitle, orderTotal)
}

private func validateAmount() -> Bool {
// TODO:
// Validate amount entered vs order total
// https://github.com/woocommerce/woocommerce-ios/issues/14749
return true
}

@StateObject private var textFieldViewModel = FormattableAmountTextFieldViewModel(size: .extraLarge,
locale: Locale.autoupdatingCurrent,
storeCurrencySettings: ServiceLocator.currencySettings,
Expand Down Expand Up @@ -54,8 +51,15 @@ struct PointOfSaleCollectCashView: View {
FormattableAmountTextField(viewModel: textFieldViewModel, style: .pos)
.onChange(of: textFieldViewModel.amount) { newValue in
textFieldAmountInput = newValue
updateChangeDueMessage()
}

if let changeDue = changeDueMessage {
Text(changeDue)
.font(.posBodyRegular)
.foregroundColor(.posTextSuccess)
}

if let errorMessage = errorMessage {
Text(errorMessage)
.font(POSFontStyle.posBodyRegular)
Expand All @@ -64,7 +68,7 @@ struct PointOfSaleCollectCashView: View {

Button(action: {
Task { @MainActor in
guard validateAmount() else {
guard validateAmountOnSubmit() else {
return
}
isLoading = true
Expand Down Expand Up @@ -102,6 +106,7 @@ struct PointOfSaleCollectCashView: View {
.background(backgroundColor)
.padding()
.animation(.easeInOut, value: errorMessage)
.animation(.easeInOut, value: changeDueMessage)
.onChange(of: textFieldAmountInput) { _ in
errorMessage = nil
}
Expand All @@ -112,6 +117,46 @@ struct PointOfSaleCollectCashView: View {
}
}

private extension PointOfSaleCollectCashView {
func parseCurrency(_ amountString: String) -> NSDecimalNumber? {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've attempted to implement this from zero using Decimal but dealing with currency was anything but fun, so I've relied on our own CurrencyFormatter implementation. That said we seem to carry our own downsides (eg: #13829 ) so I'm not sure if there's a better way we could use to handle formatting here, since we use the same currency formatter in other places in POS 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, understandable. It's okay to use what we currently have. We can make improvements internally within the formatters.

The largest issues I've seen were when converting amounts back and forth when creating and then editing Order fields again. It's not the case with this POS cash payment so it should be fine.

currencyFormatter.convertToDecimal(amountString, locale: .current)
}

private func formatAsCurrency(_ amount: NSDecimalNumber) -> String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these methods could be unit tested (with mocked currency formatter). We have calculations happening within them. These methods would likely need to change into pure functions within a helper and then View would just set appropriate labels based on the result.

currencyFormatter.formatAmount(amount) ?? "$0.00"
}

private func updateChangeDueMessage() {
guard let orderDecimal = parseCurrency(orderTotal),
let inputDecimal = parseCurrency(textFieldAmountInput) else {
changeDueMessage = nil
return
}

if inputDecimal.compare(orderDecimal) == .orderedDescending {
let changeDue = inputDecimal.subtracting(orderDecimal)
changeDueMessage = String.localizedStringWithFormat(Localization.changeDueMessage, formatAsCurrency(changeDue))
} else {
changeDueMessage = nil
}
}

private func validateAmountOnSubmit() -> Bool {
guard let orderDecimal = parseCurrency(orderTotal),
let inputDecimal = parseCurrency(textFieldAmountInput) else {
errorMessage = Localization.failedToCollectCashPayment
return false
}

if inputDecimal.compare(orderDecimal) == .orderedAscending {
errorMessage = Localization.cashPaymentAmountNotEnough
return false
}
errorMessage = nil
return true
}
}

private extension PointOfSaleCollectCashView {
enum Constants {
static let buttonSpacing: CGFloat = 12
Expand Down Expand Up @@ -147,10 +192,21 @@ private extension PointOfSaleCollectCashView {
comment: "Button to mark a cash payment as completed"
)
static let failedToCollectCashPayment = NSLocalizedString(
"pointOfSale.cashview.failedToCollectCashPayment.draft",
"pointOfSale.cashview.failedtocollectcashpayment.errormessage",
value: "Error trying to process payment. Try again.",
comment: "Error message when the system fails to collect a cash payment."
)
static let cashPaymentAmountNotEnough = NSLocalizedString(
"pointOfSale.cashview.cashpaymentamountnotenough.errormessage",
value: "Amount must be more or equal to total.",
comment: "Error message when the cash amount entered is less than the order total."
)
static let changeDueMessage = NSLocalizedString(
"pointOfSale.cashview.changedue",
value: "Change due: %1$@",
comment: "Change due when the cash amount entered exceeds the order total." +
"Reads as 'Change due: $1.23'"
)
}
}

Expand Down
9 changes: 9 additions & 0 deletions WooCommerce/Classes/POS/Utils/Color+WooCommercePOS.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ extension Color {
)
}

static var posTextSuccess: Color {
Color(
UIColor(
light: UIColor(red: 10.0/255.0, green: 148.0/255.0, blue: 0.0/255.0, alpha: 1.0),
dark: UIColor(red: 10.0/255.0, green: 148.0/255.0, blue: 0.0/255.0, alpha: 1.0)
)
)
}

// MARK: - Buttons

static var posPrimaryButtonBackground: Color {
Expand Down
Loading