Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,24 +97,6 @@ final class CreateOrderAddressFormViewModel: AddressFormViewModel, AddressFormVi
override func trackOnLoad() { }

func userDidCancelFlow() { }

/// Dispatches the searchCustomers action when the Search button is tapped.
/// The hardcoded `keyword` is temporary until we implement the rest of the feature:
/// https://github.com/woocommerce/woocommerce-ios/issues/7741
///
func customerSearchTapped() {
let action = CustomerAction.searchCustomers(
siteID: siteID,
keyword: "hello") { result in
switch result {
case .success(_):
print("Success")
case .failure(let error):
print(error)
}
}
stores.dispatch(action)
}
}

private extension CreateOrderAddressFormViewModel {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,17 @@ final class CustomerSearchUICommand: SearchUICommand {
}

func createStarterViewController() -> UIViewController? {
nil
createEmptyStateViewController()
}

func configureEmptyStateViewControllerBeforeDisplay(viewController: EmptyStateViewController, searchKeyword: String) {
let boldSearchKeyword = NSAttributedString(string: searchKeyword,
attributes: [.font: EmptyStateViewController.Config.messageFont.bold])
let format = Localization.emptySearchResults
let message = NSMutableAttributedString(string: format)

message.replaceFirstOccurrence(of: "%@", with: boldSearchKeyword)
viewController.configure(.simple(message: message, image: .emptySearchResultsImage))
}

func createCellViewModel(model: Customer) -> TitleAndSubtitleAndStatusTableViewCell.ViewModel {
Expand All @@ -61,10 +71,6 @@ final class CustomerSearchUICommand: SearchUICommand {
}

func didSelectSearchResult(model: Customer, from viewController: UIViewController, reloadData: () -> Void, updateActionButton: () -> Void) {
// Not implemented yet
print("1 - Customer tapped")
print("2 - Customer ID: \(model.customerID) - Name: \(model.firstName ?? ""))")
// Customer data will go up to EditOrderAddressForm, via OrderCustomerListView completion handler
onDidSelectSearchResult(model)
}

Expand All @@ -75,7 +81,11 @@ final class CustomerSearchUICommand: SearchUICommand {

private extension CustomerSearchUICommand {
enum Localization {
static let searchBarPlaceHolder = NSLocalizedString("Search all customers",
comment: "Customer Search Placeholder")
static let searchBarPlaceHolder = NSLocalizedString(
"Search all customers",
comment: "Customer Search Placeholder")
static let emptySearchResults = NSLocalizedString(
"We're sorry, we couldn't find results for “%@”",
comment: "Message for empty Customers search results. %@ is a placeholder for the text entered by the user.")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Yosemite
import SwiftUI

/// `SwiftUI` wrapper for `SearchViewController` using `CustomerSearchUICommand`
/// TODO: Make it generic
///
struct OrderCustomerListView: UIViewControllerRepresentable {

let siteID: Int64
Expand All @@ -16,15 +16,13 @@ struct OrderCustomerListView: UIViewControllerRepresentable {
storeID: siteID,
command: CustomerSearchUICommand(siteID: siteID, onDidSelectSearchResult: onCustomerTapped),
cellType: TitleAndSubtitleAndStatusTableViewCell.self,
// Must conform to SearchResultCell.
// TODO: Proper cell for this cellType.
cellSeparator: .none
)
let navigationController = WooNavigationController(rootViewController: viewController)
return navigationController
}

func updateUIViewController(_ uiViewController: UIViewControllerType, context: Context) {
// nope
// not implemented
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ protocol AddressFormViewModelProtocol: ObservableObject {
/// Creates a view model to be used when selecting a state for secondary fields
///
func createSecondaryStateViewModel() -> StateSelectorViewModel

/// Callback method when the Customer Search button is tapped
///
func customerSearchTapped()
}

/// Type to hold values from all the form fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,34 @@ struct EditOrderAddressForm<ViewModel: AddressFormViewModelProtocol>: View {

let isSearchCustomersEnabled = DefaultFeatureFlagService().isFeatureFlagEnabled(.orderCreationSearchCustomers)

private func fillCustomerFields(customer: Customer) {
// Primary fields:
self.viewModel.fields.firstName = customer.firstName ?? ""
self.viewModel.fields.lastName = customer.lastName ?? ""
self.viewModel.fields.email = customer.email
self.viewModel.fields.phone = customer.billing?.phone ?? ""
self.viewModel.fields.company = customer.billing?.company ?? ""
self.viewModel.fields.address1 = customer.billing?.address1 ?? ""
self.viewModel.fields.address2 = customer.billing?.address2 ?? ""
self.viewModel.fields.city = customer.billing?.city ?? ""
self.viewModel.fields.postcode = customer.billing?.postcode ?? ""
self.viewModel.fields.country = customer.billing?.country ?? ""
self.viewModel.fields.state = customer.billing?.state ?? ""
// Common secondary fields:
self.viewModel.secondaryFields.firstName = customer.firstName ?? ""
self.viewModel.secondaryFields.lastName = customer.lastName ?? ""
self.viewModel.secondaryFields.email = customer.email
// Secondary fields:
self.viewModel.secondaryFields.phone = customer.shipping?.phone ?? ""
self.viewModel.secondaryFields.company = customer.shipping?.company ?? ""
self.viewModel.secondaryFields.address1 = customer.shipping?.address1 ?? ""
self.viewModel.secondaryFields.address2 = customer.shipping?.address2 ?? ""
self.viewModel.secondaryFields.city = customer.shipping?.city ?? ""
self.viewModel.secondaryFields.postcode = customer.shipping?.postcode ?? ""
self.viewModel.secondaryFields.country = customer.shipping?.country ?? ""
self.viewModel.secondaryFields.state = customer.shipping?.state ?? ""
}

var body: some View {
Group {
ScrollView {
Expand Down Expand Up @@ -172,9 +200,8 @@ struct EditOrderAddressForm<ViewModel: AddressFormViewModelProtocol>: View {
.notice($viewModel.notice)
.sheet(isPresented: $showingCustomerSearch, content: {
OrderCustomerListView(siteID: viewModel.siteID, onCustomerTapped: { customer in
// Not implemented yet.
print("3 - Customer Callback. Fill Order data with Customer details")
print("4 - Customer ID: \(customer.customerID) - Name: \(customer.firstName ?? ""))")
fillCustomerFields(customer: customer)
showingCustomerSearch = false
Copy link
Contributor

Choose a reason for hiding this comment

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

This can move to the view model too if you like? Can wait for a future PR

})
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ final class EditOrderAddressFormViewModel: AddressFormViewModel, AddressFormView
func userDidCancelFlow() {
analytics.track(event: WooAnalyticsEvent.OrderDetailsEdit.orderDetailEditFlowCanceled(subject: self.analyticsFlowType()))
}

func customerSearchTapped() {}
}

extension EditOrderAddressFormViewModel {
Expand Down
12 changes: 12 additions & 0 deletions WooCommerce/WooCommerce.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,7 @@
57F2C6CD246DECC10074063B /* SummaryTableViewCellViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57F2C6CC246DECC10074063B /* SummaryTableViewCellViewModelTests.swift */; };
57F42E40253768D600EA87F7 /* TitleAndEditableValueTableViewCellViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57F42E3F253768D600EA87F7 /* TitleAndEditableValueTableViewCellViewModelTests.swift */; };
581D5052274AA2480089B6AD /* View+AutofocusTextModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 581D5051274AA2480089B6AD /* View+AutofocusTextModifier.swift */; };
682210ED2909666600814E14 /* CustomerSearchUICommandTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 682210EC2909666600814E14 /* CustomerSearchUICommandTests.swift */; };
6827140F28A3988300E6E3F6 /* DismissableNoticeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6827140E28A3988300E6E3F6 /* DismissableNoticeView.swift */; };
6827141128A5410D00E6E3F6 /* NewSimplePaymentsLocationNoticeViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6827141028A5410D00E6E3F6 /* NewSimplePaymentsLocationNoticeViewModel.swift */; };
6827141528A671B900E6E3F6 /* NewSimplePaymentsLocationNoticeViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6827141428A671B900E6E3F6 /* NewSimplePaymentsLocationNoticeViewController.swift */; };
Expand Down Expand Up @@ -2890,6 +2891,7 @@
57F2C6CC246DECC10074063B /* SummaryTableViewCellViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SummaryTableViewCellViewModelTests.swift; sourceTree = "<group>"; };
57F42E3F253768D600EA87F7 /* TitleAndEditableValueTableViewCellViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TitleAndEditableValueTableViewCellViewModelTests.swift; sourceTree = "<group>"; };
581D5051274AA2480089B6AD /* View+AutofocusTextModifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+AutofocusTextModifier.swift"; sourceTree = "<group>"; };
682210EC2909666600814E14 /* CustomerSearchUICommandTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomerSearchUICommandTests.swift; sourceTree = "<group>"; };
6827140E28A3988300E6E3F6 /* DismissableNoticeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DismissableNoticeView.swift; sourceTree = "<group>"; };
6827141028A5410D00E6E3F6 /* NewSimplePaymentsLocationNoticeViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewSimplePaymentsLocationNoticeViewModel.swift; sourceTree = "<group>"; };
6827141428A671B900E6E3F6 /* NewSimplePaymentsLocationNoticeViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewSimplePaymentsLocationNoticeViewController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5990,6 +5992,7 @@
573D0ACC2458665C004DE614 /* Search */ = {
isa = PBXGroup;
children = (
682210EB2909664800814E14 /* Customer */,
020C908224C84638001E2BEB /* Product */,
);
path = Search;
Expand Down Expand Up @@ -6165,6 +6168,14 @@
path = TitleAndEditableValueTableViewCell;
sourceTree = "<group>";
};
682210EB2909664800814E14 /* Customer */ = {
isa = PBXGroup;
children = (
682210EC2909666600814E14 /* CustomerSearchUICommandTests.swift */,
);
path = Customer;
sourceTree = "<group>";
};
6856D6E9B1C3C89938DCAD5C /* Testing */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -10593,6 +10604,7 @@
03FBDAFD263EE4E800ACE257 /* CouponListViewModelTests.swift in Sources */,
036F6EA6281847D5006D84F8 /* PaymentCaptureOrchestratorTests.swift in Sources */,
B555531321B57E8800449E71 /* MockUserNotificationsCenterAdapter.swift in Sources */,
682210ED2909666600814E14 /* CustomerSearchUICommandTests.swift in Sources */,
4590B652261C8D1E00A6FCE0 /* WeightFormatterTests.swift in Sources */,
D8C11A6022E2479800D4A88D /* OrderPaymentDetailsViewModelTests.swift in Sources */,
B622BC74289CF19400B10CEC /* WaitingTimeTrackerTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import XCTest
@testable import WooCommerce
import Yosemite

final class CustomerSearchUICommandTests: XCTestCase {
private let sampleSiteID: Int64 = 123

override func setUp() {
super.setUp()
}

override func tearDown() {
super.tearDown()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can delete these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! 7b58531


func test_searchResultsPredicate_includes_siteID_and_keyword_when_keyword() {
// Given
let command = CustomerSearchUICommand(siteID: sampleSiteID) { _ in }

// When
let predicate = command.searchResultsPredicate(keyword: "some")
let expectedQuery = "siteID == 123 AND ANY searchResults.keyword == \"some\""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Double check if this actually filters down multiple sites.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed 👍 , for 2 customers with the same name and customer ID, in two different sites, only one is shown.


// Then
XCTAssertEqual(predicate?.predicateFormat, expectedQuery)
}

func test_cellViewModel_display_correct_customer_details() {
let command = CustomerSearchUICommand(siteID: sampleSiteID) { _ in }
let customer = Customer(
siteID: sampleSiteID,
customerID: 1,
email: "[email protected]",
firstName: "John",
lastName: "W",
billing: nil,
shipping: nil
)

let cellViewModel = command.createCellViewModel(model: customer)

XCTAssertEqual(cellViewModel.id, String(customer.customerID))
XCTAssertEqual(cellViewModel.title, "\(customer.firstName ?? "") \(customer.lastName ?? "")")
XCTAssertEqual(cellViewModel.subtitle, String(customer.email))
}
}
6 changes: 6 additions & 0 deletions Yosemite/Yosemite/Stores/CustomerStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ public final class CustomerStore: Store {
var customers = [Customer]()
let group = DispatchGroup()
for result in searchResults {
// At the moment, we're not searching through non-registered customers
// As we only search by customer ID, calls to /wc/v3/customers/0 will always fail
// https://github.com/woocommerce/woocommerce-ios/issues/7741
if result.userID == 0 {
continue
}
group.enter()
self.retrieveCustomer(for: siteID, with: result.userID, onCompletion: { result in
if let customer = try? result.get() {
Expand Down
19 changes: 19 additions & 0 deletions Yosemite/YosemiteTests/Stores/CustomerStoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,23 @@ final class CustomerStoreTests: XCTestCase {
XCTAssertEqual(storedCustomer?.customerID, dummyCustomerID)
XCTAssertEqual(storedCustomer?.firstName, "John")
}

func test_searchCustomers_returns_no_customers_when_customer_is_not_registered() throws {
// Given
let customerID = 0
network.simulateResponse(requestUrlSuffix: "customers", filename: "wc-analytics-customers")
network.simulateResponse(requestUrlSuffix: "customers/\(customerID)", filename: "customer")

// When
let response: Result<[Networking.Customer], Error> = waitFor { promise in
let action = CustomerAction.searchCustomers(siteID: self.dummySiteID, keyword: self.dummyKeyword) { result in
promise(result)
}
self.dispatcher.dispatch(action)
}

// Then
let customers = try response.get()
XCTAssertEqual(customers.count, 0)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test! This is business logic which is well worth checking.

However... it doesn't really test what it looks like. I had a go at making it fail (by removing the if result.userID == 0 check in the store) and it still passed.... When you're testing a negative, it's especially worthwhile making sure that the test fails when you expect. Think Red-Green-Refactor, even when you're not actually doing TDD.

The reason is that we don't have anyone in wc-analytics-customer.json with user_id: 0, you'll need to add another entry to that file to get the test to work.

We can improve it further though... here's another way to do it, which more directly checks that the request is not sent, without needing to see what the customer response is:

(N.B. This still needs an analytics customer with user_id: 0 in the json file)

Suggested change
func test_searchCustomers_returns_no_customers_when_customer_is_not_registered() throws {
// Given
let customerID = 0
network.simulateResponse(requestUrlSuffix: "customers", filename: "wc-analytics-customers")
network.simulateResponse(requestUrlSuffix: "customers/\(customerID)", filename: "customer")
// When
let response: Result<[Networking.Customer], Error> = waitFor { promise in
let action = CustomerAction.searchCustomers(siteID: self.dummySiteID, keyword: self.dummyKeyword) { result in
promise(result)
}
self.dispatcher.dispatch(action)
}
// Then
let customers = try response.get()
XCTAssertEqual(customers.count, 0)
}
func test_searchCustomers_returns_no_customers_when_customer_is_not_registered() throws {
// Given
network.simulateResponse(requestUrlSuffix: "customers", filename: "wc-analytics-customers")
// When
waitFor { promise in
let action = CustomerAction.searchCustomers(siteID: self.dummySiteID, keyword: self.dummyKeyword) { result in
promise(())
}
self.dispatcher.dispatch(action)
}
// Then
let requests = network.requestsForResponseData.compactMap { $0 as? JetpackRequest }
XCTAssertFalse(requests.contains(where: { request in
request.path == "customers/0"
}))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's nice! Made the changes on 7b58531

}