fix: changing profile avatar doesn't reflect update (WPB-5494) (#2444)

* fix: changing profile avatar doesn't reflect update (WPB-5494) (#2443) * fix: remove SELECT changes from updateUser query * fix: add transactionWithResult when updateUser and return select changes * test: add tests for observing user details by id * fix: add UPDATE OR FAIL to update query and remove select changes query as it is not used anymore * fix: remove unused select changes query and change return type of updateUser to Unit * fix: adjust existing tests --------- Co-authored-by: Alexandre Ferris <[email protected]>
wireapp · Feb 5, 2024 · 5e25744 · 5e25744
1 parent f11edf5
commit 5e25744
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 35 deletions.
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt
@@ -50,6 +50,8 @@ import com.wire.kalium.logic.functional.foldToEitherWhileRight
 import com.wire.kalium.logic.functional.getOrNull
 import com.wire.kalium.logic.functional.map
 import com.wire.kalium.logic.functional.mapRight
+import com.wire.kalium.logic.functional.onFailure
+import com.wire.kalium.logic.functional.onSuccess
 import com.wire.kalium.logic.kaliumLogger
 import com.wire.kalium.logic.wrapApiRequest
 import com.wire.kalium.logic.wrapStorageRequest
@@ -500,12 +502,10 @@ internal class UserDataSource internal constructor(
 
     override suspend fun updateUserFromEvent(event: Event.User.Update): Either<CoreFailure, Unit> = wrapStorageRequest {
         userDAO.updateUser(userMapper.fromUserUpdateEventToPartialUserEntity(event))
-    }.flatMap { updated ->
-        if (!updated) {
-            Either.Left(StorageFailure.DataNotFound)
-        } else {
-            Either.Right(Unit)
-        }
+    }.onFailure {
+        Either.Left(StorageFailure.DataNotFound)
+    }.onSuccess {
+        Either.Right(Unit)
     }
 
     override suspend fun markUserAsDeletedAndRemoveFromGroupConversations(

diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt
@@ -134,7 +134,7 @@ class UserRepositoryTest {
     @Test
     fun givenAUserEvent_whenPersistingTheUser_thenShouldSucceed() = runTest {
         val (arrangement, userRepository) = Arrangement()
-            .withUpdateUserReturning(true)
+            .withUpdateUserReturning()
             .arrange()
 
         val result = userRepository.updateUserFromEvent(TestEvent.updateUser(userId = SELF_USER.id))
@@ -148,23 +148,6 @@ class UserRepositoryTest {
         }
     }
 
-    @Test
-    fun givenAUserEvent_whenPersistingTheUserAndNotExists_thenShouldFail() = runTest {
-        val (arrangement, userRepository) = Arrangement()
-            .withUpdateUserReturning(false)
-            .arrange()
-
-        val result = userRepository.updateUserFromEvent(TestEvent.updateUser(userId = SELF_USER.id))
-
-        with(result) {
-            shouldFail()
-            verify(arrangement.userDAO)
-                .suspendFunction(arrangement.userDAO::updateUser, KFunction1<PartialUserEntity>())
-                .with(any())
-                .wasInvoked(exactly = once)
-        }
-    }
-
     @Test
     fun givenAnEmptyUserIdList_whenFetchingUsers_thenShouldNotFetchFromApiAndSucceed() = runTest {
         // given
@@ -770,11 +753,11 @@ class UserRepositoryTest {
                 .thenReturn(flowOf(userEntities))
         }
 
-        fun withUpdateUserReturning(updated: Boolean) = apply {
+        fun withUpdateUserReturning() = apply {
             given(userDAO)
                 .suspendFunction(userDAO::updateUser, KFunction1<PartialUserEntity>())
                 .whenInvokedWith(any())
-                .thenReturn(updated)
+                .thenReturn(Unit)
         }
 
         fun withSuccessfulGetUsersInfo(result: UserProfileDTO = TestUser.USER_PROFILE_DTO) = apply {

diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq
@@ -60,8 +60,8 @@ insertOrIgnoreUser:
 INSERT OR IGNORE INTO User(qualified_id, name, handle, email, phone, accent_id, team, connection_status, preview_asset_id, complete_asset_id, user_type, bot_service, deleted, incomplete_metadata, expires_at, supported_protocols)
 VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);
 
-updateUser {
-UPDATE User
+updateUser:
+UPDATE OR FAIL User
 SET
 name = coalesce(:name, name),
 handle = coalesce(:handle, handle),
@@ -71,8 +71,6 @@ preview_asset_id = :preview_asset_id, preview_asset_id = coalesce(:preview_asset
 complete_asset_id = :complete_asset_id, complete_asset_id = coalesce(:complete_asset_id, complete_asset_id),
 supported_protocols = :supported_protocols, supported_protocols = coalesce(:supported_protocols, supported_protocols)
 WHERE qualified_id = ?;
-SELECT changes();
-}
 
 updatePartialUserInformation:
 UPDATE User
@@ -199,9 +197,6 @@ UPDATE User SET handle = ? WHERE qualified_id = ?;
 updateUserAsset:
 UPDATE User SET complete_asset_id = ?, preview_asset_id = ? WHERE complete_asset_id = ?;
 
-selectChanges:
-SELECT changes();
-
 updateUserAvailabilityStatus:
 UPDATE User SET user_availability_status = ? WHERE qualified_id = ?;
 

diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt
@@ -214,7 +214,7 @@ interface UserDAO {
      *
      * @return true if the user was updated
      */
-    suspend fun updateUser(update: PartialUserEntity): Boolean
+    suspend fun updateUser(update: PartialUserEntity)
 
     suspend fun updateUser(users: List<PartialUserEntity>)
 

diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt
@@ -202,7 +202,7 @@ class UserDAOImpl internal constructor(
             complete_asset_id = update.completeAssetId,
             supported_protocols = update.supportedProtocols,
             update.id
-        ).executeAsOne() > 0
+        )
     }
 
     override suspend fun updateUser(users: List<PartialUserEntity>) = withContext(queriesContext) {

diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/UserDAOTest.kt
@@ -56,6 +56,42 @@ class UserDAOTest : BaseDatabaseTest() {
         db = createDatabase(selfUserId, encryptedDBSecret, true)
     }
 
+    @Test
+    fun givenUser_whenUpdatingProfileAvatar_thenChangesAreEmittedCorrectly() = runTest(dispatcher) {
+        //given
+        val updatedUser = PartialUserEntity(
+            id = user1.id,
+            name = "newName",
+            handle = user1.handle,
+            email = user1.email,
+            accentId = user1.accentId,
+            previewAssetId = UserAssetIdEntity(
+                value ="newAvatar",
+                domain = "newAvatarDomain"
+            ),
+            completeAssetId = UserAssetIdEntity(
+                value ="newAvatar",
+                domain = "newAvatarDomain"
+            ),
+            supportedProtocols = user1.supportedProtocols
+        )
+
+        db.userDAO.upsertUser(user1)
+
+        db.userDAO.observeUserDetailsByQualifiedID(user1.id).test {
+            assertEquals(user1, (awaitItem() as UserDetailsEntity).toSimpleEntity())
+
+            // when
+            db.userDAO.updateUser(updatedUser)
+
+            // then
+            val newItem = (awaitItem() as UserDetailsEntity)
+            assertEquals(updatedUser.previewAssetId, newItem.previewAssetId)
+            assertEquals(updatedUser.completeAssetId, newItem.completeAssetId)
+            cancelAndIgnoreRemainingEvents()
+        }
+    }
+
     @Test
     fun givenUser_ThenUserCanBeInserted() = runTest(dispatcher) {
         db.userDAO.upsertUser(user1)