Separate validationResult and constraintValidationResult

collect_app/src/main/java/org/odk/collect/android/activities/FormEntryViewModelFactory.kt

@@ -1,9 +1,9 @@
 package org.odk.collect.android.activities
 
-import androidx.lifecycle.AbstractSavedStateViewModelFactory
-import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.ViewModel
-import androidx.savedstate.SavedStateRegistryOwner
+import androidx.lifecycle.ViewModelProvider
+import androidx.lifecycle.createSavedStateHandle
+import androidx.lifecycle.viewmodel.CreationExtras
 import org.javarosa.core.model.actions.recordaudio.RecordAudioActions
 import org.javarosa.core.model.instance.TreeReference
 import org.odk.collect.android.entities.EntitiesRepositoryProvider
@@ -28,6 +28,7 @@ import org.odk.collect.android.utilities.FormsRepositoryProvider
 import org.odk.collect.android.utilities.InstancesRepositoryProvider
 import org.odk.collect.android.utilities.MediaUtils
 import org.odk.collect.android.utilities.SavepointsRepositoryProvider
+import org.odk.collect.android.widgets.viewmodels.QuestionViewModel
 import org.odk.collect.async.Scheduler
 import org.odk.collect.audiorecorder.recording.AudioRecorder
 import org.odk.collect.location.LocationClient
@@ -39,7 +40,6 @@ import org.odk.collect.settings.SettingsProvider
 import java.util.function.BiConsumer
 
 class FormEntryViewModelFactory(
-    owner: SavedStateRegistryOwner,
     private val mode: String?,
     private val sessionId: String,
     private val scheduler: Scheduler,
@@ -60,13 +60,9 @@ class FormEntryViewModelFactory(
     private val htmlPrinter: HtmlPrinter,
     private val instancesDataService: InstancesDataService,
     private val changeLockProvider: ChangeLockProvider
-) : AbstractSavedStateViewModelFactory(owner, null) {
+) : ViewModelProvider.Factory {
 
-    override fun <T : ViewModel> create(
-        key: String,
-        modelClass: Class<T>,
-        handle: SavedStateHandle
-    ): T {
+    override fun <T : ViewModel> create(modelClass: Class<T>, extras: CreationExtras): T {
         val projectId = projectsDataService.requireCurrentProject().uuid
 
         return when (modelClass) {
@@ -81,7 +77,7 @@ class FormEntryViewModelFactory(
 
             FormSaveViewModel::class.java -> {
                 FormSaveViewModel(
-                    handle,
+                    extras.createSavedStateHandle(),
                     System::currentTimeMillis,
                     DiskFormSaver(),
                     mediaUtils,
@@ -152,6 +148,8 @@ class FormEntryViewModelFactory(
 
             PrinterWidgetViewModel::class.java -> PrinterWidgetViewModel(scheduler, qrCodeCreator, htmlPrinter)
 
+            QuestionViewModel::class.java -> QuestionViewModel(scheduler, formSessionRepository, sessionId)
+
             else -> throw IllegalArgumentException()
         } as T
     }

collect_app/src/main/java/org/odk/collect/android/activities/FormFillingActivity.java

@@ -412,7 +412,7 @@ public void onCreate(Bundle savedInstanceState) {
             sessionId = savedInstanceState.getString(KEY_SESSION_ID);
         }
 
-        viewModelFactory = new FormEntryViewModelFactory(this,
+        viewModelFactory = new FormEntryViewModelFactory(
                 getIntent().getStringExtra(FormOpeningMode.FORM_MODE_KEY),
                 sessionId,
                 scheduler,

collect_app/src/main/java/org/odk/collect/android/formentry/FormEntryViewModel.java

@@ -391,17 +391,6 @@ public void exit() {
         changeLocks.getFormsLock().unlock(FORM_ENTRY_TOKEN);
     }
 
-    public void validateAnswerConstraint(FormIndex index, IAnswerData answer) {
-        worker.immediate(() -> {
-            ValidationResult result = formController.validateAnswerConstraint(index, answer);
-            if (result instanceof FailedValidationResult) {
-                validationResult.postValue(new Consumable<>(result));
-            } else {
-                validationResult.postValue(new Consumable<>(null));
-            }
-        });
-    }
-
     public void validateForm() {
         worker.immediate(
                 () -> {

collect_app/src/main/java/org/odk/collect/android/formhierarchy/FormHierarchyFragmentHostActivity.kt

@@ -86,7 +86,6 @@ class FormHierarchyFragmentHostActivity : LocalizedActivity() {
     private val sessionId by lazy { intent.getStringExtra(EXTRA_SESSION_ID)!! }
     private val viewModelFactory by lazy {
         FormEntryViewModelFactory(
-            this,
             FormOpeningMode.EDIT_SAVED,
             sessionId,
             scheduler,

collect_app/src/main/java/org/odk/collect/android/widgets/utilities/GeoPolyDialogFragment.kt

@@ -7,7 +7,6 @@ import org.javarosa.core.model.Constants
 import org.javarosa.core.model.data.GeoShapeData
 import org.javarosa.core.model.data.GeoTraceData
 import org.javarosa.core.model.data.IAnswerData
-import org.javarosa.form.api.FormEntryController
 import org.javarosa.form.api.FormEntryPrompt
 import org.odk.collect.android.javarosawrapper.FailedValidationResult
 import org.odk.collect.android.utilities.FormEntryPromptUtils
@@ -65,12 +64,9 @@ class GeoPolyDialogFragment(viewModelFactory: ViewModelProvider.Factory) :
             prompt.isReadOnly,
             retainMockAccuracy,
             inputPolygon,
-            validationResult.map {
+            constraintValidationResult.map {
                 val validationResult = it.value
-                if (validationResult is FailedValidationResult &&
-                    validationResult.index == prompt.index &&
-                    validationResult.status == FormEntryController.ANSWER_CONSTRAINT_VIOLATED
-                ) {
+                if (validationResult is FailedValidationResult && validationResult.index == prompt.index) {
                     validationResult.customErrorMessage ?: getString(validationResult.defaultErrorMessage)
                 } else {
                     null

collect_app/src/main/java/org/odk/collect/android/widgets/utilities/WidgetAnswerDialogFragment.kt

@@ -9,13 +9,15 @@ import androidx.appcompat.widget.Toolbar
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
 import androidx.fragment.app.commit
+import androidx.fragment.app.viewModels
 import androidx.lifecycle.ViewModelProvider
 import org.javarosa.core.model.FormIndex
 import org.javarosa.core.model.data.IAnswerData
 import org.javarosa.form.api.FormEntryPrompt
 import org.odk.collect.android.R
 import org.odk.collect.android.databinding.WidgetAnswerDialogLayoutBinding
 import org.odk.collect.android.formentry.FormEntryViewModel
+import org.odk.collect.android.widgets.viewmodels.QuestionViewModel
 import org.odk.collect.androidshared.ui.FragmentFactoryBuilder
 import org.odk.collect.material.MaterialFullScreenDialogFragment
 import kotlin.reflect.KClass
@@ -26,18 +28,18 @@ abstract class WidgetAnswerDialogFragment<T : Fragment>(
 ) : MaterialFullScreenDialogFragment() {
 
     private val formEntryViewModel: FormEntryViewModel by activityViewModels { viewModelFactory }
+    private val questionViewModel: QuestionViewModel by viewModels { viewModelFactory }
     private val prompt: FormEntryPrompt by lazy {
         formEntryViewModel.getQuestionPrompt(requireArguments().getSerializable(ARG_FORM_INDEX) as FormIndex)
     }
-    protected val validationResult by lazy {
-        formEntryViewModel.validationResult
+    protected val constraintValidationResult by lazy {
+        questionViewModel.constraintValidationResult
     }
 
     abstract fun onCreateFragment(prompt: FormEntryPrompt): T
 
     override fun onAttach(context: Context) {
         super.onAttach(context)
-
         childFragmentManager.fragmentFactory = FragmentFactoryBuilder()
             .forClass(type) { onCreateFragment(prompt) }
             .build()
@@ -73,7 +75,7 @@ abstract class WidgetAnswerDialogFragment<T : Fragment>(
     }
 
     fun onValidate(answer: IAnswerData?) {
-        formEntryViewModel.validateAnswerConstraint(prompt.index, answer)
+        questionViewModel.validate(prompt.index, answer)
     }
 
     fun onAnswer(answer: IAnswerData?) {

collect_app/src/main/java/org/odk/collect/android/widgets/viewmodels/QuestionViewModel.kt

@@ -0,0 +1,41 @@
+package org.odk.collect.android.widgets.viewmodels
+
+import androidx.lifecycle.LiveData
+import androidx.lifecycle.MutableLiveData
+import androidx.lifecycle.ViewModel
+import org.javarosa.core.model.FormIndex
+import org.javarosa.core.model.data.IAnswerData
+import org.odk.collect.android.formentry.FormSession
+import org.odk.collect.android.formentry.FormSessionRepository
+import org.odk.collect.android.javarosawrapper.FormController
+import org.odk.collect.android.javarosawrapper.ValidationResult
+import org.odk.collect.androidshared.data.Consumable
+import org.odk.collect.androidshared.livedata.LiveDataUtils
+import org.odk.collect.async.Scheduler
+
+class QuestionViewModel(
+    private val scheduler: Scheduler,
+    formSessionRepository: FormSessionRepository,
+    sessionId: String
+) : ViewModel() {
+    private val _constraintValidationResult: MutableLiveData<Consumable<ValidationResult>> = MutableLiveData()
+    val constraintValidationResult: LiveData<Consumable<ValidationResult>> = _constraintValidationResult
+    private var formController: FormController? = null
+    private var formSessionObserver = LiveDataUtils.observe(
+        formSessionRepository.get(sessionId)
+    ) { formSession: FormSession ->
+        formController = formSession.formController
+    }
+
+    fun validate(index: FormIndex, answer: IAnswerData?) {
+        scheduler.immediate {
+            formController?.validateAnswerConstraint(index, answer)?.let {
+                _constraintValidationResult.postValue(Consumable(it))
+            }
+        }
+    }
+
+    override fun onCleared() {
+        formSessionObserver.cancel()
+    }
+}

collect_app/src/test/java/org/odk/collect/android/formentry/FormEntryViewModelTest.java

@@ -4,7 +4,6 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.core.Is.is;
 import static org.javarosa.core.model.Constants.CONTROL_SELECT_ONE;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -456,43 +455,6 @@ public void answerQuestion_savesAnswerToFormController() {
         assertThat(formController.getAnswer(formIndex.getReference()).getValue(), equalTo("answer"));
     }
 
-    @Test
-    public void validateAnswerConstraint_updatesValidationResult_ifIsIsFailedValidationResult() {
-        FormDef formDef = mock();
-        when(formDef.evaluateConstraint(any(), any())).thenReturn(false);
-        formController.setFormDef(formDef);
-
-        TreeReference reference = new TreeReference();
-        reference.add("blah", TreeReference.INDEX_UNBOUND);
-        FormIndex formIndex = new FormIndex(null, 1, 1, reference);
-        FormEntryPrompt prompt = new MockFormEntryPromptBuilder().build();
-        formController.setPrompt(formIndex, prompt);
-
-        FailedValidationResult failedValidationResult = new FailedValidationResult(formIndex, 0, null, org.odk.collect.strings.R.string.invalid_answer_error);
-        formController.setFailedConstraint(failedValidationResult);
-
-        viewModel.validateAnswerConstraint(formIndex, new StringData("answer"));
-        scheduler.flush(true);
-        assertThat(viewModel.getValidationResult().getValue().getValue(), equalTo(failedValidationResult));
-    }
-
-    @Test
-    public void validateAnswerConstraint_clearsResult_ifItIsSuccessValidationResult() {
-        FormDef formDef = mock();
-        when(formDef.evaluateConstraint(any(), any())).thenReturn(false);
-        formController.setFormDef(formDef);
-
-        TreeReference reference = new TreeReference();
-        reference.add("blah", TreeReference.INDEX_UNBOUND);
-        FormIndex formIndex = new FormIndex(null, 1, 1, reference);
-        FormEntryPrompt prompt = new MockFormEntryPromptBuilder().build();
-        formController.setPrompt(formIndex, prompt);
-
-        viewModel.validateAnswerConstraint(formIndex, new StringData("answer"));
-        scheduler.flush(true);
-        assertThat(viewModel.getValidationResult().getValue().getValue(), equalTo(null));
-    }
-
     @Test
     public void answerQuestion_whenQuestionIsAutoAdvance_movesForward() {
         TreeReference reference = new TreeReference();

collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoPolyDialogFragmentTest.kt

@@ -33,6 +33,7 @@ import org.odk.collect.android.support.CollectHelpers
 import org.odk.collect.android.support.MockFormEntryPromptBuilder
 import org.odk.collect.android.widgets.utilities.AdditionalAttributes.INCREMENTAL
 import org.odk.collect.android.widgets.utilities.WidgetAnswerDialogFragment.Companion.ARG_FORM_INDEX
+import org.odk.collect.android.widgets.viewmodels.QuestionViewModel
 import org.odk.collect.androidshared.data.Consumable
 import org.odk.collect.androidshared.ui.FragmentFactoryBuilder
 import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule
@@ -45,17 +46,24 @@ import org.odk.collect.testshared.getOrAwaitValue
 class GeoPolyDialogFragmentTest {
 
     private var prompt = MockFormEntryPromptBuilder().build()
-    private val validationResult = MutableLiveData<Consumable<ValidationResult>>(
+    private val constraintValidationResult = MutableLiveData<Consumable<ValidationResult>>(
         Consumable(SuccessValidationResult)
     )
     private val formEntryViewModel = mock<FormEntryViewModel> {
         on { getQuestionPrompt(prompt.index) } doReturn prompt
-        on { validationResult } doReturn validationResult
+    }
+
+    private val questionViewModel = mock<QuestionViewModel> {
+        on { constraintValidationResult } doReturn constraintValidationResult
     }
 
     private val viewModelFactory = object : ViewModelProvider.Factory {
         override fun <T : ViewModel> create(modelClass: Class<T>, extras: CreationExtras): T {
-            return formEntryViewModel as T
+            return when (modelClass) {
+                FormEntryViewModel::class.java -> formEntryViewModel as T
+                QuestionViewModel::class.java -> questionViewModel as T
+                else -> throw IllegalArgumentException()
+            }
         }
     }
 
@@ -333,7 +341,7 @@ class GeoPolyDialogFragmentTest {
             )
         }
 
-        verify(formEntryViewModel).validateAnswerConstraint(prompt.index, geoTraceOf(answer))
+        verify(questionViewModel).validate(prompt.index, geoTraceOf(answer))
     }
 
     @Test
@@ -354,7 +362,7 @@ class GeoPolyDialogFragmentTest {
             )
         }
 
-        verify(formEntryViewModel).validateAnswerConstraint(prompt.index, geoShapeOf(answer))
+        verify(questionViewModel).validate(prompt.index, geoShapeOf(answer))
     }
 
     @Test
@@ -390,7 +398,7 @@ class GeoPolyDialogFragmentTest {
             )
         }
 
-        verify(formEntryViewModel, never()).validateAnswerConstraint(prompt.index, geoTraceOf(answer))
+        verify(questionViewModel, never()).validate(prompt.index, geoTraceOf(answer))
     }
 
     @Test
@@ -475,22 +483,7 @@ class GeoPolyDialogFragmentTest {
             0,
             TreeReference()
         )
-        validationResult.value = Consumable(FailedValidationResult(anotherQuestionFormIndex, FormEntryController.ANSWER_CONSTRAINT_VIOLATED, "blah", 0))
-        launcherRule.launchAndAssertOnChild<GeoPolyFragment>(
-            GeoPolyDialogFragment::class,
-            bundleOf(ARG_FORM_INDEX to prompt.index)
-        ) {
-            assertThat(it.invalidMessage.getOrAwaitValue(), equalTo(null))
-        }
-    }
-
-    @Test
-    fun `ignores the validation message if it was triggered by a required but empty answer`() {
-        prompt = MockFormEntryPromptBuilder(prompt)
-            .withDataType(Constants.DATATYPE_GEOTRACE)
-            .build()
-
-        validationResult.value = Consumable(FailedValidationResult(prompt.index, FormEntryController.ANSWER_REQUIRED_BUT_EMPTY, "blah", 0))
+        constraintValidationResult.value = Consumable(FailedValidationResult(anotherQuestionFormIndex, FormEntryController.ANSWER_CONSTRAINT_VIOLATED, "blah", 0))
         launcherRule.launchAndAssertOnChild<GeoPolyFragment>(
             GeoPolyDialogFragment::class,
             bundleOf(ARG_FORM_INDEX to prompt.index)
@@ -512,7 +505,7 @@ class GeoPolyDialogFragmentTest {
             assertThat(it.invalidMessage.getOrAwaitValue(), equalTo(null))
         }
 
-        validationResult.value = Consumable(FailedValidationResult(prompt.index, FormEntryController.ANSWER_CONSTRAINT_VIOLATED, "blah", 0))
+        constraintValidationResult.value = Consumable(FailedValidationResult(prompt.index, FormEntryController.ANSWER_CONSTRAINT_VIOLATED, "blah", 0))
         launcherRule.launchAndAssertOnChild<GeoPolyFragment>(
             GeoPolyDialogFragment::class,
             bundleOf(ARG_FORM_INDEX to prompt.index)
@@ -534,7 +527,7 @@ class GeoPolyDialogFragmentTest {
             assertThat(it.invalidMessage.getOrAwaitValue(), equalTo(null))
         }
 
-        validationResult.value =
+        constraintValidationResult.value =
             Consumable(FailedValidationResult(prompt.index, FormEntryController.ANSWER_CONSTRAINT_VIOLATED, null, R.string.cancel))
         launcherRule.launchAndAssertOnChild<GeoPolyFragment>(
             GeoPolyDialogFragment::class,