From ded9327d6ac6cbc62966c71bb90fc197a6c7181a Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 30 Mar 2023 15:25:28 +0200 Subject: [PATCH 1/4] Added failing tests --- .../widgets/GeoPointMapWidgetTest.java | 28 ++++++++++++++++++ .../android/widgets/GeoPointWidgetTest.java | 29 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java index be3798b8fc1..03d619116ea 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java @@ -5,6 +5,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import org.javarosa.core.model.data.GeoPointData; +import org.javarosa.core.model.data.StringData; import org.javarosa.form.api.FormEntryPrompt; import org.junit.Before; import org.junit.Test; @@ -53,6 +54,19 @@ public void getAnswer_whenPromptHasAnswer_returnsPromptAnswer() { new GeoPointData(GeoWidgetUtils.parseGeometryPoint(answer.getDisplayText())).getDisplayText()); } + @Test + public void getAnswer_whenPromptHasInvalidAnswer_returnsNull() { + GeoPointMapWidget widget = createWidget(promptWithAnswer(new StringData("blah"))); + assertNull(widget.getAnswer()); + } + + @Test + public void creatingWidgetWithInvalidValue_doesNotUpdateWidgetDisplayedAnswer() { + GeoPointMapWidget widget = createWidget(promptWithAnswer(new StringData("blah"))); + assertEquals(widget.binding.geoAnswerText.getText(), ""); + assertEquals(widget.binding.simpleButton.getText(), widget.getContext().getString(R.string.get_point)); + } + @Test public void whenPromptHasAnswer_answerTextViewShowsCorrectAnswer() { GeoPointMapWidget widget = createWidget(promptWithAnswer(answer)); @@ -108,6 +122,12 @@ public void setData_updatesWidgetAnswer() { assertEquals(widget.getAnswer().getDisplayText(), answer.getDisplayText()); } + @Test + public void setDataWithInvalidValue_doesNotUpdateWidgetAnswer() { + GeoPointMapWidget widget = createWidget(promptWithAnswer(null)); + widget.setData("blah"); + assertEquals(widget.getAnswer(), null); + } @Test public void setData_updatesWidgetDisplayedAnswer() { @@ -116,6 +136,14 @@ public void setData_updatesWidgetDisplayedAnswer() { assertEquals(widget.binding.geoAnswerText.getText(), GeoWidgetUtils.getGeoPointAnswerToDisplay(widget.getContext(), answer.getDisplayText())); } + @Test + public void setDataWithInvalidValue_doesNotUpdateWidgetDisplayedAnswer() { + GeoPointMapWidget widget = createWidget(promptWithAnswer(null)); + widget.setData("blah"); + assertEquals(widget.binding.geoAnswerText.getText(), ""); + assertEquals(widget.binding.simpleButton.getText(), widget.getContext().getString(R.string.get_point)); + } + @Test public void setData_whenDataIsNull_updatesButtonLabel() { GeoPointMapWidget widget = createWidget(promptWithAnswer(answer)); diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointWidgetTest.java b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointWidgetTest.java index eace680f634..d9e58956876 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointWidgetTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointWidgetTest.java @@ -5,6 +5,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import org.javarosa.core.model.data.GeoPointData; +import org.javarosa.core.model.data.StringData; import org.javarosa.form.api.FormEntryPrompt; import org.junit.Before; import org.junit.Test; @@ -57,6 +58,19 @@ public void getAnswer_whenPromptHasAnswer_returnsAnswer() { assertEquals(widget.getAnswer().getDisplayText(), answer.getDisplayText()); } + @Test + public void getAnswer_whenPromptHasInvalidAnswer_returnsNull() { + GeoPointWidget widget = createWidget(promptWithAnswer(new StringData("blah"))); + assertNull(widget.getAnswer()); + } + + @Test + public void creatingWidgetWithInvalidValue_doesNotUpdateWidgetDisplayedAnswer() { + GeoPointWidget widget = createWidget(promptWithAnswer(new StringData("blah"))); + assertEquals(widget.binding.geoAnswerText.getText(), ""); + assertEquals(widget.binding.simpleButton.getText(), widget.getContext().getString(R.string.get_point)); + } + @Test public void answerTextViewShouldShowCorrectAnswer() { GeoPointWidget widget = createWidget(promptWithAnswer(answer)); @@ -113,6 +127,13 @@ public void setData_updatesWidgetAnswer() { assertEquals(widget.getAnswer().getDisplayText(), answer.getDisplayText()); } + @Test + public void setDataWithInvalidValue_doesNotUpdateWidgetAnswer() { + GeoPointWidget widget = createWidget(promptWithAnswer(null)); + widget.setData("blah"); + assertEquals(widget.getAnswer(), null); + } + @Test public void setData_updatesWidgetDisplayedAnswer() { GeoPointWidget widget = createWidget(promptWithAnswer(null)); @@ -120,6 +141,14 @@ public void setData_updatesWidgetDisplayedAnswer() { assertEquals(widget.binding.geoAnswerText.getText(), GeoWidgetUtils.getGeoPointAnswerToDisplay(widget.getContext(), answer.getDisplayText())); } + @Test + public void setDataWithInvalidValue_doesNotUpdateWidgetDisplayedAnswer() { + GeoPointWidget widget = createWidget(promptWithAnswer(null)); + widget.setData("blah"); + assertEquals(widget.binding.geoAnswerText.getText(), ""); + assertEquals(widget.binding.simpleButton.getText(), widget.getContext().getString(R.string.get_point)); + } + @Test public void setData_whenDataIsNull_updatesButtonLabel() { GeoPointWidget widget = createWidget(promptWithAnswer(answer)); From adf37d26192b54d5fc48cf2857e77831990a149b Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 30 Mar 2023 16:46:06 +0200 Subject: [PATCH 2/4] Improved geo widgets --- .../android/widgets/GeoPointMapWidget.java | 38 ++++++++++++------- .../android/widgets/GeoPointWidget.java | 28 +++++++++----- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java index 7afe047e8b6..0b18aae3478 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java @@ -60,21 +60,23 @@ protected View onCreateAnswerView(Context context, FormEntryPrompt prompt, int a binding.simpleButton.setOnClickListener(v -> geoDataRequester.requestGeoPoint(prompt, answerText, waitingForDataRegistry)); answerText = prompt.getAnswerText(); - binding.geoAnswerText.setText(GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answerText)); + String answerToDisplay = GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answerText); - boolean dataAvailable = answerText != null && !answerText.isEmpty(); - if (getFormEntryPrompt().isReadOnly()) { - if (dataAvailable) { - binding.simpleButton.setText(R.string.geopoint_view_read_only); - } else { + if (answerToDisplay.isEmpty()) { + if (getFormEntryPrompt().isReadOnly()) { binding.simpleButton.setVisibility(View.GONE); + } else { + binding.simpleButton.setText(R.string.get_point); } + answerText = null; } else { - if (dataAvailable) { - binding.simpleButton.setText(R.string.view_change_location); + if (getFormEntryPrompt().isReadOnly()) { + binding.simpleButton.setText(R.string.geopoint_view_read_only); } else { - binding.simpleButton.setText(R.string.get_point); + binding.simpleButton.setText(R.string.view_change_location); } + + binding.geoAnswerText.setText(answerToDisplay); } return binding.getRoot(); @@ -82,9 +84,10 @@ protected View onCreateAnswerView(Context context, FormEntryPrompt prompt, int a @Override public IAnswerData getAnswer() { - return answerText == null || answerText.isEmpty() + double[] parsedGeometryPoint = GeoWidgetUtils.parseGeometryPoint(answerText); + return parsedGeometryPoint == null ? null - : new GeoPointData(GeoWidgetUtils.parseGeometryPoint(answerText)); + : new GeoPointData(parsedGeometryPoint); } @Override @@ -110,9 +113,16 @@ public void cancelLongPress() { @Override public void setData(Object answer) { - answerText = answer.toString(); - binding.geoAnswerText.setText(GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answerText)); - binding.simpleButton.setText(answerText == null || answerText.isEmpty() ? R.string.get_point : R.string.view_change_location); + String answerToDisplay = GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answer.toString()); + if (answerToDisplay.isEmpty()) { + answerText = null; + binding.geoAnswerText.setText(""); + binding.simpleButton.setText(R.string.get_point); + } else { + answerText = answer.toString(); + binding.geoAnswerText.setText(answerToDisplay); + binding.simpleButton.setText(R.string.view_change_location); + } widgetValueChanged(); } } diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java index 22b45316292..e0ce6c329f6 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java @@ -63,11 +63,13 @@ protected View onCreateAnswerView(Context context, FormEntryPrompt prompt, int a answerText = prompt.getAnswerText(); - if (answerText != null && !answerText.isEmpty()) { - binding.geoAnswerText.setText(GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answerText)); - binding.simpleButton.setText(R.string.change_location); - } else { + String answerToDisplay = GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answerText); + if (answerToDisplay.isEmpty()) { binding.simpleButton.setText(R.string.get_point); + answerText = null; + } else { + binding.geoAnswerText.setText(answerToDisplay); + binding.simpleButton.setText(R.string.change_location); } return binding.getRoot(); @@ -75,9 +77,10 @@ protected View onCreateAnswerView(Context context, FormEntryPrompt prompt, int a @Override public IAnswerData getAnswer() { - return answerText == null || answerText.isEmpty() + double[] parsedGeometryPoint = GeoWidgetUtils.parseGeometryPoint(answerText); + return parsedGeometryPoint == null ? null - : new GeoPointData(GeoWidgetUtils.parseGeometryPoint(answerText)); + : new GeoPointData(parsedGeometryPoint); } @Override @@ -103,9 +106,16 @@ public void cancelLongPress() { @Override public void setData(Object answer) { - answerText = answer.toString(); - binding.geoAnswerText.setText(GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answerText)); - binding.simpleButton.setText(answerText == null || answerText.isEmpty() ? R.string.get_point : R.string.change_location); + String answerToDisplay = GeoWidgetUtils.getGeoPointAnswerToDisplay(getContext(), answer.toString()); + if (answerToDisplay.isEmpty()) { + answerText = null; + binding.geoAnswerText.setText(""); + binding.simpleButton.setText(R.string.get_point); + } else { + answerText = answer.toString(); + binding.geoAnswerText.setText(answerToDisplay); + binding.simpleButton.setText(R.string.change_location); + } widgetValueChanged(); } } From b318d266261e608c02e39cab1576260f2a116ab2 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 30 Mar 2023 16:57:38 +0200 Subject: [PATCH 3/4] Added failing test for ActivityGeoDataRequester --- .../utilities/ActivityGeoDataRequesterTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequesterTest.java b/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequesterTest.java index 4e99aa54f5f..46817f269ed 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequesterTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequesterTest.java @@ -147,6 +147,18 @@ public void requestGeoPoint_whenAnswerIsPresent_addsToIntent() { assertThat(bundle.getParcelable(GeoPointMapActivity.EXTRA_LOCATION), equalTo(new MapPoint(1.0, 2.0, 3, 4))); } + @Test + public void requestGeoPoint_whenAnswerIsPresentButInvalid_doesNotAddToIntent() { + activityGeoDataRequester.requestGeoPoint(prompt, "something", waitingForDataRegistry); + Intent startedIntent = shadowActivity.getNextStartedActivity(); + + assertEquals(startedIntent.getComponent(), new ComponentName(testActivity, GeoPointActivity.class)); + assertEquals(shadowActivity.getNextStartedActivityForResult().requestCode, LOCATION_CAPTURE); + + Bundle bundle = startedIntent.getExtras(); + assertThat(bundle.getParcelable(GeoPointMapActivity.EXTRA_LOCATION), equalTo(null)); + } + @Test public void whenWidgetHasAccuracyValue_requestGeoPoint_launchesCorrectIntent() { when(questionDef.getAdditionalAttribute(null, "accuracyThreshold")).thenReturn("10"); From a125b4f797be62dcb483a5f356087f838aef3386 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 30 Mar 2023 16:59:42 +0200 Subject: [PATCH 4/4] Fixed ActivityGeoDataRequester --- .../android/widgets/utilities/ActivityGeoDataRequester.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt index d706437ce6b..9375ca56975 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt @@ -35,10 +35,11 @@ class ActivityGeoDataRequester( waitingForDataRegistry.waitForData(prompt.index) val bundle = Bundle().also { - if (!answerText.isNullOrEmpty()) { + val parsedGeometry = GeoWidgetUtils.parseGeometry(answerText) + if (parsedGeometry.isNotEmpty()) { it.putParcelable( GeoPointMapActivity.EXTRA_LOCATION, - GeoWidgetUtils.parseGeometry(answerText)[0], + parsedGeometry[0], ) }