fix: Duplicated system message about degraded conversation (WPB-5878) (…

…#2310) * fix: Duplicated system message about degraded conversation * Updated tests --------- Co-authored-by: Mojtaba Chenani <[email protected]>
wireapp · Dec 15, 2023 · 4ace8cb · 4ace8cb
1 parent fecddaa
commit 4ace8cb
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 8 deletions.
diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Members.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Members.sq
@@ -16,6 +16,12 @@ insertMember:
 INSERT OR IGNORE INTO Member(user, conversation, role)
 VALUES (?, ?, ?);
 
+insertOrUpdateMember:
+INSERT INTO Member(user, conversation, role)
+VALUES (?, ?, ?)
+ON CONFLICT(user, conversation) DO UPDATE SET
+role = excluded.role;
+
 deleteMember:
 DELETE FROM Member WHERE conversation = ? AND user = ?;
 

diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/member/MemberDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/member/MemberDAO.kt
@@ -176,10 +176,17 @@ internal class MemberDAOImpl internal constructor(
     override suspend fun updateFullMemberList(memberList: List<MemberEntity>, conversationID: QualifiedIDEntity) =
         withContext(coroutineContext) {
             memberQueries.transaction {
-                memberQueries.deleteMembersFromConversation(conversationID)
-                for (member: MemberEntity in memberList) {
+                val membersToDelete = memberQueries.selectAllMembersByConversation(conversationID)
+                    .executeAsList()
+                    .filter { member -> memberList.none { it.user == member.user } }
+
+                memberList.forEach { member ->
                     userQueries.insertOrIgnoreUserId(member.user)
-                    memberQueries.insertMember(member.user, conversationID, member.role)
+                    memberQueries.insertOrUpdateMember(member.user, conversationID, member.role)
+                }
+
+                membersToDelete.forEach { member ->
+                    memberQueries.deleteMember(conversationID, member.user)
                 }
             }
         }

diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/MemberDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/MemberDAOTest.kt
@@ -342,16 +342,23 @@ class MemberDAOTest : BaseDatabaseTest() {
     fun givenConversationWithExistingMember_whenUpdateFullMemberListIsCalled_thenExistingMemberIsRemovedAndNewMembersAreAdded() = runTest {
         // Given
         val conversationID = TestStubs.conversationEntity1.id
+        val memberToUpdate = MemberEntity(TestStubs.user1.id, MemberEntity.Role.Member)
         val memberList = listOf(
-            MemberEntity(TestStubs.user1.id, MemberEntity.Role.Admin),
+            memberToUpdate,
             MemberEntity(TestStubs.user2.id, MemberEntity.Role.Member)
         )
 
         // Insert a conversation, user, and a member into the conversation to test the deletion operation
-        val oldMember = MemberEntity(TestStubs.user3.id, MemberEntity.Role.Member)
+        val memberToRemove = MemberEntity(TestStubs.user3.id, MemberEntity.Role.Member)
+        val oldMembers = listOf(
+            memberToRemove,
+            memberToUpdate.copy(role = MemberEntity.Role.Admin),
+        )
         userDAO.upsertUser(TestStubs.user3)
         conversationDAO.insertConversation(TestStubs.conversationEntity1)
-        memberDAO.insertMember(oldMember, conversationID)
+        oldMembers.forEach { oldMember ->
+            memberDAO.insertMember(oldMember, conversationID)
+        }
 
         // Ensure all new users are inserted before calling updateFullMemberList
         memberList.forEach { member ->
@@ -366,9 +373,9 @@ class MemberDAOTest : BaseDatabaseTest() {
         val fetchedMembers = memberDAO.observeConversationMembers(conversationID).first()
 
         // Assert that the old member was deleted
-        assertFalse(fetchedMembers.any { it.user == oldMember.user })
+        assertFalse(fetchedMembers.any { it.user == memberToRemove.user })
 
-        // Assert that the new members were inserted
+        // Assert that the new members were inserted and the old updated
         assertTrue(fetchedMembers.containsAll(memberList))
     }