From 10f05effd44055c89827242769a943e0421c0982 Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Wed, 19 Jul 2023 13:14:59 +0100 Subject: [PATCH 1/5] Ensure sync error is set before handling sync errors --- .../StoreStatsAndTopPerformersViewController.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift index e0c21105131..bcff03cda23 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift @@ -238,9 +238,9 @@ private extension StoreStatsAndTopPerformersViewController { DDLogError("⛔️ Error synchronizing order stats: \(error)") periodSyncError = error } - group.leave() periodGroup.leave() periodStoreStatsGroup.leave() + group.leave() } group.enter() @@ -254,9 +254,9 @@ private extension StoreStatsAndTopPerformersViewController { DDLogError("⛔️ Error synchronizing visitor stats: \(error)") periodSyncError = error } - group.leave() periodGroup.leave() periodStoreStatsGroup.leave() + group.leave() } group.enter() @@ -270,9 +270,9 @@ private extension StoreStatsAndTopPerformersViewController { DDLogError("⛔️ Error synchronizing summary stats: \(error)") periodSyncError = error } - group.leave() periodGroup.leave() periodStoreStatsGroup.leave() + group.leave() } group.enter() @@ -286,8 +286,8 @@ private extension StoreStatsAndTopPerformersViewController { DDLogError("⛔️ Error synchronizing top earners stats: \(error)") periodSyncError = error } - group.leave() periodGroup.leave() + group.leave() vc.removeTopPerformersGhostContent() } From c4b645970ed17d00eee98c7153a61c5fbf26b0d1 Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Wed, 19 Jul 2023 13:32:28 +0100 Subject: [PATCH 2/5] Prevent stats sync error for stores not connected to WPCom --- .../Dashboard/DashboardViewModel.swift | 10 +++++++ .../Dashboard/DashboardViewModelTests.swift | 30 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift index 824a8beff9e..e8e103e8ec8 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift @@ -113,6 +113,11 @@ final class DashboardViewModel { timeRange: StatsTimeRangeV4, latestDateToInclude: Date, onCompletion: ((Result) -> Void)? = nil) { + guard stores.isAuthenticatedWithoutWPCom == false else { // Visit stats are only available for stores connected to WPCom + onCompletion?(.success(())) + return + } + let action = StatsActionV4.retrieveSiteVisitStats(siteID: siteID, siteTimezone: siteTimezone, timeRange: timeRange, @@ -132,6 +137,11 @@ final class DashboardViewModel { timeRange: StatsTimeRangeV4, latestDateToInclude: Date, onCompletion: ((Result) -> Void)? = nil) { + guard stores.isAuthenticatedWithoutWPCom == false else { // Summary stats are only available for stores connected to WPCom + onCompletion?(.success(())) + return + } + let action = StatsActionV4.retrieveSiteSummaryStats(siteID: siteID, siteTimezone: siteTimezone, period: timeRange.summaryStatsGranularity, diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift index 89ab03a220d..4bb5f1921d6 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/DashboardViewModelTests.swift @@ -773,6 +773,36 @@ final class DashboardViewModelTests: XCTestCase { let dictionary = try XCTUnwrap(userDefaults[.hasDismissedBlazeBanner] as? [String: Bool]) XCTAssertEqual(dictionary["\(sampleSiteID)"], true) } + + func test_wpcom_stats_not_synced_when_authenticated_without_wpcom() { + // Given + stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false)) + stores.whenReceivingAction(ofType: StatsActionV4.self) { action in + switch action { + case .retrieveSiteVisitStats, .retrieveSiteSummaryStats: + XCTFail("WPCom stats should not be synced when store is authenticated without WPCom") + default: + XCTFail("Received unsupported action: \(action)") + } + } + let viewModel = DashboardViewModel(siteID: sampleSiteID, stores: stores) + + // When + let siteVisitStatsResult: Result = waitFor { promise in + viewModel.syncSiteVisitStats(for: self.sampleSiteID, siteTimezone: .current, timeRange: .thisMonth, latestDateToInclude: .init()) { result in + promise(result) + } + } + let siteSummaryStatsResult: Result = waitFor { promise in + viewModel.syncSiteSummaryStats(for: self.sampleSiteID, siteTimezone: .current, timeRange: .thisMonth, latestDateToInclude: .init()) { result in + promise(result) + } + } + + // Then + XCTAssertTrue(siteVisitStatsResult.isSuccess) + XCTAssertTrue(siteSummaryStatsResult.isSuccess) + } } private extension DashboardViewModelTests { From 5804ef92fe51fa43a1ab68d464af955107cfd48c Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Wed, 19 Jul 2023 14:11:27 +0100 Subject: [PATCH 3/5] Hide error banner optimistically on pull to refresh --- .../Classes/ViewRelated/Dashboard/DashboardViewController.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewController.swift index b479018a56b..383b41bbb46 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewController.swift @@ -931,6 +931,7 @@ private extension DashboardViewController { func onPullToRefresh() async { ServiceLocator.analytics.track(.dashboardPulledToRefresh) + hideTopBannerView() // Hide error banner optimistically on pull to refresh await withTaskGroup(of: Void.self) { group in group.addTask { [weak self] in guard let self else { return } From b7ca30400272b4551822771fec6448223f8062af Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Wed, 19 Jul 2023 15:34:34 +0100 Subject: [PATCH 4/5] Add error banner fix to 14.6 release notes --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 6030ccd265c..f6898c2197c 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -6,6 +6,7 @@ - [Internal] Media picker flow was refactored to support interactive dismissal for device photo picker and WordPress media picker sources. Affected flows: product form > images, and virtual product form > downloadable files. [https://github.com/woocommerce/woocommerce-ios/pull/10236] - [Internal] Orders: Improved error message when orders can't be loaded due to a parsing (decoding) error. [https://github.com/woocommerce/woocommerce-ios/pull/10252] - [**] Product discounts: Users can now add discounts to products when creating an order. [https://github.com/woocommerce/woocommerce-ios/pull/10244] +- [Internal] Fixed a bug preventing the "We couldn't load your data" error banner from appearing on the My store dashboard. [https://github.com/woocommerce/woocommerce-ios/pull/10262] 14.5 From fd8a89b4f925a2ae67588dce1b4e4ae82b25af29 Mon Sep 17 00:00:00 2001 From: Rachel McRoberts Date: Thu, 20 Jul 2023 10:12:40 +0100 Subject: [PATCH 5/5] Add documentation --- .../StoreStatsAndTopPerformersViewController.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift index bcff03cda23..8e73c3e527a 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift @@ -240,7 +240,7 @@ private extension StoreStatsAndTopPerformersViewController { } periodGroup.leave() periodStoreStatsGroup.leave() - group.leave() + group.leave() // Leave this group last so `syncError` is set, if needed } group.enter() @@ -256,7 +256,7 @@ private extension StoreStatsAndTopPerformersViewController { } periodGroup.leave() periodStoreStatsGroup.leave() - group.leave() + group.leave() // Leave this group last so `syncError` is set, if needed } group.enter() @@ -272,7 +272,7 @@ private extension StoreStatsAndTopPerformersViewController { } periodGroup.leave() periodStoreStatsGroup.leave() - group.leave() + group.leave() // Leave this group last so `syncError` is set, if needed } group.enter() @@ -287,7 +287,7 @@ private extension StoreStatsAndTopPerformersViewController { periodSyncError = error } periodGroup.leave() - group.leave() + group.leave() // Leave this group last so `syncError` is set, if needed vc.removeTopPerformersGhostContent() }