Skip to content

Commit 3fd0911

Browse files
grzesiek2010grzesiek2010
authored
Merge pull request #7016 from grzesiek2010/COLLECT-6984_2
Improve saving and validating answers for GeoTrace and GeoShape questions
2 parents d574284 + 8c266c0 commit 3fd0911

File tree

19 files changed

+318
-144
lines changed

19 files changed

+318
-144
lines changed

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@
158158
import org.odk.collect.android.utilities.MediaUtils;
159159
import org.odk.collect.android.utilities.SavepointsRepositoryProvider;
160160
import org.odk.collect.android.utilities.SoftKeyboardController;
161+
import org.odk.collect.android.widgets.GeoShapeWidget;
162+
import org.odk.collect.android.widgets.GeoTraceWidget;
161163
import org.odk.collect.android.widgets.QuestionWidget;
162164
import org.odk.collect.android.widgets.datetime.DateTimeWidget;
163165
import org.odk.collect.android.widgets.datetime.pickers.CustomDatePickerDialog;
@@ -562,11 +564,16 @@ private void setupViewModels(FormEntryViewModelFactory formEntryViewModelFactory
562564

563565
formEntryViewModel.getCurrentIndex().observe(this, indexAndValidationResult -> {
564566
if (indexAndValidationResult != null) {
565-
FormIndex formIndex = indexAndValidationResult.component1();
566-
FailedValidationResult validationResult = indexAndValidationResult.component2();
567-
formIndexAnimationHandler.handle(formIndex);
567+
FormIndex screenIndex = indexAndValidationResult.getFirst();
568+
FormIndex questionIndex = indexAndValidationResult.getSecond();
569+
FailedValidationResult validationResult = indexAndValidationResult.getThird();
570+
formIndexAnimationHandler.handle(screenIndex);
568571
if (validationResult != null) {
569572
handleValidationResult(validationResult);
573+
} else {
574+
if (odkView != null) {
575+
odkView.scrollToTopOf(questionIndex);
576+
}
570577
}
571578
}
572579
});
@@ -973,7 +980,9 @@ public void setWidgetData(Object data) {
973980
* Clears the answer on the screen.
974981
*/
975982
private void clearAnswer(QuestionWidget qw) {
976-
if (qw.getAnswer() != null || qw instanceof DateTimeWidget) {
983+
if (qw instanceof GeoTraceWidget || qw instanceof GeoShapeWidget) {
984+
formEntryViewModel.answerQuestion(qw.getFormEntryPrompt().getIndex(), null);
985+
} else if (qw.getAnswer() != null || qw instanceof DateTimeWidget) {
977986
qw.clearAnswer();
978987
}
979988
}

‎collect_app/src/main/java/org/odk/collect/android/formentry/FormEntryMenuProvider.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ class FormEntryMenuProvider(
154154
}
155155
R.id.menu_validate -> {
156156
formEntryViewModel.saveScreenAnswersToFormController(answersProvider.answers, false)
157-
formEntryViewModel.validate()
157+
formEntryViewModel.validateForm()
158158
true
159159
}
160160
R.id.menu_languages -> {

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

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
import java.util.Objects;
5454
import java.util.function.Supplier;
5555

56-
import kotlin.Pair;
56+
import kotlin.Triple;
5757
import timber.log.Timber;
5858

5959
public class FormEntryViewModel extends ViewModel implements SelectChoiceLoader {
@@ -62,7 +62,7 @@ public class FormEntryViewModel extends ViewModel implements SelectChoiceLoader
6262

6363
private final MutableLiveData<FormError> error = new MutableLiveData<>(null);
6464
private final MutableNonNullLiveData<Boolean> hasBackgroundRecording = new MutableNonNullLiveData<>(false);
65-
private final MutableLiveData<Pair<FormIndex, FailedValidationResult>> currentIndex = new MutableLiveData<>(null);
65+
private final MutableLiveData<Triple<FormIndex, FormIndex, FailedValidationResult>> currentIndex = new MutableLiveData<>(null);
6666
private final MutableLiveData<Consumable<ValidationResult>>
6767
validationResult = new MutableLiveData<>(new Consumable<>(null));
6868
@NonNull
@@ -117,7 +117,7 @@ public FormController getFormController() {
117117
return formController;
118118
}
119119

120-
public LiveData<Pair<FormIndex, FailedValidationResult>> getCurrentIndex() {
120+
public LiveData<Triple<FormIndex, FormIndex, FailedValidationResult>> getCurrentIndex() {
121121
return currentIndex;
122122
}
123123

@@ -289,24 +289,21 @@ public FormEntryPrompt getQuestionPrompt(FormIndex formIndex) {
289289
}
290290

291291
public void answerQuestion(FormIndex index, IAnswerData answer) {
292-
answerQuestion(index, answer, false);
293-
}
294-
295-
public void answerQuestion(FormIndex index, IAnswerData answer, Boolean validate) {
296292
worker.immediate(() -> {
297293
try {
298294
FormEntryPrompt prompt = formController.getQuestionPrompt(index);
299-
boolean autoAdvance = Appearances.hasAppearance(prompt, Appearances.QUICK);
300-
ValidationResult result = formController.saveOneScreenAnswer(index, answer, validate || autoAdvance);
295+
boolean autoAdvance = Appearances.isQuick(prompt);
296+
ValidationResult result = formController.saveOneScreenAnswer(index, answer, autoAdvance);
301297

302298
if (result instanceof FailedValidationResult) {
303299
updateIndex(true, (FailedValidationResult) result);
304300
} else {
305301
if (autoAdvance) {
306302
formController.stepToNextScreenEvent();
303+
updateIndex(true, null);
304+
} else {
305+
updateIndex(true, null, index);
307306
}
308-
309-
updateIndex(true, null);
310307
}
311308
} catch (JavaRosaException e) {
312309
throw new RuntimeException(e);
@@ -348,6 +345,10 @@ public void refresh() {
348345
}
349346

350347
private void updateIndex(boolean isAsync, @Nullable FailedValidationResult validationResult) {
348+
updateIndex(isAsync, validationResult, null);
349+
}
350+
351+
private void updateIndex(boolean isAsync, @Nullable FailedValidationResult validationResult, @Nullable FormIndex questionIndex) {
351352
choices.clear();
352353

353354
if (formController != null) {
@@ -377,9 +378,9 @@ private void updateIndex(boolean isAsync, @Nullable FailedValidationResult valid
377378

378379
AuditUtils.logCurrentScreen(formController, formController.getAuditEventLogger(), clock.get());
379380
if (isAsync) {
380-
currentIndex.postValue(new Pair<>(formController.getFormIndex(), validationResult));
381+
currentIndex.postValue(new Triple<>(formController.getFormIndex(), questionIndex, validationResult));
381382
} else {
382-
currentIndex.setValue(new Pair<>(formController.getFormIndex(), validationResult));
383+
currentIndex.setValue(new Triple<>(formController.getFormIndex(), questionIndex, validationResult));
383384
}
384385
}
385386
}
@@ -390,7 +391,14 @@ public void exit() {
390391
changeLocks.getFormsLock().unlock(FORM_ENTRY_TOKEN);
391392
}
392393

393-
public void validate() {
394+
public void validateAnswerConstraint(FormIndex index, IAnswerData answer) {
395+
worker.immediate(() -> {
396+
ValidationResult result = formController.validateAnswerConstraint(index, answer);
397+
validationResult.postValue(new Consumable<>(result));
398+
});
399+
}
400+
401+
public void validateForm() {
394402
worker.immediate(
395403
() -> {
396404
ValidationResult result = null;
@@ -486,8 +494,4 @@ public FormEntryPrompt[] saveFieldList(HashMap<FormIndex, IAnswerData> answers)
486494
private boolean isQuestionRecalculated(FormEntryPrompt mutableQuestionBeforeSave, ImmutableDisplayableQuestion immutableQuestionBeforeSave) {
487495
return !Objects.equals(mutableQuestionBeforeSave.getAnswerText(), immutableQuestionBeforeSave.getAnswerText());
488496
}
489-
490-
public interface AnswerListener {
491-
void onAnswer(FormIndex index, IAnswerData answer);
492-
}
493497
}

‎collect_app/src/main/java/org/odk/collect/android/formentry/ODKView.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -562,9 +562,18 @@ public boolean isDisplayed(QuestionWidget qw) {
562562
return qw.getLocalVisibleRect(scrollBounds);
563563
}
564564

565+
public void scrollToTopOf(FormIndex index) {
566+
for (QuestionWidget widget : widgets) {
567+
if (widget.getFormEntryPrompt().getIndex().equals(index)) {
568+
scrollToTopOf(widget);
569+
break;
570+
}
571+
}
572+
}
573+
565574
public void scrollToTopOf(@Nullable QuestionWidget qw) {
566575
if (qw != null && widgets.contains(qw)) {
567-
findViewById(R.id.odk_view_container).scrollTo(0, qw.getTop());
576+
postDelayed(() -> findViewById(R.id.odk_view_container).scrollTo(0, qw.getTop()), 400);
568577
}
569578
}
570579

@@ -691,12 +700,7 @@ public void setErrorForQuestionWithIndex(FormIndex formIndex, String errorMessag
691700
for (QuestionWidget questionWidget : getWidgets()) {
692701
if (formIndex.equals(questionWidget.getFormEntryPrompt().getIndex())) {
693702
questionWidget.displayError(errorMessage);
694-
// postDelayed is needed because otherwise scrolling may not work as expected in case when
695-
// answers are validated during form finalization.
696-
postDelayed(() -> {
697-
questionWidget.setFocus(getContext());
698-
scrollToTopOf(questionWidget);
699-
}, 400);
703+
scrollToTopOf(questionWidget);
700704
} else {
701705
questionWidget.hideError();
702706
}

‎collect_app/src/main/java/org/odk/collect/android/javarosawrapper/FormController.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,14 @@ interface FormController {
129129
@Throws(JavaRosaException::class)
130130
fun answerQuestion(index: FormIndex?, data: IAnswerData?): Int
131131

132+
/**
133+
* Validates a single answer against its constraint.
134+
*
135+
* @return SuccessValidationResult if the answer is valid, or an error result describing
136+
* the violated constraint.
137+
*/
138+
fun validateAnswerConstraint(index: FormIndex, answer: IAnswerData?): ValidationResult
139+
132140
/**
133141
* Goes through the entire form to make sure all entered answers comply with their constraints.
134142
* Constraints are ignored on 'jump to', so answers can be outside of constraints. We don't

‎collect_app/src/main/java/org/odk/collect/android/javarosawrapper/JavaRosaFormController.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,15 @@ public int answerQuestion(FormIndex index, IAnswerData data) throws JavaRosaExce
393393
}
394394
}
395395

396+
public ValidationResult validateAnswerConstraint(FormIndex index, IAnswerData answer) {
397+
boolean isAnswerValid = getFormDef().evaluateConstraint(index.getReference(), answer);
398+
if (isAnswerValid) {
399+
return SuccessValidationResult.INSTANCE;
400+
} else {
401+
return getFailedValidationResult(index, FormEntryController.ANSWER_CONSTRAINT_VIOLATED);
402+
}
403+
}
404+
396405
public ValidationResult validateAnswers(boolean moveToInvalidIndex) throws JavaRosaException {
397406
try {
398407
ValidateOutcome validateOutcome = getFormDef().validate();

‎collect_app/src/main/java/org/odk/collect/android/utilities/Appearances.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,4 +208,14 @@ object Appearances {
208208
val appearance = getSanitizedAppearanceHint(prompt)
209209
return appearance.contains(MULTILINE)
210210
}
211+
212+
@JvmStatic
213+
fun isQuick(prompt: FormEntryPrompt): Boolean {
214+
return if (prompt.controlType == Constants.CONTROL_SELECT_ONE) {
215+
val appearance = getSanitizedAppearanceHint(prompt)
216+
appearance.contains(QUICK)
217+
} else {
218+
false
219+
}
220+
}
211221
}

‎collect_app/src/main/java/org/odk/collect/android/widgets/GeoShapeWidget.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,7 @@ public IAnswerData getAnswer() {
7777
}
7878

7979
@Override
80-
public void clearAnswer() {
81-
binding.geoAnswerText.setText(null);
82-
binding.geoAnswerText.setVisibility(GONE);
83-
binding.simpleButton.setText(org.odk.collect.strings.R.string.get_polygon);
84-
widgetValueChanged();
85-
}
80+
public void clearAnswer() {}
8681

8782
@Override
8883
public void setOnLongClickListener(OnLongClickListener l) {
@@ -96,8 +91,4 @@ public void cancelLongPress() {
9691
binding.simpleButton.cancelLongPress();
9792
binding.geoAnswerText.cancelLongPress();
9893
}
99-
100-
private String getAnswerText() {
101-
return binding.geoAnswerText.getText().toString();
102-
}
10394
}

‎collect_app/src/main/java/org/odk/collect/android/widgets/GeoTraceWidget.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,21 +97,12 @@ public void setOnLongClickListener(OnLongClickListener l) {
9797
}
9898

9999
@Override
100-
public void clearAnswer() {
101-
binding.geoAnswerText.setText(null);
102-
binding.geoAnswerText.setVisibility(GONE);
103-
binding.simpleButton.setText(org.odk.collect.strings.R.string.get_line);
104-
widgetValueChanged();
105-
}
100+
public void clearAnswer() {}
106101

107102
@Override
108103
public void cancelLongPress() {
109104
super.cancelLongPress();
110105
binding.simpleButton.cancelLongPress();
111106
binding.geoAnswerText.cancelLongPress();
112107
}
113-
114-
private String getAnswerText() {
115-
return binding.geoAnswerText.getText().toString();
116-
}
117108
}

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

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import androidx.lifecycle.map
66
import org.javarosa.core.model.Constants
77
import org.javarosa.core.model.data.GeoShapeData
88
import org.javarosa.core.model.data.GeoTraceData
9+
import org.javarosa.core.model.data.IAnswerData
910
import org.javarosa.form.api.FormEntryPrompt
11+
import org.odk.collect.android.javarosawrapper.FailedValidationResult
1012
import org.odk.collect.android.utilities.FormEntryPromptUtils
1113
import org.odk.collect.android.widgets.utilities.AdditionalAttributes.INCREMENTAL
1214
import org.odk.collect.android.widgets.utilities.BindAttributes.ALLOW_MOCK_ACCURACY
@@ -37,10 +39,10 @@ class GeoPolyDialogFragment(viewModelFactory: ViewModelProvider.Factory) :
3739
if (geopolyChange != null) {
3840
val incremental = FormEntryPromptUtils.getAdditionalAttribute(prompt, INCREMENTAL)
3941
if (incremental == "true") {
40-
onAnswer(geopolyChange, outputMode, dismiss = false, validate = true)
42+
onValidate(geopolyChange, outputMode)
4143
}
4244
} else if (geopoly != null) {
43-
onAnswer(geopoly, outputMode, dismiss = true, validate = false)
45+
onAnswer(geopoly, outputMode)
4446
} else {
4547
dismiss()
4648
}
@@ -62,29 +64,35 @@ class GeoPolyDialogFragment(viewModelFactory: ViewModelProvider.Factory) :
6264
prompt.isReadOnly,
6365
retainMockAccuracy,
6466
inputPolygon,
65-
currentIndex.map {
66-
val validationResult = it.second
67-
if (validationResult != null) {
68-
validationResult.customErrorMessage
69-
?: getString(validationResult.defaultErrorMessage)
67+
validationResult.map {
68+
val validationResult = it.value
69+
if (validationResult is FailedValidationResult && validationResult.index == prompt.index) {
70+
validationResult.customErrorMessage ?: getString(validationResult.defaultErrorMessage)
7071
} else {
7172
null
7273
}
7374
}
7475
)
7576
}
7677

77-
private fun onAnswer(
78-
geoString: String,
79-
outputMode: OutputMode,
80-
dismiss: Boolean,
81-
validate: Boolean
82-
) {
83-
val answer = when (outputMode) {
84-
OutputMode.GEOTRACE -> GeoTraceData().also { it.value = geoString }
85-
OutputMode.GEOSHAPE -> GeoShapeData().also { it.value = geoString }
86-
}
78+
private fun onValidate(geoString: String, outputMode: OutputMode) {
79+
val answer = getAnswerData(geoString, outputMode)
80+
onValidate(answer)
81+
}
8782

88-
onAnswer(answer, dismiss, validate)
83+
private fun onAnswer(geoString: String, outputMode: OutputMode) {
84+
val answer = getAnswerData(geoString, outputMode)
85+
onAnswer(answer)
86+
}
87+
88+
private fun getAnswerData(geoString: String, outputMode: OutputMode): IAnswerData? {
89+
return if (geoString.isBlank()) {
90+
null
91+
} else {
92+
when (outputMode) {
93+
OutputMode.GEOTRACE -> GeoTraceData().also { it.value = geoString }
94+
OutputMode.GEOSHAPE -> GeoShapeData().also { it.value = geoString }
95+
}
96+
}
8997
}
9098
}

0 commit comments

Comments
 (0)