Merge branch 'trunk' into fix/explat-include-anonid-in-loggedin-requests

Author: jaclync

.rubocop.yml

@@ -1,3 +1,10 @@
+AllCops:
+  Exclude:
+    - DerivedData/**/*
+    - Pods/**/*
+    - vendor/**/*
+  NewCops: enable
+
 Metrics/BlockLength:
   Exclude:
     - fastlane/Fastfile
@@ -12,6 +19,7 @@ Style/AsciiComments:
 Layout/LineLength:
   Exclude:
     - fastlane/Fastfile
+    - Podfile
 
 Naming/FileName:
   Exclude:

Networking/Networking/Network/MockNetwork.swift

@@ -165,7 +165,10 @@ private extension MockNetwork {
                 return responseQueue[keyAndQueue.key]?.dequeue()
             }
         } else {
-            if let filename = responseMap.filter({ searchPath.hasSuffix($0.key) }).first?.value {
+            if let filename = responseMap.filter({ searchPath.hasSuffix($0.key) })
+                // In cases where a suffix is a substring of another suffix, the longer suffix is preferred in matched results.
+                .sorted(by: { $0.key.count > $1.key.count })
+                .first?.value {
                 return filename
             }
         }

Networking/Networking/Remote/LeaderboardsRemote.swift

@@ -29,14 +29,42 @@ public class LeaderboardsRemote: Remote {
         let mapper = LeaderboardListMapper()
         enqueue(request, mapper: mapper, completion: completion)
     }
+
+    /// Fetch the leaderboards with the deprecated API for a given site under WooCommerce version 6.7,
+    /// depending on the given granularity of the `unit` parameter.
+    ///
+    /// - Parameters:
+    ///   - siteID: The site ID
+    ///   - unit: Defines the granularity of the stats we are fetching (one of 'hour', 'day', 'week', 'month', or 'year')
+    ///   - earliestDateToInclude: The earliest date to include in the results. This string is ISO8601 compliant
+    ///   - latestDateToInclude: The latest date to include in the results. This string is ISO8601 compliant
+    ///   - quantity: Number of results to fetch
+    ///   - completion: Closure to be executed upon completion
+    ///
+    public func loadLeaderboardsDeprecated(for siteID: Int64,
+                                           unit: StatsGranularityV4,
+                                           earliestDateToInclude: String,
+                                           latestDateToInclude: String,
+                                           quantity: Int,
+                                           completion: @escaping (Result<[Leaderboard], Error>) -> Void) {
+        let parameters = [ParameterKeys.interval: unit.rawValue,
+                          ParameterKeys.after: earliestDateToInclude,
+                          ParameterKeys.before: latestDateToInclude,
+                          ParameterKeys.quantity: String(quantity)]
+
+        let request = JetpackRequest(wooApiVersion: .wcAnalytics, method: .get, siteID: siteID, path: Constants.pathDeprecated, parameters: parameters)
+        let mapper = LeaderboardListMapper()
+        enqueue(request, mapper: mapper, completion: completion)
+    }
 }
 
 
 // MARK: - Constants
 //
 private extension LeaderboardsRemote {
     enum Constants {
-        static let path = "leaderboards"
+        static let pathDeprecated = "leaderboards"
+        static let path = "leaderboards/products"
     }
 
     enum ParameterKeys {

Networking/NetworkingTests/Remote/LeaderboardsRemoteTests.swift

@@ -5,34 +5,70 @@ import XCTest
 ///
 final class LeaderboardsRemoteV4Tests: XCTestCase {
 
-    let network = MockNetwork()
-    let sampleSiteID: Int64 = 1234
+    private let network = MockNetwork()
+    private let sampleSiteID: Int64 = 1234
 
     override func setUp() {
         super.setUp()
         network.removeAllSimulatedResponses()
     }
 
-    func testLeaderboardReturnsCorrectParsedValues() throws {
+    func test_leaderboard_returns_correctly_parsed_values() throws {
         // Given
         let remote = LeaderboardsRemote(network: network)
-        network.simulateResponse(requestUrlSuffix: "leaderboards", filename: "leaderboards-year")
+        network.simulateResponse(requestUrlSuffix: "leaderboards/products", filename: "leaderboards-year")
 
         // When
-        var remoteResult: Result<[Leaderboard], Error>?
-        waitForExpectation { exp in
-            remote.loadLeaderboards(for: sampleSiteID,
+        let result = waitFor { promise in
+            remote.loadLeaderboards(for: self.sampleSiteID,
                                     unit: .yearly,
                                     earliestDateToInclude: "2020-01-01T00:00:00",
                                     latestDateToInclude: "2020-12-31T23:59:59",
                                     quantity: 3) { result in
-                                        remoteResult = result
-                                        exp.fulfill()
+                promise(result)
+            }
+        }
+
+        // Then
+        let leaderboards = try XCTUnwrap(result.get())
+
+        // API Returns 4 leaderboards
+        XCTAssertEqual(leaderboards.count, 4)
+
+        // The 4th leaderboard contains the top products and should not be empty
+        let topProducts = leaderboards[3]
+        XCTAssertFalse(topProducts.rows.isEmpty)
+
+        // Each product should have non-empty values
+        let expectedValues = [(quantity: 4, total: 20000.0), (quantity: 1, total: 15.99)]
+        zip(topProducts.rows, expectedValues).forEach { product, expectedValue in
+            XCTAssertFalse(product.subject.display.isEmpty)
+            XCTAssertFalse(product.subject.value.isEmpty)
+            XCTAssertFalse(product.quantity.display.isEmpty)
+            XCTAssertEqual(product.quantity.value, expectedValue.quantity)
+            XCTAssertFalse(product.total.display.isEmpty)
+            XCTAssertEqual(product.total.value, expectedValue.total)
+        }
+    }
+
+    func test_leaderboardDeprecated_returns_correctly_parsed_values() throws {
+        // Given
+        let remote = LeaderboardsRemote(network: network)
+        network.simulateResponse(requestUrlSuffix: "leaderboards", filename: "leaderboards-year")
+
+        // When
+        let result = waitFor { promise in
+            remote.loadLeaderboardsDeprecated(for: self.sampleSiteID,
+                                              unit: .yearly,
+                                              earliestDateToInclude: "2020-01-01T00:00:00",
+                                              latestDateToInclude: "2020-12-31T23:59:59",
+                                              quantity: 3) { result in
+                promise(result)
             }
         }
 
         // Then
-        let leaderboards = try XCTUnwrap(remoteResult?.get())
+        let leaderboards = try XCTUnwrap(result.get())
 
         // API Returns 4 leaderboards
         XCTAssertEqual(leaderboards.count, 4)
@@ -41,7 +77,7 @@ final class LeaderboardsRemoteV4Tests: XCTestCase {
         let topProducts = leaderboards[3]
         XCTAssertFalse(topProducts.rows.isEmpty)
 
-        // Each prodcut should have non-empty values
+        // Each product should have non-empty values
         let expectedValues = [(quantity: 4, total: 20000.0), (quantity: 1, total: 15.99)]
         zip(topProducts.rows, expectedValues).forEach { product, expectedValue in
             XCTAssertFalse(product.subject.display.isEmpty)
@@ -53,7 +89,7 @@ final class LeaderboardsRemoteV4Tests: XCTestCase {
         }
     }
 
-    func testLeaderboardsProperlyRelaysNetwokingErrors() {
+    func testLeaderboardsProperlyRelaysNetworkingErrors() {
         // Given
         let remote = LeaderboardsRemote(network: network)
 

Podfile

@@ -332,6 +332,7 @@ post_install do |installer|
 
   # Flag Alpha builds for Tracks
   # ============================
+  # rubocop:disable Style/CombinableLoops
   installer.pods_project.targets.each do |target|
     next unless target.name == 'Automattic-Tracks-iOS'
 
@@ -341,4 +342,5 @@ post_install do |installer|
       config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] ||= ['$(inherited)', 'ALPHA=1']
     end
   end
+  # rubocop:enable Style/CombinableLoops
 end

WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift

@@ -370,6 +370,8 @@ extension WooAnalyticsEvent {
             static let orderID = "id"
             static let hasMultipleShippingLines = "has_multiple_shipping_lines"
             static let hasMultipleFeeLines = "has_multiple_fee_lines"
+            static let errorMessage = "error_message"
+            static let validationScenario = "validation_scenario"
         }
 
         static func orderOpen(order: Order) -> WooAnalyticsEvent {
@@ -488,6 +490,25 @@ extension WooAnalyticsEvent {
         static func pluginsNotSyncedYet() -> WooAnalyticsEvent {
             WooAnalyticsEvent(statName: .pluginsNotSyncedYet, properties: [:])
         }
+
+        /// Tracked when the Order details view detects a malformed shipping address data.
+        ///
+        static func addressValidationFailed(error: AddressValidator.AddressValidationError, orderID: Int64) -> WooAnalyticsEvent {
+            switch error {
+            case .local(let errorMessage):
+                return WooAnalyticsEvent(statName: .orderAddressValidationError, properties: [
+                    Keys.errorMessage: errorMessage,
+                    Keys.validationScenario: "local",
+                    Keys.orderID: orderID
+                ])
+            case .remote(let error):
+                return WooAnalyticsEvent(statName: .orderAddressValidationError, properties: [
+                    Keys.errorMessage: "\(String(describing: error?.addressError ?? "Unknown"))",
+                    Keys.validationScenario: "remote",
+                    Keys.orderID: orderID
+                ])
+            }
+        }
     }
 }
 

WooCommerce/Classes/Analytics/WooAnalyticsStat.swift

@@ -269,6 +269,7 @@ public enum WooAnalyticsStat: String {
     case collectPaymentTapped = "payments_flow_order_collect_payment_tapped"
     case orderViewCustomFieldsTapped = "order_view_custom_fields_tapped"
     case orderDetailWaitingTimeLoaded = "order_detail_waiting_time_loaded"
+    case orderAddressValidationError = "order_address_validation_error"
 
     // MARK: Order List Sorting/Filtering
     //

WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift

@@ -15,6 +15,7 @@ final class OrderDetailsViewModel {
     private let stores: StoresManager
     private let storageManager: StorageManagerType
     private let currencyFormatter: CurrencyFormatter
+    private let addressValidator: AddressValidator
 
     private(set) var order: Order
 
@@ -34,6 +35,7 @@ final class OrderDetailsViewModel {
         self.stores = stores
         self.storageManager = storageManager
         self.currencyFormatter = currencyFormatter
+        addressValidator = AddressValidator(siteID: order.siteID, stores: stores)
     }
 
     func update(order newOrder: Order) {
@@ -544,6 +546,11 @@ extension OrderDetailsViewModel {
     func syncShippingLabels(onCompletion: ((Error?) -> ())? = nil) {
         // If the plugin is not active, there is no point on continuing with a request that will fail.
         isPluginActive(SitePlugin.SupportedPlugin.WCShip) { [weak self] isActive in
+            // We're looking to measure how often an order contains an invalid address
+            // and validate it through the Shipping label plugin, so we need to trigger
+            // this call when it's possible to verify the plugin presence
+            self?.startAddressValidation(shippingLabelPluginIsActive: isActive)
+
             guard let self = self, isActive else {
                 onCompletion?(nil)
                 return
@@ -569,6 +576,20 @@ extension OrderDetailsViewModel {
         }
     }
 
+    func startAddressValidation(shippingLabelPluginIsActive: Bool) {
+        guard let orderShippingAddress = order.shippingAddress, orderShippingAddress != Address.empty else {
+            return
+        }
+
+        let orderID = order.orderID
+
+        addressValidator.validate(address: orderShippingAddress, onlyLocally: !shippingLabelPluginIsActive) { result in
+            if let error = result.failure {
+                ServiceLocator.analytics.track(event: WooAnalyticsEvent.Orders.addressValidationFailed(error: error, orderID: orderID))
+            }
+        }
+    }
+
     func syncSavedReceipts(onCompletion: ((Error?) -> ())? = nil) {
         let action = ReceiptAction.loadReceipt(order: order) { [weak self] result in
             switch result {

WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Shipping Address Validation/AddressValidator.swift

@@ -0,0 +1,88 @@
+import Yosemite
+
+/// Reusable implementation of the Address validation introduced by the
+/// `ShippingLabelAddressFormViewModel`, but striped of the specifics to serve the form UI
+/// 
+class AddressValidator {
+    let siteID: Int64
+    private let stores: StoresManager
+
+    init(siteID: Int64, stores: StoresManager) {
+        self.siteID = siteID
+        self.stores = stores
+    }
+
+    func validate(address: Address, onlyLocally: Bool, onCompletion: @escaping (Result<Void, AddressValidationError>) -> Void) {
+        let convertedAddress = ShippingLabelAddress(company: address.company ?? "",
+                                                    name: address.fullName,
+                                                    phone: address.phone ?? "",
+                                                    country: address.country,
+                                                    state: address.state,
+                                                    address1: address.address1 ,
+                                                    address2: address.address2 ?? "",
+                                                    city: address.city,
+                                                    postcode: address.postcode)
+
+        let localErrors = validateAddressLocally(addressToBeValidated: convertedAddress)
+        if localErrors.isNotEmpty {
+            let localErrorMessage = localErrors.map { $0.rawValue }.joined(separator: ", ")
+            onCompletion(.failure(.local(localErrorMessage)))
+            return
+        }
+
+        if onlyLocally {
+            onCompletion(.success(()))
+            return
+        }
+
+        let addressToBeVerified = ShippingLabelAddressVerification(address: convertedAddress, type: .destination)
+        let action = ShippingLabelAction.validateAddress(siteID: siteID, address: addressToBeVerified) { result in
+            switch result {
+            case .success:
+                onCompletion(.success(()))
+            case .failure(let error):
+                onCompletion(.failure(.remote(error as? ShippingLabelAddressValidationError)))
+            }
+        }
+        stores.dispatch(action)
+    }
+
+    private func validateAddressLocally(addressToBeValidated: ShippingLabelAddress) -> [LocalValidationError] {
+        var errors: [LocalValidationError] = []
+
+        if addressToBeValidated.name.isEmpty && addressToBeValidated.company.isEmpty {
+            errors.append(.name)
+        }
+        if addressToBeValidated.address1.isEmpty {
+            errors.append(.address)
+        }
+        if addressToBeValidated.city.isEmpty {
+            errors.append(.city)
+        }
+        if addressToBeValidated.postcode.isEmpty {
+            errors.append(.postcode)
+        }
+        if addressToBeValidated.state.isEmpty {
+            errors.append(.state)
+        }
+        if addressToBeValidated.country.isEmpty {
+            errors.append(.country)
+        }
+
+        return errors
+    }
+
+    enum LocalValidationError: String {
+        case name = "Customer name and company are empty"
+        case address = "Address is empty"
+        case city = "City is empty"
+        case postcode = "Postcode is empty"
+        case state = "State is empty"
+        case country = "Country is empty"
+    }
+
+    enum AddressValidationError: Error {
+        case local(String)
+        case remote(ShippingLabelAddressValidationError?)
+    }
+}