Correct property update behaviour

getodk · Apr 24, 2024 · f2acf4e · f2acf4e
1 parent f252141
commit f2acf4e
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 16 deletions.
diff --git a/entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt b/entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt
@@ -27,15 +27,6 @@ object LocalEntityUseCases {
 
             val existing = entitiesRepository.get(dataset, id)
             if (existing == null || existing.version < version) {
-                val entity = Entity(
-                    dataset,
-                    id,
-                    label,
-                    version
-                )
-
-                entitiesRepository.save(entity)
-            } else {
                 val properties = 0.until(item.numChildren)
                     .fold(emptyList<Pair<String, String>>()) { properties, index ->
                         val child = item.getChildAt(index)
@@ -52,7 +43,7 @@ object LocalEntityUseCases {
                         }
                     }
 
-                val entity = existing.copy(properties = properties)
+                val entity = Entity(dataset, id, label, version, properties)
                 entitiesRepository.save(entity)
             }
         }

diff --git a/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt b/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt
@@ -44,16 +44,30 @@ class LocalEntityUseCasesTest {
     }
 
     @Test
-    fun `updateLocalEntities adds properties not in local version from older list version`() {
-        entitiesRepository.save(Entity("songs", "noah", "Noah", 2))
+    fun `updateLocalEntities ignores properties not in local version from older list version`() {
+        entitiesRepository.save(Entity("songs", "noah", "Noah", 3))
+        val csv =
+            createEntityList(Entity("songs", "noah", "Noah", 2, listOf(Pair("length", "6:38"))))
+
+        LocalEntityUseCases.updateLocalEntities("songs", csv, entitiesRepository)
+        val songs = entitiesRepository.getEntities("songs")
+        assertThat(
+            songs,
+            containsInAnyOrder(Entity("songs", "noah", "Noah", 3))
+        )
+    }
+
+    @Test
+    fun `updateLocalEntities overrides properties in local version from newer list version`() {
+        entitiesRepository.save(Entity("songs", "noah", "Noah", 1, listOf(Pair("length", "6:38"))))
         val csv =
-            createEntityList(Entity("songs", "noah", "Noa", 1, listOf(Pair("length", "6:38"))))
+            createEntityList(Entity("songs", "noah", "Noah", 2, listOf(Pair("length", "4:58"))))
 
         LocalEntityUseCases.updateLocalEntities("songs", csv, entitiesRepository)
         val songs = entitiesRepository.getEntities("songs")
         assertThat(
             songs,
-            containsInAnyOrder(Entity("songs", "noah", "Noah", 2, listOf(Pair("length", "6:38"))))
+            containsInAnyOrder(Entity("songs", "noah", "Noah", 2, listOf(Pair("length", "4:58"))))
         )
     }
 
@@ -95,13 +109,13 @@ class LocalEntityUseCasesTest {
 
     @Test
     fun `updateLocalEntities adds list entity when its label is blank`() {
-        val csv = createEntityList(Entity("songs", "cathedrals", ""))
+        val csv = createEntityList(Entity("songs", "cathedrals", label = ""))
 
         LocalEntityUseCases.updateLocalEntities("songs", csv, entitiesRepository)
         val songs = entitiesRepository.getEntities("songs")
         assertThat(
             songs,
-            containsInAnyOrder(Entity("songs", "cathedrals", ""))
+            containsInAnyOrder(Entity("songs", "cathedrals", label = ""))
         )
     }