From 605d2c2757fb7d31af00a7b47ae6bc43e9a80995 Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 7 Aug 2024 13:25:01 -0600 Subject: [PATCH 1/4] add shouldAllCardsBePresent param to last update filter --- .../analytics/hub/sync/AnalyticsUpdateDataStore.kt | 14 ++++++++++---- .../ui/dashboard/domain/ObserveLastUpdate.kt | 6 ++++-- .../ui/dashboard/stats/DashboardStatsViewModel.kt | 3 ++- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt index 051e83b0d28..a43507e010a 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt @@ -98,22 +98,28 @@ class AnalyticsUpdateDataStore @Inject constructor( */ fun observeLastUpdate( rangeSelection: StatsTimeRangeSelection, - analyticData: List + analyticData: List, + shouldAllCardsBePresent: Boolean = true ): Flow { val timestampKeys = analyticData.map { data -> getTimeStampKey(rangeSelection.identifier, data) } - return observeLastUpdate(timestampKeys) + return observeLastUpdate(timestampKeys, shouldAllCardsBePresent) } private fun observeLastUpdate( - timestampKeys: List + timestampKeys: List, + shouldAllCardsBePresent: Boolean ): Flow { val flows = timestampKeys.map { timestampKey -> dataStore.data.map { prefs -> prefs[longPreferencesKey(timestampKey)] } } return combine(flows) { lastUpdateMillisArray -> lastUpdateMillisArray.filterNotNull() } - .filter { notNullValues -> notNullValues.size == timestampKeys.size } + .filter { notNullValues -> + if (shouldAllCardsBePresent) { + notNullValues.size == timestampKeys.size + } else true + } .map { lastUpdateValues -> lastUpdateValues.min() } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt index dcb45c6c439..7b091e3d924 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt @@ -10,11 +10,13 @@ class ObserveLastUpdate @Inject constructor( ) { operator fun invoke( selectedRange: StatsTimeRangeSelection, - analyticDataList: List + analyticDataList: List, + shouldAllCardsBePresent: Boolean = true ): Flow { return analyticsUpdateDataStore.observeLastUpdate( rangeSelection = selectedRange, - analyticData = analyticDataList + analyticData = analyticDataList, + shouldAllCardsBePresent = shouldAllCardsBePresent ) } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModel.kt index 5707febe953..047d02d821e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModel.kt @@ -261,7 +261,8 @@ class DashboardStatsViewModel @AssistedInject constructor( listOf( AnalyticsUpdateDataStore.AnalyticData.REVENUE, AnalyticsUpdateDataStore.AnalyticData.VISITORS - ) + ), + false ).collect { lastUpdateMillis -> _lastUpdateStats.value = lastUpdateMillis } } } From f366de06c3366515abf14c54f8b718426c9b9f97 Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 7 Aug 2024 14:32:33 -0600 Subject: [PATCH 2/4] refactor shouldAllCardsBePresent -> shouldAllDataBePresent --- .../analytics/hub/sync/AnalyticsUpdateDataStore.kt | 12 +++++++----- .../android/ui/dashboard/domain/ObserveLastUpdate.kt | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt index a43507e010a..dd835112d22 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStore.kt @@ -99,26 +99,28 @@ class AnalyticsUpdateDataStore @Inject constructor( fun observeLastUpdate( rangeSelection: StatsTimeRangeSelection, analyticData: List, - shouldAllCardsBePresent: Boolean = true + shouldAllDataBePresent: Boolean = true ): Flow { val timestampKeys = analyticData.map { data -> getTimeStampKey(rangeSelection.identifier, data) } - return observeLastUpdate(timestampKeys, shouldAllCardsBePresent) + return observeLastUpdate(timestampKeys, shouldAllDataBePresent) } private fun observeLastUpdate( timestampKeys: List, - shouldAllCardsBePresent: Boolean + shouldAllDataBePresent: Boolean ): Flow { val flows = timestampKeys.map { timestampKey -> dataStore.data.map { prefs -> prefs[longPreferencesKey(timestampKey)] } } return combine(flows) { lastUpdateMillisArray -> lastUpdateMillisArray.filterNotNull() } .filter { notNullValues -> - if (shouldAllCardsBePresent) { + if (shouldAllDataBePresent) { notNullValues.size == timestampKeys.size - } else true + } else { + true + } } .map { lastUpdateValues -> lastUpdateValues.min() } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt index 7b091e3d924..4b284bbfaa4 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/domain/ObserveLastUpdate.kt @@ -11,12 +11,12 @@ class ObserveLastUpdate @Inject constructor( operator fun invoke( selectedRange: StatsTimeRangeSelection, analyticDataList: List, - shouldAllCardsBePresent: Boolean = true + shouldAllDataBePresent: Boolean = true ): Flow { return analyticsUpdateDataStore.observeLastUpdate( rangeSelection = selectedRange, analyticData = analyticDataList, - shouldAllCardsBePresent = shouldAllCardsBePresent + shouldAllDataBePresent = shouldAllDataBePresent ) } From ac11bc6cb672c52e97edfe51b72ed0b7d9590e19 Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 7 Aug 2024 14:32:51 -0600 Subject: [PATCH 3/4] add unit tests --- .../hub/sync/AnalyticsUpdateDataStoreTest.kt | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStoreTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStoreTest.kt index 3fe4d9ace4a..0b94fee7ff2 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStoreTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/hub/sync/AnalyticsUpdateDataStoreTest.kt @@ -3,6 +3,7 @@ package com.woocommerce.android.ui.analytics.hub.sync import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.longPreferencesKey import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.analytics.ranges.StatsTimeRangeSelection.SelectionType import com.woocommerce.android.ui.analytics.ranges.StatsTimeRangeSelection.SelectionType.LAST_MONTH @@ -172,6 +173,127 @@ class AnalyticsUpdateDataStoreTest : BaseUnitTest() { assertThat(timestampUpdate).isEqualTo(2000) } + @Test + fun `given observe should emit last update for all data sources, when a data source is missing then return null`() = testBlocking { + // Given + val selectedSiteId = 1 + val lastUpdateTimestamp = 2000L + val rangeId = defaultSelectionData.selectionType.identifier + val presentKey = + "${selectedSiteId}${AnalyticsUpdateDataStore.AnalyticData.REVENUE}$rangeId" + + val analyticsPreferences = mock { + on { get(longPreferencesKey(presentKey)) } doReturn lastUpdateTimestamp + } + + createAnalyticsUpdateScenarioWith(analyticsPreferences, selectedSiteId) + + // When + var timestampUpdate: Long? = null + sut.observeLastUpdate( + rangeSelection = defaultSelectionData, + analyticData = listOf( + AnalyticsUpdateDataStore.AnalyticData.REVENUE, + AnalyticsUpdateDataStore.AnalyticData.VISITORS + ) + ).onEach { + timestampUpdate = it + }.launchIn(this) + + // Then + assertThat(timestampUpdate).isNull() + } + + @Test + fun `given observe should emit last update for all data sources, when all data source are present then return the oldest timestamp`() = testBlocking { + // Given + val selectedSiteId = 1 + val oldLastUpdateTimestamp = 2000L + val newLastUpdateTimestamp = 2500L + val rangeId = defaultSelectionData.selectionType.identifier + val keyRevenue = + "${selectedSiteId}${AnalyticsUpdateDataStore.AnalyticData.REVENUE}$rangeId" + val keyVisitors = + "${selectedSiteId}${AnalyticsUpdateDataStore.AnalyticData.VISITORS}$rangeId" + + val analyticsPreferences = mock { + on { get(longPreferencesKey(keyRevenue)) } doReturn newLastUpdateTimestamp + on { get(longPreferencesKey(keyVisitors)) } doReturn oldLastUpdateTimestamp + } + + createAnalyticsUpdateScenarioWith(analyticsPreferences, selectedSiteId) + + // When + var timestampUpdate: Long? = null + sut.observeLastUpdate( + rangeSelection = defaultSelectionData, + analyticData = listOf( + AnalyticsUpdateDataStore.AnalyticData.REVENUE, + AnalyticsUpdateDataStore.AnalyticData.VISITORS + ) + ).onEach { + timestampUpdate = it + }.launchIn(this) + + // Then + assertThat(timestampUpdate).isNotNull() + assertThat(timestampUpdate).isEqualTo(oldLastUpdateTimestamp) + } + + @Test + fun `given observe should emit last update, when all data sources are not required, if a data source is missing then return the available last update`() = testBlocking { + // Given + val selectedSiteId = 1 + val lastUpdateTimestamp = 2000L + val rangeId = defaultSelectionData.selectionType.identifier + val presentKey = + "${selectedSiteId}${AnalyticsUpdateDataStore.AnalyticData.REVENUE}$rangeId" + + val analyticsPreferences = mock { + on { get(longPreferencesKey(presentKey)) } doReturn lastUpdateTimestamp + } + + createAnalyticsUpdateScenarioWith(analyticsPreferences, selectedSiteId) + + // When + var timestampUpdate: Long? = null + sut.observeLastUpdate( + rangeSelection = defaultSelectionData, + analyticData = listOf( + AnalyticsUpdateDataStore.AnalyticData.REVENUE, + AnalyticsUpdateDataStore.AnalyticData.VISITORS + ), + shouldAllDataBePresent = false + ).onEach { + timestampUpdate = it + }.launchIn(this) + + // Then + assertThat(timestampUpdate).isNotNull() + assertThat(timestampUpdate).isEqualTo(lastUpdateTimestamp) + } + + private fun createAnalyticsUpdateScenarioWith( + analyticsPreferences: Preferences, + selectedSiteId: Int + ) { + dataStore = mock { + on { data } doReturn flowOf(analyticsPreferences) + } + + currentTimeProvider = mock() + + val selectedSite: SelectedSite = mock { + on { getSelectedSiteId() } doReturn selectedSiteId + } + + sut = AnalyticsUpdateDataStore( + dataStore = dataStore, + currentTimeProvider = currentTimeProvider, + selectedSite = selectedSite + ) + } + private fun createAnalyticsUpdateScenarioWith( lastUpdateTimestamp: Long?, currentTimestamp: Long From 93bfe17c903475da94296db6ed82de0c84684919 Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 13 Aug 2024 21:35:00 -0600 Subject: [PATCH 4/4] fix unit tests --- .../ui/analytics/AnalyticsHubViewModelTest.kt | 27 +++++++++++++++---- .../stats/DashboardStatsViewModelTest.kt | 2 +- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/AnalyticsHubViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/AnalyticsHubViewModelTest.kt index d1f7297b360..58e4765b5ea 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/AnalyticsHubViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/analytics/AnalyticsHubViewModelTest.kt @@ -748,7 +748,7 @@ class AnalyticsHubViewModelTest : BaseUnitTest() { fun `when last update information changes, then update view state as expected`() = testBlocking { val lastUpdateTimestamp = 123456789L whenever( - observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any()) + observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any(), shouldAllDataBePresent = eq(true)) ).thenReturn(flowOf(lastUpdateTimestamp)) whenever(dateUtils.getDateOrTimeFromMillis(lastUpdateTimestamp)).thenReturn("9:35 AM") configureVisibleCards() @@ -845,7 +845,13 @@ class AnalyticsHubViewModelTest : BaseUnitTest() { isVisible = true ) ) - whenever(observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any())).thenReturn(flowOf(null)) + whenever( + observeLastUpdate.invoke( + selectedRange = any(), + analyticDataList = any(), + shouldAllDataBePresent = eq(true) + ) + ).thenReturn(flowOf(null)) configureVisibleCards(configuration) configureSuccessfulStatsResponse() @@ -889,8 +895,11 @@ class AnalyticsHubViewModelTest : BaseUnitTest() { title = AnalyticsCards.Session.name, isVisible = true ) + ) - whenever(observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any())).thenReturn(flowOf(null)) + whenever( + observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any(), shouldAllDataBePresent = eq(true)) + ).thenReturn(flowOf(null)) configureVisibleCards(configuration) configureSuccessfulStatsResponse() @@ -930,7 +939,9 @@ class AnalyticsHubViewModelTest : BaseUnitTest() { val expectedVisibleCards = configuration.filter { it.isVisible }.map { it.card } - whenever(observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any())).thenReturn(flowOf(null)) + whenever( + observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any(), shouldAllDataBePresent = eq(true)) + ).thenReturn(flowOf(null)) configureVisibleCards(configuration) configureSuccessfulStatsResponse() @@ -972,7 +983,13 @@ class AnalyticsHubViewModelTest : BaseUnitTest() { val expectedVisibleCards = configuration.filter { it.isVisible }.map { it.card } - whenever(observeLastUpdate.invoke(selectedRange = any(), analyticDataList = any())).thenReturn(flowOf(null)) + whenever( + observeLastUpdate.invoke( + selectedRange = any(), + analyticDataList = any(), + shouldAllDataBePresent = eq(true) + ) + ).thenReturn(flowOf(null)) configureVisibleCards(configuration) configureSuccessfulStatsResponse() diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModelTest.kt index 83631b192ec..4e6af805c93 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/dashboard/stats/DashboardStatsViewModelTest.kt @@ -72,7 +72,7 @@ class DashboardStatsViewModelTest : BaseUnitTest() { } private val timezoneProvider: TimezoneProvider = mock() private val observeLastUpdate: ObserveLastUpdate = mock { - onBlocking { invoke(any(), ArgumentMatchers.anyList()) } doReturn flowOf(DEFAULT_LAST_UPDATE) + onBlocking { invoke(any(), ArgumentMatchers.anyList(), eq(false)) } doReturn flowOf(DEFAULT_LAST_UPDATE) } private val dateUtils: DateUtils = mock() private val parentViewModel: DashboardViewModel = mock {