Skip to content

Commit

Permalink
Merge pull request #6440 from grzesiek2010/COLLECT-6232
Browse files Browse the repository at this point in the history
Prevent opening forms that use entities during form updates, and block form updates while filling out a form that uses entities
  • Loading branch information
grzesiek2010 authored Oct 28, 2024
2 parents 15f6845 + cc46949 commit 2e63b33
Show file tree
Hide file tree
Showing 41 changed files with 819 additions and 350 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,14 @@ class EntityFormTest {
.assertTextInDialog(R.string.unrecognized_entity_version, "2020.1.0")
.clickOKOnDialog(MainMenuPage())
}

@Test
fun closingEntityForm_releasesTheLockAndLetsOtherEntityFormsToBeStarted() {
rule.startAtFirstLaunch()
.clickTryCollect()
.copyForm("one-question-entity-registration.xml")
.startBlankForm("One Question Entity Registration")
.pressBackAndDiscardForm()
.startBlankForm("One Question Entity Registration")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,45 @@ class MatchExactlyTest {

assertThat(testDependencies.scheduler.getDeferredTasks(), `is`(empty()))
}

@Test
fun whenMatchExactlyEnabled_getsLatestFormsFromServerDuringFillingAFormWithoutEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.startAtMainMenu()
.copyForm("one-question.xml")
.setServer(testDependencies.server.url)
.enableMatchExactly()
.startBlankForm("One Question")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormExists("One Question Updated")
}

@Test
fun whenMatchExactlyEnabled_doesNotGetLatestFormsFromServerDuringFillingAFormWithEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.startAtMainMenu()
.copyForm("one-question-entity-registration.xml")
.copyForm("one-question.xml")
.setServer(testDependencies.server.url)
.enableMatchExactly()
.startBlankForm("One Question Entity Registration")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormDoesNotExist("One Question Updated")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,40 @@ class PreviouslyDownloadedOnlyTest {

assertThat(testDependencies.scheduler.getDeferredTasks(), equalTo(emptyList()))
}

@Test
fun whenPreviouslyDownloadedOnlyEnabledWithAutomaticDownload_getsLatestFormsFromServerDuringFillingAFormWithoutEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.withProject(testDependencies.server, "one-question.xml")
.enablePreviouslyDownloadedOnlyUpdatesWithAutomaticDownload()
.startBlankForm("One Question")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormExists("One Question Updated")
}

@Test
fun whenPreviouslyDownloadedOnlyEnabledWithAutomaticDownload_doesNotGetLatestFormsFromServerDuringFillingAFormWithEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.withProject(testDependencies.server, "one-question-entity-registration.xml", "one-question.xml")
.enablePreviouslyDownloadedOnlyUpdatesWithAutomaticDownload()
.startBlankForm("One Question Entity Registration")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormDoesNotExist("One Question Updated")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.odk.collect.android.instancemanagement.InstancesDataService
import org.odk.collect.android.instancemanagement.autosend.AutoSendSettingsProvider
import org.odk.collect.android.projects.ProjectsDataService
import org.odk.collect.android.utilities.ApplicationConstants
import org.odk.collect.android.utilities.ChangeLockProvider
import org.odk.collect.android.utilities.FormsRepositoryProvider
import org.odk.collect.android.utilities.InstancesRepositoryProvider
import org.odk.collect.android.utilities.MediaUtils
Expand Down Expand Up @@ -57,7 +58,8 @@ class FormEntryViewModelFactory(
private val savepointsRepositoryProvider: SavepointsRepositoryProvider,
private val qrCodeCreator: QRCodeCreator,
private val htmlPrinter: HtmlPrinter,
private val instancesDataService: InstancesDataService
private val instancesDataService: InstancesDataService,
private val changeLockProvider: ChangeLockProvider
) : AbstractSavedStateViewModelFactory(owner, null) {

override fun <T : ViewModel> create(
Expand All @@ -73,7 +75,8 @@ class FormEntryViewModelFactory(
scheduler,
formSessionRepository,
sessionId,
formsRepositoryProvider.create(projectId)
formsRepositoryProvider.create(projectId),
changeLockProvider.create(projectId)
)

FormSaveViewModel::class.java -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
import org.odk.collect.android.formhierarchy.FormHierarchyActivity;
import org.odk.collect.android.formhierarchy.ViewOnlyFormHierarchyActivity;
import org.odk.collect.android.fragments.MediaLoadingFragment;
import org.odk.collect.android.utilities.ChangeLockProvider;
import org.odk.collect.android.widgets.datetime.pickers.CustomDatePickerDialog;
import org.odk.collect.android.widgets.datetime.pickers.CustomTimePickerDialog;
import org.odk.collect.android.fragments.dialogs.LocationProvidersDisabledDialog;
Expand Down Expand Up @@ -366,6 +367,9 @@ public void allowSwiping(boolean doSwipe) {
@Inject
public SavepointsRepositoryProvider savepointsRepositoryProvider;

@Inject
public ChangeLockProvider changeLockProvider;

private final LocationProvidersReceiver locationProvidersReceiver = new LocationProvidersReceiver();

private SwipeHandler swipeHandler;
Expand Down Expand Up @@ -434,7 +438,8 @@ public void onCreate(Bundle savedInstanceState) {
savepointsRepositoryProvider,
new QRCodeCreatorImpl(),
new HtmlPrinter(),
instancesDataService
instancesDataService,
changeLockProvider
);

this.getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public final class DatabaseConstants {
public static final String FORMS_DATABASE_NAME = "forms.db";
public static final String FORMS_TABLE_NAME = "forms";
// Please always test upgrades manually when you change this value
public static final int FORMS_DATABASE_VERSION = 13;
public static final int FORMS_DATABASE_VERSION = 14;

public static final String INSTANCES_DATABASE_NAME = "instances.db";
public static final String INSTANCES_TABLE_NAME = "instances";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,46 +35,10 @@ object DatabaseObjectMapper {
values.put(DatabaseFormColumns.AUTO_DELETE, form.autoDelete)
values.put(DatabaseFormColumns.GEOMETRY_XPATH, form.geometryXpath)
values.put(DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE, form.lastDetectedAttachmentsUpdateDate)
values.put(DatabaseFormColumns.USES_ENTITIES, Boolean.toString(form.usesEntities()))
return values
}

@JvmStatic
fun getFormFromValues(values: ContentValues, formsPath: String, cachePath: String): Form {
val formFilePath = getAbsoluteFilePath(
formsPath,
values.getAsString(DatabaseFormColumns.FORM_FILE_PATH)
)

val cacheFilePath = values.getAsString(DatabaseFormColumns.JRCACHE_FILE_PATH)?.let {
getAbsoluteFilePath(cachePath, it)
}

val mediaPath = values.getAsString(DatabaseFormColumns.FORM_MEDIA_PATH)?.let {
getAbsoluteFilePath(formsPath, it)
}

return Form.Builder()
.dbId(values.getAsLong(BaseColumns._ID))
.displayName(values.getAsString(DatabaseFormColumns.DISPLAY_NAME))
.description(values.getAsString(DatabaseFormColumns.DESCRIPTION))
.formId(values.getAsString(DatabaseFormColumns.JR_FORM_ID))
.version(values.getAsString(DatabaseFormColumns.JR_VERSION))
.formFilePath(formFilePath)
.submissionUri(values.getAsString(DatabaseFormColumns.SUBMISSION_URI))
.base64RSAPublicKey(values.getAsString(DatabaseFormColumns.BASE64_RSA_PUBLIC_KEY))
.md5Hash(values.getAsString(DatabaseFormColumns.MD5_HASH))
.date(values.getAsLong(DatabaseFormColumns.DATE))
.jrCacheFilePath(cacheFilePath)
.formMediaPath(mediaPath)
.language(values.getAsString(DatabaseFormColumns.LANGUAGE))
.autoSend(values.getAsString(DatabaseFormColumns.AUTO_SEND))
.autoDelete(values.getAsString(DatabaseFormColumns.AUTO_DELETE))
.geometryXpath(values.getAsString(DatabaseFormColumns.GEOMETRY_XPATH))
.deleted(values.getAsLong(DatabaseFormColumns.DELETED_DATE) != null)
.lastDetectedAttachmentsUpdateDate(values.getAsLong(DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE))
.build()
}

@JvmStatic
fun getFormFromCurrentCursorPosition(
cursor: Cursor,
Expand All @@ -101,6 +65,7 @@ object DatabaseObjectMapper {
val geometryXpathColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.GEOMETRY_XPATH)
val deletedDateColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.DELETED_DATE)
val lastDetectedAttachmentsUpdateDateColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE)
val usesEntitiesColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.USES_ENTITIES)
return Form.Builder()
.dbId(cursor.getLong(idColumnIndex))
.displayName(cursor.getString(displayNameColumnIndex))
Expand Down Expand Up @@ -135,6 +100,7 @@ object DatabaseObjectMapper {
.geometryXpath(cursor.getString(geometryXpathColumnIndex))
.deleted(!cursor.isNull(deletedDateColumnIndex))
.lastDetectedAttachmentsUpdateDate(if (cursor.isNull(lastDetectedAttachmentsUpdateDateColumnIndex)) null else cursor.getLong(lastDetectedAttachmentsUpdateDateColumnIndex))
.usesEntities(Boolean.valueOf(cursor.getString(usesEntitiesColumnIndex)))
.build()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ object DatabaseFormColumns : BaseColumns {
const val SUBMISSION_URI = "submissionUri" // can be null
const val BASE64_RSA_PUBLIC_KEY = "base64RsaPublicKey" // can be null
const val AUTO_DELETE = "autoDelete" // can be null
const val USES_ENTITIES = "usesEntities" // can be null

// Column is called autoSubmit for legacy support but the attribute is auto-send
const val AUTO_SEND = "autoSubmit" // can be null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,24 @@
import static org.odk.collect.android.database.DatabaseConstants.FORMS_TABLE_NAME;
import static org.odk.collect.android.database.DatabaseObjectMapper.getFormFromCurrentCursorPosition;
import static org.odk.collect.android.database.DatabaseObjectMapper.getValuesFromForm;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.AUTO_DELETE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.AUTO_SEND;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.BASE64_RSA_PUBLIC_KEY;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DATE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DELETED_DATE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DESCRIPTION;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DISPLAY_NAME;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.USES_ENTITIES;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.FORM_FILE_PATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.FORM_MEDIA_PATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.GEOMETRY_XPATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.JRCACHE_FILE_PATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.JR_FORM_ID;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.JR_VERSION;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.LANGUAGE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.MD5_HASH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.SUBMISSION_URI;
import static org.odk.collect.shared.PathUtils.getRelativeFilePath;

import android.content.ContentValues;
Expand Down Expand Up @@ -230,6 +240,37 @@ private Cursor queryAndReturnCursor(Map<String, String> projectionMap, String[]
SQLiteQueryBuilder qb = new SQLiteQueryBuilder();
qb.setTables(FORMS_TABLE_NAME);

if (projection == null) {
/*
For some reason passing null as the projection doesn't always give us all the
columns so we hardcode them here so it's explicit that we need these all back.
The problem can occur, for example, when a new column is added to a database and the
database needs to be updated. After the upgrade, the new column might not be returned,
even though it already exists.
*/
projection = new String[]{
_ID,
DISPLAY_NAME,
DESCRIPTION,
JR_FORM_ID,
JR_VERSION,
MD5_HASH,
DATE,
FORM_MEDIA_PATH,
FORM_FILE_PATH,
LANGUAGE,
SUBMISSION_URI,
BASE64_RSA_PUBLIC_KEY,
JRCACHE_FILE_PATH,
AUTO_SEND,
AUTO_DELETE,
GEOMETRY_XPATH,
DELETED_DATE,
LAST_DETECTED_ATTACHMENTS_UPDATE_DATE,
USES_ENTITIES
};
}

if (projectionMap != null) {
qb.setProjectionMap(projectionMap);
}
Expand Down
Loading

0 comments on commit 2e63b33

Please sign in to comment.