From 1cc9f977025d603944aae767b5ddcb4d668b20b1 Mon Sep 17 00:00:00 2001
From: Callum Stott <callum@seadowg.com>
Date: Wed, 24 Apr 2024 18:04:21 +0100
Subject: [PATCH] Correct property update behaviour

---
 .../collect/entities/LocalEntityUseCases.kt   | 11 +-------
 .../entities/LocalEntityUseCasesTest.kt       | 26 ++++++++++++++-----
 2 files changed, 21 insertions(+), 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
index 8f772b8703d..3894b8b9899 100644
--- 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
index 2730a53e73c..016e9f4ed80 100644
--- 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 = ""))
         )
     }