[Product] Update product tag store to migrate Core Data usage and fix a few issues

RELEASE-NOTES.txt

@@ -4,6 +4,7 @@
 -----
 - [*] Jetpack Setup: Fixed an issue where the WordPress.com authentication fails when using a passwordless account that's already connected to Jetpack [https://github.com/woocommerce/woocommerce-ios/pull/14501]
 - [*] Jetpack Setup: Fixed an issue with magic link handling when the app is cold started. [https://github.com/woocommerce/woocommerce-ios/pull/14502]
+- [*] Product Tags: Improved the tag list screen by displaying the cached items on first load and fixed issue clearing all tags [https://github.com/woocommerce/woocommerce-ios/pull/14511]
 - [**] Media Library: On sites logged in with application password, when picking image from WordPress Media Library, all images will now load correctly. [https://github.com/woocommerce/woocommerce-ios/pull/14444]
 
 21.2

WooCommerce/Classes/ViewRelated/Products/Edit Product/Edit Tags/ProductTagsViewController.swift

@@ -105,7 +105,7 @@ private extension ProductTagsViewController {
         navigationItem.setRightBarButton(UIBarButtonItem(title: Strings.saveButton,
                                                          style: .done,
                                                          target: self,
-                                                         action: #selector(addTagsRemotely)),
+                                                         action: #selector(confirmTags)),
                                          animated: true)
         navigationItem.rightBarButtonItem?.isEnabled = true
     }
@@ -207,7 +207,14 @@ extension ProductTagsViewController: KeyboardScrollable {
 //
 private extension ProductTagsViewController {
     func loadTags() {
-        displayGhostContent(over: tableView)
+        let cachedTags = self.fetchedTags
+        if cachedTags.isEmpty {
+            /// Display the loading state only cached items are unavailable.
+            displayGhostContent(over: tableView)
+        } else {
+            /// Otherwise, display cached items right away.
+            tagsLoaded(tags: cachedTags.map { $0.name } )
+        }
 
         let action = ProductTagAction.synchronizeAllProductTags(siteID: product.siteID) { [weak self] error in
             if let error = error {
@@ -232,12 +239,21 @@ private extension ProductTagsViewController {
                                            searchQuery: partialTag)
     }
 
-    @objc func addTagsRemotely() {
+    @objc func confirmTags() {
         ServiceLocator.analytics.track(.productTagSettingsDoneButtonTapped, withProperties: [
             "has_changed_data": hasUnsavedChanges()
         ])
 
         textView.resignFirstResponder()
+
+        guard allTags.isNotEmpty else {
+            return onCompletion([])
+        }
+
+        createNewTagsRemotely()
+    }
+
+    func createNewTagsRemotely() {
         configureRightBarButtonItemAsSpinner()
 
         let action = ProductTagAction.addProductTags(siteID: product.siteID, tags: allTags) { [weak self] (result) in

Yosemite/Yosemite/Stores/ProductTagStore.swift

@@ -7,10 +7,6 @@ import Storage
 public final class ProductTagStore: Store {
     private let remote: ProductTagsRemote
 
-    private lazy var sharedDerivedStorage: StorageType = {
-        return storageManager.writerDerivedStorage
-    }()
-
     public override init(dispatcher: Dispatcher, storageManager: StorageManagerType, network: Network) {
         self.remote = ProductTagsRemote(network: network)
         super.init(dispatcher: dispatcher, storageManager: storageManager, network: network)
@@ -80,10 +76,6 @@ private extension ProductTagStore {
 
             switch result {
             case .success(let productTags):
-                if pageNumber == Default.firstPageNumber {
-                    self?.deleteUnusedStoredProductTags(siteID: siteID)
-                }
-
                 self?.upsertStoredProductTagsInBackground(productTags, siteID: siteID) {
                     onCompletion(.success(productTags))
                 }
@@ -96,6 +88,9 @@ private extension ProductTagStore {
     /// Create new product tags associated with a given Site ID.
     ///
     func addProductTags(siteID: Int64, tags: [String], onCompletion: @escaping (Result<[ProductTag], Error>) -> Void) {
+        guard tags.isEmpty == false else {
+            return onCompletion(.success([]))
+        }
         remote.createProductTags(for: siteID, names: tags) { [weak self] (result) in
             switch result {
             case .success(let productTags):
@@ -114,8 +109,9 @@ private extension ProductTagStore {
         remote.deleteProductTags(for: siteID, ids: ids) { [weak self] (result) in
             switch result {
             case .success(let productTags):
-                self?.deleteStoredProductTags(siteID: siteID, ids: ids)
-                onCompletion(.success(productTags))
+                self?.deleteStoredProductTags(siteID: siteID, ids: ids) {
+                    onCompletion(.success(productTags))
+                }
             case .failure(let error):
                 onCompletion(.failure(error))
             }
@@ -130,32 +126,19 @@ private extension ProductTagStore {
     /// onCompletion will be called on the main thread!
     ///
     func upsertStoredProductTagsInBackground(_ readOnlyProductTags: [Networking.ProductTag],
-                                                   siteID: Int64,
-                                                   onCompletion: @escaping () -> Void) {
-        let derivedStorage = sharedDerivedStorage
-        derivedStorage.perform { [weak self] in
-            self?.upsertStoredProductTags(readOnlyProductTags, in: derivedStorage, siteID: siteID)
-        }
-
-        storageManager.saveDerivedType(derivedStorage: derivedStorage) {
-            DispatchQueue.main.async(execute: onCompletion)
-        }
-    }
-
-    /// Deletes any Storage.ProductTag  that is not associated to a product on the specified `siteID`
-    ///
-    func deleteUnusedStoredProductTags(siteID: Int64) {
-        let storage = storageManager.viewStorage
-        storage.deleteUnusedProductTags(siteID: siteID)
-        storage.saveIfNeeded()
+                                             siteID: Int64,
+                                             onCompletion: @escaping () -> Void) {
+        storageManager.performAndSave({ [weak self] storage in
+            self?.upsertStoredProductTags(readOnlyProductTags, in: storage, siteID: siteID)
+        }, completion: onCompletion, on: .main)
     }
 
-    /// Deletes any Storage.ProductTag with the specified `siteID` and `productID`
+    /// Deletes any Storage.ProductTag with the specified `siteID` and `tagID`'s
     ///
-    func deleteStoredProductTags(siteID: Int64, ids: [Int64]) {
-        let storage = storageManager.viewStorage
-        storage.deleteProductTags(siteID: siteID, ids: ids)
-        storage.saveIfNeeded()
+    func deleteStoredProductTags(siteID: Int64, ids: [Int64], onCompletion: @escaping () -> Void) {
+        storageManager.performAndSave({ storage in
+            storage.deleteProductTags(siteID: siteID, ids: ids)
+        }, completion: onCompletion, on: .main)
     }
 
 }
@@ -169,12 +152,13 @@ private extension ProductTagStore {
     ///     - siteID: site ID for looking up the ProductTag.
     ///
     func upsertStoredProductTags(_ readOnlyProductTags: [Networking.ProductTag],
-                                       in storage: StorageType,
-                                       siteID: Int64) {
+                                 in storage: StorageType,
+                                 siteID: Int64) {
+        let storedItems = storage.loadProductTags(siteID: siteID)
         // Upserts the ProductTag models from the read-only version
         for readOnlyProductTag in readOnlyProductTags {
             let storageProductTag: Storage.ProductTag = {
-                if let storedTag = storage.loadProductTag(siteID: siteID, tagID: readOnlyProductTag.tagID) {
+                if let storedTag = storedItems.first(where: { $0.tagID == readOnlyProductTag.tagID }) {
                     return storedTag
                 }
                 return storage.insertNewObject(ofType: Storage.ProductTag.self)

Yosemite/YosemiteTests/Stores/ProductTagStoreTests.swift

@@ -58,7 +58,7 @@ final class ProductTagStoreTests: XCTestCase {
         super.tearDown()
     }
 
-    func testSynchronizeProductTagsReturnsTagsUponSuccessfulResponse() throws {
+    func test_synchronizeAllProductTags_saves_all_tags_to_storage() throws {
         // Given a stubed product-tags network response
         network.simulateResponse(requestUrlSuffix: "products/tags", filename: "product-tags-all")
         network.simulateResponse(requestUrlSuffix: "products/tags", filename: "product-tags-empty")
@@ -238,51 +238,42 @@ final class ProductTagStoreTests: XCTestCase {
         XCTAssertNotNil(result?.failure)
     }
 
-    func testAddProductTagReturnsErrorUponEmptyResponse() {
-        // Given an empty network response
-        XCTAssertEqual(storedProductTagsCount, 0)
-
-        // When dispatching a `addProductTags` action
+    func test_addProductTags_returns_success_for_empty_tag_list() throws {
+        // When
         var result: Result<[Networking.ProductTag], Error>?
         waitForExpectation { (exp) in
-            let action = ProductTagAction.addProductTags(siteID: sampleSiteID, tags: ["Round toe", "Flat"]) { (aResult) in
+            let action = ProductTagAction.addProductTags(siteID: sampleSiteID, tags: []) { (aResult) in
                 result = aResult
                 exp.fulfill()
             }
             store.onAction(action)
         }
 
-        // Then no tags should be stored
-        XCTAssertEqual(storedProductTagsCount, 0)
-        XCTAssertNotNil(result?.failure)
-    }
+        // Then
+        let receivedResult = try XCTUnwrap(result)
+        XCTAssertTrue(receivedResult.isSuccess)
 
-    func testSynchronizeProductTagsDeletesUnusedTags() {
-        // Given some stored product tags without product relationships
-        let sampleTags = (1...5).map { id in
-            return sampleTag(tagID: id)
-        }
-        sampleTags.forEach { tag in
-            storageManager.insertSampleProductTag(readOnlyProductTag: tag)
-        }
-        XCTAssertEqual(storedProductTagsCount, sampleTags.count)
+        let addedTags = try receivedResult.get()
+        XCTAssertTrue(addedTags.isEmpty)
+    }
 
-        // When dispatching a `synchronizeAllProductTags` action
-        network.simulateResponse(requestUrlSuffix: "products/tags", filename: "product-tags-all")
-        network.simulateResponse(requestUrlSuffix: "products/tags", filename: "product-tags-empty")
+    func testAddProductTagReturnsErrorUponEmptyResponse() {
+        // Given an empty network response
+        XCTAssertEqual(storedProductTagsCount, 0)
 
-        var errorResponse: ProductTagActionError?
+        // When dispatching a `addProductTags` action
+        var result: Result<[Networking.ProductTag], Error>?
         waitForExpectation { (exp) in
-            let action = ProductTagAction.synchronizeAllProductTags(siteID: sampleSiteID) { error in
-                errorResponse = error
+            let action = ProductTagAction.addProductTags(siteID: sampleSiteID, tags: ["Round toe", "Flat"]) { (aResult) in
+                result = aResult
                 exp.fulfill()
             }
             store.onAction(action)
         }
 
-        // Then new tag should be stored and old tags should be deleted
-        XCTAssertEqual(storedProductTagsCount, 4)
-        XCTAssertNil(errorResponse)
+        // Then no tags should be stored
+        XCTAssertEqual(storedProductTagsCount, 0)
+        XCTAssertNotNil(result?.failure)
     }
 
     func testDeleteProductTagDeleteStoredTagSuccessfulResponse() {