From fdf550a9e4963dbdadb6fc89e4a2daf65d4909c2 Mon Sep 17 00:00:00 2001 From: andreia Date: Fri, 14 Aug 2026 15:52:22 +0200 Subject: [PATCH 1/6] make DataCollectionInitializer check for draft data before initializing --- .../DataCollectionInitializer.kt | 69 ++++++++++++++----- 1 file changed, 51 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt index c920921145..aa514242e5 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt @@ -25,10 +25,19 @@ import org.groundplatform.android.ui.common.LocationOfInterestHelper import org.groundplatform.android.ui.datacollection.DataCollectionInitializer.Companion.TASK_POSITION_ID import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.job.Job +import org.groundplatform.domain.model.submission.DraftSubmission +import org.groundplatform.domain.model.submission.TaskData +import org.groundplatform.domain.model.submission.ValueDelta import org.groundplatform.domain.model.task.Task import org.groundplatform.domain.repository.LocationOfInterestRepositoryInterface +import org.groundplatform.domain.repository.SubmissionRepositoryInterface import org.groundplatform.domain.repository.SurveyRepositoryInterface +data class DataCollectionInitializerResult( + val uiState: DataCollectionUiState, + val collectedData: Map = emptyMap(), +) + /** * DataCollectionInitializer * @@ -38,7 +47,8 @@ import org.groundplatform.domain.repository.SurveyRepositoryInterface * - Load active survey (with timeout). * - Resolve job by id. * - Pick displayable tasks (exclude Add-LOI when LOI exists). - * - Choose initial task id from SavedStateHandle (if valid) or first task. + * - Restore the data collected so far from the draft of an interrupted session, if any. + * - Choose initial task id from SavedStateHandle or the draft (if valid), or first task. * - Compute a simple [TaskPosition] from list order. * - Resolve a user-visible LOI name (typed for Add-LOI, formatted for existing LOI). */ @@ -47,31 +57,33 @@ class DataCollectionInitializer constructor( private val locationOfInterestHelper: LocationOfInterestHelper, private val locationOfInterestRepository: LocationOfInterestRepositoryInterface, + private val submissionRepository: SubmissionRepositoryInterface, private val surveyRepository: SurveyRepositoryInterface, ) { /** - * Computes the initial [DataCollectionUiState] without building any sequence. + * Computes the initial [DataCollectionInitializerResult] without building any sequence. * * Reads from [savedStateHandle]: * - [TASK_POSITION_ID]: previously visited task id (optional). - * - [TASK_LOI_NAME_KEY]: user-typed LOI name for Add-LOI flows (optional). */ suspend fun initialize( savedStateHandle: SavedStateHandle, jobId: String, loiId: String?, loiName: String?, - ): DataCollectionUiState = + ): DataCollectionInitializerResult = try { val survey = loadSurveyOrThrow() val job = resolveJobOrThrow(survey, jobId) val tasks = pickTasks(job, loiId) if (tasks.isEmpty()) throw DataCollectionException.NoValidTasks + val draft = findDraft(survey, jobId, loiId) + val savedTaskId: String? = savedStateHandle[TASK_POSITION_ID] val currentTaskId = - resolveInitialTaskId(tasks, savedTaskId) + resolveInitialTaskId(tasks, savedTaskId, draft?.currentTaskId) ?: throw DataCollectionException.Wrapped( DataCollectionErrorCode.INITIAL_TASK_RESOLUTION_FAILED, IllegalStateException("No valid initial task id"), @@ -89,21 +101,24 @@ constructor( val loiName = computeLoiName(survey.id, loiId, loiName) - DataCollectionUiState.Ready( - surveyId = survey.id, - job = job, - loiName = loiName, - tasks = tasks, - isAddLoiFlow = loiId == null, - currentTaskId = currentTaskId, - position = position, + DataCollectionInitializerResult( + DataCollectionUiState.Ready( + surveyId = survey.id, + job = job, + loiName = loiName, + tasks = tasks, + isAddLoiFlow = loiId == null, + currentTaskId = currentTaskId, + position = position, + ), + collectedData = restoreData(tasks, draft?.deltas.orEmpty()), ) } catch (e: DataCollectionException) { - DataCollectionUiState.Error(e.code, e) + DataCollectionInitializerResult(DataCollectionUiState.Error(e.code, e)) } catch (c: CancellationException) { throw c } catch (t: Throwable) { - DataCollectionUiState.Error(mapThrowableToCode(t), t) + DataCollectionInitializerResult(DataCollectionUiState.Error(mapThrowableToCode(t), t)) } private suspend fun loadSurveyOrThrow(): Survey = @@ -122,14 +137,32 @@ constructor( private fun pickTasks(job: Job, loiId: String?): List = if (loiId == null) job.tasksSorted else job.tasksSorted.filterNot { it.isAddLoiTask } + /** + * Returns the draft of an interrupted data collection session for the given [survey], [jobId] and + * [loiId], or `null` when there is none, or the stored one belongs to a different session. + */ + private suspend fun findDraft(survey: Survey, jobId: String, loiId: String?): DraftSubmission? { + val draftId = submissionRepository.getDraftSubmissionsId() + val draft = + if (draftId.isEmpty()) null else submissionRepository.getDraftSubmission(draftId, survey) + return draft?.takeIf { it.surveyId == survey.id && it.jobId == jobId && it.loiId == loiId } + } + + private fun restoreData(tasks: List, deltas: List): Map { + val deltaMap = deltas.associateBy { it.taskId to it.taskType } + return tasks + .mapNotNull { task -> deltaMap[task.id to task.type]?.newTaskData?.let { task to it } } + .toMap() + } + /** * Choose initial task: - * - Use [saved] if it exists within [tasks]. + * - Use the first of [candidates] which exists within [tasks]. * - Otherwise, first task in list. */ - private fun resolveInitialTaskId(tasks: List, saved: String?): String? { + private fun resolveInitialTaskId(tasks: List, vararg candidates: String?): String? { val validIds = tasks.map { it.id }.toSet() - return saved?.takeIf { it in validIds } ?: tasks.firstOrNull()?.id + return candidates.filterNotNull().firstOrNull { it in validIds } ?: tasks.firstOrNull()?.id } /** From ef1e0584382d7221972708248504fadfaa823df6 Mon Sep 17 00:00:00 2001 From: andreia Date: Fri, 14 Aug 2026 15:53:31 +0200 Subject: [PATCH 2/6] update TaskSequenceHandler to provide the right task to resume --- .../ui/datacollection/TaskSequenceHandler.kt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandler.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandler.kt index 69bbc571d6..7949cd5820 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandler.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandler.kt @@ -82,6 +82,21 @@ class TaskSequenceHandler( return validTasks } + /** + * Returns the task ID to resume. Ensures sequences never skip past tasks with required answers, + * so incomplete answers get recollected instead of omitted. + */ + fun getResumeTask(taskId: String): String { + validateTaskId(taskId) + val sequence = getValidTasks() + val target = sequence.firstOrNull { it.id == taskId } ?: sequence.first() + val unanswered = + sequence + .takeWhile { it != target } + .firstOrNull { (it.isRequired || it.isAddLoiTask) && taskDataHandler.getData(it) == null } + return (unanswered ?: target).id + } + /** Resets the local cache of the task list. */ fun invalidateCache() { isTaskListReady = false From 597acc064756a3a8173ea8d30f00a22266b0d95e Mon Sep 17 00:00:00 2001 From: andreia Date: Fri, 14 Aug 2026 15:53:50 +0200 Subject: [PATCH 3/6] update DataCollectionViewModel with the new logic --- .../datacollection/DataCollectionViewModel.kt | 78 ++++++++----------- .../android/ui/home/HomeScreenFragment.kt | 4 - .../HomeScreenMapContainerFragment.kt | 6 -- app/src/main/res/navigation/nav_graph.xml | 22 ------ 4 files changed, 31 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt index c99116a8e3..55e599b929 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt @@ -32,7 +32,6 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import org.groundplatform.android.data.local.room.converter.SubmissionDeltasConverter import org.groundplatform.android.data.uuid.OfflineUuidGenerator import org.groundplatform.android.di.coroutines.ApplicationScope import org.groundplatform.android.di.coroutines.IoDispatcher @@ -51,7 +50,6 @@ import org.groundplatform.android.ui.datacollection.tasks.point.DropPinTaskViewM import org.groundplatform.android.ui.datacollection.tasks.polygon.DrawAreaTaskViewModel import org.groundplatform.android.ui.datacollection.tasks.text.TextTaskViewModel import org.groundplatform.android.ui.datacollection.tasks.time.TimeTaskViewModel -import org.groundplatform.domain.model.job.Job import org.groundplatform.domain.model.submission.TaskData import org.groundplatform.domain.model.submission.ValueDelta import org.groundplatform.domain.model.submission.isNotNullOrEmpty @@ -108,8 +106,6 @@ internal constructor( private val _loiNameDraft = MutableStateFlow("") val loiNameDraft: StateFlow = _loiNameDraft - private var shouldLoadFromDraft: Boolean = savedStateHandle[TASK_SHOULD_LOAD_FROM_DRAFT] ?: false - private val jobId: String = requireNotNull(savedStateHandle[TASK_JOB_ID_KEY]) private val loiId: String? = savedStateHandle[TASK_LOI_ID_KEY] @@ -129,20 +125,32 @@ internal constructor( getTypedLoiNameOrEmpty(), ) - if (initResult is DataCollectionUiState.Ready) { - if (shouldLoadFromDraft) { - initializeDraftValues(initResult.job, initResult.tasks) + _uiState.value = + when (val state = initResult.uiState) { + is DataCollectionUiState.Ready -> { + taskDataHandler.setData(initResult.collectedData) + taskSequenceHandler = TaskSequenceHandler(state.tasks, taskDataHandler) + setupSession(state) + } + is DataCollectionUiState.Error -> { + Timber.e(state.cause, "Initialization failed code=%s", state.code) + state + } + else -> { + state + } } - taskSequenceHandler = TaskSequenceHandler(initResult.tasks, taskDataHandler) - } - - if (initResult is DataCollectionUiState.Error) { - Timber.e(initResult.cause, "Initialization failed code=%s", initResult.code) - } - _uiState.value = initResult } } + private fun setupSession(state: DataCollectionUiState.Ready): DataCollectionUiState.Ready { + val taskId = taskSequenceHandler.getResumeTask(state.currentTaskId) + if (taskId == state.currentTaskId) return state + + Timber.w("No data restored for task %s; resuming at %s", state.currentTaskId, taskId) + return state.withTask(taskId) + } + private fun setLoiName(name: String) { savedStateHandle[TASK_LOI_NAME_KEY] = name _uiState.update { state -> @@ -410,11 +418,9 @@ internal constructor( val validIds = taskSequenceHandler.getValidTasks().map { it.id }.toSet() val safeId = if (taskId in validIds) taskId else validIds.first() - savedStateHandle[TASK_POSITION_ID] = safeId + val newState = st.withTask(safeId) saveDraft(safeId) - - val newPos = taskSequenceHandler.getTaskPosition(safeId) - _uiState.value = st.copy(currentTaskId = safeId, position = newPos) + _uiState.value = newState } private fun getDeltas(): List { @@ -464,33 +470,6 @@ internal constructor( return block(s) } - private fun initializeDraftValues(job: Job, tasks: List) { - val serialized: String? = savedStateHandle[TASK_DRAFT_VALUES] - if (serialized.isNullOrBlank()) { - Timber.w("No draft values found; skipping load") - return - } - - val parsed = - try { - SubmissionDeltasConverter.fromString(job, serialized) - } catch (e: Exception) { - Timber.e(e, "Failed to parse draft submission") - emptyList() - } - - val deltaMap = parsed.associateBy { it.taskId to it.taskType } - - val draftValues = - tasks - .mapNotNull { task -> deltaMap[task.id to task.type]?.newTaskData?.let { task to it } } - .toMap() - - if (draftValues.isNotEmpty()) { - taskDataHandler.setData(draftValues) - } - } - private inline fun validateOrShow(taskVm: AbstractTaskViewModel, onValid: () -> Unit) { val error = taskVm.validate() if (error != null) { @@ -505,13 +484,18 @@ internal constructor( .map { (it as? DataCollectionUiState.Ready)?.currentTaskId == taskId } .distinctUntilChanged() + private fun DataCollectionUiState.Ready.withTask( + taskId: String + ): DataCollectionUiState.Ready { + savedStateHandle[TASK_POSITION_ID] = taskId + return copy(currentTaskId = taskId, position = taskSequenceHandler.getTaskPosition(taskId)) + } + companion object { private const val TASK_JOB_ID_KEY = "jobId" private const val TASK_LOI_ID_KEY = "locationOfInterestId" private const val TASK_LOI_NAME_KEY = "locationOfInterestName" private const val TASK_POSITION_ID = "currentTaskId" - private const val TASK_DRAFT_VALUES = "draftValues" - private const val TASK_SHOULD_LOAD_FROM_DRAFT = "shouldLoadFromDraft" fun getViewModelClass(taskType: Task.Type): Class = when (taskType) { diff --git a/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenFragment.kt b/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenFragment.kt index cf212a92d9..03adbf731b 100644 --- a/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenFragment.kt @@ -33,7 +33,6 @@ import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject import kotlinx.coroutines.launch import org.groundplatform.android.R -import org.groundplatform.android.data.local.room.converter.SubmissionDeltasConverter import org.groundplatform.android.databinding.HomeScreenFragBinding import org.groundplatform.android.ui.common.AbstractFragment import org.groundplatform.android.ui.common.BackPressListener @@ -152,9 +151,6 @@ class HomeScreenFragment : AbstractFragment(), BackPressListener { draft.loiId, draft.loiName ?: "", draft.jobId, - true, - SubmissionDeltasConverter.toString(draft.deltas), - draft.currentTaskId ?: "", ) ) diff --git a/app/src/main/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt b/app/src/main/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt index f249c00f20..c13c897b2c 100644 --- a/app/src/main/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt @@ -192,9 +192,6 @@ class HomeScreenMapContainerFragment : AbstractMapContainerFragment() { cardUiData.loi.id, cardUiData.loi.properties[LOI_NAME_PROPERTY] as? String?, cardUiData.loi.job.id, - false, - null, - "", ) ) is AdHocDataCollectionButtonData -> @@ -204,9 +201,6 @@ class HomeScreenMapContainerFragment : AbstractMapContainerFragment() { null, null, cardUiData.job.id, - false, - null, - "", ) ) } diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index 30d7db288b..1d08ac3b28 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -98,17 +98,6 @@ - - - - - - Date: Fri, 14 Aug 2026 15:54:05 +0200 Subject: [PATCH 4/6] update unit tests --- .../DataCollectionFragmentTest.kt | 70 ++++++++++++------- .../DataCollectionViewModelTest.kt | 3 +- .../datacollection/TaskSequenceHandlerTest.kt | 44 +++++++++++- 3 files changed, 89 insertions(+), 28 deletions(-) diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt index 2a98dc2823..f47a4f9dc4 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt @@ -39,7 +39,6 @@ import org.groundplatform.android.FakeData.LOCATION_OF_INTEREST import org.groundplatform.android.FakeData.LOCATION_OF_INTEREST_NAME import org.groundplatform.android.FakeData.USER import org.groundplatform.android.R -import org.groundplatform.android.data.local.room.converter.SubmissionDeltasConverter import org.groundplatform.android.data.remote.FakeRemoteDataStore import org.groundplatform.android.data.sync.MutationSyncWorkManager import org.groundplatform.android.di.PdfModule @@ -327,9 +326,8 @@ class DataCollectionFragmentTest : BaseHiltTest() { setupFragment( loiId = null, loiName = null, - shouldLoadFromDraft = true, - draftValues = SubmissionDeltasConverter.toString(expectedDeltas), - currentTaskId = TASK_ID_2, + draftDeltas = expectedDeltas, + draftCurrentTaskId = TASK_ID_2, ) runner() @@ -343,6 +341,19 @@ class DataCollectionFragmentTest : BaseHiltTest() { ) } + @Test + fun `Resumes at the first task whose data could not be restored`() = runWithTestDispatcher { + // The draft holds no answer for the add LOI task, e.g. because the task changed in the survey. + setupFragment( + loiId = null, + loiName = null, + draftDeltas = listOf(TASK_1_VALUE_DELTA), + draftCurrentTaskId = TASK_ID_2, + ) + + runner().validateTextIsDisplayed(TASK_0_NAME) + } + @Test fun `Does not load draft if it references missing job`() = runWithTestDispatcher { setupFragment() @@ -664,11 +675,15 @@ class DataCollectionFragmentTest : BaseHiltTest() { } @Test - fun `Loading tasks from draft with invalid data handles gracefully`() = runWithTestDispatcher { - setupFragment(shouldLoadFromDraft = true, draftValues = "invalid-json-data") + fun `Draft of another LOI is ignored`() = runWithTestDispatcher { + setupFragment( + draftDeltas = listOf(TASK_1_VALUE_DELTA), + draftLoiId = "another loi", + draftCurrentTaskId = TASK_ID_2, + ) - // Should still load first task even with invalid draft data - runner().validateTextIsDisplayed(TASK_1_NAME) + // The first task is shown, with nothing restored into it. + runner().validateTextIsDisplayed(TASK_1_NAME).assertButtonIsDisabled("Next") } @Test @@ -937,10 +952,7 @@ class DataCollectionFragmentTest : BaseHiltTest() { } private fun setupFragmentWithDraft(expectedValues: List) { - setupFragment( - shouldLoadFromDraft = true, - draftValues = SubmissionDeltasConverter.toString(expectedValues), - ) + setupFragment(draftDeltas = expectedValues) } private fun setupFragmentWithNoLoi() { @@ -951,27 +963,35 @@ class DataCollectionFragmentTest : BaseHiltTest() { viewModel.updateCameraPosition(CameraPosition(TASK_0_RESPONSE)) } + /** + * Launches the fragment, optionally with [draftDeltas] stored as the draft of an interrupted data + * collection session for the same job and LOI. + */ private fun setupFragment( loiId: String? = LOCATION_OF_INTEREST.id, loiName: String? = LOCATION_OF_INTEREST_NAME, tasks: List = TASKS, - shouldLoadFromDraft: Boolean = false, - draftValues: String? = null, - currentTaskId: String = "", + draftDeltas: List? = null, + draftLoiId: String? = loiId, + draftCurrentTaskId: String = "", ) { setupSubmission(tasks) - val argsBundle = - DataCollectionFragmentArgs.Builder( - loiId, - loiName, - JOB.id, - shouldLoadFromDraft, - draftValues, - currentTaskId, + draftDeltas?.let { + runWithTestDispatcher { + submissionRepository.saveDraftSubmission( + jobId = JOB.id, + loiId = draftLoiId, + surveyId = SURVEY.id, + deltas = it, + loiName = loiName, + currentTaskId = draftCurrentTaskId, ) - .build() - .toBundle() + } + } + + val argsBundle = + DataCollectionFragmentArgs.Builder(loiId, loiName, JOB.id).build().toBundle() fragmentScenario.launchFragmentWithNavController( argsBundle, diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModelTest.kt index 6e508f80de..a25effb1a0 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModelTest.kt @@ -107,7 +107,8 @@ class DataCollectionViewModelTest : BaseHiltTest() { private suspend fun setupViewModel(result: DataCollectionUiState): DataCollectionViewModel { val initializer = mock().apply { - whenever(initialize(any(), any(), anyOrNull(), anyOrNull())) doReturn result + whenever(initialize(any(), any(), anyOrNull(), anyOrNull())) doReturn + DataCollectionInitializerResult(result) } return DataCollectionViewModel( diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandlerTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandlerTest.kt index fe4d0a89e8..eb1702b273 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandlerTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/TaskSequenceHandlerTest.kt @@ -54,13 +54,18 @@ class TaskSequenceHandlerTest { private val taskDataHandler = TaskDataHandler() private val taskSequenceHandler = TaskSequenceHandler(allTasks, taskDataHandler) - private fun createTask(taskId: String, index: Int, condition: Condition? = null) = + private fun createTask( + taskId: String, + index: Int, + condition: Condition? = null, + isRequired: Boolean = true, + ) = Task( taskId, index, Type.MULTIPLE_CHOICE, label = "", - true, + isRequired, multipleChoice = multipleChoice, condition = condition, ) @@ -289,6 +294,41 @@ class TaskSequenceHandlerTest { } } + @Test + fun `getResumeTask returns the given task when the ones before it are answered`() { + taskDataHandler.setData(task1, MultipleChoiceTaskData(multipleChoice, listOf(option1.id))) + + assertThat(taskSequenceHandler.getResumeTask(task2.id)).isEqualTo(task2.id) + } + + @Test + fun `getResumeTask returns the first unanswered task before the given one`() { + assertThat(taskSequenceHandler.getResumeTask(task2.id)).isEqualTo(task1.id) + } + + @Test + fun `getResumeTask ignores unanswered optional tasks`() { + val optionalTask = createTask(taskId = "optional", index = 0, isRequired = false) + val handler = TaskSequenceHandler(listOf(optionalTask, task2), taskDataHandler) + + assertThat(handler.getResumeTask(task2.id)).isEqualTo(task2.id) + } + + @Test + fun `getResumeTask returns the given task when it is the first one`() { + assertThat(taskSequenceHandler.getResumeTask(task1.id)).isEqualTo(task1.id) + } + + @Test + fun `getResumeTask returns the first task when the given one is not in the sequence`() { + assertThat(taskSequenceHandler.getResumeTask(conditionalTask.id)).isEqualTo(task1.id) + } + + @Test + fun `getResumeTask throws error for invalid task id`() { + assertThrows(IllegalArgumentException::class.java) { taskSequenceHandler.getResumeTask("") } + } + @Test fun `generateValidTasksList handles large chain of conditional tasks`() { // Create a chain of 100 tasks, each depending on the previous one. From e06267a20b37d6af74bb32df4e7fe94e649c2b54 Mon Sep 17 00:00:00 2001 From: andreia Date: Mon, 17 Aug 2026 11:56:26 +0200 Subject: [PATCH 5/6] apply code review: add getDraftSubmissionForSession to SubmissionRepository --- .../repository/SubmissionRepository.kt | 21 +++-- .../DataCollectionInitializer.kt | 14 +-- .../android/ui/home/HomeScreenViewModel.kt | 19 +--- .../repository/SubmissionRepositoryTest.kt | 86 +++++++++++++------ .../DataCollectionFragmentTest.kt | 17 ++-- .../SubmissionRepositoryInterface.kt | 20 ++++- .../testing/FakeSubmissionRepository.kt | 21 +++-- 7 files changed, 114 insertions(+), 84 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/repository/SubmissionRepository.kt b/app/src/main/java/org/groundplatform/android/repository/SubmissionRepository.kt index 352cbfdf34..f3c42499d4 100644 --- a/app/src/main/java/org/groundplatform/android/repository/SubmissionRepository.kt +++ b/app/src/main/java/org/groundplatform/android/repository/SubmissionRepository.kt @@ -70,16 +70,27 @@ constructor( } ?: run { Timber.w("Job not found for survey $surveyId and LOI $locationOfInterestId") } } - override suspend fun getDraftSubmission( - draftSubmissionId: String, + override suspend fun getDraftSubmission(survey: Survey): DraftSubmission? { + val draftId = localValueStore.draftSubmissionId + val draft = + if (draftId.isNullOrEmpty()) null + else localSubmissionStore.getDraftSubmission(draftSubmissionId = draftId, survey = survey) + if (draft != null && draft.surveyId != survey.id) { + Timber.e("Skipping draft submission, survey id doesn't match") + return null + } + return draft + } + + override suspend fun getDraftSubmissionForSession( survey: Survey, + jobId: String, + loiId: String?, ): DraftSubmission? = - localSubmissionStore.getDraftSubmission(draftSubmissionId = draftSubmissionId, survey = survey) + getDraftSubmission(survey)?.takeIf { it.jobId == jobId && it.loiId == loiId } override suspend fun countDraftSubmissions() = localSubmissionStore.countDraftSubmissions() - override fun getDraftSubmissionsId() = localValueStore.draftSubmissionId ?: "" - override suspend fun saveDraftSubmission( jobId: String, loiId: String?, diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt index aa514242e5..35c2e101a8 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionInitializer.kt @@ -25,7 +25,6 @@ import org.groundplatform.android.ui.common.LocationOfInterestHelper import org.groundplatform.android.ui.datacollection.DataCollectionInitializer.Companion.TASK_POSITION_ID import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.job.Job -import org.groundplatform.domain.model.submission.DraftSubmission import org.groundplatform.domain.model.submission.TaskData import org.groundplatform.domain.model.submission.ValueDelta import org.groundplatform.domain.model.task.Task @@ -79,7 +78,7 @@ constructor( val tasks = pickTasks(job, loiId) if (tasks.isEmpty()) throw DataCollectionException.NoValidTasks - val draft = findDraft(survey, jobId, loiId) + val draft = submissionRepository.getDraftSubmissionForSession(survey, jobId, loiId) val savedTaskId: String? = savedStateHandle[TASK_POSITION_ID] val currentTaskId = @@ -137,17 +136,6 @@ constructor( private fun pickTasks(job: Job, loiId: String?): List = if (loiId == null) job.tasksSorted else job.tasksSorted.filterNot { it.isAddLoiTask } - /** - * Returns the draft of an interrupted data collection session for the given [survey], [jobId] and - * [loiId], or `null` when there is none, or the stored one belongs to a different session. - */ - private suspend fun findDraft(survey: Survey, jobId: String, loiId: String?): DraftSubmission? { - val draftId = submissionRepository.getDraftSubmissionsId() - val draft = - if (draftId.isEmpty()) null else submissionRepository.getDraftSubmission(draftId, survey) - return draft?.takeIf { it.surveyId == survey.id && it.jobId == jobId && it.loiId == loiId } - } - private fun restoreData(tasks: List, deltas: List): Map { val deltaMap = deltas.associateBy { it.taskId to it.taskType } return tasks diff --git a/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenViewModel.kt index 9312ecc799..ffd71ed586 100644 --- a/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/home/HomeScreenViewModel.kt @@ -48,7 +48,6 @@ import org.groundplatform.domain.repository.OfflineAreaRepositoryInterface import org.groundplatform.domain.repository.SubmissionRepositoryInterface import org.groundplatform.domain.repository.SurveyRepositoryInterface import org.groundplatform.domain.repository.UserRepositoryInterface -import timber.log.Timber data class HomeDrawerState(val user: User, val survey: Survey?, val appVersion: String) @@ -128,24 +127,10 @@ internal constructor( /** Attempts to return draft submission for the currently active active survey. */ suspend fun getDraftSubmission(): DraftSubmission? { - val draftId = submissionRepository.getDraftSubmissionsId() - val survey = surveyRepository.activeSurveyFlow.first() - - if (survey == null || draftId.isEmpty()) { - // No active survey or draft submission. - return null - } - - val draft = submissionRepository.getDraftSubmission(draftId, survey) ?: return null - - if (draft.surveyId != survey.id) { - Timber.e("Skipping draft submission, survey id doesn't match") - return null - } - // TODO: Check whether the previous user id matches with current user or not. // Issue URL: https://github.com/google/ground-android/issues/2903 - return draft + val survey = surveyRepository.activeSurveyFlow.first() ?: return null + return submissionRepository.getDraftSubmission(survey) } fun openNavDrawer() { diff --git a/app/src/test/java/org/groundplatform/android/repository/SubmissionRepositoryTest.kt b/app/src/test/java/org/groundplatform/android/repository/SubmissionRepositoryTest.kt index 65377363fe..2138cc3716 100644 --- a/app/src/test/java/org/groundplatform/android/repository/SubmissionRepositoryTest.kt +++ b/app/src/test/java/org/groundplatform/android/repository/SubmissionRepositoryTest.kt @@ -115,56 +115,92 @@ class SubmissionRepositoryTest { @Test fun `getDraftSubmission gets the draft from the local store`() = runTest { setupMocks() - val result = repository.getDraftSubmission(DRAFT_SUBMISSION.id, TEST_SURVEY) + val result = repository.getDraftSubmission(TEST_SURVEY) assertThat(result).isEqualTo(DRAFT_SUBMISSION) + verify(localSubmissionStore).getDraftSubmission(DRAFT_SUBMISSION.id, TEST_SURVEY) + assertThat(repository.getDraftSubmission(TEST_SURVEY)?.id).isEqualTo(DRAFT_SUBMISSION.id) } @Test fun `getDraftSubmission returns null when not found`() = runTest { setupMocks(draftSubmissions = null) - assertThat(repository.getDraftSubmission("missing", TEST_SURVEY)).isNull() + assertThat(repository.getDraftSubmission(TEST_SURVEY)).isNull() } @Test - fun `countDraftSubmissions counts the draft submissions in the local store`() = runTest { - setupMocks( - draftSubmissions = - listOf( - DRAFT_SUBMISSION, - DRAFT_SUBMISSION.copy(id = "draft-2", surveyId = "survey2"), - DRAFT_SUBMISSION.copy(id = "draft-3", surveyId = "survey3"), - ) - ) + fun `getDraftSubmission returns null when there is no draftSubmissionId stored`() = runTest { + setupMocks(draftSubmissions = listOf(DRAFT_SUBMISSION), selectedDraftId = null) - assertThat(repository.countDraftSubmissions()).isEqualTo(3) + assertThat(repository.getDraftSubmission(TEST_SURVEY)).isNull() + } + + @Test + fun `getDraftSubmission returns null when the draft belongs to a different survey`() = runTest { + setupMocks(draftSubmissions = listOf(DRAFT_SUBMISSION.copy(surveyId = "some-other-survey"))) + + assertThat(repository.getDraftSubmission(TEST_SURVEY)).isNull() + } + + @Test + fun `getDraftSubmissionForSession returns the draft when job and LOI match`() = runTest { + setupMocks() + + assertThat(repository.getDraftSubmissionForSession(TEST_SURVEY, TEST_JOB.id, TEST_LOI.id)) + .isEqualTo(DRAFT_SUBMISSION) + } + + @Test + fun `getDraftSubmissionForSession returns null when the draft is for a different job`() = + runTest { + setupMocks() + + assertThat(repository.getDraftSubmissionForSession(TEST_SURVEY, "other-job", TEST_LOI.id)) + .isNull() + } + + @Test + fun `getDraftSubmissionForSession returns null when the draft is for a different LOI`() = + runTest { + setupMocks() + + assertThat(repository.getDraftSubmissionForSession(TEST_SURVEY, TEST_JOB.id, "other-loi")) + .isNull() + } + + @Test + fun `getDraftSubmissionForSession matches an add-LOI draft on a null LOI id`() = runTest { + val addLoiDraft = DRAFT_SUBMISSION.copy(loiId = null) + setupMocks(draftSubmissions = listOf(addLoiDraft)) + + assertThat(repository.getDraftSubmissionForSession(TEST_SURVEY, TEST_JOB.id, null)) + .isEqualTo(addLoiDraft) } @Test - fun `getDraftSubmissionsId returns id from local value store`() = runTest { - val selectedDraftId = "draft-3" + fun `getDraftSubmissionForSession returns null when the draft is for a different survey`() = + runTest { + setupMocks(draftSubmissions = listOf(DRAFT_SUBMISSION.copy(surveyId = "some-other-survey"))) + + assertThat(repository.getDraftSubmissionForSession(TEST_SURVEY, TEST_JOB.id, TEST_LOI.id)) + .isNull() + } + + @Test + fun `countDraftSubmissions counts the draft submissions in the local store`() = runTest { setupMocks( draftSubmissions = listOf( DRAFT_SUBMISSION, DRAFT_SUBMISSION.copy(id = "draft-2", surveyId = "survey2"), DRAFT_SUBMISSION.copy(id = "draft-3", surveyId = "survey3"), - ), - selectedDraftId = selectedDraftId, + ) ) - assertThat(repository.getDraftSubmissionsId()).isEqualTo(selectedDraftId) + assertThat(repository.countDraftSubmissions()).isEqualTo(3) } - @Test - fun `getDraftSubmissionsId returns empty string when there is no draftSubmissionId stored`() = - runTest { - setupMocks(draftSubmissions = listOf(DRAFT_SUBMISSION), selectedDraftId = null) - - assertThat(repository.getDraftSubmissionsId()).isEmpty() - } - @Test fun `saveDraftSubmission saves and updates the id in the local store`() = runTest { setupMocks() diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt index f47a4f9dc4..5fe3fa2a66 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/DataCollectionFragmentTest.kt @@ -361,15 +361,14 @@ class DataCollectionFragmentTest : BaseHiltTest() { runner().inputText(TASK_1_RESPONSE).clickNextButton() // Verify draft was saved - val draftId = submissionRepository.getDraftSubmissionsId() - assertThat(draftId).isNotEmpty() + assertThat(submissionRepository.getDraftSubmission(SURVEY)).isNotNull() assertThat(submissionRepository.countDraftSubmissions()).isEqualTo(1) // Simulate deleting the job from the submission val surveyWithMissingJob = SURVEY.copy(jobMap = emptyMap()) // Attempt to get draft with the survey that's missing the job - val result = submissionRepository.getDraftSubmission(draftId, surveyWithMissingJob) + val result = submissionRepository.getDraftSubmission(surveyWithMissingJob) assertThat(result).isNull() } @@ -918,15 +917,14 @@ class DataCollectionFragmentTest : BaseHiltTest() { } private suspend fun assertDraftSaved(valueDeltas: List, currentTaskId: String) { - val draftId = submissionRepository.getDraftSubmissionsId() - assertThat(draftId).isNotEmpty() + val draft = checkNotNull(submissionRepository.getDraftSubmission(SURVEY)) // Exactly 1 draft should be present always. assertThat(submissionRepository.countDraftSubmissions()).isEqualTo(1) - assertThat(submissionRepository.getDraftSubmission(draftId, SURVEY)) + assertThat(draft) .isEqualTo( DraftSubmission( - id = draftId, + id = draft.id, jobId = JOB.id, loiId = LOCATION_OF_INTEREST.id, loiName = LOCATION_OF_INTEREST_NAME, @@ -938,7 +936,7 @@ class DataCollectionFragmentTest : BaseHiltTest() { } private suspend fun assertNoDraftSaved() { - assertThat(submissionRepository.getDraftSubmissionsId()).isEmpty() + assertThat(submissionRepository.getDraftSubmission(SURVEY)).isNull() assertThat(submissionRepository.countDraftSubmissions()).isEqualTo(0) } @@ -990,8 +988,7 @@ class DataCollectionFragmentTest : BaseHiltTest() { } } - val argsBundle = - DataCollectionFragmentArgs.Builder(loiId, loiName, JOB.id).build().toBundle() + val argsBundle = DataCollectionFragmentArgs.Builder(loiId, loiName, JOB.id).build().toBundle() fragmentScenario.launchFragmentWithNavController( argsBundle, diff --git a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SubmissionRepositoryInterface.kt b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SubmissionRepositoryInterface.kt index 7b198accb6..a1faa413c1 100644 --- a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SubmissionRepositoryInterface.kt +++ b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SubmissionRepositoryInterface.kt @@ -35,11 +35,25 @@ interface SubmissionRepositoryInterface { collectionId: String, ) - suspend fun getDraftSubmission(draftSubmissionId: String, survey: Survey): DraftSubmission? + /** + * Returns the draft submission of an interrupted data collection session for [survey], whichever + * job and LOI it belongs to, or `null` when none is stored or the stored one belongs to a + * different survey. + */ + suspend fun getDraftSubmission(survey: Survey): DraftSubmission? - suspend fun countDraftSubmissions(): Int + /** + * Returns the draft submission of an interrupted data collection session, but only when it + * belongs to the session identified by [survey], [jobId] and [loiId]. Returns `null` when there + * is no draft, or the stored one was left by a different session. + */ + suspend fun getDraftSubmissionForSession( + survey: Survey, + jobId: String, + loiId: String?, + ): DraftSubmission? - fun getDraftSubmissionsId(): String + suspend fun countDraftSubmissions(): Int suspend fun saveDraftSubmission( jobId: String, diff --git a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSubmissionRepository.kt b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSubmissionRepository.kt index 399ae61171..f76202c033 100644 --- a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSubmissionRepository.kt +++ b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSubmissionRepository.kt @@ -23,8 +23,7 @@ import org.groundplatform.domain.model.submission.ValueDelta import org.groundplatform.domain.repository.SubmissionRepositoryInterface class FakeSubmissionRepository : SubmissionRepositoryInterface { - var draftSubmission: List = emptyList() - var latestDraftSubmissionId: String = "" + var draftSubmission: DraftSubmission? = null var pendingCreateCount: Int = 0 var pendingDeleteCount: Int = 0 var submissions: List = emptyList() @@ -39,14 +38,15 @@ class FakeSubmissionRepository : SubmissionRepositoryInterface { onSaveSubmissionCall(SaveSubmissionParams(surveyId, locationOfInterestId, deltas, collectionId)) } - override suspend fun getDraftSubmission( - draftSubmissionId: String, - survey: Survey, - ): DraftSubmission? = draftSubmission.firstOrNull { it.id == draftSubmissionId } + override suspend fun getDraftSubmission(survey: Survey): DraftSubmission? = draftSubmission - override suspend fun countDraftSubmissions(): Int = draftSubmission.count() + override suspend fun getDraftSubmissionForSession( + survey: Survey, + jobId: String, + loiId: String?, + ): DraftSubmission? = draftSubmission - override fun getDraftSubmissionsId(): String = latestDraftSubmissionId + override suspend fun countDraftSubmissions(): Int = if (draftSubmission == null) 0 else 1 override suspend fun saveDraftSubmission( jobId: String, @@ -56,7 +56,7 @@ class FakeSubmissionRepository : SubmissionRepositoryInterface { loiName: String?, currentTaskId: String, ) { - draftSubmission += + draftSubmission = FakeDataGenerator.newDraftSubmission( jobId = jobId, loiId = loiId, @@ -68,8 +68,7 @@ class FakeSubmissionRepository : SubmissionRepositoryInterface { } override suspend fun deleteDraftSubmission() { - draftSubmission = emptyList() - latestDraftSubmissionId = "" + draftSubmission = null } override suspend fun getTotalSubmissionCount(loi: LocationOfInterest): Int = From 0221d1045464bc389f1167198333fb7ae47059ee Mon Sep 17 00:00:00 2001 From: andreia Date: Thu, 20 Aug 2026 17:42:14 +0200 Subject: [PATCH 6/6] fix code format after ktfmt update --- .../android/ui/datacollection/DataCollectionViewModel.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt index 55e599b929..fe5381bf7d 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt @@ -484,9 +484,7 @@ internal constructor( .map { (it as? DataCollectionUiState.Ready)?.currentTaskId == taskId } .distinctUntilChanged() - private fun DataCollectionUiState.Ready.withTask( - taskId: String - ): DataCollectionUiState.Ready { + private fun DataCollectionUiState.Ready.withTask(taskId: String): DataCollectionUiState.Ready { savedStateHandle[TASK_POSITION_ID] = taskId return copy(currentTaskId = taskId, position = taskSequenceHandler.getTaskPosition(taskId)) }