Merge branch 'trunk' into issue/6128-basic-local-implementation

Author: Ecarrion

Networking/Networking/Model/ShippingLine.swift

@@ -6,7 +6,13 @@ import Codegen
 public struct ShippingLine: Codable, Equatable, GeneratedFakeable {
     public let shippingID: Int64
     public let methodTitle: String
-    public let methodID: String
+
+    /// Shipping Method ID
+    ///
+    /// Sending a null value to the REST API removes the Shipping Line.
+    ///
+    public let methodID: String?
+
     public let total: String
     public let totalTax: String
     public let taxes: [ShippingLineTax]
@@ -15,7 +21,7 @@ public struct ShippingLine: Codable, Equatable, GeneratedFakeable {
     ///
     public init(shippingID: Int64,
                 methodTitle: String,
-                methodID: String,
+                methodID: String?,
                 total: String,
                 totalTax: String,
                 taxes: [ShippingLineTax]) {

Networking/Networking/Remote/OrdersRemote.swift

@@ -274,10 +274,13 @@ public extension OrdersRemote {
         private static let commonOrderFieldValues = [
             "id", "parent_id", "number", "status", "currency", "customer_id", "customer_note", "date_created_gmt", "date_modified_gmt", "date_paid_gmt",
             "discount_total", "discount_tax", "shipping_total", "shipping_tax", "total", "total_tax", "payment_method", "payment_method_title",
-            "billing", "coupon_lines", "shipping_lines", "refunds", "fee_lines", "order_key", "tax_lines"
+            "billing", "coupon_lines", "shipping_lines", "refunds", "fee_lines", "order_key", "tax_lines", "meta_data"
         ]
+        // Use with caution. Any fields in here will be overwritten with empty values by
+        // `Order+ReadOnlyConvertible.swift: Order.update(with:)` when the list of orders is fetched.
+        // See p91TBi-7yL-p2 for discussion.
         private static let singleOrderExtraFieldValues = [
-            "line_items", "shipping", "meta_data"
+            "line_items", "shipping"
         ]
     }
 

Networking/NetworkingTests/Remote/OrdersRemoteTests.swift

@@ -211,6 +211,27 @@ final class OrdersRemoteTests: XCTestCase {
         wait(for: [expectation], timeout: Constants.expectationTimeout)
     }
 
+    func test_update_order_properly_encodes_shipping_lines_for_removal_from_order() throws {
+        // Given
+        let remote = OrdersRemote(network: network)
+        let shipping = ShippingLine(shippingID: 333, methodTitle: "Shipping", methodID: nil, total: "1.23", totalTax: "", taxes: [])
+        let order = Order.fake().copy(shippingLines: [shipping])
+
+        // When
+        remote.updateOrder(from: 123, order: order, fields: [.shippingLines]) { result in }
+
+        // Then
+        let request = try XCTUnwrap(network.requestsForResponseData.last as? JetpackRequest)
+        let received = try XCTUnwrap(request.parameters["shipping_lines"] as? [[String: AnyHashable]]).first
+        let expected: [String: AnyHashable] = [
+            "id": shipping.shippingID,
+            "method_title": shipping.methodTitle,
+            "method_id": NSNull(),
+            "total": shipping.total
+        ]
+        assertEqual(received, expected)
+    }
+
 
     // MARK: - Load Order Notes Tests
 
@@ -394,7 +415,7 @@ final class OrdersRemoteTests: XCTestCase {
         let expected: [String: AnyHashable] = [
             "id": shipping.shippingID,
             "method_title": shipping.methodTitle,
-            "method_id": shipping.methodID,
+            "method_id": shipping.methodID ?? "",
             "total": shipping.total
         ]
         assertEqual(received, expected)

RELEASE-NOTES.txt

@@ -4,6 +4,7 @@
 -----
 - [*] Orders: In the experimental Order Creation feature, product variations added to a new order now show a list of their attributes. [https://github.com/woocommerce/woocommerce-ios/pull/6131]
 - [*] Enlarged the tap area for the action button on the notice view. [https://github.com/woocommerce/woocommerce-ios/pull/6146]
+- [*] Reviews: Fixed crash on iPad when tapping the More button. [https://github.com/woocommerce/woocommerce-ios/pull/6187]
 
 8.5
 -----

WooCommerce/Classes/Analytics/WooAnalyticsStat.swift

@@ -565,6 +565,12 @@ public enum WooAnalyticsStat: String {
     case hubMenuSwitchStoreTapped = "hub_menu_switch_store_tapped"
     case hubMenuOptionTapped = "hub_menu_option_tapped"
     case hubMenuSettingsTapped = "hub_menu_settings_tapped"
+
+    // MARK: Coupons
+    case couponsLoaded = "coupons_loaded"
+    case couponsLoadedFailed = "coupons_loaded_failed"
+    case couponsListSearchTapped = "coupons_list_search_tapped"
+    case couponDetails = "coupon_details"
 }
 
 public extension WooAnalyticsStat {

WooCommerce/Classes/ViewRelated/Coupons/CouponDetails/CouponDetails.swift

@@ -34,6 +34,8 @@ struct CouponDetails: View {
         self.noticePresenter = DefaultNoticePresenter()
         viewModel.syncCoupon()
         viewModel.loadCouponReport()
+
+        ServiceLocator.analytics.track(.couponDetails, withProperties: ["action": "loaded"])
     }
 
     private var detailRows: [DetailRow] {
@@ -60,9 +62,11 @@ struct CouponDetails: View {
                                         UIPasteboard.general.string = viewModel.couponCode
                                         let notice = Notice(title: Localization.couponCopied, feedbackType: .success)
                                         noticePresenter.enqueue(notice: notice)
+                                        ServiceLocator.analytics.track(.couponDetails, withProperties: ["action": "copied_code"])
                                     }),
                                     .default(Text(Localization.shareCoupon), action: {
                                         showingShareSheet = true
+                                        ServiceLocator.analytics.track(.couponDetails, withProperties: ["action": "shared_code"])
                                     }),
                                     .cancel()
                                 ]

WooCommerce/Classes/ViewRelated/Coupons/CouponListViewController.swift

@@ -167,7 +167,7 @@ private extension CouponListViewController {
     /// Shows `SearchViewController`.
     ///
     @objc private func displaySearchCoupons() {
-        // TODO: add analytics
+        ServiceLocator.analytics.track(.couponsListSearchTapped)
         let searchViewController = SearchViewController<TitleAndSubtitleAndStatusTableViewCell, CouponSearchUICommand>(
             storeID: siteID,
             command: CouponSearchUICommand(),

WooCommerce/Classes/ViewRelated/Coupons/CouponListViewModel.swift

@@ -151,20 +151,23 @@ extension CouponListViewModel: SyncingCoordinatorDelegate {
                                 pageNumber: pageNumber,
                                 pageSize: pageSize) { [weak self] result in
                 guard let self = self else { return }
-                self.handleCouponSyncResult(result: result)
+                self.handleCouponSyncResult(result: result, pageNumber: pageNumber)
                 onCompletion?(result.isSuccess)
         }
 
         storesManager.dispatch(action)
     }
 
-    func handleCouponSyncResult(result: Result<Bool, Error>) {
+    func handleCouponSyncResult(result: Result<Bool, Error>, pageNumber: Int) {
         switch result {
         case .success:
             DDLogInfo("Synchronized coupons")
+            ServiceLocator.analytics.track(.couponsLoaded,
+                                           withProperties: ["is_loading_more": pageNumber != SyncingCoordinator.Defaults.pageFirstIndex])
 
         case .failure(let error):
             DDLogError("⛔️ Error synchronizing coupons: \(error)")
+            ServiceLocator.analytics.track(.couponsLoadedFailed, withError: error)
         }
 
         self.transitionToResultsUpdatedState(hasData: couponViewModels.isNotEmpty)

WooCommerce/Classes/ViewRelated/Inbox/Inbox.swift

@@ -18,14 +18,9 @@ struct Inbox: View {
                 InfiniteScrollList(isLoading: viewModel.shouldShowBottomActivityIndicator,
                                    loadAction: viewModel.onLoadNextPageAction) {
                     ForEach(viewModel.noteRowViewModels) { rowViewModel in
-                        if #available(iOS 15.0, *) {
-                            // In order to show full-width separator, the default list separator is hidden and a `Divider` is shown inside the row.
-                            InboxNoteRow(viewModel: rowViewModel)
-                                .listRowSeparator(.hidden)
-                        } else {
-                            InboxNoteRow(viewModel: rowViewModel)
-                        }
+                        InboxNoteRow(viewModel: rowViewModel)
                     }
+                    .background(Constants.listForeground)
                 }
             case .empty:
                 // TODO: 5954 - update empty state
@@ -34,17 +29,19 @@ struct Inbox: View {
                            image: .emptyProductsTabImage)
                     .frame(maxHeight: .infinity)
             case .syncingFirstPage:
-                List {
-                    ForEach(viewModel.placeholderRowViewModels) { rowViewModel in
-                        InboxNoteRow(viewModel: rowViewModel)
-                            .redacted(reason: .placeholder)
-                            .shimmering()
+                ScrollView {
+                    LazyVStack(spacing: 0) {
+                        ForEach(viewModel.placeholderRowViewModels) { rowViewModel in
+                            InboxNoteRow(viewModel: rowViewModel)
+                                .redacted(reason: .placeholder)
+                                .shimmering()
+                        }
+                        .background(Constants.listForeground)
                     }
                 }
-                .listStyle(PlainListStyle())
             }
         }
-        .background(Color(.listBackground).ignoresSafeArea())
+        .background(Constants.listBackground.ignoresSafeArea())
         .navigationTitle(Localization.title)
         .onAppear {
             viewModel.onLoadTrigger.send()
@@ -53,6 +50,12 @@ struct Inbox: View {
 }
 
 private extension Inbox {
+
+    enum Constants {
+        static let listForeground: Color = Color(.listForeground)
+        static let listBackground: Color = Color(.listBackground)
+    }
+
     enum Localization {
         static let title = NSLocalizedString("Inbox", comment: "Title for the screen that shows inbox notes.")
         static let emptyStateTitle = NSLocalizedString("Congrats, you’ve read everything!",

WooCommerce/Classes/ViewRelated/Inbox/InboxNoteRow.swift

@@ -5,57 +5,71 @@ import Yosemite
 struct InboxNoteRow: View {
     let viewModel: InboxNoteRowViewModel
 
+    // Tracks the scale of the view due to accessibility changes.
+    @ScaledMetric private var scale: CGFloat = 1
+
     var body: some View {
         VStack(spacing: 0) {
-            VStack(alignment: .leading,
-                   spacing: Constants.verticalSpacing) {
+            VStack(alignment: .leading, spacing: Constants.spacingBetweenTopHStackAndContentVStack) {
                 // HStack with type icon and relative date.
-                // TODO: 5954 - type icon and relative date
+                HStack {
+                    Circle()
+                        .frame(width: scale * Constants.typeIconDimension, height: scale * Constants.typeIconDimension, alignment: .center)
+                        .foregroundColor(Color(Constants.typeIconCircleColor))
+                        .overlay(
+                            viewModel.typeIcon
+                                    .resizable()
+                                    .scaledToFit()
+                                .padding(scale * Constants.typeIconPadding)
+                        )
+                    Text(viewModel.date)
+                        .font(.subheadline)
+                        .foregroundColor(Color(Constants.dateTextColor))
+                    Spacer()
+                }
 
-                // Title.
-                Text(viewModel.title)
-                    .bodyStyle()
-                    .fixedSize(horizontal: false, vertical: true)
+                VStack(alignment: .leading, spacing: Constants.verticalSpacing) {
+                    // Title.
+                    Text(viewModel.title)
+                        .bodyStyle()
+                        .fixedSize(horizontal: false, vertical: true)
 
-                // Content.
-                AttributedText(viewModel.attributedContent)
-                    .attributedTextLinkColor(Color(.accent))
+                    // Content.
+                    AttributedText(viewModel.attributedContent)
+                        .attributedTextLinkColor(Color(.accent))
 
-                // HStack with actions and dismiss action.
-                HStack(spacing: Constants.spacingBetweenActions) {
-                    ForEach(viewModel.actions) { action in
-                        if let url = action.url {
-                            Button(action.title) {
-                                // TODO: 5955 - handle action
-                                print("Handling action with URL: \(url)")
+                    // HStack with actions and dismiss action.
+                    HStack(spacing: Constants.spacingBetweenActions) {
+                        ForEach(viewModel.actions) { action in
+                            if let url = action.url {
+                                Button(action.title) {
+                                    // TODO: 5955 - handle action
+                                    print("Handling action with URL: \(url)")
+                                }
+                                .foregroundColor(Color(.accent))
+                                .font(.body)
+                                .buttonStyle(PlainButtonStyle())
+                            } else {
+                                Text(action.title)
                             }
-                            .foregroundColor(Color(.accent))
-                            .font(.body)
-                            .buttonStyle(PlainButtonStyle())
-                        } else {
-                            Text(action.title)
                         }
-                    }
-                    Button(Localization.dismiss) {
-                        // TODO: 5955 - handle dismiss action
-                        print("Handling dismiss action")
-                    }
-                    .foregroundColor(Color(.withColorStudio(.gray, shade: .shade30)))
-                    .font(.body)
-                    .buttonStyle(PlainButtonStyle())
+                        Button(Localization.dismiss) {
+                            // TODO: 5955 - handle dismiss action
+                            print("Handling dismiss action")
+                        }
+                        .foregroundColor(Color(.withColorStudio(.gray, shade: .shade30)))
+                        .font(.body)
+                        .buttonStyle(PlainButtonStyle())
 
-                    Spacer()
+                        Spacer()
+                    }
                 }
             }
                    .padding(Constants.defaultPadding)
 
-            if #available(iOS 15.0, *) {
-                // In order to show full-width separator, the default list separator is hidden and a `Divider` is shown inside the row.
-                Divider()
-                    .frame(height: Constants.dividerHeight)
-            }
+            Divider()
+                .frame(height: Constants.dividerHeight)
         }
-        .listRowInsets(.zero)
     }
 }
 
@@ -66,18 +80,25 @@ private extension InboxNoteRow {
 
     enum Constants {
         static let spacingBetweenActions: CGFloat = 16
+        static let spacingBetweenTopHStackAndContentVStack: CGFloat = 8
         static let verticalSpacing: CGFloat = 14
         static let defaultPadding: CGFloat = 16
         static let dividerHeight: CGFloat = 1
+        static let dateTextColor: UIColor = .withColorStudio(.gray, shade: .shade30)
+        static let typeIconDimension: CGFloat = 29
+        static let typeIconPadding: CGFloat = 5
+        static let typeIconCircleColor: UIColor = .init(light: .withColorStudio(.gray, shade: .shade0), dark: .withColorStudio(.gray, shade: .shade70))
     }
 }
 
 struct InboxNoteRow_Previews: PreviewProvider {
     static var previews: some View {
+        // Monday, February 14, 2022 1:04:42 PM
+        let today = Date(timeIntervalSince1970: 1644843882)
         let note = InboxNote(siteID: 2,
                              id: 6,
                              name: "",
-                             type: "",
+                             type: "marketing",
                              status: "",
                              actions: [.init(id: 2, name: "", label: "Let your customers know about Apple Pay", status: "", url: "https://wordpress.org"),
                                        .init(id: 6, name: "", label: "No URL", status: "", url: "")],
@@ -91,13 +112,33 @@ struct InboxNoteRow_Previews: PreviewProvider {
                              isRemoved: false,
                              isRead: false,
                              dateCreated: .init())
-        let viewModel = InboxNoteRowViewModel(note: note)
+        let shortNote = InboxNote(siteID: 2,
+                             id: 6,
+                             name: "",
+                             type: "",
+                             status: "",
+                             actions: [.init(id: 2, name: "", label: "Learn Apple Pay", status: "", url: "https://wordpress.org"),
+                                       .init(id: 6, name: "", label: "No URL", status: "", url: "")],
+                             title: "Boost sales this holiday season with Apple Pay!",
+                             content: "Increase your conversion rate.",
+                             isRemoved: false,
+                             isRead: false,
+                             dateCreated: today)
         Group {
-            InboxNoteRow(viewModel: viewModel)
+            List {
+                InboxNoteRow(viewModel: .init(note: note.copy(type: "marketing", dateCreated: today), today: today))
+                InboxNoteRow(viewModel: .init(note: shortNote.copy(type: "error").copy(dateCreated: today.addingTimeInterval(-6*60)), today: today))
+                InboxNoteRow(viewModel: .init(note: shortNote.copy(type: "warning").copy(dateCreated: today.addingTimeInterval(-6*3600)), today: today))
+                InboxNoteRow(viewModel: .init(note: shortNote.copy(type: "update").copy(dateCreated: today.addingTimeInterval(-6*86400)), today: today))
+                InboxNoteRow(viewModel: .init(note: shortNote.copy(type: "info").copy(dateCreated: today.addingTimeInterval(-14*86400)), today: today))
+                InboxNoteRow(viewModel: .init(note: shortNote.copy(type: "survey").copy(dateCreated: today.addingTimeInterval(-1.5*86400)), today: today))
+            }
                 .preferredColorScheme(.dark)
-            InboxNoteRow(viewModel: viewModel)
+                .environment(\.sizeCategory, .extraSmall)
+                .previewLayout(.sizeThatFits)
+            InboxNoteRow(viewModel: .init(note: note.copy(dateCreated: today.addingTimeInterval(-86400*2)), today: today))
                 .preferredColorScheme(.light)
-            InboxNoteRow(viewModel: viewModel)
+            InboxNoteRow(viewModel: .init(note: note.copy(dateCreated: today.addingTimeInterval(-6*60)), today: today))
                 .preferredColorScheme(.light)
                 .environment(\.sizeCategory, .extraExtraExtraLarge)
         }