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

Jump to bottom

[Woo POS] Extract totals logic to TotalsViewModel #13211

Merged
merged 9 commits into from
Jul 2, 2024
Merged

[Woo POS] Extract totals logic to TotalsViewModel #13211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a1f5322
Create `TotalsViewModel` and extract sync. Unit tests
iamgabrielma Jul 1, 2024
701dda3
Move methods from dashboard to TotalsViewModel
iamgabrielma Jul 1, 2024
2944fa9
Pass items from cart to calculate and sync order
iamgabrielma Jul 1, 2024
e62d9a2
Merge branch 'trunk' into task/12998-pos-extract-totals-vm
jaclync Jul 1, 2024
4fe2aed
Move derived variables from TotalsView to TotalsViewModel.
jaclync Jul 1, 2024
c56f34e
Move payment related code from `PointOfSaleDashboardViewModel` to `To…
jaclync Jul 1, 2024
f3c427a
Remove unused `POSOrderService.updateOrderStatus`.
jaclync Jul 1, 2024
85a0626
Minor polishes to privatize and remove dashboard VM dependency as muc…
jaclync Jul 1, 2024
2e8b444
Fix unit testing build failure.
jaclync Jul 1, 2024
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
3 changes: 2 additions & 1 deletion WooCommerce/Classes/POS/Presentation/CartView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ import Combine
let orderStagePublisher = orderStageSubject.eraseToAnyPublisher()
let dashboardViewModel = PointOfSaleDashboardViewModel(itemProvider: POSItemProviderPreview(),
cardPresentPaymentService: CardPresentPaymentPreviewService(),
orderService: POSOrderPreviewService())
orderService: POSOrderPreviewService(),
currencyFormatter: .init(currencySettings: .init()))
let cartViewModel = CartViewModel(orderStage: orderStagePublisher)

return CartView(viewModel: dashboardViewModel, cartViewModel: cartViewModel)
Expand Down
38 changes: 7 additions & 31 deletions WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,6 @@ struct PointOfSaleDashboardView: View {
}
.toolbarBackground(Color.toolbarBackground, for: .bottomBar)
.toolbarBackground(.visible, for: .bottomBar)
.sheet(isPresented: $viewModel.showsCardReaderSheet, content: {
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be here because now the "Connect now" button in toolbar does not work.

Copy link
Contributor

Choose a reason for hiding this comment

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

now it only works if the totals are presented.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohh great catch! I will fix this today.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was looking at this now and even if I change to how it was the sheet does not show, did we change something behind the scenes on how we set showsCardReaderSheet?

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 was looking at this now and even if I change to how it was the sheet does not show, did we change something behind the scenes on how we set showsCardReaderSheet?

It does work for me with the changes on #13226, can you check if you can still reproduce the issue on that branch?

// Might be the only way unless we make the type conform to `Identifiable`
if let alertType = viewModel.cardPresentPaymentAlertViewModel {
PointOfSaleCardPresentPaymentAlert(alertType: alertType)
} else {
switch viewModel.cardPresentPaymentEvent {
case .idle,
.show, // handled above
.showOnboarding:
Text(viewModel.cardPresentPaymentEvent.temporaryEventDescription)
}
}
})
}
}

Expand All @@ -68,10 +55,11 @@ private extension PointOfSaleDashboardView {
}

var totalsView: some View {
TotalsView(viewModel: viewModel)
.background(Color(UIColor.systemBackground))
.frame(maxWidth: .infinity)
.cornerRadius(16)
TotalsView(viewModel: viewModel,
totalsViewModel: viewModel.totalsViewModel)
.background(Color(UIColor.systemBackground))
.frame(maxWidth: .infinity)
.cornerRadius(16)
}

var productListView: some View {
Expand All @@ -80,26 +68,14 @@ private extension PointOfSaleDashboardView {
}
}

fileprivate extension CardPresentPaymentEvent {
var temporaryEventDescription: String {
switch self {
case .idle:
return "Idle"
case .show:
return "Event"
case .showOnboarding(let onboardingViewModel):
return "Onboarding: \(onboardingViewModel.state.reasonForAnalytics)" // This will only show the initial onboarding state
}
}
}

#if DEBUG
#Preview {
NavigationStack {
PointOfSaleDashboardView(
viewModel: PointOfSaleDashboardViewModel(itemProvider: POSItemProviderPreview(),
cardPresentPaymentService: CardPresentPaymentPreviewService(),
orderService: POSOrderPreviewService()))
orderService: POSOrderPreviewService(),
currencyFormatter: .init(currencySettings: .init())))
}
}
#endif
10 changes: 7 additions & 3 deletions WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.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
import protocol Yosemite.POSItemProvider
import protocol Yosemite.POSOrderServiceProtocol

Expand All @@ -10,13 +11,15 @@ struct PointOfSaleEntryPointView: View {
init(itemProvider: POSItemProvider,
hideAppTabBar: @escaping ((Bool) -> Void),
cardPresentPaymentService: CardPresentPaymentFacade,
orderService: POSOrderServiceProtocol) {
orderService: POSOrderServiceProtocol,
currencyFormatter: CurrencyFormatter) {
self.hideAppTabBar = hideAppTabBar

_viewModel = StateObject(wrappedValue: PointOfSaleDashboardViewModel(
itemProvider: itemProvider,
cardPresentPaymentService: cardPresentPaymentService,
orderService: orderService)
orderService: orderService,
currencyFormatter: currencyFormatter)
)
}

Expand All @@ -36,6 +39,7 @@ struct PointOfSaleEntryPointView: View {
PointOfSaleEntryPointView(itemProvider: POSItemProviderPreview(),
hideAppTabBar: { _ in },
cardPresentPaymentService: CardPresentPaymentPreviewService(),
orderService: POSOrderPreviewService())
orderService: POSOrderPreviewService(),
currencyFormatter: .init(currencySettings: .init()))
}
#endif
74 changes: 52 additions & 22 deletions WooCommerce/Classes/POS/Presentation/TotalsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import SwiftUI

struct TotalsView: View {
@ObservedObject private var viewModel: PointOfSaleDashboardViewModel
@ObservedObject private var totalsViewModel: TotalsViewModel

init(viewModel: PointOfSaleDashboardViewModel) {
init(viewModel: PointOfSaleDashboardViewModel, totalsViewModel: TotalsViewModel) {
self.viewModel = viewModel
self.totalsViewModel = totalsViewModel
}

var body: some View {
Expand All @@ -19,21 +21,21 @@ struct TotalsView: View {
HStack {
VStack(spacing: Constants.totalsVerticalSpacing) {
priceFieldView(title: "Subtotal",
formattedPrice: viewModel.formattedCartTotalPrice,
formattedPrice: totalsViewModel.formattedCartTotalPrice,
shimmeringActive: false,
redacted: false)
Divider()
.overlay(Color.posTotalsSeparator)
priceFieldView(title: "Taxes",
formattedPrice:
viewModel.formattedOrderTotalTaxPrice,
shimmeringActive: viewModel.isSyncingOrder,
redacted: viewModel.formattedOrderTotalTaxPrice == nil || viewModel.isSyncingOrder)
totalsViewModel.formattedOrderTotalTaxPrice,
shimmeringActive: totalsViewModel.isShimmering,
redacted: totalsViewModel.isPriceFieldRedacted)
Divider()
.overlay(Color.posTotalsSeparator)
totalPriceView(formattedPrice: viewModel.formattedOrderTotalPrice,
shimmeringActive: viewModel.isSyncingOrder,
redacted: viewModel.formattedOrderTotalPrice == nil || viewModel.isSyncingOrder)
totalPriceView(formattedPrice: totalsViewModel.formattedOrderTotalPrice,
shimmeringActive: totalsViewModel.isShimmering,
redacted: totalsViewModel.isTotalPriceFieldRedacted)
}
.padding()
.frame(idealWidth: Constants.pricesIdealWidth)
Expand All @@ -42,9 +44,11 @@ struct TotalsView: View {
RoundedRectangle(cornerRadius: Constants.defaultBorderLineCornerRadius)
.stroke(Color.posTotalsSeparator, lineWidth: Constants.defaultBorderLineWidth)
)
if viewModel.showRecalculateButton {
if totalsViewModel.showRecalculateButton {
Button("Calculate amounts") {
viewModel.calculateAmountsTapped()
totalsViewModel.calculateAmountsTapped(
with: viewModel.cartViewModel.itemsInCart,
allItems: viewModel.itemSelectorViewModel.items)
}
}
}
Expand All @@ -60,12 +64,25 @@ struct TotalsView: View {
}
}
.onDisappear {
viewModel.onTotalsViewDisappearance()
totalsViewModel.onTotalsViewDisappearance()
}
.sheet(isPresented: $totalsViewModel.showsCardReaderSheet, content: {
// Might be the only way unless we make the type conform to `Identifiable`
if let alertType = totalsViewModel.cardPresentPaymentAlertViewModel {
PointOfSaleCardPresentPaymentAlert(alertType: alertType)
} else {
switch totalsViewModel.cardPresentPaymentEvent {
case .idle,
.show, // handled above
.showOnboarding:
Text(totalsViewModel.cardPresentPaymentEvent.temporaryEventDescription)
}
}
})
}

private var gradientStops: [Gradient.Stop] {
if viewModel.paymentState == .cardPaymentSuccessful {
if totalsViewModel.paymentState == .cardPaymentSuccessful {
return [
Gradient.Stop(color: Color.clear, location: 0.0),
Gradient.Stop(color: Color.posTotalsGradientGreen, location: 1.0)
Expand All @@ -78,10 +95,6 @@ struct TotalsView: View {
]
}
}

private var paymentButtonsDisabled: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to be unused, thanks for cleaning it up.

return !viewModel.areAmountsFullyCalculated
}
}

private extension TotalsView {
Expand All @@ -107,7 +120,7 @@ private extension TotalsView {

@ViewBuilder
private var paymentsActionButtons: some View {
if viewModel.paymentState == .cardPaymentSuccessful {
if totalsViewModel.paymentState == .cardPaymentSuccessful {
newTransactionButton
}
else {
Expand All @@ -116,19 +129,19 @@ private extension TotalsView {
}

@ViewBuilder private var cardReaderView: some View {
switch viewModel.cardReaderConnectionViewModel.connectionStatus {
switch totalsViewModel.connectionStatus {
case .connected:
if let inlinePaymentMessage = viewModel.cardPresentPaymentInlineMessage {
if let inlinePaymentMessage = totalsViewModel.cardPresentPaymentInlineMessage {
PointOfSaleCardPresentPaymentInLineMessage(messageType: inlinePaymentMessage)
} else {
Text("Reader connected")
Button(action: viewModel.cardPaymentTapped) {
Button(action: totalsViewModel.cardPaymentTapped) {
Text("Collect Payment")
}
}
case .disconnected:
Text("Reader disconnected")
Button(action: viewModel.cardPaymentTapped) {
Button(action: totalsViewModel.cardPaymentTapped) {
Text("Collect Payment")
}
}
Expand Down Expand Up @@ -181,10 +194,27 @@ private extension TotalsView {
}
}

fileprivate extension CardPresentPaymentEvent {
var temporaryEventDescription: String {
switch self {
case .idle:
return "Idle"
case .show:
return "Event"
case .showOnboarding(let onboardingViewModel):
return "Onboarding: \(onboardingViewModel.state.reasonForAnalytics)" // This will only show the initial onboarding state
}
}
}

#if DEBUG
#Preview {
TotalsView(viewModel: .init(itemProvider: POSItemProviderPreview(),
cardPresentPaymentService: CardPresentPaymentPreviewService(),
orderService: POSOrderPreviewService()))
orderService: POSOrderPreviewService(),
currencyFormatter: .init(currencySettings: .init())),
totalsViewModel: .init(orderService: POSOrderPreviewService(),
cardPresentPaymentService: CardPresentPaymentPreviewService(),
currencyFormatter: .init(currencySettings: .init())))
}
#endif
Loading