CMM-1952: stats insights: tags card and some fixes

WordPress/src/main/java/org/wordpress/android/ui/newstats/InsightsViewModel.kt

@@ -3,6 +3,7 @@ package org.wordpress.android.ui.newstats
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -17,8 +18,10 @@ import org.wordpress.android.ui.newstats.repository.InsightsResult
 import org.wordpress.android.ui.newstats.repository.StatsSummaryResult
 import org.wordpress.android.ui.newstats.repository.StatsSummaryUseCase
 import org.wordpress.android.ui.newstats.repository.StatsInsightsUseCase
+import org.wordpress.android.ui.newstats.repository.StatsTagsUseCase
 import org.wordpress.android.util.AppLog
 import org.wordpress.android.util.NetworkUtilsWrapper
+import java.util.concurrent.atomic.AtomicBoolean
 import kotlin.coroutines.cancellation.CancellationException
 import javax.inject.Inject
 
@@ -30,7 +33,8 @@ class InsightsViewModel @Inject constructor(
         InsightsCardsConfigurationRepository,
     private val networkUtilsWrapper: NetworkUtilsWrapper,
     private val statsSummaryUseCase: StatsSummaryUseCase,
-    private val statsInsightsUseCase: StatsInsightsUseCase
+    private val statsInsightsUseCase: StatsInsightsUseCase,
+    private val statsTagsUseCase: StatsTagsUseCase
 ) : ViewModel() {
     private val _visibleCards =
         MutableStateFlow<List<InsightsCardType>>(
@@ -68,13 +72,20 @@ class InsightsViewModel @Inject constructor(
     val isDataRefreshing: StateFlow<Boolean> =
         _isDataRefreshing.asStateFlow()
 
-    @Volatile
-    private var isDataLoaded = false
-
-    @Volatile
-    private var isDataLoading = false
+    private val isDataLoaded = AtomicBoolean(false)
+    private val isDataLoading = AtomicBoolean(false)
+    private val summaryFetched = AtomicBoolean(false)
+    private val insightsFetched = AtomicBoolean(false)
+    // Main-thread-confined: only accessed from
+    // viewModelScope (Dispatchers.Main).
+    private var fetchJob: Job? = null
 
     init {
+        viewModelScope.launch {
+            statsSummaryUseCase.clearCache()
+            statsInsightsUseCase.clearCache()
+            statsTagsUseCase.clearCache()
+        }
         checkNetworkStatus()
         loadConfiguration()
         observeConfigurationChanges()
@@ -90,30 +101,45 @@ class InsightsViewModel @Inject constructor(
     // region Data fetching
 
     fun loadDataIfNeeded() {
-        if (isDataLoaded || isDataLoading) return
-        isDataLoading = true
+        if (isDataLoaded.get() ||
+            !isDataLoading.compareAndSet(false, true)
+        ) return
         fetchData()
     }
 
     fun fetchData(forceRefresh: Boolean = false) {
         val siteId = resolvedSiteId() ?: run {
-            isDataLoading = false
+            isDataLoading.set(false)
             _isDataRefreshing.value = false
             return
         }
-        viewModelScope.launch {
+        val cards = _cardsToLoad.value
+        val shouldFetchSummary = cards.needsSummary()
+        val shouldFetchInsights = cards.needsInsights()
+        if (!shouldFetchSummary && !shouldFetchInsights) {
+            isDataLoading.set(false)
+            _isDataRefreshing.value = false
+            return
+        }
+        fetchJob = viewModelScope.launch {
             try {
                 coroutineScope {
-                    launch {
-                        fetchSummary(siteId, forceRefresh)
+                    if (shouldFetchSummary) {
+                        launch {
+                            fetchSummary(siteId, forceRefresh)
+                        }
                     }
-                    launch {
-                        fetchInsights(siteId, forceRefresh)
+                    if (shouldFetchInsights) {
+                        launch {
+                            fetchInsights(
+                                siteId, forceRefresh
+                            )
+                        }
                     }
                 }
-                isDataLoaded = true
+                isDataLoaded.set(true)
             } finally {
-                isDataLoading = false
+                isDataLoading.set(false)
                 _isDataRefreshing.value = false
             }
         }
@@ -131,6 +157,9 @@ class InsightsViewModel @Inject constructor(
             val result = statsSummaryUseCase(
                 siteId, forceRefresh
             )
+            if (result is StatsSummaryResult.Success) {
+                summaryFetched.set(true)
+            }
             _summaryResult.emit(result)
         } catch (e: Exception) {
             if (e is CancellationException) throw e
@@ -160,6 +189,9 @@ class InsightsViewModel @Inject constructor(
             val result = statsInsightsUseCase(
                 siteId, forceRefresh
             )
+            if (result is InsightsResult.Success) {
+                insightsFetched.set(true)
+            }
             _insightsResult.emit(result)
         } catch (e: Exception) {
             if (e is CancellationException) throw e
@@ -178,8 +210,11 @@ class InsightsViewModel @Inject constructor(
     }
 
     fun refreshData() {
-        isDataLoaded = false
-        isDataLoading = true
+        fetchJob?.cancel()
+        isDataLoaded.set(false)
+        summaryFetched.set(false)
+        insightsFetched.set(false)
+        isDataLoading.set(true)
         _isDataRefreshing.value = true
         fetchData(forceRefresh = true)
     }
@@ -224,7 +259,17 @@ class InsightsViewModel @Inject constructor(
     ) {
         _visibleCards.value = config.visibleCards
         _hiddenCards.value = config.computeHiddenCards()
+        val cards = config.visibleCards
+        val needsNewFetch =
+            (cards.needsSummary() &&
+                !summaryFetched.get()) ||
+                (cards.needsInsights() &&
+                    !insightsFetched.get())
         _cardsToLoad.value = config.visibleCards
+        if (needsNewFetch) {
+            isDataLoaded.set(false)
+            loadDataIfNeeded()
+        }
     }
 
     fun removeCard(cardType: InsightsCardType) {
@@ -287,4 +332,22 @@ class InsightsViewModel @Inject constructor(
             null
         }
     }
+
+    companion object {
+        // TAGS_AND_CATEGORIES is intentionally absent
+        // from both checks: it has its own dedicated
+        // fetch path via StatsTagsUseCase in
+        // TagsAndCategoriesViewModel.
+        private fun List<InsightsCardType>.needsSummary():
+            Boolean = any {
+            it == InsightsCardType.ALL_TIME_STATS ||
+                it == InsightsCardType.MOST_POPULAR_DAY
+        }
+
+        private fun List<InsightsCardType>.needsInsights():
+            Boolean = any {
+            it == InsightsCardType.YEAR_IN_REVIEW ||
+                it == InsightsCardType.MOST_POPULAR_TIME
+        }
+    }
 }

WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt

@@ -279,7 +279,6 @@ private fun StatsTabContent(
         )
         StatsTab.INSIGHTS -> InsightsTabContent()
         StatsTab.SUBSCRIBERS -> SubscribersTabContent()
-        else -> PlaceholderTabContent(tab)
     }
 }
 
@@ -1227,14 +1226,8 @@ private fun InsightsTabContent(
                             uiState =
                                 tagsAndCategoriesUiState,
                             onShowAllClick = {
-                                val items =
-                                    tagsAndCategoriesViewModel
-                                        .getDetailData()
                                 TagsAndCategoriesDetailActivity
-                                    .start(
-                                        context,
-                                        items
-                                    )
+                                    .start(context)
                             },
                             onRemoveCard = {
                                 insightsViewModel
@@ -1357,65 +1350,6 @@ private fun AddInsightsCardBottomSheet(
     }
 }
 
-@Composable
-private fun NoConnectionContent(
-    onRetry: () -> Unit
-) {
-    Box(
-        modifier = Modifier.fillMaxSize(),
-        contentAlignment = Alignment.Center
-    ) {
-        Column(
-            modifier = Modifier
-                .fillMaxWidth()
-                .padding(horizontal = 60.dp),
-            horizontalAlignment = Alignment.CenterHorizontally,
-            verticalArrangement = Arrangement.Center
-        ) {
-            Icon(
-                painter = painterResource(R.drawable.ic_wifi_off_24px),
-                contentDescription = null,
-                modifier = Modifier
-                    .size(48.dp)
-                    .background(
-                        color = MaterialTheme.colorScheme.surfaceVariant,
-                        shape = CircleShape
-                    )
-                    .padding(12.dp),
-                tint = MaterialTheme.colorScheme.onSurfaceVariant
-            )
-            Spacer(modifier = Modifier.height(24.dp))
-            Text(
-                text = stringResource(R.string.no_connection_error_title),
-                style = MaterialTheme.typography.titleMedium,
-                color = MaterialTheme.colorScheme.onSurface,
-                textAlign = TextAlign.Center
-            )
-            Spacer(modifier = Modifier.height(8.dp))
-            Text(
-                text = stringResource(R.string.no_connection_error_description),
-                style = MaterialTheme.typography.bodyMedium,
-                color = MaterialTheme.colorScheme.onSurfaceVariant,
-                textAlign = TextAlign.Center
-            )
-            Spacer(modifier = Modifier.height(24.dp))
-            Button(onClick = onRetry) {
-                Text(stringResource(R.string.retry))
-            }
-        }
-    }
-}
-
-@Composable
-private fun PlaceholderTabContent(tab: StatsTab) {
-    Box(
-        modifier = Modifier.fillMaxSize(),
-        contentAlignment = Alignment.Center
-    ) {
-        Text(text = "${stringResource(id = tab.titleResId)} - Coming Soon")
-    }
-}
-
 @Composable
 private fun StatsPeriodMenu(
     expanded: Boolean,

WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsInsightsUseCase.kt

@@ -44,4 +44,8 @@ class StatsInsightsUseCase @Inject constructor(
             result
         }
     }
+
+    suspend fun clearCache() {
+        mutex.withLock { cachedInsights = null }
+    }
 }

WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsSummaryUseCase.kt

@@ -44,4 +44,8 @@ class StatsSummaryUseCase @Inject constructor(
             result
         }
     }
+
+    suspend fun clearCache() {
+        mutex.withLock { cachedSummary = null }
+    }
 }

WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsTagsUseCase.kt

@@ -0,0 +1,105 @@
+package org.wordpress.android.ui.newstats.repository
+
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
+import org.wordpress.android.fluxc.store.AccountStore
+import org.wordpress.android.ui.newstats.datasource.StatsTagsData
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class StatsTagsUseCase @Inject constructor(
+    private val statsRepository: StatsRepository,
+    private val accountStore: AccountStore
+) {
+    private val mutex = Mutex()
+    private var cachedTags:
+        Triple<Long, Int, StatsTagsData>? = null
+
+    // In-flight request keyed by (siteId, max).
+    // Concurrent callers with the same params join
+    // the existing request instead of duplicating it.
+    private var inFlight:
+        Pair<Pair<Long, Int>,
+            CompletableDeferred<TagsResult>>? = null
+
+    @Suppress("ReturnCount")
+    suspend operator fun invoke(
+        siteId: Long,
+        max: Int = DEFAULT_MAX_ITEMS,
+        forceRefresh: Boolean = false
+    ): TagsResult {
+        val token = accountStore.accessToken
+        if (token.isNullOrEmpty()) {
+            return TagsResult.Error("No access token")
+        }
+        statsRepository.init(token)
+
+        val key = siteId to max
+
+        // Under lock: check cache, then check/create
+        // an in-flight deferred. The actual network
+        // call runs outside the lock so concurrent
+        // callers with different params aren't blocked.
+        val (deferred, isOwner) = mutex.withLock {
+            val cached = cachedTags
+            if (!forceRefresh &&
+                isCacheHit(cached, siteId, max)
+            ) {
+                return TagsResult.Success(
+                    cached!!.third
+                )
+            }
+
+            val existing = inFlight
+            if (!forceRefresh &&
+                existing != null &&
+                existing.first == key
+            ) {
+                return@withLock existing.second to false
+            }
+
+            val newDeferred =
+                CompletableDeferred<TagsResult>()
+            inFlight = key to newDeferred
+            newDeferred to true
+        }
+
+        if (isOwner) {
+            val result = statsRepository.fetchTags(
+                siteId = siteId,
+                max = max
+            )
+            mutex.withLock {
+                if (result is TagsResult.Success) {
+                    cachedTags =
+                        Triple(siteId, max, result.data)
+                }
+                inFlight = null
+            }
+            deferred.complete(result)
+        }
+
+        return deferred.await()
+    }
+
+    suspend fun clearCache() {
+        mutex.withLock {
+            cachedTags = null
+            inFlight = null
+        }
+    }
+
+    private fun isCacheHit(
+        cached: Triple<Long, Int, StatsTagsData>?,
+        siteId: Long,
+        max: Int
+    ): Boolean = cached != null &&
+        cached.first == siteId &&
+        cached.second == max
+
+    companion object {
+        private const val DEFAULT_MAX_ITEMS = 10
+    }
+}