Skip to content

Commit 4e170c4

Browse files
pmusolinopmusolino
authored
Merge pull request #5337 from toupper/issue/5159-sync-productCategory-remotely
Filter Products by Category: synchronize remotely the Filter Product Category
2 parents bc0c0cc + 289f043 commit 4e170c4

File tree

8 files changed

+355
-105
lines changed

8 files changed

+355
-105
lines changed

‎Networking/Networking/Model/DotcomError.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ public enum DotcomError: Error, Decodable, Equatable, GeneratedFakeable {
3939
/// Jetpack site stats module disabled
4040
case statsModuleDisabled
4141

42+
/// The requested resourced does not exist remotely
43+
case resourceDoesNotExist
44+
4245
/// Decodable Initializer.
4346
///
4447
public init(from decoder: Decoder) throws {
@@ -59,6 +62,8 @@ public enum DotcomError: Error, Decodable, Equatable, GeneratedFakeable {
5962
self = .noRestRoute
6063
case Constants.invalidBlog where message == ErrorMessages.statsModuleDisabled:
6164
self = .statsModuleDisabled
65+
case Constants.restTermInvalid where message == ErrorMessages.resourceDoesNotExist:
66+
self = .resourceDoesNotExist
6267
default:
6368
self = .unknown(code: error, message: message)
6469
}
@@ -73,6 +78,7 @@ public enum DotcomError: Error, Decodable, Equatable, GeneratedFakeable {
7378
static let invalidToken = "invalid_token"
7479
static let requestFailed = "http_request_failed"
7580
static let noRestRoute = "rest_no_route"
81+
static let restTermInvalid = "woocommerce_rest_term_invalid"
7682
}
7783

7884
/// Coding Keys
@@ -87,6 +93,7 @@ public enum DotcomError: Error, Decodable, Equatable, GeneratedFakeable {
8793
private enum ErrorMessages {
8894
static let statsModuleDisabled = "This blog does not have the Stats module enabled"
8995
static let noStatsPermission = "user cannot view stats"
96+
static let resourceDoesNotExist = "Resource does not exist."
9097
}
9198
}
9299

@@ -111,6 +118,8 @@ extension DotcomError: CustomStringConvertible {
111118
return NSLocalizedString("Dotcom No Stats Permission", comment: "WordPress.com error thrown when the user has no permission to view site stats.")
112119
case .statsModuleDisabled:
113120
return NSLocalizedString("Dotcom Stats Module Disabled", comment: "WordPress.com error thrown when the Jetpack site stats module is disabled.")
121+
case .resourceDoesNotExist:
122+
return NSLocalizedString("Dotcom Resource does not exist", comment: "WordPress.com error thrown when a requested resource does not exist remotely.")
114123
case .unknown(let code, let message):
115124
let theMessage = message ?? String()
116125
let messageFormat = NSLocalizedString(

‎Networking/Networking/Remote/ProductCategoriesRemote.swift‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,23 @@ public final class ProductCategoriesRemote: Remote {
3131
enqueue(request, mapper: mapper, completion: completion)
3232
}
3333

34+
/// Loads a remote `ProductCategory`
35+
///
36+
/// - Parameters:
37+
/// - categoryID: Category Id of the requested product category.
38+
/// - siteID: Site from which we'll fetch the remote product category.
39+
/// - completion: Closure to be executed upon completion.
40+
///
41+
public func loadProductCategory(with categoryID: Int64,
42+
siteID: Int64,
43+
completion: @escaping (Result<ProductCategory, Error>) -> Void) -> Void {
44+
let path = Path.categories + "/\(categoryID)"
45+
let request = JetpackRequest(wooApiVersion: .mark3, method: .get, siteID: siteID, path: path)
46+
let mapper = ProductCategoryMapper(siteID: siteID)
47+
48+
enqueue(request, mapper: mapper, completion: completion)
49+
}
50+
3451
/// Create a new `ProductCategory`.
3552
///
3653
/// - Parameters:

‎Networking/NetworkingTests/Remote/ProductCategoriesRemoteTests.swift‎

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,22 @@ final class ProductCategoriesRemoteTests: XCTestCase {
2727

2828
/// Verifies that loadAllProductCategories properly parses the `categories-all` sample response.
2929
///
30-
func testLoadAllProductCategoriesProperlyReturnsParsedProductCategories() {
30+
func test_loadAllProductCategories_properly_then_it_returns_parsed_product_categories() {
3131
// Given
3232
let remote = ProductCategoriesRemote(network: network)
33-
let expectation = self.expectation(description: "Load All Product Categories")
3433

3534
network.simulateResponse(requestUrlSuffix: "products/categories", filename: "categories-all")
3635

3736
// When
38-
var result: (categories: [ProductCategory]?, error: Error?)
39-
remote.loadAllProductCategories(for: sampleSiteID) { categories, error in
40-
result = (categories, error)
41-
expectation.fulfill()
42-
}
37+
let result: (categories: [ProductCategory]?, error: Error?) = waitFor { [weak self] promise in
38+
guard let self = self else {
39+
return
40+
}
4341

44-
wait(for: [expectation], timeout: Constants.expectationTimeout)
42+
remote.loadAllProductCategories(for: self.sampleSiteID) { categories, error in
43+
promise((categories, error))
44+
}
45+
}
4546

4647
// Then
4748
XCTAssertNil(result.error)
@@ -51,19 +52,20 @@ final class ProductCategoriesRemoteTests: XCTestCase {
5152

5253
/// Verifies that loadAllProductCategories properly relays Networking Layer errors.
5354
///
54-
func testLoadAllProductCategoriesProperlyRelaysNetwokingErrors() {
55+
func test_loadAllProductCategories_properly_then_it_relays_networking_errors() {
5556
// Given
5657
let remote = ProductCategoriesRemote(network: network)
57-
let expectation = self.expectation(description: "Load All Product Categories returns error")
5858

5959
// When
60-
var result: (categories: [ProductCategory]?, error: Error?)
61-
remote.loadAllProductCategories(for: sampleSiteID) { categories, error in
62-
result = (categories, error)
63-
expectation.fulfill()
64-
}
60+
let result: (categories: [ProductCategory]?, error: Error?) = waitFor { [weak self] promise in
61+
guard let self = self else {
62+
return
63+
}
6564

66-
wait(for: [expectation], timeout: Constants.expectationTimeout)
65+
remote.loadAllProductCategories(for: self.sampleSiteID) { categories, error in
66+
promise((categories, error))
67+
}
68+
}
6769

6870
// Then
6971
XCTAssertNil(result.categories)
@@ -74,7 +76,7 @@ final class ProductCategoriesRemoteTests: XCTestCase {
7476

7577
/// Verifies that createProductCategory properly parses the `category` sample response.
7678
///
77-
func testCreateProductCategoryProperlyReturnsParsedProductCategory() {
79+
func test_createProductCategory_properly_then_it_returns_parsed_product_category() {
7880
// Given
7981
let remote = ProductCategoriesRemote(network: network)
8082

@@ -97,23 +99,68 @@ final class ProductCategoriesRemoteTests: XCTestCase {
9799

98100
/// Verifies that createProductCategory properly relays Networking Layer errors.
99101
///
100-
func testCreateProductCategoryProperlyRelaysNetwokingErrors() {
102+
func test_createProductCategory_properly_then_it_relays_networking_errors() {
101103
// Given
102104
let remote = ProductCategoriesRemote(network: network)
103-
let expectation = self.expectation(description: "Create Product Category returns error")
104105

105106
// When
106-
var result: Result<ProductCategory, Error>?
107-
remote.createProductCategory(for: sampleSiteID, name: "Dress", parentID: 0) { aResult in
108-
result = aResult
109-
expectation.fulfill()
110-
}
107+
let result: Result<ProductCategory, Error>? = waitFor { [weak self] promise in
108+
guard let self = self else {
109+
return
110+
}
111111

112-
wait(for: [expectation], timeout: Constants.expectationTimeout)
112+
remote.createProductCategory(for: self.sampleSiteID, name: "Dress", parentID: 0) { aResult in
113+
promise(aResult)
114+
}
115+
}
113116

114117
// Then
115118
XCTAssertNil(try? result?.get())
116119
XCTAssertNotNil(result?.failure)
117120
}
118121

122+
func test_loadProductCategory_then_returns_parsed_ProductCategory() {
123+
// Given
124+
let remote = ProductCategoriesRemote(network: network)
125+
let categoryID: Int64 = 44
126+
127+
network.simulateResponse(requestUrlSuffix: "products/categories/\(categoryID)", filename: "category")
128+
129+
// When
130+
let result: Result<ProductCategory, Error>? = waitFor { [weak self] promise in
131+
guard let self = self else {
132+
return
133+
}
134+
135+
remote.loadProductCategory(with: categoryID, siteID: self.sampleSiteID) { aResult in
136+
promise(aResult)
137+
}
138+
}
139+
140+
// Then
141+
XCTAssertNil(result?.failure)
142+
XCTAssertNotNil(try result?.get())
143+
XCTAssertEqual(try result?.get().name, "Dress")
144+
}
145+
146+
func test_loadProductCategory_network_fails_then_returns_error() {
147+
// Given
148+
let remote = ProductCategoriesRemote(network: network)
149+
let categoryID: Int64 = 44
150+
151+
// When
152+
let result: Result<ProductCategory, Error>? = waitFor { [weak self] promise in
153+
guard let self = self else {
154+
return
155+
}
156+
157+
remote.loadProductCategory(with: categoryID, siteID: self.sampleSiteID) { aResult in
158+
promise(aResult)
159+
}
160+
}
161+
162+
// Then
163+
XCTAssertNil(try? result?.get())
164+
XCTAssertNotNil(result?.failure)
165+
}
119166
}

‎WooCommerce/Classes/ViewRelated/Products/Categories/ProductCategoryListViewModel.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ private extension ProductCategoryListViewModel {
225225
let retryToken = RetryToken(fromPageNumber: pageNumber)
226226
syncCategoriesState = .failed(retryToken)
227227
DDLogError("⛔️ Error fetching product categories: \(rawError.localizedDescription)")
228+
default:
229+
break
228230
}
229231
}
230232
}

‎WooCommerce/Classes/ViewRelated/Products/ProductsViewController.swift‎

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,9 @@ final class ProductsViewController: UIViewController {
113113

114114
private var filters: FilterProductListViewModel.Filters = FilterProductListViewModel.Filters() {
115115
didSet {
116-
if filters != oldValue {
116+
let contentIsNotSyncedYet = syncingCoordinator.highestPageBeingSynced ?? 0 == 0
117+
if filters != oldValue ||
118+
contentIsNotSyncedYet {
117119
updateLocalProductSettings(sort: sortOrder,
118120
filters: filters)
119121
updateFilterButtonTitle(filters: filters)
@@ -169,16 +171,7 @@ final class ProductsViewController: UIViewController {
169171
registerTableViewCells()
170172

171173
showTopBannerViewIfNeeded()
172-
173-
/// We sync the local product settings for configuring local sorting and filtering.
174-
/// If there are some info stored when this screen is loaded, the data will be updated using the stored sort/filters.
175-
/// If no info are stored (so there is a failure), we resynchronize the syncingCoordinator for updating the screen using the default sort/filters.
176-
///
177-
syncLocalProductsSettings { [weak self] (result) in
178-
if result.isFailure {
179-
self?.syncingCoordinator.resynchronize()
180-
}
181-
}
174+
syncProductsSettings()
182175
}
183176

184177
override func viewWillAppear(_ animated: Bool) {
@@ -566,6 +559,19 @@ private extension ProductsViewController {
566559
}
567560
displayNoResultsOverlay()
568561
}
562+
563+
/// We sync the local product settings for configuring local sorting and filtering.
564+
/// If there are some info stored when this screen is loaded, the data will be updated using the stored sort/filters.
565+
/// If any of the filters has to be synchronize remotely, it is done so after the filters are loaded, and the data updated if necessary.
566+
/// If no info are stored (so there is a failure), we resynchronize the syncingCoordinator for updating the screen using the default sort/filters.
567+
///
568+
func syncProductsSettings() {
569+
syncLocalProductsSettings { [weak self] (result) in
570+
if result.isFailure {
571+
self?.syncingCoordinator.resynchronize()
572+
}
573+
}
574+
}
569575
}
570576

571577
// MARK: - UITableViewDataSource Conformance
@@ -871,21 +877,65 @@ extension ProductsViewController: SyncingCoordinatorDelegate {
871877
let action = AppSettingsAction.loadProductsSettings(siteID: siteID) { [weak self] (result) in
872878
switch result {
873879
case .success(let settings):
874-
if let sort = settings.sort {
875-
self?.sortOrder = ProductsSortOrder(rawValue: sort) ?? .default
880+
self?.syncProductCategoryFilterRemotely(from: settings) { settings in
881+
if let sort = settings.sort {
882+
self?.sortOrder = ProductsSortOrder(rawValue: sort) ?? .default
883+
}
884+
885+
self?.filters = FilterProductListViewModel.Filters(stockStatus: settings.stockStatusFilter,
886+
productStatus: settings.productStatusFilter,
887+
productType: settings.productTypeFilter,
888+
productCategory: settings.productCategoryFilter,
889+
numberOfActiveFilters: settings.numberOfActiveFilters())
890+
876891
}
877-
self?.filters = FilterProductListViewModel.Filters(stockStatus: settings.stockStatusFilter,
878-
productStatus: settings.productStatusFilter,
879-
productType: settings.productTypeFilter,
880-
productCategory: settings.productCategoryFilter,
881-
numberOfActiveFilters: settings.numberOfActiveFilters())
882892
case .failure:
883893
break
884894
}
885895
onCompletion(result)
886896
}
887897
ServiceLocator.stores.dispatch(action)
888898
}
899+
900+
/// Syncs the Product Category filter of settings remotely. This is necessary in case the category information was updated
901+
/// or the category itself removed.
902+
///
903+
private func syncProductCategoryFilterRemotely(from settings: StoredProductSettings.Setting,
904+
onCompletion: @escaping (StoredProductSettings.Setting) -> Void) {
905+
guard let productCategory = settings.productCategoryFilter else {
906+
onCompletion(settings)
907+
return
908+
}
909+
910+
let action = ProductCategoryAction.synchronizeProductCategory(siteID: siteID, categoryID: productCategory.categoryID) { result in
911+
var updatingProductCategory: ProductCategory? = productCategory
912+
913+
switch result {
914+
case .success(let productCategory):
915+
updatingProductCategory = productCategory
916+
case .failure(let error):
917+
if let error = error as? ProductCategoryActionError,
918+
case .categoryDoesNotExistRemotely = error {
919+
// The product category was removed
920+
updatingProductCategory = nil
921+
}
922+
}
923+
924+
var completionSettings = settings
925+
if updatingProductCategory != productCategory {
926+
completionSettings = StoredProductSettings.Setting(siteID: settings.siteID,
927+
sort: settings.sort,
928+
stockStatusFilter: settings.stockStatusFilter,
929+
productStatusFilter: settings.productStatusFilter,
930+
productTypeFilter: settings.productTypeFilter,
931+
productCategoryFilter: updatingProductCategory)
932+
}
933+
934+
onCompletion(completionSettings)
935+
}
936+
937+
ServiceLocator.stores.dispatch(action)
938+
}
889939
}
890940

891941
// MARK: - Finite State Machine Management

‎Yosemite/Yosemite/Actions/ProductCategoryAction.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ public enum ProductCategoryAction: Action {
1515
///
1616
case addProductCategory(siteID: Int64, name: String, parentID: Int64?, onCompletion: (Result<ProductCategory, Error>) -> Void)
1717

18+
/// Synchronizes the ProductCategory matching the specified categoryID.
19+
/// `onCompletion` will be invoked when the sync operation finishes. `error` will be nill if the operation succeed.
20+
///
21+
case synchronizeProductCategory(siteID: Int64, categoryID: Int64, onCompletion: (Result<ProductCategory, Error>) -> Void)
1822
}
1923

2024
/// Defines all errors that a `ProductCategoryAction` can return
@@ -23,4 +27,8 @@ public enum ProductCategoryActionError: Error {
2327
/// Represents a product category synchronization failed state
2428
///
2529
case categoriesSynchronization(pageNumber: Int, rawError: Error)
30+
31+
/// The requested category cannot be found remotely
32+
///
33+
case categoryDoesNotExistRemotely
2634
}

0 commit comments

Comments
 (0)