Stop Change Reason prompt displaying on rotation after it's dismissed

grzesiek2010 · Dec 4, 2019 · 1ed71c3 · 1ed71c3
1 parent 993fadf
commit 1ed71c3
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 24 deletions.
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
@@ -1879,8 +1879,8 @@ private boolean saveDataToDisk(boolean exit, boolean complete, String updatedSav
         if (exit) {
             getFormController().getAuditEventLogger().exitView();
             changesReasonPromptViewModel.savingForm();
-            changesReasonPromptViewModel.requiresReasonToContinue().observe(this, requiresReason -> {
-                if (!requiresReason) {
+            changesReasonPromptViewModel.readyToSave().observe(this, readyToSave -> {
+                if (readyToSave) {
                     saveToDiskTask = new SaveToDiskTask(getIntent().getData(), true, complete,
                             updatedSaveName);
                     saveToDiskTask.setFormSavedListener(this);

diff --git a/.../main/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptDialogFragment.java b/.../main/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptDialogFragment.java
@@ -82,20 +82,20 @@ public void onAttach(@NonNull Context context) {
         super.onAttach(context);
 
         viewModel = ViewModelProviders.of(requireActivity(), viewModelFactory).get(ChangesReasonPromptViewModel.class);
-        viewModel.requiresReasonToContinue().observe(this, requiresIdentity -> {
-            if (!requiresIdentity) {
+        viewModel.requiresReasonToContinue().observe(this, requiresReason -> {
+            if (!requiresReason) {
                 dismiss();
             }
         });
     }
 
     @Override
     protected void onBackPressed() {
-        dismiss();
+        viewModel.promptDismissed();
     }
 
     @Override
     protected void onCloseClicked() {
-        dismiss();
+        viewModel.promptDismissed();
     }
 }
diff --git a/...p/src/main/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptViewModel.java b/...p/src/main/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptViewModel.java
@@ -11,26 +11,39 @@
 public class ChangesReasonPromptViewModel extends ViewModel {
 
     private final MutableLiveData<Boolean> requiresReasonToContinue = new MutableLiveData<>(false);
+    private final MutableLiveData<Boolean> readyToSave = new MutableLiveData<>(false);
+
     private AuditEventLogger auditEventLogger;
     private String reason;
 
     public LiveData<Boolean> requiresReasonToContinue() {
         return requiresReasonToContinue;
     }
 
+    public LiveData<Boolean> readyToSave() {
+        return readyToSave;
+    }
+
     public void setAuditEventLogger(AuditEventLogger auditEventLogger) {
         this.auditEventLogger = auditEventLogger;
-        updateRequiresReasonToContinue();
     }
 
     public void savingForm() {
-        updateRequiresReasonToContinue();
+        if (!requiresReasonToSave()) {
+            readyToSave.setValue(true);
+        } else {
+            requiresReasonToContinue.setValue(true);
+        }
     }
 
     public void editingForm() {
         auditEventLogger.setEditing(true);
     }
 
+    public void promptDismissed() {
+        requiresReasonToContinue.setValue(false);
+    }
+
     public void setReason(String reason) {
         this.reason = reason;
     }
@@ -39,20 +52,19 @@ public void saveReason(Long currentTime) {
         if (reason != null && !isBlank(reason)) {
             this.auditEventLogger.logEvent(AuditEvent.AuditEventType.CHANGE_REASON, null, true, null, currentTime, reason);
             requiresReasonToContinue.setValue(false);
+            readyToSave.setValue(true);
         }
     }
 
     public String getReason() {
         return reason;
     }
 
-    private void updateRequiresReasonToContinue() {
-        requiresReasonToContinue.setValue(
-                auditEventLogger != null
-                        && auditEventLogger.isEditing()
-                        && auditEventLogger.isChangeReasonRequired()
-                        && auditEventLogger.isChangesMade()
-        );
+    private boolean requiresReasonToSave() {
+        return auditEventLogger != null
+                && auditEventLogger.isEditing()
+                && auditEventLogger.isChangeReasonRequired()
+                && auditEventLogger.isChangesMade();
     }
 
     public static class Factory implements ViewModelProvider.Factory {

diff --git a/...t/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptDialogFragmentTest.java b/...t/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptDialogFragmentTest.java
@@ -7,6 +7,8 @@
 import androidx.lifecycle.ViewModel;
 import androidx.lifecycle.ViewModelProvider;
 
+import org.jetbrains.annotations.NotNull;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.odk.collect.android.support.RobolectricHelpers;
@@ -15,31 +17,57 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(RobolectricTestRunner.class)
 public class ChangesReasonPromptDialogFragmentTest {
 
-    @Test
-    public void show_onlyEverOpensOneDialog() {
+    private FragmentManager fragmentManager;
+    private ChangesReasonPromptViewModel viewModel;
+    private ViewModelProvider.Factory testFactory;
+
+    @Before
+    public void setup() {
         FragmentActivity activity = RobolectricHelpers.createThemedActivity(FragmentActivity.class);
-        FragmentManager fragmentManager = activity.getSupportFragmentManager();
+        fragmentManager = activity.getSupportFragmentManager();
 
-        ChangesReasonPromptViewModel viewModel = mock(ChangesReasonPromptViewModel.class);
+        viewModel = mock(ChangesReasonPromptViewModel.class);
         when(viewModel.requiresReasonToContinue()).thenReturn(new MutableLiveData<>(true));
-        ViewModelProvider.Factory factory = new TestFactory(viewModel);
+        testFactory = new TestFactory(viewModel);
+    }
 
-        ChangesReasonPromptDialogFragment dialog1 = new ChangesReasonPromptDialogFragment();
-        dialog1.viewModelFactory = factory;
+    @Test
+    public void show_onlyEverOpensOneDialog() {
+        ChangesReasonPromptDialogFragment dialog1 = createFragment();
         dialog1.show("Best Form", fragmentManager);
 
-        ChangesReasonPromptDialogFragment dialog2 = new ChangesReasonPromptDialogFragment();
-        dialog2.viewModelFactory = factory;
+        ChangesReasonPromptDialogFragment dialog2 = createFragment();
         dialog2.show("Best Form", fragmentManager);
 
         assertThat(fragmentManager.getFragments().size(), equalTo(1));
     }
 
+    @Test
+    public void onBackPressed_and_onCloseClicked_callPromptDismissed() {
+        ChangesReasonPromptDialogFragment dialog = createFragment();
+        dialog.show("Best Form", fragmentManager);
+
+        dialog.onBackPressed();
+        verify(viewModel, times(1)).promptDismissed();
+
+        dialog.onCloseClicked();
+        verify(viewModel, times(2)).promptDismissed();
+    }
+
+    @NotNull
+    private ChangesReasonPromptDialogFragment createFragment() {
+        ChangesReasonPromptDialogFragment dialog = new ChangesReasonPromptDialogFragment();
+        dialog.viewModelFactory = testFactory;
+        return dialog;
+    }
+
     private static class TestFactory implements ViewModelProvider.Factory {
 
         private final ChangesReasonPromptViewModel viewModel;

diff --git a/...c/test/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptViewModelTest.java b/...c/test/java/org/odk/collect/android/formentry/audit/ChangesReasonPromptViewModelTest.java
@@ -4,6 +4,8 @@
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -24,4 +26,20 @@ public void save_logsChangeReasonAuditEvent() {
 
         verify(logger).logEvent(AuditEvent.AuditEventType.CHANGE_REASON, null, true, null, 123L, "Blah");
     }
+
+    @Test
+    public void promptDismissed_sets_isChangeReasonRequired_false() {
+        AuditEventLogger logger = mock(AuditEventLogger.class);
+        when(logger.isChangeReasonRequired()).thenReturn(true);
+        when(logger.isChangesMade()).thenReturn(true);
+        when(logger.isEditing()).thenReturn(true);
+
+        ChangesReasonPromptViewModel viewModel = new ChangesReasonPromptViewModel();
+        viewModel.setAuditEventLogger(logger);
+        viewModel.savingForm();
+        assertThat(viewModel.requiresReasonToContinue().getValue(), equalTo(true));
+
+        viewModel.promptDismissed();
+        assertThat(viewModel.requiresReasonToContinue().getValue(), equalTo(false));
+    }
 }