From 975cbc413ff412093b2d31fa3ac3149127883bea Mon Sep 17 00:00:00 2001 From: sufyanAbbasi Date: Tue, 11 Jun 2024 18:57:18 -0400 Subject: [PATCH] Filter saved data to those present in the task sequence (#2489) * Filter only to data in task sequence * Add DataCollectionViewModelTest * Add tests for conditional task rendering and hidden task saving logic * Rename hidden to conditional * Formatting --------- Co-authored-by: Sufyan Abbasi --- .../datacollection/DataCollectionViewModel.kt | 5 +- .../DataCollectionFragmentTest.kt | 123 +++++++++++++++++- .../ui/datacollection/TaskFragmentRunner.kt | 6 + 3 files changed, 126 insertions(+), 8 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewModel.kt b/ground/src/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewModel.kt index 1ad9ac2eba..ee78c4f783 100644 --- a/ground/src/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewModel.kt +++ b/ground/src/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewModel.kt @@ -256,7 +256,10 @@ internal constructor( } private fun getDeltas(): List = - data.map { (task, value) -> ValueDelta(task.id, task.type, value) } + // Filter deltas to valid tasks. + data + .filter { (task) -> task in getTaskSequence() } + .map { (task, value) -> ValueDelta(task.id, task.type, value) } /** Persists the changes locally and enqueues a worker to sync with remote datastore. */ private fun saveChanges(deltas: List) { diff --git a/ground/src/test/java/com/google/android/ground/ui/datacollection/DataCollectionFragmentTest.kt b/ground/src/test/java/com/google/android/ground/ui/datacollection/DataCollectionFragmentTest.kt index 3678474112..1ae12bbe22 100644 --- a/ground/src/test/java/com/google/android/ground/ui/datacollection/DataCollectionFragmentTest.kt +++ b/ground/src/test/java/com/google/android/ground/ui/datacollection/DataCollectionFragmentTest.kt @@ -22,8 +22,13 @@ import com.google.android.ground.R import com.google.android.ground.capture import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase import com.google.android.ground.launchFragmentWithNavController +import com.google.android.ground.model.submission.MultipleChoiceTaskData import com.google.android.ground.model.submission.TextTaskData import com.google.android.ground.model.submission.ValueDelta +import com.google.android.ground.model.task.Condition +import com.google.android.ground.model.task.Expression +import com.google.android.ground.model.task.MultipleChoice +import com.google.android.ground.model.task.Option import com.google.android.ground.model.task.Task import com.google.android.ground.persistence.local.room.converter.SubmissionDeltasConverter import com.google.android.ground.repository.SubmissionRepository @@ -35,6 +40,7 @@ import com.sharedtest.persistence.remote.FakeRemoteDataStore import dagger.hilt.android.testing.BindValue import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject +import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Test @@ -134,7 +140,7 @@ class DataCollectionFragmentTest : BaseHiltTest() { runner() .inputText(TASK_1_RESPONSE) .clickNextButton() - .inputText(TASK_2_RESPONSE) + .selectMultipleChoiceOption(TASK_2_OPTION_LABEL) .clickPreviousButton() // Both deletion and creating happens twice as we do it on every previous/next step @@ -187,7 +193,7 @@ class DataCollectionFragmentTest : BaseHiltTest() { runner() .validateTextIsDisplayed(TASK_1_RESPONSE) .clickNextButton() - .validateTextIsDisplayed(TASK_2_RESPONSE) + .validateTextIsDisplayed(TASK_2_OPTION_LABEL) } @Test @@ -197,7 +203,7 @@ class DataCollectionFragmentTest : BaseHiltTest() { .clickNextButton() .validateTextIsNotDisplayed(TASK_1_NAME) .validateTextIsDisplayed(TASK_2_NAME) - .inputText(TASK_2_RESPONSE) + .selectMultipleChoiceOption(TASK_2_OPTION_LABEL) .clickDoneButton() // Click "done" on final task verify(submissionRepository) @@ -216,6 +222,60 @@ class DataCollectionFragmentTest : BaseHiltTest() { verify(submissionRepository, times(1)).deleteDraftSubmission() } + @Test + fun `Clicking done after triggering conditional task saves task data`() = runWithTestDispatcher { + runner() + .inputText(TASK_1_RESPONSE) + .clickNextButton() + .validateTextIsDisplayed(TASK_2_NAME) + // Select the option to unhide the conditional task. + .selectMultipleChoiceOption(TASK_2_OPTION_CONDITIONAL_LABEL) + // TODO(#2394): Next button should be rendered here. + .clickDoneButton() + // Conditional task is rendered. + .validateTextIsDisplayed(TASK_CONDITIONAL_NAME) + .inputText(TASK_CONDITIONAL_RESPONSE) + .clickNextButton() + + verify(submissionRepository) + .saveSubmission(eq(SURVEY.id), eq(LOCATION_OF_INTEREST.id), capture(deltaCaptor)) + + // Conditional task data is submitted. + listOf(TASK_1_VALUE_DELTA, TASK_2_CONDITIONAL_VALUE_DELTA, TASK_CONDITIONAL_VALUE_DELTA) + .forEach { value -> assertThat(deltaCaptor.value).contains(value) } + } + + @Test + fun `Clicking done after editing conditional task state doesn't save inputted conditional task`() = + runWithTestDispatcher { + runner() + .inputText(TASK_1_RESPONSE) + .clickNextButton() + .validateTextIsDisplayed(TASK_2_NAME) + // Select the option to unhide the conditional task. + .selectMultipleChoiceOption(TASK_2_OPTION_CONDITIONAL_LABEL) + // TODO(#2394): Next button should be rendered here. + .clickDoneButton() + .validateTextIsDisplayed(TASK_CONDITIONAL_NAME) + // Input a value, then go back to hide the task again. + .inputText(TASK_CONDITIONAL_RESPONSE) + .clickPreviousButton() + .validateTextIsDisplayed(TASK_2_NAME) + // Unselect the option to hide the conditional task. + .selectMultipleChoiceOption(TASK_2_OPTION_CONDITIONAL_LABEL) + .selectMultipleChoiceOption(TASK_2_OPTION_LABEL) + .clickDoneButton() + .validateTextIsNotDisplayed(TASK_CONDITIONAL_NAME) + + verify(submissionRepository) + .saveSubmission(eq(SURVEY.id), eq(LOCATION_OF_INTEREST.id), capture(deltaCaptor)) + + // Conditional task data is not submitted. + listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA).forEach { value -> + assertThat(deltaCaptor.value).contains(value) + } + } + private fun setupSubmission() = runWithTestDispatcher { whenever(submissionRepository.createSubmission(SURVEY.id, LOCATION_OF_INTEREST.id)) .thenReturn(SUBMISSION) @@ -258,14 +318,63 @@ class DataCollectionFragmentTest : BaseHiltTest() { private const val TASK_ID_2 = "2" const val TASK_2_NAME = "task 2" - private const val TASK_2_RESPONSE = "response 2" - private val TASK_2_VALUE = TextTaskData.fromString(TASK_2_RESPONSE) - private val TASK_2_VALUE_DELTA = ValueDelta(TASK_ID_2, Task.Type.TEXT, TASK_2_VALUE) + private const val TASK_2_OPTION = "option 1" + private const val TASK_2_OPTION_LABEL = "Option 1" + private const val TASK_2_OPTION_CONDITIONAL = "option 2" + private const val TASK_2_OPTION_CONDITIONAL_LABEL = "Option 2" + private val TASK_2_MULTIPLE_CHOICE = + MultipleChoice( + persistentListOf( + Option(TASK_2_OPTION, "code1", TASK_2_OPTION_LABEL), + Option(TASK_2_OPTION_CONDITIONAL, "code2", TASK_2_OPTION_CONDITIONAL_LABEL), + ), + MultipleChoice.Cardinality.SELECT_MULTIPLE, + ) + private val TASK_2_VALUE = + MultipleChoiceTaskData.fromList(TASK_2_MULTIPLE_CHOICE, listOf(TASK_2_OPTION)) + private val TASK_2_CONDITIONAL_VALUE = + MultipleChoiceTaskData.fromList(TASK_2_MULTIPLE_CHOICE, listOf(TASK_2_OPTION_CONDITIONAL)) + private val TASK_2_VALUE_DELTA = ValueDelta(TASK_ID_2, Task.Type.MULTIPLE_CHOICE, TASK_2_VALUE) + private val TASK_2_CONDITIONAL_VALUE_DELTA = + ValueDelta(TASK_ID_2, Task.Type.MULTIPLE_CHOICE, TASK_2_CONDITIONAL_VALUE) + + private const val TASK_ID_CONDITIONAL = "conditional" + const val TASK_CONDITIONAL_NAME = "conditional task" + private const val TASK_CONDITIONAL_RESPONSE = "conditional response" + private val TASK_CONDITIONAL_VALUE = TextTaskData.fromString(TASK_CONDITIONAL_RESPONSE) + private val TASK_CONDITIONAL_VALUE_DELTA = + ValueDelta(TASK_ID_CONDITIONAL, Task.Type.TEXT, TASK_CONDITIONAL_VALUE) private val TASKS = listOf( Task(TASK_ID_1, 0, Task.Type.TEXT, TASK_1_NAME, true), - Task(TASK_ID_2, 1, Task.Type.TEXT, TASK_2_NAME, true), + Task( + TASK_ID_2, + 1, + Task.Type.MULTIPLE_CHOICE, + TASK_2_NAME, + true, + multipleChoice = TASK_2_MULTIPLE_CHOICE, + ), + Task( + TASK_ID_CONDITIONAL, + 2, + Task.Type.TEXT, + TASK_CONDITIONAL_NAME, + true, + condition = + Condition( + Condition.MatchType.MATCH_ANY, + expressions = + listOf( + Expression( + Expression.ExpressionType.ANY_OF_SELECTED, + TASK_ID_2, + optionIds = setOf(TASK_2_OPTION_CONDITIONAL), + ) + ), + ), + ), ) private val JOB = FakeData.JOB.copy(tasks = TASKS.associateBy { it.id }) diff --git a/ground/src/test/java/com/google/android/ground/ui/datacollection/TaskFragmentRunner.kt b/ground/src/test/java/com/google/android/ground/ui/datacollection/TaskFragmentRunner.kt index af4a19cbb0..ffef049fac 100644 --- a/ground/src/test/java/com/google/android/ground/ui/datacollection/TaskFragmentRunner.kt +++ b/ground/src/test/java/com/google/android/ground/ui/datacollection/TaskFragmentRunner.kt @@ -27,6 +27,7 @@ import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.action.ViewActions.typeText import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.matcher.ViewMatchers.isDisplayed @@ -74,6 +75,11 @@ class TaskFragmentRunner( return this } + internal fun selectMultipleChoiceOption(optionText: String): TaskFragmentRunner { + onView(withText(optionText)).perform(click()) + return this + } + internal fun validateTextIsDisplayed(text: String): TaskFragmentRunner { onView(withText(text)).check(matches(isDisplayed())) return this