woocommerce · joshheald · Nov 7, 2024 · Nov 7, 2024 · Nov 7, 2024 · Nov 7, 2024
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -6,6 +6,7 @@
 
 - [internal] Updated Xcode version to 16.1 [https://github.com/woocommerce/woocommerce-ios/pull/14225]
 - [**] Fixed: properly open and show order details in Watch app [https://github.com/woocommerce/woocommerce-ios/pull/14306]
+- [*] Order editing: disable editing orders in other currencies, due to risk of showing incorrect information [https://github.com/woocommerce/woocommerce-ios/pull/14345]
 - [*] Payments: Improved error messages for reader connections when using site credentials for login [https://github.com/woocommerce/woocommerce-ios/pull/14336]
 
 21.0

diff --git a/WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift b/WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift
@@ -654,6 +654,10 @@ extension WooAnalyticsEvent {
             ])
         }
 
+        static func orderEditButtonTappedWhileDisabledForCurrencyConflict() -> WooAnalyticsEvent {
+            WooAnalyticsEvent(statName: .orderEditButtonTappedWhileDisabledForCurrencyConflict, properties: [:])
+        }
+
         static func orderProductAdd(flow: Flow,
                                     source: BarcodeScanning.Source,
                                     addedVia: OrderProductAdditionVia,

diff --git a/WooCommerce/Classes/Analytics/WooAnalyticsStat.swift b/WooCommerce/Classes/Analytics/WooAnalyticsStat.swift
@@ -474,6 +474,7 @@ enum WooAnalyticsStat: String {
     case orderContactAction = "order_contact_action"
     case orderCustomerAdd = "order_customer_add"
     case orderEditButtonTapped = "order_edit_button_tapped"
+    case orderEditButtonTappedWhileDisabledForCurrencyConflict = "order_edit_button_tapped_while_disabled_for_currency_conflict"
     case ordersListFilter = "orders_list_filter"
     case ordersListSearch = "orders_list_search"
     case ordersListLoaded = "orders_list_loaded"

diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsSyncStateController.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsSyncStateController.swift
@@ -0,0 +1,16 @@
+import Foundation
+
+protocol OrderDetailsSyncStateControlling {
+    var syncState: OrderDetailsSyncState { get set }
+}
+
+struct OrderDetailsSyncStateController: OrderDetailsSyncStateControlling {
+    var syncState: OrderDetailsSyncState
+}
+
+/// Defines the possible sync states of the view model data.
+///
+enum OrderDetailsSyncState {
+    case notSynced
+    case synced
+}
diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift
@@ -21,7 +21,7 @@ final class OrderDetailsViewModel {
 
     /// Defines the current sync states of the view model data.
     ///
-    private var syncState: SyncState = .notSynced
+    private var syncStateController: OrderDetailsSyncStateControlling
 
     var orderStatus: OrderStatus? {
         return lookUpOrderStatus(for: order)
@@ -31,12 +31,14 @@ final class OrderDetailsViewModel {
          stores: StoresManager = ServiceLocator.stores,
          storageManager: StorageManagerType = ServiceLocator.storageManager,
          currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings),
-         featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) {
+         featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService,
+         syncStateController: OrderDetailsSyncStateControlling = OrderDetailsSyncStateController(syncState: .notSynced)) {
         self.order = order
         self.stores = stores
         self.storageManager = storageManager
         self.currencyFormatter = currencyFormatter
         self.featureFlagService = featureFlagService
+        self.syncStateController = syncStateController
         self.configurationLoader = CardPresentConfigurationLoader(stores: stores)
         self.dataSource = OrderDetailsDataSource(order: order,
                                                  cardPresentPaymentsConfiguration: configurationLoader.configuration)
@@ -168,8 +170,23 @@ final class OrderDetailsViewModel {
 
     /// Returns edit action availability given the internal state.
     ///
-    var editButtonIsEnabled: Bool {
-        syncState == .synced
+    var editButtonBehaviour: EditButtonBehaviour {
+        guard syncStateController.syncState == .synced else {
+            return .disabledForSyncing
+        }
+
+        guard let orderCurrency = CurrencyCode(caseInsensitiveRawValue: order.currency),
+              orderCurrency == ServiceLocator.currencySettings.currencyCode else {
+            return .showNoticeForCurrencyConflict
+        }
+
+        return .enabled
+    }
+
+    enum EditButtonBehaviour {
+        case enabled
+        case disabledForSyncing
+        case showNoticeForCurrencyConflict
     }
 
     var paymentMethodsViewModel: PaymentMethodsViewModel {
@@ -297,7 +314,7 @@ extension OrderDetailsViewModel {
 
             /// Update state to synced
             ///
-            self?.syncState = .synced
+            self?.syncStateController.syncState = .synced
 
             onReloadSections?()
             onCompletion?()
@@ -908,21 +925,11 @@ private extension OrderDetailsViewModel {
     }
 }
 
-// MARK: Definitions
-private extension OrderDetailsViewModel {
-    /// Defines the possible sync states of the view model data.
-    ///
-    enum SyncState {
-        case notSynced
-        case synced
-    }
-}
-
 // MARK: - Notices
-private extension OrderDetailsViewModel {
-    func displayReceiptRetrievalErrorNotice(for order: Order,
-                                            with error: Error?,
-                                            in viewController: UIViewController) {
+extension OrderDetailsViewModel {
+    private func displayReceiptRetrievalErrorNotice(for order: Order,
+                                                    with error: Error?,
+                                                    in viewController: UIViewController) {
         let noticePresenter = DefaultNoticePresenter()
         let notice = Notice(title: Localization.failedReceiptRetrievalNoticeText,
                             feedbackType: .error)
@@ -932,11 +939,31 @@ private extension OrderDetailsViewModel {
         DDLogError("Failed to retrieve receipt for order: \(order.orderID). Site \(order.siteID). Error: \(String(describing: error))")
     }
 
+    func showNoticeForEditingWithCurrencyConflict(in viewController: UIViewController) {
+        let siteCurrency = ServiceLocator.currencySettings.currencyCode.rawValue
+        let noticePresenter = DefaultNoticePresenter()
+        let title = String(format: Localization.editingOrderWithCurrencyConflictNoticeTitle, order.currency, siteCurrency)
+        let notice = Notice(title: title,
+                            feedbackType: .error)
+        noticePresenter.presentingViewController = viewController
+        noticePresenter.enqueue(notice: notice)
+
+        DDLogError("Attempt to edit order \(order.orderID) with currency \(order.currency), but did not match site's currency \(siteCurrency).")
+    }
+
     enum Localization {
         static let failedReceiptRetrievalNoticeText = NSLocalizedString(
             "OrderDetailsViewModel.displayReceiptRetrievalErrorNotice.notice",
             value: "Unable to retrieve receipt.",
             comment: "Notice that appears when no receipt can be retrieved upon tapping on 'See receipt' in the Order Details view.")
+
+        static let editingOrderWithCurrencyConflictNoticeTitle = NSLocalizedString(
+            "OrderDetailsViewModel.editingOrderWithCurrencyConflictNotice.title",
+            value: "Sorry, you can only edit this order on the web, as it uses %1$@, and your site's currency is %2$@.",
+            comment: "Title for notice that's shown when trying to edit an order that's in a different currency. " +
+            "This action isn't supported in the app. Placeholders: %1$@ is the order currency code (e.g. USD), " +
+            "%2$@ is the site currency code (e.g. GBP.)"
+        )
     }
 }
 

diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/OrderDetailsViewController.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/OrderDetailsViewController.swift
@@ -159,9 +159,9 @@ private extension OrderDetailsViewController {
         let editButton = UIBarButtonItem(title: Localization.NavBar.editOrder,
                                          style: .plain,
                                          target: self,
-                                         action: #selector(editOrder))
+                                         action: #selector(editButtonTapped))
         editButton.accessibilityIdentifier = "order-details-edit-button"
-        editButton.isEnabled = viewModel.editButtonIsEnabled
+        editButton.isEnabled = viewModel.editButtonBehaviour != .disabledForSyncing
         navigationItem.rightBarButtonItems = [editButton] + orderNavigationRightBarButtonItems()
 
         navigationItem.largeTitleDisplayMode = .never
@@ -388,9 +388,21 @@ private extension OrderDetailsViewController {
         }
     }
 
+    @objc private func editButtonTapped() {
+        switch viewModel.editButtonBehaviour {
+        case .enabled:
+            editOrder()
+        case .showNoticeForCurrencyConflict:
+            viewModel.showNoticeForEditingWithCurrencyConflict(in: self)
+            ServiceLocator.analytics.track(event: WooAnalyticsEvent.Orders.orderEditButtonTappedWhileDisabledForCurrencyConflict())
+        case .disabledForSyncing:
+            return
+        }
+    }
+
     /// Presents the order edit form
     ///
-    @objc private func editOrder() {
+    private func editOrder() {
         let viewModel = EditableOrderViewModel(siteID: viewModel.order.siteID, flow: .editing(initialOrder: viewModel.order))
         let viewController = OrderFormHostingController(viewModel: viewModel)
         if ServiceLocator.featureFlagService.isFeatureFlagEnabled(.sideBySideViewForOrderForm) {

diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj
@@ -853,6 +853,7 @@
 		20D5CB532AFCF8E7009A39C3 /* PaymentsToggleRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20D5CB522AFCF8E7009A39C3 /* PaymentsToggleRow.swift */; };
 		20DA6DDB2B681175002AA0FB /* AdaptiveModalContainer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DA6DDA2B681175002AA0FB /* AdaptiveModalContainer.swift */; };
 		20E188842AD059A50053E945 /* AboutTapToPayView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20E188832AD059A50053E945 /* AboutTapToPayView.swift */; };
+		20FA73882CDCC3A900554BE3 /* OrderDetailsSyncStateController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20FA73872CDCC3A900554BE3 /* OrderDetailsSyncStateController.swift */; };
 		247CE89C2583402A00F9D9D1 /* Embassy in Frameworks */ = {isa = PBXBuildFile; productRef = 247CE89B2583402A00F9D9D1 /* Embassy */; };
 		247CE8A6258340E600F9D9D1 /* ScreenshotImages.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 247CE8A5258340E600F9D9D1 /* ScreenshotImages.xcassets */; };
 		24C5AC7625A53021008FD769 /* Embassy in Frameworks */ = {isa = PBXBuildFile; productRef = 247CE89B2583402A00F9D9D1 /* Embassy */; };
@@ -3961,6 +3962,7 @@
 		20D5CB522AFCF8E7009A39C3 /* PaymentsToggleRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentsToggleRow.swift; sourceTree = "<group>"; };
 		20DA6DDA2B681175002AA0FB /* AdaptiveModalContainer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdaptiveModalContainer.swift; sourceTree = "<group>"; };
 		20E188832AD059A50053E945 /* AboutTapToPayView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AboutTapToPayView.swift; sourceTree = "<group>"; };
+		20FA73872CDCC3A900554BE3 /* OrderDetailsSyncStateController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OrderDetailsSyncStateController.swift; sourceTree = "<group>"; };
 		247CE8A5258340E600F9D9D1 /* ScreenshotImages.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = ScreenshotImages.xcassets; sourceTree = "<group>"; };
 		24C579D124F476300076E1B4 /* Woo-Alpha.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = "Woo-Alpha.entitlements"; sourceTree = "<group>"; };
 		24F98C4F2502AEE200F49B68 /* EventLogging.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventLogging.swift; sourceTree = "<group>"; };
@@ -12349,6 +12351,7 @@
 				CECC758D23D2260E00486676 /* Refunded Products */,
 				02619856256B539400E321E9 /* Shipping Labels */,
 				CE85535C209B5BB700938BDC /* OrderDetailsViewModel.swift */,
+				20FA73872CDCC3A900554BE3 /* OrderDetailsSyncStateController.swift */,
 				0375799828201F750083F2E1 /* CardPresentPaymentsReadinessUseCase.swift */,
 				D817585D22BB5E8700289CFE /* OrderEmailComposer.swift */,
 				D817586122BB64C300289CFE /* OrderDetailsNotices.swift */,
@@ -16262,6 +16265,7 @@
 				DA0DBE2F2C4FC61D00DF14C0 /* POSFloatingControlView.swift in Sources */,
 				DE02ABAD2B55288D008E0AC4 /* BlazeTargetLocationSearchView.swift in Sources */,
 				45BBFBC3274FDA6400213001 /* HubMenuViewController.swift in Sources */,
+				20FA73882CDCC3A900554BE3 /* OrderDetailsSyncStateController.swift in Sources */,
 				451B1747258BD7B600836277 /* AddAttributeOptionsViewModel.swift in Sources */,
 				B958640A2A657F44002C4C6E /* EnhancedCouponListView.swift in Sources */,
 				B9E4364E287589E200883CFA /* BadgeView.swift in Sources */,

diff --git a/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift
@@ -213,7 +213,48 @@ final class OrderDetailsViewModelTests: XCTestCase {
         let viewModel = OrderDetailsViewModel(order: order)
 
         // Then
-        XCTAssertFalse(viewModel.editButtonIsEnabled)
+        XCTAssertEqual(viewModel.editButtonBehaviour, OrderDetailsViewModel.EditButtonBehaviour.disabledForSyncing)
+    }
+
+    // Context: https://github.com/woocommerce/woocommerce-ios/issues/14304
+    func test_there_should_not_be_an_edit_order_action_if_order_currency_doesnt_match_site_currency() {
+        // Given
+        let gbp = CurrencySettings(currencyCode: .GBP,
+                                   currencyPosition: .left,
+                                   thousandSeparator: "",
+                                   decimalSeparator: ".",
+                                   numberOfDecimals: 2)
+        ServiceLocator.setCurrencySettings(gbp)
+
+        let usdOrder = Order.fake().copy(currency: "usd", total: "10.0")
+
+        let syncStateController = OrderDetailsSyncStateController(syncState: .synced)
+
+        // When
+        let viewModel = OrderDetailsViewModel(order: usdOrder, syncStateController: syncStateController)
+
+        // Then
+        XCTAssertEqual(viewModel.editButtonBehaviour, OrderDetailsViewModel.EditButtonBehaviour.showNoticeForCurrencyConflict)
+    }
+
+    func test_the_edit_order_action_should_be_enabled_when_the_order_is_synced_and_matches_site_currency() {
+        // Given
+        let usd = CurrencySettings(currencyCode: .USD,
+                                   currencyPosition: .left,
+                                   thousandSeparator: "",
+                                   decimalSeparator: ".",
+                                   numberOfDecimals: 2)
+        ServiceLocator.setCurrencySettings(usd)
+
+        let usdOrder = Order.fake().copy(currency: "usd", total: "10.0")
+
+        let syncStateController = OrderDetailsSyncStateController(syncState: .synced)
+
+        // When
+        let viewModel = OrderDetailsViewModel(order: usdOrder, syncStateController: syncStateController)
+
+        // Then
+        XCTAssertEqual(viewModel.editButtonBehaviour, OrderDetailsViewModel.EditButtonBehaviour.enabled)
     }
 
     func test_paymentMethodsViewModel_title_contains_formatted_order_amount() {

diff --git a/...e/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsViewControllerTests.swift b/...e/WooCommerceTests/ViewRelated/Orders/Order Details/OrderDetailsViewControllerTests.swift
@@ -77,14 +77,17 @@ final class OrderDetailsViewControllerTests: XCTestCase {
     }
 
     @MainActor
-    func test_edit_order_button_is_visible_and_navigates_to_edit_order_screen() throws {
+    func test_edit_order_button_is_visible_and_navigates_to_edit_order_screen_when_order_synced() throws {
         // Given
         let presentationVerifier = PresentationVerifier()
         let storageManager = MockStorageManager()
         let order = MockOrders().sampleOrder()
         let storesManager = OrderDetailStoreManagerFactory.createManager(order: order)
 
-        let viewModel = OrderDetailsViewModel(order: order, stores: storesManager, storageManager: storageManager)
+        let viewModel = OrderDetailsViewModel(order: order,
+                                              stores: storesManager,
+                                              storageManager: storageManager,
+                                              syncStateController: OrderDetailsSyncStateController(syncState: .synced))
         let viewController = OrderDetailsViewController(viewModel: viewModel)
 
         // When