From 301474782b16c3d5ed83ae1862a7c46c9a253ec1 Mon Sep 17 00:00:00 2001 From: Alejo Date: Thu, 25 Jan 2024 10:46:22 -0600 Subject: [PATCH 1/3] visitor stats optional --- .../android/ui/appwidgets/stats/GetWidgetStats.kt | 14 ++++++-------- .../stats/today/TodayStatsWidgetUIHelper.kt | 9 +++++---- .../android/ui/mystore/data/StatsRepository.kt | 10 +++++++++- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStats.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStats.kt index e383e0495a3..018cc7e4d85 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStats.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStats.kt @@ -35,9 +35,9 @@ class GetWidgetStats @Inject constructor( // If siteModel is null, exit the function with WidgetStatsFailure siteModel == null -> WidgetStatsResult.WidgetStatsFailure("No site selected") else -> { - // Fetch stats, always force to refresh data val areVisitorStatsSupported = siteModel.connectionType == SiteConnectionType.Jetpack + // Fetch stats, always force to refresh data. val fetchedStats = statsRepository.fetchStats( granularity = granularity, forced = true, @@ -47,7 +47,7 @@ class GetWidgetStats @Inject constructor( if (fetchedStats.isError) { WidgetStatsResult.WidgetStatsFailure(fetchedStats.error.message) } else { - WidgetStatsResult.WidgetStats(fetchedStats.model!!, areVisitorStatsSupported) + WidgetStatsResult.WidgetStats(fetchedStats.model!!) } } } @@ -62,15 +62,13 @@ class GetWidgetStats @Inject constructor( data class WidgetStats( private val revenueModel: WCRevenueStatsModel?, private val visitorsMap: Map?, - val currencyCode: String, - val areVisitorStatsSupported: Boolean + val currencyCode: String ) : WidgetStatsResult() { constructor( stats: StatsRepository.SiteStats, - areVisitorStatsSupported: Boolean - ) : this(stats.revenue, stats.visitors, stats.currencyCode, areVisitorStatsSupported) + ) : this(stats.revenue, stats.visitors, stats.currencyCode) - val visitorsTotal: Int + val visitorsTotal: Int? val ordersTotal: Int val revenueGross: Double @@ -82,7 +80,7 @@ class GetWidgetStats @Inject constructor( orderCount = total.ordersCount ?: 0 } - visitorsTotal = visitorsMap?.values?.sum() ?: 0 + visitorsTotal = visitorsMap?.values?.sum() ordersTotal = orderCount revenueGross = grossRevenue } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/today/TodayStatsWidgetUIHelper.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/today/TodayStatsWidgetUIHelper.kt index f91ca767c40..0a011f51ed7 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/today/TodayStatsWidgetUIHelper.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/appwidgets/stats/today/TodayStatsWidgetUIHelper.kt @@ -67,8 +67,9 @@ class TodayStatsWidgetUIHelper @Inject constructor( remoteViews.setTextViewText(R.id.widget_revenue_value, revenue) remoteViews.setTextViewText(R.id.widget_orders_value, stats.ordersTotal.toString()) - remoteViews.setTextViewText(R.id.widget_visitors_value, stats.visitorsTotal.toString()) - + stats.visitorsTotal?.let { + remoteViews.setTextViewText(R.id.widget_visitors_value, it.toString()) + } remoteViews.setViewVisibility(R.id.widget_revenue_value, View.VISIBLE) remoteViews.setViewVisibility(R.id.widget_revenue_skeleton, View.INVISIBLE) @@ -80,11 +81,11 @@ class TodayStatsWidgetUIHelper @Inject constructor( remoteViews.setViewVisibility( R.id.widget_visitors_title, - if (stats.areVisitorStatsSupported) View.VISIBLE else View.GONE + if (stats.visitorsTotal != null) View.VISIBLE else View.GONE ) remoteViews.setViewVisibility( R.id.widget_visitors_value, - if (stats.areVisitorStatsSupported) View.VISIBLE else View.GONE + if (stats.visitorsTotal != null) View.VISIBLE else View.GONE ) remoteViews.setTextViewText( diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt index ce796ed17ce..ce662b97352 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt @@ -285,6 +285,11 @@ class StatsRepository @Inject constructor( } } + /** + * This function will return the site stats optional including visitor stats. + * Even if the includeVisitorStats flag is set to true, errors fetching visitor + * will be handled as null and only errors fetching the revenue stats will be processed. + */ suspend fun fetchStats( granularity: StatsGranularity, forced: Boolean, @@ -314,7 +319,10 @@ class StatsRepository @Inject constructor( val revenueStats = fetchRevenueStats.await() val siteCurrencyCode = wooCommerceStore.getSiteSettings(site)?.currencyCode.orEmpty() - return@coroutineScope if (visitorStats.isError || revenueStats.isError) { + // If there was an error fetching the visitor stats chances are that is because + // jetpack is not properly configure to return stats. So we take into account + // only revenue stats to return process the error response. + return@coroutineScope if (revenueStats.isError) { val error = WooError( type = WooErrorType.GENERIC_ERROR, original = BaseRequest.GenericErrorType.UNKNOWN, From cc5e0751738877d7a8b5fae9d251582a76c730b3 Mon Sep 17 00:00:00 2001 From: Alejo Date: Thu, 25 Jan 2024 10:47:45 -0600 Subject: [PATCH 2/3] fix unit tests --- .../android/ui/appwidgets/stats/GetWidgetStatsTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStatsTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStatsTest.kt index ff7e24e9e29..d37c442e403 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStatsTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/appwidgets/stats/GetWidgetStatsTest.kt @@ -133,7 +133,7 @@ class GetWidgetStatsTest : BaseUnitTest() { } @Test - fun `when fetchStats succeed then get stats respond with WidgetStatsFailure`() = + fun `when fetchStats succeed then get stats respond with WidgetStats`() = testBlocking { // Given the user is logged, v4 stats is supported and network is working fine whenever(accountRepository.isUserLoggedIn()).thenReturn(true) @@ -146,7 +146,7 @@ class GetWidgetStatsTest : BaseUnitTest() { // When GetWidgetStats is invoked val result = sut.invoke(defaultGranularity, defaultSiteModel) - val expected = GetWidgetStats.WidgetStatsResult.WidgetStats(defaultResponse, true) + val expected = GetWidgetStats.WidgetStatsResult.WidgetStats(defaultResponse) // Then the result is WidgetStatsFailure assertThat(result).isEqualTo(expected) From 502cdcc79334e9a07eefadd3269f88632dcade67 Mon Sep 17 00:00:00 2001 From: Alejo Date: Thu, 25 Jan 2024 11:51:43 -0600 Subject: [PATCH 3/3] add unit tests --- .../ui/mystore/StatsRepositoryTests.kt | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/ui/mystore/StatsRepositoryTests.kt diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/mystore/StatsRepositoryTests.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/mystore/StatsRepositoryTests.kt new file mode 100644 index 00000000000..bc28364ee44 --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/mystore/StatsRepositoryTests.kt @@ -0,0 +1,148 @@ +package com.woocommerce.android.ui.mystore + +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.mystore.data.StatsRepository +import com.woocommerce.android.viewmodel.BaseUnitTest +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.action.WCStatsAction +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.model.WCRevenueStatsModel +import org.wordpress.android.fluxc.store.WCLeaderboardsStore +import org.wordpress.android.fluxc.store.WCOrderStore +import org.wordpress.android.fluxc.store.WCStatsStore +import org.wordpress.android.fluxc.store.WooCommerceStore + +@OptIn(ExperimentalCoroutinesApi::class) +class StatsRepositoryTests : BaseUnitTest() { + + private val selectedSite: SelectedSite = mock() + private val wcStatsStore: WCStatsStore = mock() + private val wcOrderStore: WCOrderStore = mock() + private val wcLeaderboardsStore: WCLeaderboardsStore = mock() + private val wooCommerceStore: WooCommerceStore = mock() + + private lateinit var sut: StatsRepository + + private val defaultSiteModel = SiteModel() + + @Before + fun setup() { + sut = StatsRepository( + selectedSite = selectedSite, + wcStatsStore = wcStatsStore, + wcOrderStore = wcOrderStore, + wcLeaderboardsStore = wcLeaderboardsStore, + wooCommerceStore = wooCommerceStore + ) + } + + @Test + fun `when visitors and revenue requests succeed then a success response is returned containing both value`() = testBlocking { + val granularity = WCStatsStore.StatsGranularity.DAYS + val startDate = "2024-01-25 00:00:00" + val endDate = "2024-01-25 23:59:59" + val visitorStatsResponse = WCStatsStore.OnWCStatsChanged( + rowsAffected = 2, + granularity = granularity, + quantity = "5", + date = startDate + ) + + val revenueStatsResponse = WCStatsStore.OnWCRevenueStatsChanged( + rowsAffected = 2, + granularity = granularity, + startDate = startDate, + endDate = endDate + ) + + whenever(selectedSite.get()).thenReturn(defaultSiteModel) + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) + whenever(wcStatsStore.fetchNewVisitorStats(any())).thenReturn(visitorStatsResponse) + whenever(wcStatsStore.fetchRevenueStats(any())).thenReturn(revenueStatsResponse) + whenever(wcStatsStore.getRawRevenueStats(eq(defaultSiteModel), eq(granularity), eq(startDate), eq(endDate))) + .thenReturn(WCRevenueStatsModel()) + + val result = sut.fetchStats( + granularity = WCStatsStore.StatsGranularity.DAYS, + forced = true, + includeVisitorStats = true + ) + + assertThat(result.isError).isEqualTo(false) + assertThat(result.model).isNotNull + assertThat(result.model!!.revenue).isNotNull + assertThat(result.model!!.visitors).isNotNull + } + + @Test + fun `when visitors requests fails then a success response is returned with visitors null`() = testBlocking { + val granularity = WCStatsStore.StatsGranularity.DAYS + val startDate = "2024-01-25 00:00:00" + val endDate = "2024-01-25 23:59:59" + val visitorStatsResponse = WCStatsStore.OnWCStatsChanged(0, granularity).also { + it.error = WCStatsStore.OrderStatsError() + it.causeOfChange = WCStatsAction.FETCH_NEW_VISITOR_STATS + } + + val revenueStatsResponse = WCStatsStore.OnWCRevenueStatsChanged( + rowsAffected = 2, + granularity = granularity, + startDate = startDate, + endDate = endDate + ) + + whenever(selectedSite.get()).thenReturn(defaultSiteModel) + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) + whenever(wcStatsStore.fetchNewVisitorStats(any())).thenReturn(visitorStatsResponse) + whenever(wcStatsStore.fetchRevenueStats(any())).thenReturn(revenueStatsResponse) + whenever(wcStatsStore.getRawRevenueStats(eq(defaultSiteModel), eq(granularity), eq(startDate), eq(endDate))) + .thenReturn(WCRevenueStatsModel()) + + val result = sut.fetchStats( + granularity = WCStatsStore.StatsGranularity.DAYS, + forced = true, + includeVisitorStats = true + ) + + assertThat(result.isError).isEqualTo(false) + assertThat(result.model).isNotNull + assertThat(result.model!!.revenue).isNotNull + assertThat(result.model!!.visitors).isNull() + } + + @Test + fun `when revenue requests fails then an error is returned`() = testBlocking { + val granularity = WCStatsStore.StatsGranularity.DAYS + val startDate = "2024-01-25 00:00:00" + val visitorStatsResponse = WCStatsStore.OnWCStatsChanged( + rowsAffected = 2, + granularity = granularity, + quantity = "5", + date = startDate + ) + + val revenueStatsResponse = WCStatsStore.OnWCRevenueStatsChanged(0, granularity) + .also { it.error = WCStatsStore.OrderStatsError() } + + whenever(selectedSite.get()).thenReturn(defaultSiteModel) + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) + whenever(wcStatsStore.fetchNewVisitorStats(any())).thenReturn(visitorStatsResponse) + whenever(wcStatsStore.fetchRevenueStats(any())).thenReturn(revenueStatsResponse) + + val result = sut.fetchStats( + granularity = WCStatsStore.StatsGranularity.DAYS, + forced = true, + includeVisitorStats = true + ) + + assertThat(result.isError).isEqualTo(true) + assertThat(result.model).isNull() + } +}