Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ import com.simprints.infra.sync.SyncOrchestrator
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.dropWhile
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.merge
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.launch
import javax.inject.Inject
Expand Down Expand Up @@ -60,6 +64,9 @@ internal class SyncInfoViewModel @Inject constructor(
private val imageSyncStatusFlow =
syncOrchestrator.observeImageSyncStatus()

private val eventSyncButtonClickFlow = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
private val imageSyncButtonClickFlow = MutableSharedFlow<Unit>(extraBufferCapacity = 1)

val logoutEventFlow: Flow<LiveDataEventWithContent<Unit>> = combine(
eventSyncStateFlow,
imageSyncStatusFlow,
Expand All @@ -75,16 +82,63 @@ internal class SyncInfoViewModel @Inject constructor(
}.flowOn(ioDispatcher)

val syncInfoLiveData: LiveData<SyncInfo> by lazy {
observeSyncInfo(isPreLogoutUpSync)
val dataLayerDrivenSyncInfoFlow = observeSyncInfo(isPreLogoutUpSync)
.onStart {
startInitialSyncIfRequired()
syncImagesAfterEventsWhenRequired()
}.flowOn(ioDispatcher)
}

/**
* Visual sync button responsiveness optimization
*
* The problem: data layer-driven progress visualization is simple programmatically, but can be slow in the UI.
*
* How it would work without the optimization:
* The forceEventSync and toggleImageSync invoke sync purely on the data layer,
* so the UI may remain unaware of the forced sync command until data-driven evidence of sync starts appearing.
* This may take seconds on slow devices.
*
* How it works with the optimization:
* Each forced sync, invoked by forceEventSync and toggleImageSync, immediately reshapes the flow of events:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, only this paragraph is relevant; the rest can be pruned.

Also, this is not a KDoc comment, so it should not have the double star at the beginning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the double star, but let's stay a bit more descriptive than usual about this non-trivial feature. There's been a history of more comments requested in similar situations rather than less.

* At first we immediately emit a sync state that is forcefully marked as in progress, for events or images separately.
* And we start ignoring sync states that happen before the true progress in data layer appears.
* Once the true progress in data layer starts, we keep showing that true progress.
* Additionally, an initial progress is emitted on start, before any forced sync invocations. To prevent getting stuck.
*/

val eventSyncButtonResponsiveSyncInfo = eventSyncButtonClickFlow.flatMapLatest {
dataLayerDrivenSyncInfoFlow.dropWhile { syncInfo ->
!syncInfo.syncInfoSectionRecords.isProgressVisible
}.onStart {
val initialState = syncInfoLiveData.value ?: SyncInfo()
emit(initialState.forceEventSyncProgress())
}
}

val imageSyncButtonResponsiveSyncInfo = imageSyncButtonClickFlow.flatMapLatest {
dataLayerDrivenSyncInfoFlow.dropWhile { syncInfo ->
!syncInfo.syncInfoSectionImages.isProgressVisible
}.onStart {
val initialState = syncInfoLiveData.value ?: SyncInfo()
emit(initialState.forceImageSyncProgress())
}
}

merge(
eventSyncButtonResponsiveSyncInfo,
imageSyncButtonResponsiveSyncInfo,
).onStart {
emit(dataLayerDrivenSyncInfoFlow.firstOrNull() ?: SyncInfo())
}.distinctUntilChanged().flowOn(ioDispatcher)
.asLiveData(viewModelScope.coroutineContext)
}

fun forceEventSync() {
viewModelScope.launch {
val isEventSyncing = eventSyncStateFlow.firstOrNull()?.isSyncInProgress() == true
if (!isEventSyncing) {
eventSyncButtonClickFlow.emit(Unit)
}
syncOrchestrator.stopEventSync()
Comment thread
BurningAXE marked this conversation as resolved.
val isDownSyncAllowed =
!isPreLogoutUpSync && configManager.getProject(authStore.signedInProjectId).state == ProjectState.RUNNING
Expand All @@ -98,6 +152,7 @@ internal class SyncInfoViewModel @Inject constructor(
if (isImageSyncing) {
syncOrchestrator.stopImageSync()
} else {
imageSyncButtonClickFlow.emit(Unit)
syncOrchestrator.startImageSync()
}
}
Expand Down Expand Up @@ -162,6 +217,32 @@ internal class SyncInfoViewModel @Inject constructor(
}
}

private fun SyncInfo.forceEventSyncProgress() = copy(
syncInfoSectionRecords = syncInfoSectionRecords.copy(
counterTotalRecords = "",
counterRecordsToUpload = "",
counterRecordsToDownload = "",
isInstructionDefaultVisible = false,
isInstructionCommCarePermissionVisible = false,
isInstructionNoModulesVisible = false,
isInstructionOfflineVisible = false,
isInstructionErrorVisible = false,
isProgressVisible = true,
isSyncButtonEnabled = false,
footerLastSyncMinutesAgo = "",
)
)

private fun SyncInfo.forceImageSyncProgress() = copy(
syncInfoSectionImages = syncInfoSectionImages.copy(
counterImagesToUpload = "",
isInstructionDefaultVisible = false,
isInstructionOfflineVisible = false,
isProgressVisible = true,
footerLastSyncMinutesAgo = "",
)
)

private suspend fun ConfigManager.isModuleSelectionRequired() =
getProjectConfiguration().isModuleSelectionAvailable() && getDeviceConfiguration().selectedModules.isEmpty()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.simprints.infra.sync.ImageSyncStatus
import com.simprints.infra.sync.SyncOrchestrator
import com.simprints.testtools.common.coroutines.TestCoroutineRule
import com.simprints.testtools.common.livedata.getOrAwaitValue
import com.simprints.testtools.common.livedata.getOrAwaitValues
import io.mockk.*
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
Expand Down Expand Up @@ -510,6 +511,126 @@ class SyncInfoViewModelTest {
coVerify(exactly = 0) { syncOrchestrator.startEventSync(any()) }
}

// Sync button responsiveness optimization

@Test
fun `should immediately show event progress snapshot when forcing event sync`() = runTest {
createViewModel()

val values = viewModel.syncInfoLiveData.getOrAwaitValues(number = 2) {
viewModel.forceEventSync()
}

val initial = values[0]
val forced = values[1]
assertThat(initial.syncInfoSectionRecords.isProgressVisible).isFalse()
assertThat(forced.syncInfoSectionRecords.isProgressVisible).isTrue()
}

@Test
fun `should not emit forced event progress when events already syncing`() = runTest {
val mockInProgressEventSyncState = mockk<EventSyncState>(relaxed = true) {
every { isSyncInProgress() } returns true
}
every { eventSyncManager.getLastSyncState(any()) } returns MutableLiveData(mockInProgressEventSyncState)
createViewModel()

val values = viewModel.syncInfoLiveData.getOrAwaitValues(number = 1) {
viewModel.forceEventSync()
}

val initial = values[0]
assertThat(initial.syncInfoSectionRecords.isProgressVisible).isFalse()
}

@Test
fun `should immediately show image progress snapshot when starting image sync`() = runTest {
val mockNotSyncingImageStatus = mockk<ImageSyncStatus>(relaxed = true) {
every { isSyncing } returns false
}
every { syncOrchestrator.observeImageSyncStatus() } returns MutableStateFlow(mockNotSyncingImageStatus)
createViewModel()

val values = viewModel.syncInfoLiveData.getOrAwaitValues(number = 2) {
viewModel.toggleImageSync()
}

val initial = values[0]
val forced = values[1]
assertThat(initial.syncInfoSectionImages.isProgressVisible).isFalse()
assertThat(forced.syncInfoSectionImages.isProgressVisible).isTrue()
}

@Test
fun `should not emit forced image progress when stopping image sync`() = runTest {
val mockSyncingImageStatus = mockk<ImageSyncStatus>(relaxed = true) {
every { isSyncing } returns true
}
every { syncOrchestrator.observeImageSyncStatus() } returns MutableStateFlow(mockSyncingImageStatus)
createViewModel()

val values = viewModel.syncInfoLiveData.getOrAwaitValues(number = 1) {
viewModel.toggleImageSync()
}

val initial = values[0]
assertThat(initial.syncInfoSectionImages.isProgressVisible).isFalse()
}

@Test
fun `should switch from forced to data-driven event sync progress once available`() = runTest {
val base = createDefaultSyncInfo()
val dataFlow = MutableStateFlow(base)
every { observeSyncInfo(any()) } returns dataFlow
createViewModel()

val values = viewModel.syncInfoLiveData.getOrAwaitValues(number = 3) {
viewModel.forceEventSync()
dataFlow.value = base.copy(
syncInfoSectionRecords = base.syncInfoSectionRecords.copy(
isProgressVisible = true,
counterTotalRecords = "123",
),
)
}

val initial = values[0]
val forced = values[1]
val dataDriven = values[2]
assertThat(initial.syncInfoSectionRecords.isProgressVisible).isFalse()
assertThat(forced.syncInfoSectionRecords.isProgressVisible).isTrue()
assertThat(forced.syncInfoSectionRecords.counterTotalRecords).isEmpty()
assertThat(dataDriven.syncInfoSectionRecords.isProgressVisible).isTrue()
assertThat(dataDriven.syncInfoSectionRecords.counterTotalRecords).isEqualTo("123")
}

@Test
fun `should switch from forced to data-driven image sync progress once available`() = runTest {
val base = createDefaultSyncInfo()
val dataFlow = MutableStateFlow(base)
every { observeSyncInfo(any()) } returns dataFlow
createViewModel()

val values = viewModel.syncInfoLiveData.getOrAwaitValues(number = 3) {
viewModel.toggleImageSync()
dataFlow.value = base.copy(
syncInfoSectionImages = base.syncInfoSectionImages.copy(
isProgressVisible = true,
counterImagesToUpload = "123",
),
)
}

val initial = values[0]
val forced = values[1]
val dataDriven = values[2]
assertThat(initial.syncInfoSectionImages.isProgressVisible).isFalse()
assertThat(forced.syncInfoSectionImages.isProgressVisible).isTrue()
assertThat(forced.syncInfoSectionImages.counterImagesToUpload).isEmpty()
assertThat(dataDriven.syncInfoSectionImages.isProgressVisible).isTrue()
assertThat(dataDriven.syncInfoSectionImages.counterImagesToUpload).isEqualTo("123")
}

// Other/combined UX case tests

@Test
Expand Down