Merge pull request #7528 from woocommerce/issue/7464-fix-search-filter-product

Product selector: Stop clearing product store and fix issue combining search term with filter

Author: itsmeichigo

WooCommerce/Classes/ViewRelated/Products/ProductSelector/ProductSelectorViewModel.swift

@@ -33,20 +33,7 @@ final class ProductSelectorViewModel: ObservableObject {
 
     /// Selected filter for the product list
     ///
-    var filters: FilterProductListViewModel.Filters = FilterProductListViewModel.Filters() {
-        didSet {
-            let contentIsNotSyncedYet = syncingCoordinator.highestPageBeingSynced ?? 0 == 0
-            if filters != oldValue || contentIsNotSyncedYet {
-                updateFilterButtonTitle()
-                productsResultsController.updatePredicate(siteID: siteID,
-                                                          stockStatus: filters.stockStatus,
-                                                          productStatus: filters.productStatus,
-                                                          productType: filters.productType)
-                updateProductsResultsController()
-                syncingCoordinator.resynchronize {}
-            }
-        }
-    }
+    @Published var filters = FilterProductListViewModel.Filters()
 
     /// Title of the filter button, should be updated with number of active filters.
     ///
@@ -110,9 +97,9 @@ final class ProductSelectorViewModel: ObservableObject {
 
     /// Predicate for the results controller.
     ///
-    private lazy var resultsPredicate: NSPredicate? = {
+    private var resultsPredicate: NSPredicate? {
         productsResultsController.predicate
-    }()
+    }
 
     /// Current search term entered by the user.
     /// Each update will trigger a remote product search and sync.
@@ -407,6 +394,22 @@ private extension ProductSelectorViewModel {
         }
     }
 
+    func updatePredicate(searchTerm: String, filters: FilterProductListViewModel.Filters) {
+        productsResultsController.updatePredicate(siteID: siteID,
+                                                  stockStatus: filters.stockStatus,
+                                                  productStatus: filters.productStatus,
+                                                  productType: filters.productType)
+        if searchTerm.isNotEmpty {
+            // When the search query changes, also includes the original results predicate in addition to the search keyword.
+            let searchResultsPredicate = NSPredicate(format: "ANY searchResults.keyword = %@", searchTerm)
+            let subpredicates = [resultsPredicate, searchResultsPredicate].compactMap { $0 }
+            productsResultsController.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: subpredicates)
+        } else {
+            // Resets the results to the full product list when there is no search query.
+            productsResultsController.predicate = resultsPredicate
+        }
+    }
+
     /// Setup: Syncing Coordinator
     ///
     func configureSyncingCoordinator() {
@@ -428,28 +431,22 @@ private extension ProductSelectorViewModel {
     /// Updates the product results predicate & triggers a new sync when search term changes
     ///
     func configureProductSearch() {
-        $searchTerm
+        let searchTermPublisher = $searchTerm
             .dropFirst() // Drop initial value
             .removeDuplicates()
             .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main)
-            .sink { [weak self] newSearchTerm in
-                guard let self = self else { return }
-
-                if newSearchTerm.isNotEmpty {
-                    // When the search query changes, also includes the original results predicate in addition to the search keyword.
-                    let searchResultsPredicate = NSPredicate(format: "ANY searchResults.keyword = %@", newSearchTerm)
-                    let subpredicates = [self.resultsPredicate, searchResultsPredicate].compactMap { $0 }
-                    self.productsResultsController.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: subpredicates)
-                } else {
-                    // Resets the results to the full product list when there is no search query.
-                    self.productsResultsController.predicate = self.resultsPredicate
-                }
 
+        searchTermPublisher.combineLatest($filters.removeDuplicates())
+            .sink { [weak self] searchTerm, filters in
+                guard let self = self else { return }
+                self.updateFilterButtonTitle(with: filters)
+                self.updatePredicate(searchTerm: searchTerm, filters: filters)
+                self.updateProductsResultsController()
                 self.syncingCoordinator.resynchronize()
             }.store(in: &subscriptions)
     }
 
-    func updateFilterButtonTitle() {
+    func updateFilterButtonTitle(with filters: FilterProductListViewModel.Filters) {
         let activeFiltersCount = filters.numberOfActiveFilters
         if activeFiltersCount == 0 {
             filterButtonTitle = Localization.filterButtonWithoutActiveFilters

WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/ProductSelectorViewModelTests.swift

@@ -11,6 +11,7 @@ final class ProductSelectorViewModelTests: XCTestCase {
         storageManager.viewStorage
     }
     private let stores = MockStoresManager(sessionManager: .testingInstance)
+    private let searchDebounceTime: UInt64 = 600_000_000 // 500 milliseconds with buffer
 
     override func setUp() {
         super.setUp()
@@ -472,26 +473,29 @@ final class ProductSelectorViewModelTests: XCTestCase {
         XCTAssertEqual(selectedItems, [simpleProduct.productID, 12])
     }
 
-    func test_filter_button_title_shows_correct_number_of_active_filters() {
+    func test_filter_button_title_shows_correct_number_of_active_filters() async throws {
         // Given
         let viewModel = ProductSelectorViewModel(siteID: sampleSiteID)
+        let defaultTitle = NSLocalizedString("Filter", comment: "")
         // confidence check
-        XCTAssertEqual(viewModel.filterButtonTitle, NSLocalizedString("Filter", comment: ""))
+        XCTAssertEqual(viewModel.filterButtonTitle, defaultTitle)
 
         // When
+        viewModel.searchTerm = ""
         viewModel.filters = FilterProductListViewModel.Filters(
             stockStatus: ProductStockStatus.outOfStock,
             productStatus: ProductStatus.draft,
             productType: ProductType.simple,
             productCategory: nil,
             numberOfActiveFilters: 3
         )
+        try await Task.sleep(nanoseconds: searchDebounceTime)
 
         // Then
         XCTAssertEqual(viewModel.filterButtonTitle, String.localizedStringWithFormat(NSLocalizedString("Filter (%ld)", comment: ""), 3))
     }
 
-    func test_productRows_are_updated_correctly_when_filters_are_applied() {
+    func test_productRows_are_updated_correctly_when_filters_are_applied() async throws {
         // Given
         let simpleProduct = Product.fake().copy(siteID: sampleSiteID, productID: 1, productTypeKey: ProductType.simple.rawValue, purchasable: true)
         let variableProduct = Product.fake().copy(siteID: sampleSiteID, productID: 10, productTypeKey: ProductType.variable.rawValue, purchasable: true)
@@ -507,6 +511,8 @@ final class ProductSelectorViewModelTests: XCTestCase {
             productCategory: nil,
             numberOfActiveFilters: 1
         )
+        viewModel.searchTerm = ""
+        try await Task.sleep(nanoseconds: searchDebounceTime)
 
         // Then
         XCTAssertEqual(viewModel.productRows.count, 1)
@@ -552,7 +558,7 @@ final class ProductSelectorViewModelTests: XCTestCase {
         XCTAssertEqual(variableProductRow?.selectedState, .notSelected)
     }
 
-    func test_synchronizeProducts_are_triggered_with_correct_filters() {
+    func test_synchronizeProducts_are_triggered_with_correct_filters() async throws {
         // Given
         let viewModel = ProductSelectorViewModel(siteID: sampleSiteID, stores: stores)
         var filteredStockStatus: ProductStockStatus?
@@ -581,6 +587,8 @@ final class ProductSelectorViewModelTests: XCTestCase {
 
         // When
         viewModel.filters = filters
+        viewModel.searchTerm = ""
+        try await Task.sleep(nanoseconds: searchDebounceTime)
 
         // Then
         assertEqual(filteredStockStatus, filters.stockStatus)
@@ -589,7 +597,7 @@ final class ProductSelectorViewModelTests: XCTestCase {
         assertEqual(filteredProductCategory, filters.productCategory)
     }
 
-    func test_searchProducts_are_triggered_with_correct_filters() {
+    func test_searchProducts_are_triggered_with_correct_filters() async throws {
         // Given
         let viewModel = ProductSelectorViewModel(siteID: sampleSiteID, stores: stores)
         var filteredStockStatus: ProductStockStatus?
@@ -617,15 +625,60 @@ final class ProductSelectorViewModelTests: XCTestCase {
         }
 
         // When
-        viewModel.searchTerm = "hiii"
         viewModel.filters = filters
+        viewModel.searchTerm = "hiii"
+        try await Task.sleep(nanoseconds: searchDebounceTime)
 
         // Then
         assertEqual(filteredStockStatus, filters.stockStatus)
         assertEqual(filteredProductType, filters.productType)
         assertEqual(filteredProductStatus, filters.productStatus)
         assertEqual(filteredProductCategory, filters.productCategory)
     }
+
+    func test_search_term_and_filters_are_combined_to_get_correct_results() {
+        // Given
+        let bolognese = Product.fake().copy(siteID: sampleSiteID, productID: 1, name: "Bolognese spaghetti", productTypeKey: ProductType.simple.rawValue)
+        let carbonara = Product.fake().copy(siteID: sampleSiteID, productID: 23, name: "Carbonara spaghetti", productTypeKey: ProductType.simple.rawValue)
+        let pizza = Product.fake().copy(siteID: sampleSiteID, productID: 11, name: "Pizza", productTypeKey: ProductType.variable.rawValue)
+        insert(pizza)
+        insert(bolognese, withSearchTerm: "spa")
+        insert(carbonara, withSearchTerm: "spa")
+
+        let viewModel = ProductSelectorViewModel(siteID: sampleSiteID, storageManager: storageManager)
+        XCTAssertEqual(viewModel.productRows.count, 3) // Confidence check
+
+        // When
+        viewModel.searchTerm = "spa"
+        waitUntil {
+            viewModel.productRows.count != 3
+        }
+
+        // Then
+        XCTAssertEqual(viewModel.productRows.count, 2) // 2 spaghetti
+
+        // When
+        let updatedFilters = FilterProductListViewModel.Filters(
+            stockStatus: nil,
+            productStatus: nil,
+            productType: ProductType.variable,
+            productCategory: nil,
+            numberOfActiveFilters: 1
+        )
+        viewModel.filters = updatedFilters
+
+        // Then
+        XCTAssertEqual(viewModel.productRows.count, 0) // no product matches the filter and search term
+
+        // When
+        viewModel.searchTerm = ""
+        waitUntil {
+            viewModel.productRows.isNotEmpty
+        }
+
+        // Then
+        XCTAssertEqual(viewModel.productRows.count, 1) // only 1 variable product "Pizza"
+    }
 }
 
 // MARK: - Utils

Yosemite/Yosemite/Stores/ProductStore.swift

@@ -141,11 +141,9 @@ private extension ProductStore {
                                  excludedProductIDs: excludedProductIDs) { [weak self] result in
             switch result {
             case .success(let products):
-                let shouldDeleteExistingProducts = pageNumber == Default.firstPageNumber
                 self?.upsertSearchResultsInBackground(siteID: siteID,
                                                       keyword: keyword,
-                                                      readOnlyProducts: products,
-                                                      shouldDeleteExistingProducts: shouldDeleteExistingProducts) {
+                                                      readOnlyProducts: products) {
                     onCompletion(.success(()))
                 }
             case .failure(let error):
@@ -673,13 +671,9 @@ private extension ProductStore {
     private func upsertSearchResultsInBackground(siteID: Int64,
                                                  keyword: String,
                                                  readOnlyProducts: [Networking.Product],
-                                                 shouldDeleteExistingProducts: Bool = false,
                                                  onCompletion: @escaping () -> Void) {
         let derivedStorage = sharedDerivedStorage
         derivedStorage.perform { [weak self] in
-            if shouldDeleteExistingProducts {
-                derivedStorage.deleteProducts(siteID: siteID)
-            }
             self?.upsertStoredProducts(readOnlyProducts: readOnlyProducts, in: derivedStorage)
             self?.upsertStoredResults(siteID: siteID, keyword: keyword, readOnlyProducts: readOnlyProducts, in: derivedStorage)
         }