Merge pull request #5474 from seadowg/ignore-changes

seadowg · web-flow · commit fb41bbe48afa · 2023-03-14T14:03:47.000Z
Show different text for ignoring changes when editing or filling out a form
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/external/InstanceEditActionTest.kt b/collect_app/src/androidTest/java/org/odk/collect/android/feature/external/InstanceEditActionTest.kt
@@ -64,7 +64,7 @@ class InstanceEditActionTest {
         val intent = Intent(Intent.ACTION_EDIT).also { it.data = uri }
         val result = rule.launchForResult(intent, FormHierarchyPage("One Question")) {
             it.clickGoToStart()
-                .pressBackAndIgnoreChanges(AppClosedPage())
+                .pressBackAndDiscardChanges(AppClosedPage())
         }
 
         assertThat(result.resultData.data, equalTo(uri))
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/BackgroundAudioRecordingTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/BackgroundAudioRecordingTest.java
@@ -160,7 +160,7 @@ public void uncheckingRecordAudio_andConfirming_endsAndDeletesRecording() {
 
         formEntryPage.closeSoftKeyboard()
                 .pressBack(new SaveOrIgnoreDialog<>("One Question", new MainMenuPage()))
-                .clickIgnoreChanges()
+                .clickDiscardForm()
                 .startBlankForm("One Question");
 
         assertThat(stubAudioRecorderViewModel.isRecording(), is(false));
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/FormLanguageTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/FormLanguageTest.java
@@ -41,7 +41,7 @@ public void languageChoiceIsPersisted() {
                 .clickOnString(R.string.change_language)
                 .clickOnText("French (fr)")
                 .pressBack(new SaveOrIgnoreDialog<>("One Question", new MainMenuPage()))
-                .clickIgnoreChanges()
+                .clickDiscardForm()
 
                 .startBlankForm("One Question")
                 .assertQuestion("quel âge as-tu");
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/QuickSaveTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/QuickSaveTest.java
@@ -30,7 +30,7 @@ public void whenFillingForm_clickingSaveIcon_savesCurrentAnswers() {
                         new QuestionAndAnswer("What is your age?", "32")
                 )
                 .clickSave()
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question")
@@ -46,7 +46,7 @@ public void whenFillingForm_withViolatedConstraintsOnCurrentScreen_clickingSaveI
                 .answerQuestion("What is your name?", "Reuben")
                 .swipeToNextQuestion("What is your age?", true)
                 .clickSave()
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
@@ -60,15 +60,15 @@ public void whenEditingANonFinalizedForm_withViolatedConstraintsOnCurrentScreen_
                 .startBlankForm("Two Question Required")
                 .answerQuestion("What is your name?", "Reuben")
                 .clickSave()
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
                 .clickGoToStart()
                 .answerQuestion("What is your name?", "Another Reuben")
                 .swipeToNextQuestion("What is your age?", true)
                 .clickSave()
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
@@ -94,7 +94,7 @@ public void whenEditingAFinalizedForm_withViolatedConstraintsOnCurrentScreen_cli
                 .removeResponse()
                 .clickSaveWithError(R.string.required_answer_error)
 
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
@@ -123,7 +123,7 @@ public void whenEditingAFinalizedForm_withViolatedConstraintsOnAnotherScreen_cli
                 .clickSave()
                 .assertConstraintDisplayed("Sorry, this response is required!")
                 .assertQuestion("What is your age?", true)
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/QuittingFormTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/QuittingFormTest.java
@@ -49,7 +49,7 @@ public void whenFillingForm_pressingBack_andClickingIgnoreChanges_doesNotSaveFor
                 .startBlankForm("Two Question")
                 .answerQuestion("What is your name?", "Reuben")
                 .pressBack(new SaveOrIgnoreDialog<>("Two Question", new MainMenuPage()))
-                .clickIgnoreChanges()
+                .clickDiscardForm()
 
                 .assertNumberOfEditableForms(0);
     }
@@ -63,7 +63,7 @@ public void whenFillingForm_saving_andPressingBack_andClickingIgnoreChanges_save
                 .clickSave()
                 .swipeToNextQuestion("What is your age?")
                 .answerQuestion("What is your age?", "10")
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question")
@@ -130,7 +130,7 @@ public void whenEditingAFinalizedForm_withViolatedConstraintsOnCurrentScreen_pre
                 .closeSoftKeyboard()
                 .pressBack(new SaveOrIgnoreDialog<>("Two Question Required", new FormEntryPage("Two Question Required")))
                 .clickSaveChangesWithError(R.string.required_answer_error)
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
@@ -160,7 +160,7 @@ public void whenEditingAFinalizedForm_withViolatedConstraintsOnAnotherScreen_pre
                 .pressBack(new SaveOrIgnoreDialog<>("Two Question Required", new FormEntryPage("Two Question Required")))
                 .clickSaveChangesWithError(R.string.required_answer_error)
                 .assertQuestion("What is your age?", true)
-                .pressBackAndIgnoreChanges()
+                .pressBackAndDiscardChanges()
 
                 .clickEditSavedForm(1)
                 .clickOnForm("Two Question Required")
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/SaveIncompleteTest.kt b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/SaveIncompleteTest.kt
@@ -25,7 +25,7 @@ class SaveIncompleteTest {
             .startBlankForm("Two Question Save Incomplete")
             .answerQuestion("What is your name?", "Dez")
             .swipeToNextQuestion("[saveIncomplete] What is your age?")
-            .pressBackAndIgnoreChanges()
+            .pressBackAndDiscardChanges()
 
             .clickEditSavedForm(1)
             .clickOnForm("Two Question Save Incomplete")
@@ -39,7 +39,7 @@ class SaveIncompleteTest {
             .startBlankForm("Two Question Save Incomplete Required")
             .answerQuestion("What is your name?", "Dez")
             .swipeToNextQuestion("[saveIncomplete] What is your age?", true)
-            .pressBackAndIgnoreChanges()
+            .pressBackAndDiscardChanges()
 
             .clickEditSavedForm(1)
             .clickOnForm("Two Question Save Incomplete Required")
@@ -62,7 +62,7 @@ class SaveIncompleteTest {
             .clickGoToStart()
             .answerQuestion("What is your name?", "Meg")
             .swipeToNextQuestion("[saveIncomplete] What is your age?", true)
-            .pressBackAndIgnoreChanges()
+            .pressBackAndDiscardChanges()
 
             .assertNumberOfFinalizedForms(0)
             .clickEditSavedForm(1)
@@ -90,7 +90,7 @@ class SaveIncompleteTest {
             .swipeToPreviousQuestion("What is your name?")
             .answerQuestion("What is your name?", "Bradley")
             .swipeToNextQuestion("[saveIncomplete] What is your age?")
-            .pressBackAndIgnoreChanges()
+            .pressBackAndDiscardChanges()
 
             .assertNumberOfFinalizedForms(0)
             .clickEditSavedForm(1)
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/audit/TrackChangesReasonTest.kt b/collect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/audit/TrackChangesReasonTest.kt
@@ -141,7 +141,7 @@ class TrackChangesReasonTest {
             .clickGoToStart()
             .closeSoftKeyboard()
             .pressBack(SaveOrIgnoreDialog("Track Changes Reason", MainMenuPage()))
-            .clickIgnoreChanges()
+            .clickDiscardChanges()
     }
 
     @Test
@@ -161,4 +161,15 @@ class TrackChangesReasonTest {
             .clickSave(FormEntryPage("Track Changes Reason"))
             .assertQuestion("What up?")
     }
+
+    @Test
+    fun fillingABlankForm_andClickingSave_andClickingSaveAndExit_doesNotPromptForReason() {
+        rule.startAtMainMenu()
+            .copyForm("track-changes-reason-on-edit.xml")
+            .startBlankForm("Track Changes Reason")
+            .clickSave()
+            .closeSoftKeyboard()
+            .pressBack(SaveOrIgnoreDialog("Track Changes Reason", MainMenuPage()))
+            .clickSaveChanges()
+    }
 }
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/settings/FormMetadataSettingsTest.kt b/collect_app/src/androidTest/java/org/odk/collect/android/feature/settings/FormMetadataSettingsTest.kt
@@ -81,7 +81,7 @@ class FormMetadataSettingsTest {
             .startBlankForm("Metadata")
             .assertText("Chino")
             .pressBack(SaveOrIgnoreDialog("Metadata", MainMenuPage()))
-            .clickIgnoreChanges()
+            .clickDiscardForm()
             .openProjectSettingsDialog()
             .clickSettings()
             .clickUserAndDeviceIdentity()
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/regression/DrawWidgetTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/regression/DrawWidgetTest.java
@@ -34,7 +34,7 @@ public void saveIgnoreDialog_ShouldUseBothOptions() {
                 .clickOnId(R.id.simple_button)
                 .waitForRotationToEnd()
                 .pressBack(new SaveOrIgnoreDialog<>("Sketch Image", new FormEntryPage("All widgets")))
-                .clickIgnoreChanges()
+                .clickDiscardForm()
                 .waitForRotationToEnd()
                 .clickOnId(R.id.simple_button)
                 .waitForRotationToEnd()
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/regression/FormValidationTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/regression/FormValidationTest.java
@@ -50,6 +50,6 @@ public void openHierarchyView_ShouldSeeShortForms() {
                 .pressBack(new FormEntryPage("OnePageFormShort"))
                 .closeSoftKeyboard()
                 .pressBack(new SaveOrIgnoreDialog<>("OnePageFormShort", new MainMenuPage()))
-                .clickIgnoreChanges();
+                .clickDiscardForm();
     }
 }
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/regression/SignatureWidgetTest.java b/collect_app/src/androidTest/java/org/odk/collect/android/regression/SignatureWidgetTest.java
@@ -37,7 +37,7 @@ public void saveIgnoreDialog_ShouldUseBothOptions() {
                 .pressBack(new SaveOrIgnoreDialog<>("Gather Signature", new FormEntryPage("All widgets")))
                 .checkIsTranslationDisplayed("Exit Gather Signature", "Salir Adjuntar firma")
                 .assertText(R.string.keep_changes)
-                .clickIgnoreChanges()
+                .clickDiscardForm()
                 .waitForRotationToEnd()
                 .clickWidgetButton()
                 .waitForRotationToEnd()
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/FormEntryPage.java b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/FormEntryPage.java
@@ -365,16 +365,28 @@ public FormEntryPage assertConstraintDisplayed(String constraintText) {
         return this;
     }
 
-    public MainMenuPage pressBackAndIgnoreChanges() {
+    public MainMenuPage pressBackAndDiscardChanges() {
         return closeSoftKeyboard()
                 .pressBack(new SaveOrIgnoreDialog<>(formName, new MainMenuPage()))
-                .clickIgnoreChanges();
+                .clickDiscardChanges();
     }
 
-    public <D extends Page<D>> D pressBackAndIgnoreChanges(D destination) {
+    public <D extends Page<D>> D pressBackAndDiscardChanges(D destination) {
+        return closeSoftKeyboard()
+                .pressBack(new SaveOrIgnoreDialog<>(formName, destination))
+                .clickDiscardChanges();
+    }
+
+    public MainMenuPage pressBackAndDiscardForm() {
+        return closeSoftKeyboard()
+                .pressBack(new SaveOrIgnoreDialog<>(formName, new MainMenuPage()))
+                .clickDiscardForm();
+    }
+
+    public <D extends Page<D>> D pressBackAndDiscardForm(D destination) {
         return closeSoftKeyboard()
                 .pressBack(new SaveOrIgnoreDialog<D>(formName, destination))
-                .clickIgnoreChanges();
+                .clickDiscardForm();
     }
 
     public FormEntryPage assertBackgroundLocationSnackbarShown() {
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/SaveOrIgnoreDialog.java b/collect_app/src/androidTest/java/org/odk/collect/android/support/pages/SaveOrIgnoreDialog.java
@@ -39,8 +39,13 @@ public D clickSaveChangesWithError(int errorMsg) {
         return destination.assertOnPage();
     }
 
-    public D clickIgnoreChanges() {
+    public D clickDiscardForm() {
         clickOnString(R.string.do_not_save);
         return destination.assertOnPage();
     }
+
+    public D clickDiscardChanges() {
+        clickOnString(R.string.discard_changes);
+        return destination.assertOnPage();
+    }
 }
diff --git a/collect_app/src/main/java/org/odk/collect/android/activities/FormEntryActivity.java b/collect_app/src/main/java/org/odk/collect/android/activities/FormEntryActivity.java
@@ -2272,8 +2272,6 @@ && new PlayServicesChecker().isGooglePlayServicesAvailable(this)) {
                                 startActivityForResult(intent, RequestCodes.HIERARCHY_ACTIVITY);
                             }
                         });
-
-                        formController.getAuditEventLogger().setEditing(true);
                     } else {
                         formControllerAvailable(formController);
                         if (ApplicationConstants.FormModes.VIEW_SENT.equalsIgnoreCase(formMode)) {
diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/QuitFormDialog.kt b/collect_app/src/main/java/org/odk/collect/android/formentry/QuitFormDialog.kt
@@ -60,10 +60,10 @@ object QuitFormDialog {
         ) {
             ImmutableList.of(
                 IconMenuItem(R.drawable.ic_save, R.string.keep_changes),
-                IconMenuItem(R.drawable.ic_delete, R.string.do_not_save)
+                getDiscardItem(formSaveViewModel)
             )
         } else {
-            ImmutableList.of(IconMenuItem(R.drawable.ic_delete, R.string.do_not_save))
+            ImmutableList.of(getDiscardItem(formSaveViewModel))
         }
 
         val listView = DialogUtils.createActionListView(activity)
@@ -116,4 +116,12 @@ object QuitFormDialog {
 
         return dialog
     }
+
+    private fun getDiscardItem(formSaveViewModel: FormSaveViewModel): IconMenuItem {
+        return if (formSaveViewModel.hasSaved()) {
+            IconMenuItem(R.drawable.ic_delete, R.string.discard_changes)
+        } else {
+            IconMenuItem(R.drawable.ic_delete, R.string.do_not_save)
+        }
+    }
 }
diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/audit/AuditEventLogger.java b/collect_app/src/main/java/org/odk/collect/android/formentry/audit/AuditEventLogger.java
@@ -41,7 +41,6 @@ public class AuditEventLogger {
     private final AuditConfig auditConfig;
     private final FormController formController;
     private String user;
-    private boolean editing;
 
     public AuditEventLogger(AuditConfig auditConfig, AuditEventWriter writer, FormController formController) {
         this.auditConfig = auditConfig;
@@ -295,14 +294,6 @@ public boolean isChangeReasonRequired() {
         return auditConfig != null && auditConfig.isTrackChangesReasonEnabled();
     }
 
-    public void setEditing(boolean editing) {
-        this.editing = editing;
-    }
-
-    public boolean isEditing() {
-        return editing;
-    }
-
     public interface AuditEventWriter {
 
         void writeEvents(List<AuditEvent> auditEvents);
diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaveViewModel.java b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaveViewModel.java
@@ -102,14 +102,6 @@ public FormSaveViewModel(SavedStateHandle stateHandle, Supplier<Long> clock, For
         });
     }
 
-    public void editingForm() {
-        if (formController == null) {
-            return;
-        }
-
-        formController.getAuditEventLogger().setEditing(true);
-    }
-
     public void saveForm(Uri instanceContentURI, boolean shouldFinalize, String updatedSaveName, boolean viewExiting) {
         if (isSaving() || formController == null) {
             return;
@@ -286,7 +278,7 @@ public void resumeFormEntry() {
 
     private boolean requiresReasonToSave() {
         return formController != null
-                && formController.getAuditEventLogger().isEditing()
+                && formController.isEditing()
                 && formController.getAuditEventLogger().isChangeReasonRequired();
     }
 
@@ -297,6 +289,11 @@ public String getFormName() {
         return formController.getFormTitle();
     }
 
+    public boolean hasSaved() {
+        File instanceFile = formController.getInstanceFile();
+        return instanceFile != null && instanceFile.exists();
+    }
+
     @Override
     public void deleteAnswerFile(String questionIndex, String fileName) {
         if (questionIndex != null && fileName != null) {
diff --git a/collect_app/src/main/java/org/odk/collect/android/javarosawrapper/FormController.kt b/collect_app/src/main/java/org/odk/collect/android/javarosawrapper/FormController.kt
@@ -33,6 +33,8 @@ interface FormController {
 
     fun getAuditEventLogger(): AuditEventLogger?
 
+    fun isEditing(): Boolean
+
     /**
      * For logging purposes...
      *
diff --git a/collect_app/src/main/java/org/odk/collect/android/javarosawrapper/JavaRosaFormController.java b/collect_app/src/main/java/org/odk/collect/android/javarosawrapper/JavaRosaFormController.java
@@ -90,6 +90,7 @@ public class JavaRosaFormController implements FormController {
      */
     private static final String AUDIT = "audit";
     public static final String AUDIT_FILE_NAME = "audit.csv";
+    private final boolean isEditing;
 
     /*
      * Store the auditEventLogger object with the form controller state
@@ -106,6 +107,12 @@ public JavaRosaFormController(File mediaFolder, FormEntryController fec, File in
         this.mediaFolder = mediaFolder;
         formEntryController = fec;
         this.instanceFile = instanceFile;
+        isEditing = instanceFile != null;
+    }
+
+    @Override
+    public boolean isEditing() {
+        return isEditing;
     }
 
     public FormDef getFormDef() {
diff --git a/collect_app/src/test/java/org/odk/collect/android/formentry/audit/FormSaveViewModelTest.java b/collect_app/src/test/java/org/odk/collect/android/formentry/audit/FormSaveViewModelTest.java
diff --git a/collect_app/src/test/java/org/odk/collect/android/utilities/DummyFormController.kt b/collect_app/src/test/java/org/odk/collect/android/utilities/DummyFormController.kt
diff --git a/strings/src/main/res/values/strings.xml b/strings/src/main/res/values/strings.xml

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ class InstanceEditActionTest {`
`64`	`64`	`val intent = Intent(Intent.ACTION_EDIT).also { it.data = uri }`
`65`	`65`	`val result = rule.launchForResult(intent, FormHierarchyPage("One Question")) {`
`66`	`66`	`it.clickGoToStart()`
`67`		`- .pressBackAndIgnoreChanges(AppClosedPage())`
	`67`	`+ .pressBackAndDiscardChanges(AppClosedPage())`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`assertThat(result.resultData.data, equalTo(uri))`
Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,6 @@ public void openHierarchyView_ShouldSeeShortForms() {`
`50`	`50`	`.pressBack(new FormEntryPage("OnePageFormShort"))`
`51`	`51`	`.closeSoftKeyboard()`
`52`	`52`	`.pressBack(new SaveOrIgnoreDialog<>("OnePageFormShort", new MainMenuPage()))`
`53`		`- .clickIgnoreChanges();`
	`53`	`+ .clickDiscardForm();`
`54`	`54`	`}`
`55`	`55`	`}`