diff --git a/mobile/feature/stop-detail/src/main/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModel.kt b/mobile/feature/stop-detail/src/main/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModel.kt index 3e6fd009..3c539466 100644 --- a/mobile/feature/stop-detail/src/main/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModel.kt +++ b/mobile/feature/stop-detail/src/main/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModel.kt @@ -274,7 +274,16 @@ class StopDetailViewModel } } + /** + * One-shot header fetch. Idempotent on a successful load: once the header is + * [HeaderState.Loaded] we don't re-fetch, so a config change (rotation) that recreates the + * ViewModel — or simply re-enters `init` — never re-hits `stops/{id}/route_type/{rt}` + * (issue #161). The genuine first load still runs (header is `Loading`), and + * [retryHeader] resets the state back to `Loading` before calling, so retry-after-error + * still re-fetches. + */ private fun loadHeader() { + if (_uiState.value.header is HeaderState.Loaded) return viewModelScope.launch { val result: Result = getStopDetail(stopId, routeType) _uiState.update { current -> @@ -292,7 +301,15 @@ class StopDetailViewModel private fun StopDetailUiState.applyDepartureResult(result: Result>): StopDetailUiState = when (result) { - is Result.Loading -> copy(departures = DeparturesState.Loading) + is Result.Loading -> + // Don't blow away an already-loaded list when the poll restarts. On a config + // change (rotation) `repeatOnLifecycle(RESUMED)` re-subscribes, and the + // repository's first re-emission is `Loading`; flipping back to the loading + // skeleton there is the visible "reload flicker" from issue #161. Keep the + // last-good `Loaded` rows on screen — the fresh data lands on the next + // `Success` tick and replaces them in place. Only the genuine first load (no + // rows yet) shows the skeleton. + if (departures is DeparturesState.Loaded) this else copy(departures = DeparturesState.Loading) is Result.Success -> { lastHeadPoll = result.data val merged = mergeDepartures(headPoll = result.data) diff --git a/mobile/feature/stop-detail/src/test/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModelTest.kt b/mobile/feature/stop-detail/src/test/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModelTest.kt index c5158974..3960c816 100644 --- a/mobile/feature/stop-detail/src/test/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModelTest.kt +++ b/mobile/feature/stop-detail/src/test/kotlin/ac/jfx/openptv/feature/stopdetail/StopDetailViewModelTest.kt @@ -100,6 +100,40 @@ class StopDetailViewModelTest { assertThat(viewModel.uiState.value.header).isEqualTo(HeaderState.Loaded(detail)) } + @Test + fun `header is fetched exactly once on first load — issue 161`() = + runTest(dispatcher) { + stopDetailRepository.enqueueSuccess(StopDetailMother.aStopDetail().build()) + val viewModel = newViewModel() + advanceUntilIdle() + + assertThat(viewModel.uiState.value.header).isInstanceOf(HeaderState.Loaded::class.java) + assertThat(stopDetailRepository.requestedKeys).hasSize(1) + } + + @Test + fun `restarting the departures poll does not refetch the header — issue 161`() = + runTest(dispatcher) { + // A config change (rotation) re-runs the repeatOnLifecycle(RESUMED) block, which calls + // startObserving() again. That must not drag the one-shot header fetch along with it — + // the header stays Loaded and `stops/{id}/route_type/{rt}` is hit only once. + stopDetailRepository.enqueueSuccess(StopDetailMother.aStopDetail().build()) + val viewModel = newViewModel() + advanceUntilIdle() + viewModel.startObserving() + advanceUntilIdle() + departureRepository.emitSuccess(listOf(DepartureMother.aDeparture().build())) + advanceUntilIdle() + + // Simulate the rotation Pause -> Resume: the lifecycle owner re-enters the block. + viewModel.stopObserving() + viewModel.startObserving() + advanceUntilIdle() + + assertThat(viewModel.uiState.value.header).isInstanceOf(HeaderState.Loaded::class.java) + assertThat(stopDetailRepository.requestedKeys).hasSize(1) + } + @Test fun `header Error renders the user-facing reason and retry restores Loading`() = runTest(dispatcher) { @@ -246,7 +280,7 @@ class StopDetailViewModelTest { } @Test - fun `loading emission flips departures back to Loading`() = + fun `first loading emission shows the loading skeleton`() = runTest(dispatcher) { stopDetailRepository.enqueueSuccess(StopDetailMother.aStopDetail().build()) val viewModel = newViewModel() @@ -254,14 +288,34 @@ class StopDetailViewModelTest { viewModel.startObserving() advanceUntilIdle() + // No rows have landed yet — a Loading emission should surface the skeleton. + departureRepository.emit(Result.Loading) + advanceUntilIdle() + assertThat(viewModel.uiState.value.departures).isEqualTo(DeparturesState.Loading) + } + + @Test + fun `loading emission after data keeps the last-good list on screen — issue 161`() = + runTest(dispatcher) { + // Rotation re-subscribes the poll via repeatOnLifecycle(RESUMED); the repository's + // first re-emission is Loading. We must NOT flip back to the skeleton — that's the + // reload flicker issue #161 fixes. The previously-loaded rows stay visible until the + // next Success tick replaces them. + stopDetailRepository.enqueueSuccess(StopDetailMother.aStopDetail().build()) + val viewModel = newViewModel() + advanceUntilIdle() + viewModel.startObserving() + advanceUntilIdle() + departureRepository.emitSuccess(listOf(DepartureMother.aDeparture().build())) advanceUntilIdle() - assertThat(viewModel.uiState.value.departures) - .isInstanceOf(DeparturesState.Loaded::class.java) + val loaded = viewModel.uiState.value.departures + assertThat(loaded).isInstanceOf(DeparturesState.Loaded::class.java) departureRepository.emit(Result.Loading) advanceUntilIdle() - assertThat(viewModel.uiState.value.departures).isEqualTo(DeparturesState.Loading) + // Same Loaded list, not the skeleton. + assertThat(viewModel.uiState.value.departures).isEqualTo(loaded) } @Test