From 38d8f5915bc1f2d36f619b7f70c0b4dd6c994f9f Mon Sep 17 00:00:00 2001
From: Adrian Kuta <adrian.kuta93@gmail.com>
Date: Wed, 10 Jun 2026 13:03:09 +0200
Subject: [PATCH] fix(characters:presentation): pagination race + silent
 restore failure (review)

- Race: isLoadingNextPage was set inside the launched coroutine, so a rapid second
  OnLoadNextPage passed the guard before the flag flipped -> the same page loaded twice and
  items were appended twice. Set the loading flag synchronously before launching.
- Restore: when a middle page failed after earlier pages loaded, the error was swallowed
  (error=null, no event). Now any restore failure emits a ShowSnackbar; partial restores show
  the loaded list + snackbar, full failures show the error state.

Found by the milestone review.
---
 .../presentation/CharacterListViewModel.kt    | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/feature/characters/presentation/src/main/kotlin/com/example/architecture/feature/characters/presentation/CharacterListViewModel.kt b/feature/characters/presentation/src/main/kotlin/com/example/architecture/feature/characters/presentation/CharacterListViewModel.kt
index 3858449..c131025 100644
--- a/feature/characters/presentation/src/main/kotlin/com/example/architecture/feature/characters/presentation/CharacterListViewModel.kt
+++ b/feature/characters/presentation/src/main/kotlin/com/example/architecture/feature/characters/presentation/CharacterListViewModel.kt
@@ -52,9 +52,9 @@ class CharacterListViewModel(
     }
 
     private fun restore(targetPage: Int) {
+        // Flip the flag synchronously so a guard reading state sees it immediately.
+        _state.update { it.copy(isLoading = true, error = null) }
         viewModelScope.launch {
-            _state.update { it.copy(isLoading = true, error = null) }
-
             val accumulated = mutableListOf<CharacterUi>()
             var lastLoadedPage = 0
             var endReached = false
@@ -77,9 +77,14 @@ class CharacterListViewModel(
                 page++
             }
 
-            if (accumulated.isEmpty() && error != null) {
-                _state.update { it.copy(isLoading = false, error = error) }
+            // Always surface a failure — even a partial one where earlier pages loaded.
+            if (error != null) {
                 _events.send(CharacterListEvent.ShowSnackbar(error))
+            }
+
+            if (accumulated.isEmpty()) {
+                // Nothing loaded → full-screen error (or an empty list if the API simply had none).
+                _state.update { it.copy(isLoading = false, error = error) }
             } else {
                 val loadedPage = lastLoadedPage.coerceAtLeast(1)
                 _state.update {
@@ -88,6 +93,7 @@ class CharacterListViewModel(
                         isLoading = false,
                         currentPage = loadedPage,
                         endReached = endReached,
+                        // The list is shown; the snackbar already surfaced any partial failure.
                         error = null,
                     )
                 }
@@ -112,8 +118,11 @@ class CharacterListViewModel(
     }
 
     private fun loadPage(page: Int) {
+        // Flip the loading flag SYNCHRONOUSLY (before launching) so a rapid second OnLoadNextPage is
+        // guarded out before its coroutine starts — otherwise the same page loads twice and items
+        // get appended twice.
+        _state.update { it.copy(isLoadingNextPage = true, error = null) }
         viewModelScope.launch {
-            _state.update { it.copy(isLoadingNextPage = true, error = null) }
             characterRepository.getCharacters(page)
                 .onSuccess { pageData ->
                     _state.update { state ->