Merge pull request #6569 from woocommerce/issue/6568-product-reviews-list-empty-crash

Product Reviews: Empty list and crash when syncing remotely for the first time

Author: toupper

WooCommerce/Classes/ViewRelated/Products/Edit Product/Reviews/ProductReviewsViewController.swift

@@ -172,21 +172,6 @@ extension ProductReviewsViewController: UITableViewDelegate {
 // MARK: - Placeholders
 //
 private extension ProductReviewsViewController {
-
-    /// Renders Placeholder Reviews.
-    ///
-    func displayPlaceholderReviews() {
-        displayGhostContent()
-        viewModel.didDisplayPlaceholderReviews()
-    }
-
-    /// Removes Placeholder Reviews.
-    ///
-    func removePlaceholderReviews() {
-        removeGhostContent()
-        viewModel.didRemovePlaceholderReviews(tableView: tableView)
-    }
-
     /// Displays the EmptyStateViewController.
     ///
     func displayEmptyViewController() {
@@ -250,7 +235,7 @@ private extension ProductReviewsViewController {
         case .results:
             break
         case .placeholder:
-            displayPlaceholderReviews()
+            displayGhostContent()
         case .syncing:
             ensureFooterSpinnerIsStarted()
         }
@@ -265,7 +250,7 @@ private extension ProductReviewsViewController {
         case .results:
             break
         case .placeholder:
-            removePlaceholderReviews()
+            removeGhostContent()
         case .syncing:
             ensureFooterSpinnerIsStopped()
         }

WooCommerce/Classes/ViewRelated/Products/Edit Product/Reviews/ProductReviewsViewModel.swift

@@ -27,16 +27,6 @@ final class ProductReviewsViewModel {
         self.data = data
     }
 
-    func didDisplayPlaceholderReviews() {
-        data.stopForwardingEvents()
-    }
-
-    /// Removes Placeholder Notes (and restores the ResultsController <> UITableView link).
-    ///
-    func didRemovePlaceholderReviews(tableView: UITableView) {
-        data.startForwardingEvents(to: tableView)
-    }
-
     func configureResultsController(tableView: UITableView) {
         data.startForwardingEvents(to: tableView)
 

WooCommerce/Classes/ViewRelated/Reviews/ReviewsDataSource.swift

@@ -55,8 +55,4 @@ protocol ReviewsDataSource: UITableViewDataSource, ReviewsInteractionDelegate {
     /// Force a refresh of entities observing data collections
     ///
     func refreshDataObservers()
-
-    /// Cancels forwarding events to any previously registered table view
-    ///
-    func stopForwardingEvents()
 }

WooCommerce/Classes/ViewRelated/Reviews/ReviewsViewController.swift

@@ -169,8 +169,8 @@ final class ReviewsViewController: UIViewController, GhostableViewController {
         refreshControl.resetAnimation(in: tableView) { [unowned self] in
             // ghost animation is also removed after switching tabs
             // show make sure it's displayed again
-            self.removePlaceholderReviews()
-            self.displayPlaceholderReviews()
+            self.removeGhostContent()
+            self.displayGhostContent()
         }
     }
 
@@ -338,20 +338,6 @@ private extension ReviewsViewController {
 //
 private extension ReviewsViewController {
 
-    /// Renders Placeholder Reviews.
-    ///
-    func displayPlaceholderReviews() {
-        displayGhostContent()
-        viewModel.didDisplayPlaceholderReviews()
-    }
-
-    /// Removes Placeholder Reviews.
-    ///
-    func removePlaceholderReviews() {
-        removeGhostContent()
-        viewModel.didRemovePlaceholderReviews(tableView: tableView)
-    }
-
     /// Displays the EmptyStateViewController.
     ///
     func displayEmptyViewController() {
@@ -446,7 +432,7 @@ private extension ReviewsViewController {
         case .results:
             break
         case .placeholder:
-            displayPlaceholderReviews()
+            displayGhostContent()
         case .syncing(let pageNumber):
             if pageNumber != SyncingCoordinator.Defaults.pageFirstIndex {
                 ensureFooterSpinnerIsStarted()
@@ -463,10 +449,10 @@ private extension ReviewsViewController {
         case .results:
             break
         case .placeholder:
-            removePlaceholderReviews()
+            removeGhostContent()
         case .syncing:
             ensureFooterSpinnerIsStopped()
-            removePlaceholderReviews()
+            removeGhostContent()
         }
     }
 

WooCommerce/Classes/ViewRelated/Reviews/ReviewsViewModel.swift

@@ -26,10 +26,6 @@ protocol ReviewsViewModelOutput {
 /// Handles actions related to Reviews screen
 ///
 protocol ReviewsViewModelActionsHandler {
-    func didDisplayPlaceholderReviews()
-
-    func didRemovePlaceholderReviews(tableView: UITableView)
-
     func configureResultsController(tableView: UITableView)
 
     func refreshResults()
@@ -88,17 +84,6 @@ final class ReviewsViewModel: ReviewsViewModelOutput, ReviewsViewModelActionsHan
         self.stores = stores
     }
 
-    func didDisplayPlaceholderReviews() {
-        data.stopForwardingEvents()
-    }
-
-    /// Restores the ResultsController <> UITableView link after the placeholder was removed from the view.
-    ///
-    func didRemovePlaceholderReviews(tableView: UITableView) {
-        data.startForwardingEvents(to: tableView)
-        tableView.reloadData()
-    }
-
     func configureResultsController(tableView: UITableView) {
         data.startForwardingEvents(to: tableView)
 

WooCommerce/WooCommerceTests/Reviews/ReviewsViewModelTests.swift

@@ -40,29 +40,6 @@ final class ReviewsViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.isEmpty, mockDataSource.isEmpty)
     }
 
-    func testDisplayPlaceHolderReviewsStopsForWardingEventsInDataSource() {
-        // Given
-        let mockDataSource = MockReviewsDataSource()
-        let viewModel = ReviewsViewModel(siteID: sampleSiteID, data: mockDataSource)
-        viewModel.didDisplayPlaceholderReviews()
-
-        // Then
-        XCTAssertTrue(mockDataSource.stopsForwardingEventsWasHit)
-    }
-
-    func testRemovePlaceHolderReviewsStartsForWardingEventsInDataSource() {
-        // Given
-        let table = UITableView()
-        let mockDataSource = MockReviewsDataSource()
-        let viewModel = ReviewsViewModel(siteID: sampleSiteID, data: mockDataSource)
-
-        // When
-        viewModel.didRemovePlaceholderReviews(tableView: table)
-
-        // Then
-        XCTAssertTrue(mockDataSource.startForwardingEventsWasHit)
-    }
-
     func testConfigureResultsControllerStartsForWardingEventsAndStartsObservingReviewsInDataSource() {
         // Given
         let table = UITableView()

WooCommerce/WooCommerceTests/ViewRelated/Products/Edit Product/ProductReviewsViewModelTests.swift

@@ -34,23 +34,6 @@ final class ProductReviewsViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.isEmpty, mockDataSource.isEmpty)
     }
 
-    func test_did_display_placeHolder_reviews_stops_forwarding_events_in_dataSource() {
-        let ds = mockDataSource as! MockProductReviewsDataSource
-
-        viewModel.didDisplayPlaceholderReviews()
-
-        XCTAssertTrue(ds.stopsForwardingEventsWasHit)
-    }
-
-    func test_did_remove_placeHolder_reviews_starts_forwarding_events_in_dataSource() {
-        let table = UITableView()
-        let ds = mockDataSource as! MockProductReviewsDataSource
-
-        viewModel.didRemovePlaceholderReviews(tableView: table)
-
-        XCTAssertTrue(ds.startForwardingEventsWasHit)
-    }
-
     func test_configure_resultsController_starts_forwarding_events_and_starts_observing_reviews_in_dataSource() {
         let table = UITableView()
         let ds = mockDataSource as! MockProductReviewsDataSource