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

Use media file type to determine entity lists #6249

Merged
merged 9 commits into from
Jul 11, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ class ViewEntitiesTest {
@Test
fun canViewLocallyCreatedEntitiesInBrowser() {
testDependencies.server.addForm("one-question-entity-registration.xml")
testDependencies.server.addForm(
"one-question-entity-follow-up.xml",
listOf(StubOpenRosaServer.EntityListItem("people.csv"))
)

rule.withMatchExactlyProject(testDependencies.server.url)
.addEntityListInBrowser("people")
.startBlankForm("One Question Entity Registration")
.fillOutAndFinalize(FormEntryPage.QuestionAndAnswer("Name", "Logan Roy"))
.openEntityBrowser()
Expand All @@ -39,7 +42,6 @@ class ViewEntitiesTest {
)

rule.withMatchExactlyProject(testDependencies.server.url)
.addEntityListInBrowser("people")
.refreshForms()
.openEntityBrowser()
.clickOnList("people")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.junit.Test
import org.junit.rules.RuleChain
import org.junit.runner.RunWith
import org.odk.collect.android.support.StubOpenRosaServer.EntityListItem
import org.odk.collect.android.support.StubOpenRosaServer.MediaFileItem
import org.odk.collect.android.support.TestDependencies
import org.odk.collect.android.support.pages.FormEntryPage
import org.odk.collect.android.support.pages.MainMenuPage
Expand All @@ -27,7 +28,7 @@ class EntityFormTest {
testDependencies.server.addForm("one-question-entity-registration.xml")
testDependencies.server.addForm(
"one-question-entity-update.xml",
listOf(EntityListItem("people.csv"))
listOf(MediaFileItem("people.csv"))
)

rule.withMatchExactlyProject(testDependencies.server.url)
Expand All @@ -51,7 +52,7 @@ class EntityFormTest {
)

rule.withMatchExactlyProject(testDependencies.server.url)
.setupEntities("people")
.enableLocalEntitiesInForms()

.startBlankForm("One Question Entity Registration")
.fillOutAndFinalize(FormEntryPage.QuestionAndAnswer("Name", "Logan Roy"))
Expand All @@ -71,7 +72,7 @@ class EntityFormTest {
)

rule.withMatchExactlyProject(testDependencies.server.url)
.setupEntities("people")
.enableLocalEntitiesInForms()

.startBlankForm("One Question Entity Update") // Open to create cached form def
.pressBackAndDiscardForm()
Expand All @@ -93,7 +94,7 @@ class EntityFormTest {
)

rule.withMatchExactlyProject(testDependencies.server.url)
.setupEntities("people")
.enableLocalEntitiesInForms()

.startBlankForm("One Question Entity Update")
.assertQuestion("Select person")
Expand Down Expand Up @@ -124,7 +125,6 @@ class EntityFormTest {

rule.withProject(testDependencies.server)
.enableLocalEntitiesInForms()
.addEntityListInBrowser("people")

.clickGetBlankForm()
.clickClearAll()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.test.rule.GrantPermissionRule
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.odk.collect.android.support.StubOpenRosaServer.EntityListItem
import org.odk.collect.android.support.TestDependencies
import org.odk.collect.android.support.pages.EntitiesPage
import org.odk.collect.android.support.pages.ExperimentalPage
Expand Down Expand Up @@ -47,7 +48,16 @@ class SwitchProjectTest {

@Test
fun switchingProject_switchesSettingsFormsInstancesAndEntities() {
testDependencies.server.addForm("One Question Entity Registration", "one-question-entity", "1", "one-question-entity-registration.xml")
testDependencies.server.addForm(
"One Question Entity Registration",
"one-question-entity",
"1",
"one-question-entity-registration.xml"
)
testDependencies.server.addForm(
"one-question-entity-update.xml",
listOf(EntityListItem("people.csv"))
)

rule.startAtMainMenu()
// Copy and fill form
Expand Down Expand Up @@ -76,7 +86,6 @@ class SwitchProjectTest {
.clickOKOnDialog(MainMenuPage())

// Fill form
.addEntityListInBrowser("people")
.startBlankForm("One Question Entity Registration")
.fillOutAndFinalize(FormEntryPage.QuestionAndAnswer("Name", "Alice"))
.clickSendFinalizedForm(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,15 @@ private InputStream getManifestResponse(@NonNull URI uri) throws IOException {
mediaFileHash = Md5.getMd5Hash(getResourceAsStream("media/" + mediaFile.getFile()));
}

if (mediaFile instanceof EntityListItem) {
stringBuilder
.append("<mediaFile type=\"entityList\">");
} else {
stringBuilder
.append("<mediaFile>");
}

stringBuilder
.append("<mediaFile>")
.append("<filename>" + mediaFile.getName() + "</filename>\n");

if (noHashPrefixInMediaFiles) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.odk.collect.android.support.StorageUtils;
import org.odk.collect.android.support.TestScheduler;
import org.odk.collect.testshared.WaitFor;
import org.odk.collect.strings.R.string;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -288,30 +287,12 @@ public EntitiesPage openEntityBrowser() {
return new EntitiesPage().assertOnPage();
}

public MainMenuPage addEntityListInBrowser(String entityList) {
return openEntityBrowser()
.clickOptionsIcon(string.add_entity_list)
.clickOnTextInPopup(string.add_entity_list)
.inputText(entityList)
.clickOnTextInDialog(string.add)
.assertText(entityList)
.pressBack(new ExperimentalPage())
.pressBack(new ProjectSettingsPage())
.pressBack(new MainMenuPage());
}

public MainMenuPage refreshForms() {
return clickFillBlankForm()
.clickRefresh()
.pressBack(new MainMenuPage());
}

public MainMenuPage setupEntities(String entityList) {
return enableLocalEntitiesInForms()
.addEntityListInBrowser(entityList)
.refreshForms();
}

@NotNull
public MainMenuPage enableLocalEntitiesInForms() {
return openProjectSettingsDialog()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ object ServerFormUseCases {
}
}

val dataset = mediaFile.filename.substringBefore(".csv")
if (entitiesRepository.getLists().contains(dataset)) {
if (mediaFile.isEntityList) {
val dataset = mediaFile.filename.substringBefore(".csv")
LocalEntityUseCases.updateLocalEntitiesFromServer(dataset, tempMediaFile, entitiesRepository)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ class OpenRosaResponseParserImpl : OpenRosaResponseParser {
continue
}

val type = mediaFileElement.getAttributeValue(null, "type")

val name = mediaFileElement.name
if (name.equals("mediaFile", ignoreCase = true)) {
var filename: String? = null
Expand Down Expand Up @@ -201,7 +203,7 @@ class OpenRosaResponseParserImpl : OpenRosaResponseParser {
return null
}

files.add(MediaFile(filename, hash, downloadUrl))
files.add(MediaFile(filename, hash, downloadUrl, type == "entityList"))
}
}
return files
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,54 @@ class OpenRosaResponseParserImplTest {
val formList = OpenRosaResponseParserImpl().parseManifest(Document())
assertThat(formList, equalTo(null))
}

@Test
fun `parseManifest() when media file has type entityList returns isEntityList as true`() {
Copy link
Member

Choose a reason for hiding this comment

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

We should have a test to check if it returns false otherwise.

val response = StringBuilder()
.appendLine("<?xml version='1.0' encoding='UTF-8' ?>")
.appendLine("<manifest xmlns=\"http://openrosa.org/xforms/xformsManifest\">")
.appendLine("<mediaFile type=\"entityList\">")
.appendLine("<filename>badgers.csv</filename>")
.appendLine("<hash>blah</hash>")
.appendLine("<downloadUrl>http://funk.appspot.com/binaryData?blobKey=%3A477e3</downloadUrl>")
.appendLine("</mediaFile>")
.appendLine("</manifest>")
.toString()

val doc = StringReader(response).use { reader ->
val parser = KXmlParser()
parser.setInput(reader)
parser.setFeature(XmlPullParser.FEATURE_PROCESS_NAMESPACES, true)
Document().also { it.parse(parser) }
}

val mediaFiles = OpenRosaResponseParserImpl().parseManifest(doc)!!
assertThat(mediaFiles.size, equalTo(1))
assertThat(mediaFiles[0].isEntityList, equalTo(true))
}

@Test
fun `parseManifest() when media file does not have type returns isEntityList as false`() {
val response = StringBuilder()
.appendLine("<?xml version='1.0' encoding='UTF-8' ?>")
.appendLine("<manifest xmlns=\"http://openrosa.org/xforms/xformsManifest\">")
.appendLine("<mediaFile>")
.appendLine("<filename>badgers.csv</filename>")
.appendLine("<hash>blah</hash>")
.appendLine("<downloadUrl>http://funk.appspot.com/binaryData?blobKey=%3A477e3</downloadUrl>")
.appendLine("</mediaFile>")
.appendLine("</manifest>")
.toString()

val doc = StringReader(response).use { reader ->
val parser = KXmlParser()
parser.setInput(reader)
parser.setFeature(XmlPullParser.FEATURE_PROCESS_NAMESPACES, true)
Document().also { it.parse(parser) }
}

val mediaFiles = OpenRosaResponseParserImpl().parseManifest(doc)!!
assertThat(mediaFiles.size, equalTo(1))
assertThat(mediaFiles[0].isEntityList, equalTo(false))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,4 @@ class EntitiesViewModel(
_lists.postValue(entitiesRepository.getLists().toList())
}
}

fun addEntityList(name: String) {
scheduler.immediate {
entitiesRepository.addList(name)
_lists.postValue(entitiesRepository.getLists().toList())
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.odk.collect.entities.browser

import android.content.Context
import android.os.Bundle
import android.view.LayoutInflater
import android.view.Menu
Expand All @@ -18,9 +17,7 @@ import androidx.navigation.fragment.findNavController
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import androidx.recyclerview.widget.RecyclerView.ViewHolder
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import org.odk.collect.entities.R
import org.odk.collect.entities.databinding.AddEntitiesDialogLayoutBinding
import org.odk.collect.entities.databinding.EntityListItemLayoutBinding
import org.odk.collect.entities.databinding.ListLayoutBinding
import org.odk.collect.lists.RecyclerViewUtils
Expand Down Expand Up @@ -50,15 +47,14 @@ class EntityListsFragment(
}

menuHost().addMenuProvider(
ListsMenuProvider(entitiesViewModel, requireContext()),
ListsMenuProvider(entitiesViewModel),
viewLifecycleOwner
)
}
}

private class ListsMenuProvider(
private val entitiesViewModel: EntitiesViewModel,
private val context: Context
private val entitiesViewModel: EntitiesViewModel
) : MenuProvider {
override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) {
menuInflater.inflate(R.menu.entity_lists, menu)
Expand All @@ -71,18 +67,6 @@ private class ListsMenuProvider(
true
}

R.id.add_entity_list -> {
val binding = AddEntitiesDialogLayoutBinding.inflate(LayoutInflater.from(context))
MaterialAlertDialogBuilder(context)
.setView(binding.root)
.setTitle(org.odk.collect.strings.R.string.add_entity_list)
.setPositiveButton(org.odk.collect.strings.R.string.add) { _, _ ->
entitiesViewModel.addEntityList(binding.entityListName.text.toString())
}
.show()
true
}

else -> false
}
}
Expand Down
24 changes: 0 additions & 24 deletions entities/src/main/res/layout/add_entities_dialog_layout.xml

This file was deleted.

5 changes: 0 additions & 5 deletions entities/src/main/res/menu/entity_lists.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,4 @@
android:id="@+id/clear_entities"
android:title="@string/clear_entities"
app:showAsAction="never" />

<item
android:id="@+id/add_entity_list"
android:title="@string/add_entity_list"
app:showAsAction="never" />
</menu>
5 changes: 3 additions & 2 deletions forms/src/main/java/org/odk/collect/forms/MediaFile.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ package org.odk.collect.forms

import java.io.Serializable

data class MediaFile(
data class MediaFile @JvmOverloads constructor(
val filename: String,
val hash: String,
val downloadUrl: String
val downloadUrl: String,
val isEntityList: Boolean = false
) : Serializable
3 changes: 1 addition & 2 deletions strings/src/main/res/values-de/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -935,8 +935,7 @@
<!--Text for button that deletes all entities for the project-->
<string name="clear_entities">Alle löschen</string>
<!--Text for button that adds a new entity list-->
<string name="add_entity_list">Objektliste hinzufügen</string>
<!--Label for entity that has been created offline and is not on the server yet-->
<!--Label for entity that has been created offline and is not on the server yet-->
<string name="offline">Offline</string>
<!--##############################################
# Crash handling
Expand Down
3 changes: 1 addition & 2 deletions strings/src/main/res/values-fi/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -941,8 +941,7 @@
<!--Text for button that deletes all entities for the project-->
<string name="clear_entities">Poista kaikki</string>
<!--Text for button that adds a new entity list-->
Copy link
Member

Choose a reason for hiding this comment

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

You should also get rid of the explanation text above.

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've removed it for the string in values, but won't touch it for the translated files as that should happen automatically for when new translations are added.

<string name="add_entity_list">Lisää entiteettilista</string>
<!--Label for entity that has been created offline and is not on the server yet-->
<!--Label for entity that has been created offline and is not on the server yet-->
<string name="offline">Offline</string>
<!--##############################################
# Crash handling
Expand Down
3 changes: 1 addition & 2 deletions strings/src/main/res/values-fr/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,7 @@
<!--Text for button that deletes all entities for the project-->
<string name="clear_entities">Tout supprimer</string>
<!--Text for button that adds a new entity list-->
<string name="add_entity_list">Ajouter liste d\'entités</string>
<!--Label for entity that has been created offline and is not on the server yet-->
<!--Label for entity that has been created offline and is not on the server yet-->
<string name="offline">Hors ligne</string>
<!--##############################################
# Crash handling
Expand Down
Loading