Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove grace period restriction time restriction from editing finalized forms #5717

Merged
merged 8 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.odk.collect.android.instancemanagement.InstanceExtKt.OCTOBER_1st_2023_UTC;
import static org.odk.collect.android.support.FileUtils.copyFileFromAssets;

import android.Manifest;
Expand All @@ -28,6 +27,7 @@
import org.odk.collect.android.support.pages.FormEntryPage;
import org.odk.collect.android.support.pages.MainMenuPage;
import org.odk.collect.android.support.pages.SaveOrDiscardFormDialog;
import org.odk.collect.android.support.pages.SendFinalizedFormPage;
import org.odk.collect.android.support.rules.CollectTestRule;
import org.odk.collect.android.support.rules.TestRuleChain;
import org.odk.collect.audiorecorder.recording.AudioRecorder;
Expand Down Expand Up @@ -185,13 +185,17 @@ public void whenRecordAudioPermissionNotGranted_openingForm_andDenyingPermission

@Test
public void viewForm_doesNotRecordAudio() {
currentTimeMillis = OCTOBER_1st_2023_UTC + 1;

rule.startAtMainMenu()
.setServer(testDependencies.server.getURL())
.copyForm("one-question-background-audio.xml")
.startBlankForm("One Question")
.fillOutAndFinalize(new FormEntryPage.QuestionAndAnswer("what is your age", "17"))
.clickSendFinalizedForm(1)
.clickSelectAll()
.clickSendSelected()
.clickOK(new SendFinalizedFormPage())
.pressBack(new MainMenuPage())
.clickViewSentForm(1)
.clickOnForm("One Question");

assertThat(stubAudioRecorderViewModel.isRecording(), is(false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.odk.collect.android.R
import org.odk.collect.android.instancemanagement.OCTOBER_1st_2023_UTC
import org.odk.collect.android.support.TestDependencies
import org.odk.collect.android.support.pages.MainMenuPage
import org.odk.collect.android.support.rules.CollectTestRule
Expand Down Expand Up @@ -41,9 +40,7 @@ class FormSavedSnackbarTest {
}

@Test
fun beforeOCTOBER_1st_2023_UTC_whenDraftFinalized_displaySnackbarWithViewActionThatOpensFormForEdit() {
currentTimeMillis = OCTOBER_1st_2023_UTC - 1

fun whenDraftFinalized_displaySnackbarWithViewActionThatOpensFormForEdit() {
rule.startAtMainMenu()
.copyForm("one-question.xml")
.startBlankForm("One Question")
Expand All @@ -62,28 +59,6 @@ class FormSavedSnackbarTest {
.assertText(R.string.jump_to_end)
}

@Test
fun afterOCTOBER_1st_2023_UTC_whenDraftFinalized_displaySnackbarWithViewActionThatOpensFormForViewOnly() {
currentTimeMillis = OCTOBER_1st_2023_UTC + 1

rule.startAtMainMenu()
.copyForm("one-question.xml")
.startBlankForm("One Question")
.answerQuestion(0, "25")
.swipeToEndScreen()
.clickSaveAsDraft()
.clickEditSavedForm()
.clickOnForm("One Question")
.clickGoToEnd()
.clickFinalize()
.assertText(R.string.form_saved)
.clickOnString(R.string.view_form)
.assertText("25")
.assertTextDoesNotExist(R.string.jump_to_beginning)
.assertTextDoesNotExist(R.string.jump_to_end)
.assertText(R.string.exit)
}

@Test
fun snackbarCanBeDismissed_andWillNotBeDisplayedAgainAfterRecreatingTheActivity() {
rule.startAtMainMenu()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import org.junit.Test
import org.junit.rules.RuleChain
import org.junit.runner.RunWith
import org.odk.collect.android.R
import org.odk.collect.android.instancemanagement.OCTOBER_1st_2023_UTC
import org.odk.collect.android.support.CollectHelpers.addGDProject
import org.odk.collect.android.support.TestDependencies
import org.odk.collect.android.support.pages.FormEntryPage.QuestionAndAnswer
Expand Down Expand Up @@ -39,29 +38,22 @@ class SendFinalizedFormTest {
.around(rule)

@Test
fun beforeOCTOBER_1st_2023_UTC_canEditFormsBeforeSending() {
currentTimeMillis = OCTOBER_1st_2023_UTC - 1

fun canEditFormsBeforeSending() {
rule.withProject(testDependencies.server.url)
.copyForm("one-question.xml", projectName = testDependencies.server.hostName)
.startBlankForm("One Question")
.fillOutAndFinalize(QuestionAndAnswer("what is your age", "52"))

.clickSendFinalizedForm(1)
.clickOnFormToEdit("One Question")
.assertText("52")
}

@Test
fun afterOCTOBER_1st_2023_UTC_canViewFormsBeforeSending() {
currentTimeMillis = OCTOBER_1st_2023_UTC + 1
.clickGoToStart()
.answerQuestion("what is your age", "53")
.swipeToEndScreen()
.clickFinalize()

rule.withProject(testDependencies.server.url)
.copyForm("one-question.xml", projectName = testDependencies.server.hostName)
.startBlankForm("One Question")
.fillOutAndFinalize(QuestionAndAnswer("what is your age", "52"))
.clickSendFinalizedForm(1)
.clickOnForm("One Question")
.assertText("52")
.clickOnFormToEdit("One Question")
.assertText("53")
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@
import org.odk.collect.android.audio.AudioRecordingControllerFragment;
import org.odk.collect.android.audio.M4AAppender;
import org.odk.collect.android.backgroundwork.InstanceSubmitScheduler;
import org.odk.collect.android.dao.helpers.InstancesDaoHelper;
import org.odk.collect.android.entities.EntitiesRepositoryProvider;
import org.odk.collect.android.exception.JavaRosaException;
import org.odk.collect.android.external.FormsContract;
Expand Down Expand Up @@ -1035,7 +1034,7 @@ public boolean onOptionsItemSelected(MenuItem item) {

case R.id.menu_save:
// don't exit
saveForm(false, InstancesDaoHelper.isInstanceComplete(getFormController()), null, true);
saveForm(false, false, null, true);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a super easy change as the tests already describe the desired behaviour. I just had to get rid of the extra untested logic

return true;
}

Expand Down Expand Up @@ -1968,7 +1967,7 @@ public boolean onKeyDown(int keyCode, KeyEvent event) {
}

QuitFormDialog.show(this, formSaveViewModel, formEntryViewModel, settingsProvider, () -> {
saveForm(true, InstancesDaoHelper.isInstanceComplete(getFormController()), null, true);
saveForm(true, false, null, true);
});
return true;
case KeyEvent.KEYCODE_DPAD_RIGHT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import android.widget.ListView;
import android.widget.ProgressBar;

import androidx.activity.result.ActivityResultLauncher;
import androidx.activity.result.contract.ActivityResultContracts;
import androidx.annotation.NonNull;
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.widget.SearchView;
Expand Down Expand Up @@ -134,6 +136,11 @@ public class InstanceUploaderListActivity extends LocalizedActivity implements
private SearchView searchView;
private String savedFilterText;

private final ActivityResultLauncher<Intent> formLauncher = registerForActivityResult(new ActivityResultContracts.StartActivityForResult(), result -> {
setResult(RESULT_OK, result.getData());
finish();
});

@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Expand Down Expand Up @@ -394,7 +401,7 @@ public void onItemClick(AdapterView<?> parent, View view, int position, long row
} else {
long instanceId = c.getLong(c.getColumnIndex(DatabaseInstanceColumns._ID));
Intent intent = FormFillingIntentFactory.editInstanceIntent(this, currentProjectProvider.getCurrentProject().getUuid(), instanceId);
startActivity(intent);
formLauncher.launch(intent);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@
package org.odk.collect.android.dao.helpers;

import org.odk.collect.android.application.Collect;
import org.odk.collect.android.javarosawrapper.FormController;
import org.odk.collect.android.utilities.InstancesRepositoryProvider;
import org.odk.collect.forms.instances.Instance;
import org.odk.collect.forms.instances.InstancesRepository;

import timber.log.Timber;

/**
* Provides abstractions over database calls for instances.
*
Expand All @@ -34,34 +31,6 @@ private InstancesDaoHelper() {

}

/**
* Checks the database to determine if the current instance being edited has
* already been 'marked completed'. A form can be 'unmarked' complete and
* then resaved.
*
* @return true if form has been marked completed, false otherwise.
* <p>
* TODO: replace with method in {@link InstancesRepository}
* that returns an {@link Instance} object from a path.
*/
public static boolean isInstanceComplete(FormController formController) {
// default to false if we're mid form
boolean complete = false;

if (formController != null && formController.getInstanceFile() != null) {
// Then see if we've already marked this form as complete before
String path = formController.getInstanceFile().getAbsolutePath();
Instance instance = new InstancesRepositoryProvider(Collect.getInstance()).get().getOneByPath(path);
if (instance != null && instance.getStatus().equals(Instance.STATUS_COMPLETE)) {
complete = true;
}
} else {
Timber.w("FormController or its instanceFile field has a null value");
}

return complete;
}

// TODO: replace with method in {@link org.odk.collect.android.instances.InstancesRepository}
// that returns an {@link Instance} object from a path.
public static boolean isInstanceAvailable(String path) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import org.odk.collect.android.activities.FormFillingActivity
import org.odk.collect.android.analytics.AnalyticsEvents
import org.odk.collect.android.injection.DaggerUtils
import org.odk.collect.android.instancemanagement.InstanceDeleter
import org.odk.collect.android.instancemanagement.canBeEditedWithGracePeriod
import org.odk.collect.android.projects.CurrentProjectProvider
import org.odk.collect.android.utilities.ApplicationConstants
import org.odk.collect.android.utilities.ContentUriHelper
Expand All @@ -20,6 +19,7 @@ import org.odk.collect.forms.Form
import org.odk.collect.forms.instances.Instance
import org.odk.collect.projects.ProjectsRepository
import org.odk.collect.settings.SettingsProvider
import org.odk.collect.settings.keys.ProtectedProjectKeys
import org.odk.collect.strings.localization.LocalizedActivity
import java.io.File
import javax.inject.Inject
Expand Down Expand Up @@ -216,7 +216,7 @@ class FormUriActivity : LocalizedActivity() {

val formEditingEnabled = if (uriMimeType == InstancesContract.CONTENT_ITEM_TYPE) {
val instance = instanceRepositoryProvider.get().get(ContentUriHelper.getIdFromUri(uri))
instance!!.canBeEditedWithGracePeriod(settingsProvider)
instance!!.canBeEdited(settingsProvider)
} else {
true
}
Expand All @@ -230,7 +230,7 @@ class FormUriActivity : LocalizedActivity() {

return if (uriMimeType == InstancesContract.CONTENT_ITEM_TYPE) {
val instance = instanceRepositoryProvider.get().get(ContentUriHelper.getIdFromUri(uri))
instance!!.status == Instance.STATUS_COMPLETE && instance.canBeEditedWithGracePeriod(settingsProvider)
instance!!.status == Instance.STATUS_COMPLETE && instance.canBeEdited(settingsProvider)
} else {
false
}
Expand All @@ -245,3 +245,8 @@ class FormUriActivity : LocalizedActivity() {
private const val FORM_FILLING_ALREADY_STARTED = "FORM_FILLING_ALREADY_STARTED"
}
}

private fun Instance.canBeEdited(settingsProvider: SettingsProvider): Boolean {
return (this.status == Instance.STATUS_INCOMPLETE || this.status == Instance.STATUS_COMPLETE) &&
settingsProvider.getProtectedSettings().getBoolean(ProtectedProjectKeys.KEY_EDIT_SAVED)
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.odk.collect.android.formmanagement.InstancesAppState
import org.odk.collect.android.instancemanagement.InstanceDiskSynchronizer
import org.odk.collect.android.instancemanagement.autosend.AutoSendSettingsProvider
import org.odk.collect.android.instancemanagement.autosend.shouldFormBeSentAutomatically
import org.odk.collect.android.instancemanagement.canBeEdited
import org.odk.collect.android.preferences.utilities.FormUpdateMode
import org.odk.collect.android.preferences.utilities.SettingsUtils
import org.odk.collect.android.utilities.ContentUriHelper
Expand Down Expand Up @@ -122,7 +121,11 @@ class MainMenuViewModel(
return null
}

val action = if (instance.canBeEdited(settingsProvider)) {
val action = if (
instance.status == Instance.STATUS_INCOMPLETE &&
settingsProvider.getProtectedSettings()
.getBoolean(ProtectedProjectKeys.KEY_EDIT_SAVED)
) {
R.string.edit_form
} else {
if (instance.status == Instance.STATUS_INCOMPLETE || instance.canEditWhenComplete()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import org.mockito.kotlin.whenever
import org.odk.collect.android.R
import org.odk.collect.android.activities.FormFillingActivity
import org.odk.collect.android.injection.config.AppDependencyModule
import org.odk.collect.android.instancemanagement.OCTOBER_1st_2023_UTC
import org.odk.collect.android.projects.CurrentProjectProvider
import org.odk.collect.android.storage.StoragePathProvider
import org.odk.collect.android.support.CollectHelpers
Expand Down Expand Up @@ -375,7 +374,7 @@ class FormUriActivityTest {
}

@Test
fun `When attempting to edit a finalized form saved before October 1st 2023 UTC then display a warning and start form filling`() {
fun `When attempting to edit a finalized form display a warning and start form filling`() {
val project = Project.Saved("123", "First project", "A", "#cccccc")
projectsRepository.save(project)
whenever(currentProjectProvider.getCurrentProject()).thenReturn(project)
Expand All @@ -386,7 +385,6 @@ class FormUriActivityTest {
Instance.Builder()
.formId("1")
.formVersion("1")
.lastStatusChangeDate(OCTOBER_1st_2023_UTC - 1)
.instanceFilePath(TempFiles.createTempFile(TempFiles.createTempDir()).absolutePath)
.status(Instance.STATUS_COMPLETE)
.build()
Expand All @@ -400,31 +398,6 @@ class FormUriActivityTest {
assertStartSavedFormIntent(project.uuid, instance.dbId, true)
}

@Test
fun `When attempting to edit a finalized form saved after October 1st 2023 UTC then start form for view only`() {
val project = Project.Saved("123", "First project", "A", "#cccccc")
projectsRepository.save(project)
whenever(currentProjectProvider.getCurrentProject()).thenReturn(project)

formsRepository.save(
FormUtils.buildForm("1", "1", TempFiles.createTempDir().absolutePath).build()
)

val instance = instancesRepository.save(
Instance.Builder()
.formId("1")
.formVersion("1")
.lastStatusChangeDate(OCTOBER_1st_2023_UTC + 1)
.instanceFilePath(TempFiles.createTempFile(TempFiles.createTempDir()).absolutePath)
.status(Instance.STATUS_COMPLETE)
.build()
)

launcherRule.launchForResult<FormUriActivity>(getSavedIntent(project.uuid, instance.dbId))

assertStartSavedFormIntent(project.uuid, instance.dbId, false)
}

@Test
fun `When attempting to edit a submitted form then start form for view only`() {
val project = Project.Saved("123", "First project", "A", "#cccccc")
Expand Down
2 changes: 1 addition & 1 deletion strings/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1215,5 +1215,5 @@
<string name="view_form">View</string>
<string name="close_snackbar">Close snackbar</string>

<string name="edit_finalized_form_warning">After September 30th, you will not be able to edit finalized forms. Save forms as draft to edit them later.\n\nYou can check for errors in a draft form by tapping the three dots (⋮) and then Check for errors.</string>
<string name="edit_finalized_form_warning">In later releases, you will not be able to edit finalized forms. Save forms as draft to edit them later.\n\nYou can check for errors in a draft form by tapping the three dots (⋮) and then Check for errors.</string>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan this message. Any ideas for a better one @lognaturel?

</resources>