diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt index 90511c2..1f508bd 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt @@ -286,29 +286,30 @@ class RepositoryListViewModel( val mode = content.mode // Once this session has confirmed a star succeeded (favoriteAction == Starred), a repeat // tap on the ALL list now un-stars instead of starring again -- a real toggle, matching - // what tapping an already-filled star obviously should do. Before this, a repeat tap either - // re-called star() and double-incremented the displayed count, or (briefly) was a dead-end - // no-op with no way back to unstarred without switching to the FAVORITES tab. + // what tapping an already-filled star obviously should do. val alreadyStarredThisSession = mode == RepositoryListMode.ALL && repository.favoriteAction == FavoriteActionUiState.Starred - updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.InProgress) } + + // Applied immediately, before the network call -- this app's Gitea host is a home NAS, not + // a datacenter, so waiting for the full round trip before moving the star/count at all made + // every tap feel laggy ("좀 늦게 카운팅이 변하는 느낌"). The ALL-list star/count now update + // optimistically on tap and only get rolled back in the catch block below if the request + // actually turns out to have failed; the FAVORITES-list unstar (which removes the whole + // row, a more disruptive thing to have to undo visually) still waits for confirmation. + when { + alreadyStarredThisSession -> updateRow(repository.coordinates) { + it.copy(favoriteAction = FavoriteActionUiState.Idle, starsCount = (it.starsCount - 1).coerceAtLeast(0)) + } + mode == RepositoryListMode.ALL -> updateRow(repository.coordinates) { + it.copy(favoriteAction = FavoriteActionUiState.Starred, starsCount = it.starsCount + 1) + } + else -> updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.InProgress) } + } + viewModelScope.launch { try { when { - alreadyStarredThisSession -> { - favoritesApi.unstar(repository.coordinates) - updateRow(repository.coordinates) { - it.copy(favoriteAction = FavoriteActionUiState.Idle, starsCount = (it.starsCount - 1).coerceAtLeast(0)) - } - } - mode == RepositoryListMode.ALL -> { - favoritesApi.star(repository.coordinates) - // starsCount only ever reflects what the last full list load returned - // otherwise -- a successful star here needs its own +1, or the row keeps - // showing the pre-star count until the next reload. - updateRow(repository.coordinates) { - it.copy(favoriteAction = FavoriteActionUiState.Starred, starsCount = it.starsCount + 1) - } - } + alreadyStarredThisSession -> favoritesApi.unstar(repository.coordinates) + mode == RepositoryListMode.ALL -> favoritesApi.star(repository.coordinates) else -> { favoritesApi.unstar(repository.coordinates) removeRow(repository.coordinates) @@ -329,7 +330,17 @@ class RepositoryListViewModel( failure, ) } - updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.Failed) } + if (mode == RepositoryListMode.ALL) { + // Roll back the optimistic update above -- the tap turned out not to have + // actually succeeded server-side, so both the icon and the count need to + // revert to what they were before this attempt. + updateRow(repository.coordinates) { + val revertedCount = if (alreadyStarredThisSession) it.starsCount + 1 else (it.starsCount - 1).coerceAtLeast(0) + it.copy(favoriteAction = FavoriteActionUiState.Failed, starsCount = revertedCount) + } + } else { + updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.Failed) } + } } } } diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt index c66b104..1501920 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt @@ -353,6 +353,28 @@ class RepositoryListViewModelTest { assertEquals(5, updated.starsCount) } + @Test + fun `toggleFavorite updates the star and count immediately, before the network call resolves`() = runTest(dispatcher.scheduler) { + val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 2)))) + val viewModel = viewModel(browser) + advanceUntilIdle() + + val row = (viewModel.state.value as RepositoryListUiState.Content).repositories.single() + viewModel.toggleFavorite(row) + + // Deliberately no advanceUntilIdle() here yet: the launched coroutine that actually calls + // favoritesApi.star() hasn't run on the StandardTestDispatcher at all, but the row's + // favoriteAction/starsCount should already reflect the tap -- that immediacy is the whole + // point of the fix ("좀 늦게 카운팅이 변하는 느낌"). + val immediatelyAfterTap = (viewModel.state.value as RepositoryListUiState.Content).repositories.single() + assertEquals(FavoriteActionUiState.Starred, immediatelyAfterTap.favoriteAction) + assertEquals(3, immediatelyAfterTap.starsCount) + assertEquals(emptyList(), browser.starredCalls) + + advanceUntilIdle() + assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.starredCalls) + } + @Test fun `a repeat tap on an already-starred row from the all-repositories list un-stars it`() = runTest(dispatcher.scheduler) { val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 4)))) @@ -429,6 +451,9 @@ class RepositoryListViewModelTest { val rows = (viewModel.state.value as RepositoryListUiState.Content).repositories.associateBy { it.name } assertEquals(FavoriteActionUiState.Failed, rows.getValue("a").favoriteAction) assertEquals(FavoriteActionUiState.Idle, rows.getValue("b").favoriteAction) + // The optimistic +1 applied on tap must be rolled back once the star turned out to have + // failed -- otherwise a failed attempt would still leave the count permanently off by one. + assertEquals(0, rows.getValue("a").starsCount) browser.favoriteActionFailure = null viewModel.toggleFavorite(rows.getValue("a")) @@ -436,6 +461,28 @@ class RepositoryListViewModelTest { val retried = (viewModel.state.value as RepositoryListUiState.Content).repositories.single { it.name == "a" } assertEquals(FavoriteActionUiState.Starred, retried.favoriteAction) + assertEquals(1, retried.starsCount) + } + + @Test + fun `a failed un-star rolls back the optimistic decrement and returns the row to Starred`() = runTest(dispatcher.scheduler) { + val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 4)))) + val viewModel = viewModel(browser) + advanceUntilIdle() + + val row = (viewModel.state.value as RepositoryListUiState.Content).repositories.single() + viewModel.toggleFavorite(row) + advanceUntilIdle() + val starred = (viewModel.state.value as RepositoryListUiState.Content).repositories.single() + assertEquals(5, starred.starsCount) + + browser.favoriteActionFailure = ApiFailure.Network(java.io.IOException()) + viewModel.toggleFavorite(starred) + advanceUntilIdle() + + val afterFailedUnstar = (viewModel.state.value as RepositoryListUiState.Content).repositories.single() + assertEquals(FavoriteActionUiState.Failed, afterFailedUnstar.favoriteAction) + assertEquals(5, afterFailedUnstar.starsCount) } @Test diff --git a/docs/development-log.md b/docs/development-log.md index 25a2ef8..9634784 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -593,3 +593,15 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: still optimistic (not re-fetched from the server) in both directions; a failed *unstar* attempt still collapses to the same flat `Failed` UI state as a failed star attempt, which can render as not-starred even if the repo is still actually starred server-side — accepted as a minor, harmlessly-retryable gap rather than adding more state to track which action was in flight. - Next recommended task: get live confirmation that both starring (count up) and un-starring (count back down, tappable again) now work from the ALL list. - Next owner: Claude, continuing per the standing direction. + +## 2026-08-29 (continued) — TASK-034 third round: make the star/count update optimistic + +- Agent and role: Claude, direct continuation of the same live session — with starring and un-starring both working correctly, the human product owner reported it still felt slow: "좋은데 바로바로 반응이 안되네 좀 늦게 카운팅이 변하는 느낌" (good, but doesn't respond immediately — the count feels like it changes a bit late). +- Root cause: every state/count update in `toggleFavorite` was applied only *inside* the coroutine, after `favoritesApi.star()/unstar()` had already returned — meaning the star/count visibly did nothing until the full network round trip to this app's home-NAS-hosted Gitea completed, the same underlying "this host is not a datacenter" latency class as TASK-031's HTTP timeout fix. +- How it was implemented: moved the `favoriteAction`/`starsCount` update to run synchronously in `toggleFavorite`, before `viewModelScope.launch { ... }` even starts, so the tap feels instant; the coroutine itself now only performs the actual `star()/unstar()` call. The catch block was extended to roll the optimistic change back (restore the pre-tap count, mark `Failed`) if the request actually turns out to have failed, so a failure doesn't leave the display stuck showing an action that never really happened. The FAVORITES-list unstar (which removes the whole row) deliberately stays non-optimistic, since reverting a *removed* row on failure is more jarring than reverting a color/count and wasn't part of what was reported as slow. +- Files changed: `RepositoryListViewModel.kt`, `RepositoryListViewModelTest.kt`, `tasks/034-favorite-star-count-not-incrementing.md`. +- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug` BUILD SUCCESSFUL, all unit tests passing (new coverage: state/count updates immediately on tap, before the queued coroutine runs on the `StandardTestDispatcher`; a failed star and a failed unstar each roll the count back to its pre-tap value), 0 lint errors. Installed on the human product owner's device over wireless `adb`. +- Review findings and resolutions: None from a separate review pass. +- Known limitations: unchanged from earlier in this task — still optimistic rather than server-authoritative in both directions (accepted tradeoff), and the underlying "can't tell if already starred on load" limitation from TASK-018/TASK-030 is untouched. +- Next recommended task: get live confirmation that starring/un-starring now feels instant. +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/034-favorite-star-count-not-incrementing.md b/tasks/034-favorite-star-count-not-incrementing.md index 4473572..34a3d4f 100644 --- a/tasks/034-favorite-star-count-not-incrementing.md +++ b/tasks/034-favorite-star-count-not-incrementing.md @@ -1,4 +1,4 @@ -# TASK-034: Star count doesn't increment, then repeat tap doesn't un-star +# TASK-034: Star count doesn't increment, repeat tap doesn't un-star, then feels laggy Status: implemented; all required checks pass; live-installed, awaiting the human product owner's confirmation Owner: Claude @@ -6,7 +6,7 @@ Reviewer: Codex ## Outcome -Two rounds of live feedback on the same feature, following TASK-033's OAuth scope fix. First: the +Three rounds of live feedback on the same feature, following TASK-033's OAuth scope fix. First: the human product owner confirmed starring itself now works ("별표 눌러서 즐겨찾기는 되는데") but the displayed star count didn't go up ("카운팅은 안올라가네") — `toggleFavorite`'s ALL-list success branch updated only `favoriteAction` (to `Starred`) and never touched `starsCount`. Fixed by @@ -14,7 +14,12 @@ incrementing `starsCount` in the same state update. Second, immediately after: w repeat-tap guard in place, tapping an already-starred row no longer un-starred it either ("이제 올라가는데 다시 눌렀을때 취소가 안되네") — the first fix's guard made a repeat tap a dead no-op instead of a real toggle. Replaced that guard with an actual toggle: a repeat tap on a -row this session knows is starred now calls `unstar()` and decrements the count back down. +row this session knows is starred now calls `unstar()` and decrements the count back down. Third: +with the toggle working correctly, the human product owner reported the response still felt slow +("좀 늦게 카운팅이 변하는 느낌") — every state/count update was applied only *after* the full +network round trip to this app's home-NAS-hosted Gitea completed, so a tap visibly did nothing for +however long that took. Made the star/count update optimistically, on tap, before the network call +starts; the update is only rolled back if the request actually turns out to have failed. ## Scope @@ -45,6 +50,17 @@ row this session knows is starred now calls `unstar()` and decrements the count only tracks what happened *within this session's own interaction* with a row, same as `favoriteAction` always has (TASK-018/TASK-030). A repository already starred before the app session started still shows outlined/untapped until acted on directly. +- **Optimistic on tap, not after the network call, for the ALL list specifically**: `updateRow(...)` + for the star/unstar case now runs synchronously in `toggleFavorite`, before + `viewModelScope.launch { ... }` even starts, instead of inside the coroutine after + `favoritesApi.star()/unstar()` returns. This app's Gitea host is a home NAS (the same root cause + category as TASK-031's timeout fix), so waiting for a full round trip before moving the star or + count at all made every tap feel delayed rather than instant. The catch block now explicitly + rolls the optimistic change back (`Failed` state, count restored to what it was before the tap) + if the request turns out to have actually failed, rather than only ever moving the display forward + on success as the first two rounds of this fix did. The FAVORITES-list unstar (which removes the + whole row) deliberately stays non-optimistic — reverting a *removed* row is a more jarring failure + case than reverting a color/count, and wasn't part of what was reported as feeling slow. ## Acceptance criteria @@ -52,11 +68,14 @@ row this session knows is starred now calls `unstar()` and decrements the count - [x] Tapping an already-`Starred` row again un-stars it: calls `unstar()`, decrements the count by 1, and returns the row to `Idle` (outlined, tappable to star again). - [x] The count never displays below 0. -- [x] Unstarring from the FAVORITES list is unaffected (row removal, no count to update). +- [x] The star icon and count update immediately on tap, not after the network call resolves; a + failed request rolls the display back to its pre-tap state instead of leaving it stuck at the + optimistic value. +- [x] Unstarring from the FAVORITES list is unaffected (row removal, no count to update, still + waits for confirmation as before). - [x] Build, unit tests, and lint pass. -- [ ] Live confirmation from the human product owner that both starring (count up) and un-starring - (count back down) now work as expected from the ALL list — installed on the device but not - yet re-tested in this exact session. +- [ ] Live confirmation from the human product owner that starring/un-starring now feels instant — + installed on the device but not yet re-tested in this exact session. ## Required checks @@ -68,20 +87,23 @@ row this session knows is starred now calls `unstar()` and decrements the count ## Evidence -- `RepositoryListViewModel.kt`: `toggleFavorite` now branches on whether the row is already - `Starred` this session — un-star path (`unstar()`, `starsCount - 1` coerced at 0, → `Idle`) and - star path (`star()`, `starsCount + 1`, → `Starred`) are both explicit `when` branches, replacing - the first pass's early-return no-op guard. +- `RepositoryListViewModel.kt`: `toggleFavorite`'s ALL-list `favoriteAction`/`starsCount` update + now happens synchronously before the coroutine launches (optimistic), with the network call + itself reduced to just `favoritesApi.star()/unstar()` inside `viewModelScope.launch`; the catch + block now rolls the optimistic change back on failure (restoring the pre-tap count, marking + `Failed`) instead of only ever advancing the display on success. - `RepositoryListViewModelTest.kt`: `repo()` test helper gained a `starsCount` parameter; tests cover starring incrementing the count by 1, a repeat tap un-starring (calls both `star()` and - `unstar()`, decrements the count, returns to `Idle`), and a full star→unstar round trip landing - back at the original count. + `unstar()`, decrements the count, returns to `Idle`), a full star→unstar round trip landing back + at the original count, the state/count updating immediately on tap *before* the queued coroutine + has run on the `StandardTestDispatcher` (the optimism itself), and a failed star/unstar each + rolling the count back to its pre-tap value. - `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. - `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing. - `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors. - Installed on the human product owner's device over wireless `adb` (`adb install -r`, streamed - install succeeded) twice across this task's two rounds; not yet re-tested live against the final - toggle behavior in this exact session. + install succeeded) three times across this task's three rounds; not yet re-tested live against + the optimistic-update behavior in this exact session. ## Known limitations