fix: refresh subscriptions seamlessly

Author: Priveetee

app/src/main/java/dev/typetype/android/feature/subscriptions/SubscriptionsFeedStatusBar.kt

@@ -24,7 +24,6 @@ import dev.typetype.android.core.ui.components.RequestIdRow
 internal fun SubscriptionsFeedStatusBar(
     isRefreshing: Boolean,
     isServerRefreshing: Boolean,
-    hasPendingRefresh: Boolean,
     errorMessage: String?,
     requestId: String?,
     hasContent: Boolean,
@@ -33,7 +32,6 @@ internal fun SubscriptionsFeedStatusBar(
     when {
         errorMessage != null && hasContent -> FailureStatus(errorMessage, requestId, onRetry)
         isRefreshing && hasContent -> ProgressStatus(R.string.subscriptions_checking_updates)
-        hasPendingRefresh && hasContent -> RefreshAvailableStatus(onRetry)
         isServerRefreshing && hasContent -> ProgressStatus(R.string.subscriptions_rebuilding)
     }
 }
@@ -55,29 +53,6 @@ private fun ProgressStatus(label: Int) {
     }
 }
 
-@Composable
-private fun RefreshAvailableStatus(onRefresh: () -> Unit) {
-    Surface(
-        color = MaterialTheme.colorScheme.secondaryContainer,
-        contentColor = MaterialTheme.colorScheme.onSecondaryContainer,
-        modifier = Modifier.fillMaxWidth().semantics { liveRegion = LiveRegionMode.Polite },
-    ) {
-        Row(
-            modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = 6.dp, bottom = 6.dp),
-            verticalAlignment = Alignment.CenterVertically,
-        ) {
-            Text(
-                text = stringResource(R.string.subscriptions_update_available),
-                style = MaterialTheme.typography.bodyMedium,
-                modifier = Modifier.weight(1f),
-            )
-            TextButton(onClick = onRefresh) {
-                Text(stringResource(R.string.subscriptions_refresh))
-            }
-        }
-    }
-}
-
 @Composable
 private fun FailureStatus(message: String, requestId: String?, onRetry: () -> Unit) {
     Surface(

app/src/main/java/dev/typetype/android/feature/subscriptions/SubscriptionsGenerationAction.kt

@@ -0,0 +1,13 @@
+package dev.typetype.android.feature.subscriptions
+
+internal enum class SubscriptionsGenerationAction { Continue, Replace, Stop }
+
+internal fun subscriptionsGenerationAction(
+    currentGeneration: Long?,
+    observedGeneration: Long,
+    receivedGeneration: Long,
+): SubscriptionsGenerationAction = when {
+    currentGeneration != observedGeneration -> SubscriptionsGenerationAction.Stop
+    receivedGeneration != observedGeneration -> SubscriptionsGenerationAction.Replace
+    else -> SubscriptionsGenerationAction.Continue
+}

app/src/main/java/dev/typetype/android/feature/subscriptions/SubscriptionsScreen.kt

@@ -78,7 +78,6 @@ fun SubscriptionsScreen(
         SubscriptionsFeedStatusBar(
             isRefreshing = state.isLoading,
             isServerRefreshing = state.isServerRefreshing,
-            hasPendingRefresh = state.hasPendingRefresh,
             errorMessage = state.errorMessage,
             requestId = state.errorRequestId,
             hasContent = state.videos.isNotEmpty(),

app/src/main/java/dev/typetype/android/feature/subscriptions/SubscriptionsState.kt

@@ -9,7 +9,6 @@ data class SubscriptionsState(
     val isLoading: Boolean = false,
     val isLoadingMore: Boolean = false,
     val isServerRefreshing: Boolean = false,
-    val hasPendingRefresh: Boolean = false,
     val videos: List<Video> = emptyList(),
     val channels: List<SubscriptionSummary> = emptyList(),
     val selectedTab: SubscriptionsTab = SubscriptionsTab.Videos,

app/src/main/java/dev/typetype/android/feature/subscriptions/SubscriptionsViewModel.kt

@@ -136,7 +136,6 @@ class SubscriptionsViewModel @Inject constructor(
                 isLoading = true,
                 isLoadingMore = false,
                 isServerRefreshing = false,
-                hasPendingRefresh = false,
                 errorMessage = null,
                 errorRequestId = null,
                 loadMoreError = false,
@@ -161,7 +160,6 @@ class SubscriptionsViewModel @Inject constructor(
                 videos = page.videos.distinctBy { video -> video.url },
                 hasMore = page.hasMore,
                 isServerRefreshing = page.refreshing,
-                hasPendingRefresh = false,
                 generatedAtMillis = page.generatedAtMillis,
                 errorMessage = null,
                 errorRequestId = null,
@@ -197,7 +195,7 @@ class SubscriptionsViewModel @Inject constructor(
                 onFailure = { failure ->
                     if (failure.requiresSubscriptionsPaginationRestart()) {
                         refreshMonitorJob?.cancel()
-                        offerRefresh()
+                        requestJob = viewModelScope.launch { loadFirstPage() }
                     } else {
                         networkRecovery.schedule(failure, SubscriptionsRecoveryRequest.Pagination)
                         val details = errorMapper.details(failure, R.string.subscriptions_failed)
@@ -251,12 +249,13 @@ class SubscriptionsViewModel @Inject constructor(
                     showRefreshFailure(failure)
                     return@launch
                 }
-                if (generation != expectedGeneration) return@launch
-                if (page.generation != expectedGeneration) {
-                    offerRefresh()
-                    return@launch
-                } else {
-                    _state.update {
+                when (subscriptionsGenerationAction(generation, expectedGeneration, page.generation)) {
+                    SubscriptionsGenerationAction.Stop -> return@launch
+                    SubscriptionsGenerationAction.Replace -> {
+                        viewModelScope.launch { acceptFirstPage(page, hadCachedContent = true) }
+                        return@launch
+                    }
+                    SubscriptionsGenerationAction.Continue -> _state.update {
                         it.copy(
                             isServerRefreshing = page.refreshing,
                             generatedAtMillis = page.generatedAtMillis,
@@ -268,19 +267,6 @@ class SubscriptionsViewModel @Inject constructor(
         }
     }
 
-    private fun offerRefresh() {
-        nextCursor = null
-        _state.update {
-            it.copy(
-                isLoadingMore = false,
-                isServerRefreshing = false,
-                hasPendingRefresh = true,
-                hasMore = false,
-                loadMoreError = false,
-            )
-        }
-    }
-
     private suspend fun persistPage(page: SubscriptionsPage, append: Boolean) {
         try {
             videoMetaRepository.cacheVideos(page.videos)

app/src/main/res/values/strings.xml

@@ -26,8 +26,6 @@
     <string name="subscriptions_failed">Couldn\'t load subscriptions</string>
     <string name="subscriptions_checking_updates">Checking for new videos</string>
     <string name="subscriptions_rebuilding">Updating subscriptions in the background</string>
-    <string name="subscriptions_update_available">New subscription videos are available</string>
-    <string name="subscriptions_refresh">Refresh</string>
     <string name="subscriptions_preparing">Preparing subscriptions</string>
     <string name="subscriptions_empty_title">Your subscriptions will appear here</string>
     <string name="subscriptions_empty_description">Subscribe to channels to build a personal feed of their latest videos.</string>

app/src/test/java/dev/typetype/android/feature/subscriptions/SubscriptionsGenerationActionTest.kt

@@ -0,0 +1,30 @@
+package dev.typetype.android.feature.subscriptions
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+class SubscriptionsGenerationActionTest {
+    @Test
+    fun `new server generation replaces the visible feed`() {
+        assertEquals(
+            SubscriptionsGenerationAction.Replace,
+            subscriptionsGenerationAction(4L, 4L, 5L),
+        )
+    }
+
+    @Test
+    fun `matching generation keeps monitoring`() {
+        assertEquals(
+            SubscriptionsGenerationAction.Continue,
+            subscriptionsGenerationAction(4L, 4L, 4L),
+        )
+    }
+
+    @Test
+    fun `stale monitor cannot replace a newer feed`() {
+        assertEquals(
+            SubscriptionsGenerationAction.Stop,
+            subscriptionsGenerationAction(5L, 4L, 6L),
+        )
+    }
+}