Skip to content

POS cash payment: prefill order amount and update analytics tracking #16058

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

Merged
merged 7 commits into from
Sep 4, 2025
Merged

POS cash payment: prefill order amount and update analytics tracking #16058

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
363e0b0
Add a new `checkout_cash_payment_tapped` POS event and track it when …
jaclync Aug 29, 2025
7119219
PointOfSaleCollectCashView: prefill text field with order total, upda…
jaclync Aug 29, 2025
004b624
Update release notes.
jaclync Aug 29, 2025
f149da7
Merge branch 'trunk' into backlog/WOOMOB-1017-cash-payment-changes
jaclync Sep 4, 2025
0dff142
Fix build warnings outside of PR scope but in the same file.
jaclync Sep 4, 2025
7c576c0
Improve readability of `isPaymentButtonEnabled` function.
jaclync Sep 4, 2025
15e4a27
Merge branch 'trunk' into backlog/WOOMOB-1017-cash-payment-changes
jaclync Sep 4, 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
2 changes: 1 addition & 1 deletion RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

23.2
-----

- [*] POS: Enhanced cash payment experience with pre-filled amounts. [https://github.com/woocommerce/woocommerce-ios/pull/16058]

23.1
-----
Expand Down
1 change: 1 addition & 0 deletions WooCommerce/Classes/Analytics/TracksProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ private extension TracksProvider {
WooAnalyticsStat.pointOfSaleViewDocsTapped,
WooAnalyticsStat.pointOfSaleReaderReadyForCardPayment,
WooAnalyticsStat.pointOfSaleCashCollectPaymentSuccess,
WooAnalyticsStat.pointOfSaleCheckoutCashPaymentTapped,
WooAnalyticsStat.pointOfSaleCashPaymentTapped,
WooAnalyticsStat.pointOfSaleCashPaymentFailed,
WooAnalyticsStat.pointOfSaleItemsHeaderTapped,
Expand Down
1 change: 1 addition & 0 deletions WooCommerce/Classes/Analytics/WooAnalyticsStat.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,7 @@ enum WooAnalyticsStat: String {
case pointOfSaleItemRemovedFromCart = "item_removed_from_cart"
case pointOfSaleCheckoutTapped = "checkout_tapped"
case pointOfSaleBackToCartTapped = "back_to_cart_tapped"
case pointOfSaleCheckoutCashPaymentTapped = "checkout_cash_payment_tapped"
case pointOfSaleCashPaymentTapped = "cash_payment_tapped"
case pointOfSaleCashPaymentFailed = "cash_payment_failed"
case pointOfSaleBackToCheckoutFromCashTapped = "back_to_checkout_from_cash"
Expand Down
2 changes: 1 addition & 1 deletion WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ extension PointOfSaleAggregateModel {
// Once we get the callback from the card service, we switch to cash collection state
@MainActor
func startCashPayment() async {
analytics.track(.pointOfSaleCashPaymentTapped)
analytics.track(.pointOfSaleCheckoutCashPaymentTapped)
try? await cardPresentPaymentService.cancelPayment()
paymentState.cash = .collectingCash
}
Expand Down
27 changes: 16 additions & 11 deletions WooCommerce/Classes/POS/Presentation/PointOfSaleCollectCashView.swift
View file
Open in desktop
Copy link
Contributor

@iamgabrielma iamgabrielma Sep 1, 2025

Choose a reason for hiding this comment

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

Since we're updating this file and testing the cash payment, we could also update the warnings for iOS17 in lines 65 and 108 regarding onChange deprecation 👍

Copy link
Contributor Author

@jaclync jaclync Sep 4, 2025

Choose a reason for hiding this comment

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

Good catch, fixed in 0dff142. Hopefully we can reset build warnings to 0 and new warnings are detected in CI in the near future.

Copy link
Contributor Author

@jaclync jaclync Sep 4, 2025

Choose a reason for hiding this comment

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

Btw @iamgabrielma while checking your PR #16002 from the latest merge conflicts, the release notes entry seemed to get into 23.1 instead of 23.2. Not sure what impact it has though, maybe just for Peacock beta testing.

Copy link
Contributor

@iamgabrielma iamgabrielma Sep 4, 2025

Choose a reason for hiding this comment

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

Oh, appreciated, I did move them here then it seems messed it up when merging trunk into the branch. Thanks, I'll address it 🙇

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ struct PointOfSaleCollectCashView: View {
String.localizedStringWithFormat(Localization.backNavigationSubtitle, orderTotal)
}

private var isButtonEnabled: Bool {
viewHelper.isPaymentButtonEnabled(orderTotal: orderTotal,
textFieldAmountInput: textFieldAmountInput,
isLoading: isLoading)
}

@StateObject private var textFieldViewModel = FormattableAmountTextFieldViewModel(size: .extraLarge,
locale: Locale.autoupdatingCurrent,
storeCurrencySettings: ServiceLocator.currencySettings,
Expand Down Expand Up @@ -89,7 +95,7 @@ struct PointOfSaleCollectCashView: View {
.buttonStyle(POSFilledButtonStyle(size: .normal, isLoading: isLoading))
.frame(maxWidth: .infinity)
.dynamicTypeSize(...DynamicTypeSize.accessibility1)
.disabled(isLoading)
.disabled(!isButtonEnabled)
}
.padding([.horizontal])
.padding(.bottom, max(keyboardFrame.height - geometry.safeAreaInsets.bottom,
Expand All @@ -110,6 +116,9 @@ struct PointOfSaleCollectCashView: View {
}
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.onAppear {
prefillOrderTotal()
}
}

private func markComplete() async throws {
Expand All @@ -121,9 +130,7 @@ struct PointOfSaleCollectCashView: View {

private extension PointOfSaleCollectCashView {
private func submitCashAmount() async {
guard validateAmountOnSubmit() else {
return
}
ServiceLocator.analytics.track(.pointOfSaleCashPaymentTapped)
Copy link
Contributor

@iamgabrielma iamgabrielma Sep 1, 2025

Choose a reason for hiding this comment

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

Not necessarily for this PR, but I wonder if we should expose the analytics prop in PointOfSaleAggregateModel, so the POS Views that depend on it can use this dependency for analytics, rather than relying on calling the shared instance of service locator. Thoughts?

Copy link
Contributor Author

@jaclync jaclync Sep 4, 2025

Choose a reason for hiding this comment

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

When reviewing Povilas' POS modularization PRs in the last HACK Week, he updated all the ServiceLocator analytics usage with an Analytics environment variable from the top level view for all POS views to access. I hope his work gets merged in soon.

Copy link
Contributor

@staskus staskus Sep 4, 2025

Choose a reason for hiding this comment

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

I think I'll bring some of these things to the trunk, without the POS module itself, so it could be used within the code now.

isLoading = true
do {
try await markComplete()
Expand All @@ -140,14 +147,12 @@ private extension PointOfSaleCollectCashView {
textFieldAmountInput: textFieldAmountInput)
}

private func validateAmountOnSubmit() -> Bool {
viewHelper.validateAmountOnSubmit(
orderTotal: orderTotal,
textFieldAmountInput: textFieldAmountInput,
onError: { error in
errorMessage = error
})
private func prefillOrderTotal() {
if let orderDecimal = viewHelper.parseCurrency(orderTotal) {
textFieldViewModel.presetAmount(orderDecimal)
}
isTextFieldFocused = true
}
}

private extension PointOfSaleCollectCashView {
Expand Down
28 changes: 5 additions & 23 deletions WooCommerce/Classes/POS/ViewHelpers/CollectCashViewHelper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,17 @@ final class CollectCashViewHelper {
}
}

func validateAmountOnSubmit(orderTotal: String,
func isPaymentButtonEnabled(orderTotal: String,
textFieldAmountInput: String,
onError: (String) -> Void) -> Bool {
let userInput = textFieldAmountInput.isNotEmpty ? textFieldAmountInput : "0"

isLoading: Bool) -> Bool {
guard let orderDecimal = parseCurrency(orderTotal),
let inputDecimal = parseCurrency(userInput) else {
onError(Localization.failedToCollectCashPayment)
return false
}

if inputDecimal < orderDecimal {
onError(Localization.cashPaymentAmountNotEnough)
let inputDecimal = parseCurrency(textFieldAmountInput.isNotEmpty ? textFieldAmountInput : "0") else {
return false
}
return true
return inputDecimal >= orderDecimal && !isLoading
Copy link
Contributor

@iamgabrielma iamgabrielma Sep 1, 2025

Choose a reason for hiding this comment

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

nit: I don't find this function specially readable, perhaps just by switching the order a bit and extracting logic from the guard makes it clearer? Could be entirely personal so feel free to ignore (untested!):

    func isPaymentButtonEnabled(orderTotal: String,
                                textFieldAmountInput: String,
                                isLoading: Bool) -> Bool {
        if isLoading { return false }

        let sanitizedInput = textFieldAmountInput.isNotEmpty ? textFieldAmountInput : "0"

        guard let orderDecimal = parseCurrency(orderTotal),
              let inputDecimal = parseCurrency(sanitizedInput) else {
            return false
        }

        return inputDecimal >= orderDecimal
    }

Copy link
Contributor Author

@jaclync jaclync Sep 4, 2025

Choose a reason for hiding this comment

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

Thanks for suggesting for better readability, updated in 7c576c0.

}

private func parseCurrency(_ amountString: String) -> Decimal? {
func parseCurrency(_ amountString: String) -> Decimal? {
// Removes all leading/trailing whitespace, if any
let sanitized = amountString.trimmingCharacters(in: .whitespacesAndNewlines)

Expand Down Expand Up @@ -101,16 +93,6 @@ private extension CollectCashViewHelper {
comment: "Change due when the cash amount entered exceeds the order total." +
"Reads as 'Change due: $1.23'"
)
static let failedToCollectCashPayment = NSLocalizedString(
"collectcashviewhelper.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(
"collectcashviewhelper.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."
)
}
}

Expand Down
Loading