refactor: retain user info after deleting and remove users from groups when deleted

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt

@@ -215,9 +215,6 @@ interface ConversationRepository {
     suspend fun clearContent(conversationId: ConversationId): Either<CoreFailure, Unit>
     suspend fun observeIsUserMember(conversationId: ConversationId, userId: UserId): Flow<Either<CoreFailure, Boolean>>
     suspend fun whoDeletedMe(conversationId: ConversationId): Either<CoreFailure, UserId?>
-
-    suspend fun deleteUserFromConversations(userId: UserId): Either<CoreFailure, Unit>
-
     suspend fun getConversationsByUserId(userId: UserId): Either<CoreFailure, List<Conversation>>
     suspend fun insertConversations(conversations: List<Conversation>): Either<CoreFailure, Unit>
     suspend fun changeConversationName(
@@ -868,10 +865,6 @@ internal class ConversationDataSource internal constructor(
         )?.toModel()
     }
 
-    override suspend fun deleteUserFromConversations(userId: UserId): Either<CoreFailure, Unit> = wrapStorageRequest {
-        conversationDAO.revokeOneOnOneConversationsWithDeletedUser(userId.toDao())
-    }
-
     override suspend fun getConversationsByUserId(userId: UserId): Either<CoreFailure, List<Conversation>> {
         return wrapStorageRequest { conversationDAO.getConversationsByUserId(userId.toDao()) }
             .map { it.map { entity -> conversationMapper.fromDaoModel(entity) } }

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt

@@ -110,7 +110,7 @@ interface UserRepository {
      */
     suspend fun getAllRecipients(): Either<CoreFailure, Pair<List<Recipient>, List<Recipient>>>
     suspend fun updateUserFromEvent(event: Event.User.Update): Either<CoreFailure, Unit>
-    suspend fun removeUser(userId: UserId): Either<CoreFailure, Unit>
+    suspend fun markUserAsDeletedAndRemoveFromGroupConversations(userId: UserId): Either<CoreFailure, Unit>
 
     /**
      * Marks federated user as defederated in order to hold conversation history
@@ -502,9 +502,9 @@ internal class UserDataSource internal constructor(
         }
     }
 
-    override suspend fun removeUser(userId: UserId): Either<CoreFailure, Unit> {
+    override suspend fun markUserAsDeletedAndRemoveFromGroupConversations(userId: UserId): Either<CoreFailure, Unit> {
         return wrapStorageRequest {
-            userDAO.markUserAsDeleted(userId.toDao())
+            userDAO.markUserAsDeletedAndRemoveFromGroupConv(userId.toDao())
         }
     }
 

logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/UserEventReceiver.kt

@@ -182,28 +182,7 @@ internal class UserEventReceiverImpl internal constructor(
             logout(LogoutReason.DELETED_ACCOUNT)
             Either.Right(Unit)
         } else {
-            // TODO: those 2 steps must be done in one transaction
-            //  userRepo.markAsDeleted(event.userId) will mark user as deleted and remove from the group conversations
-            userRepository.removeUser(event.userId)
-                .onSuccess {
-                    conversationRepository.deleteUserFromConversations(event.userId)
-                        .onSuccess {
-                            kaliumLogger
-                                .logEventProcessing(
-                                    EventLoggingStatus.SUCCESS,
-                                    event
-                                )
-                        }
-                        .onFailure {
-                            kaliumLogger
-                                .logEventProcessing(
-                                    EventLoggingStatus.FAILURE,
-                                    event,
-                                    Pair("errorInfo", "$it")
-                                )
-                        }
-
-                }
+            userRepository.markUserAsDeletedAndRemoveFromGroupConversations(event.userId)
                 .onFailure {
                     kaliumLogger
                         .logEventProcessing(

logic/src/commonTest/kotlin/com/wire/kalium/logic/data/team/TeamRepositoryTest.kt

@@ -18,7 +18,7 @@
 
 package com.wire.kalium.logic.data.team
 
-import app.cash.turbine.test
+import  app.cash.turbine.test
 import com.wire.kalium.logic.NetworkFailure
 import com.wire.kalium.logic.data.conversation.LegalHoldStatus
 import com.wire.kalium.logic.data.id.TeamId
@@ -34,7 +34,6 @@ import com.wire.kalium.logic.util.shouldSucceed
 import com.wire.kalium.network.api.base.authenticated.TeamsApi
 import com.wire.kalium.network.api.base.authenticated.client.ClientIdDTO
 import com.wire.kalium.network.api.base.authenticated.keypackage.LastPreKeyDTO
-import com.wire.kalium.network.api.base.authenticated.userDetails.UserDetailsApi
 import com.wire.kalium.network.api.base.model.ErrorResponse
 import com.wire.kalium.network.api.base.model.LegalHoldStatusDTO
 import com.wire.kalium.network.api.base.model.LegalHoldStatusResponse
@@ -324,13 +323,13 @@ class TeamRepositoryTest {
             stubsUnitByDefault = true
         }
 
+        val teamMapper = MapperProvider.teamMapper()
+
         @Mock
         val userConfigDAO = configure(mock(classOf<UserConfigDAO>())) {
             stubsUnitByDefault = true
         }
 
-        val teamMapper = MapperProvider.teamMapper()
-
         @Mock
         val teamsApi = mock(classOf<TeamsApi>())
 
@@ -350,11 +349,12 @@ class TeamRepositoryTest {
             teamMapper = teamMapper,
             teamsApi = teamsApi,
             userDAO = userDAO,
-            userConfigDAO = userConfigDAO,
             selfUserId = TestUser.USER_ID,
             serviceDAO = serviceDAO,
             legalHoldHandler = legalHoldHandler,
             legalHoldRequestHandler = legalHoldRequestHandler,
+            userConfigDAO = userConfigDAO
+
         )
 
         fun withApiGetTeamInfoSuccess(teamDTO: TeamDTO) = apply {

logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt

@@ -37,7 +37,6 @@ import com.wire.kalium.logic.framework.TestUser.LIST_USERS_DTO
 import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.logic.functional.getOrNull
 import com.wire.kalium.logic.test_util.TestNetworkResponseError
-import com.wire.kalium.logic.sync.receiver.UserEventReceiverTest
 import com.wire.kalium.logic.test_util.TestNetworkException.federationNotEnabled
 import com.wire.kalium.logic.util.shouldFail
 import com.wire.kalium.logic.util.shouldSucceed

logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/UserEventReceiverTest.kt

@@ -109,20 +109,15 @@ class UserEventReceiverTest {
     fun givenUserDeleteEvent_RepoAndPersisMessageAreInvoked() = runTest {
         val event = TestEvent.userDelete(userId = OTHER_USER_ID)
         val (arrangement, eventReceiver) = arrange {
-            withRemoveUserSuccess()
-            withDeleteUserFromConversationsSuccess()
+            withMarkUserAsDeletedAndRemoveFromGroupConversationsSuccess()
+            // withDeleteUserFromConversationsSuccess()
             withConversationsByUserId(listOf(TestConversation.CONVERSATION))
         }
 
         eventReceiver.onEvent(event)
 
         verify(arrangement.userRepository)
-            .suspendFunction(arrangement.userRepository::removeUser)
-            .with(any())
-            .wasInvoked(exactly = once)
-
-        verify(arrangement.conversationRepository)
-            .suspendFunction(arrangement.conversationRepository::deleteUserFromConversations)
+            .suspendFunction(arrangement.userRepository::markUserAsDeletedAndRemoveFromGroupConversations)
             .with(any())
             .wasInvoked(exactly = once)
     }
@@ -329,11 +324,6 @@ class UserEventReceiverTest {
             given(logoutUseCase).suspendFunction(logoutUseCase::invoke).whenInvokedWith(any()).thenReturn(Unit)
         }
 
-        fun withDeleteUserFromConversationsSuccess() = apply {
-            given(conversationRepository).suspendFunction(conversationRepository::deleteUserFromConversations)
-                .whenInvokedWith(any()).thenReturn(Either.Right(Unit))
-        }
-
         fun withConversationsByUserId(conversationIds: List<Conversation>) = apply {
             given(conversationRepository).suspendFunction(conversationRepository::getConversationsByUserId)
                 .whenInvokedWith(any()).thenReturn(Either.Right(conversationIds))

logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/UserRepositoryArrangement.kt

@@ -37,7 +37,7 @@ internal interface UserRepositoryArrangement {
 
     fun withUpdateUserFailure(coreFailure: CoreFailure)
 
-    fun withRemoveUserSuccess()
+    fun withMarkUserAsDeletedAndRemoveFromGroupConversationsSuccess()
 
     fun withSelfUserReturning(selfUser: SelfUser?)
 
@@ -74,8 +74,8 @@ internal class UserRepositoryArrangementImpl: UserRepositoryArrangement {
             .whenInvokedWith(any()).thenReturn(Either.Left(coreFailure))
     }
 
-    override fun withRemoveUserSuccess() {
-        given(userRepository).suspendFunction(userRepository::removeUser)
+    override fun withMarkUserAsDeletedAndRemoveFromGroupConversationsSuccess() {
+        given(userRepository).suspendFunction(userRepository::markUserAsDeletedAndRemoveFromGroupConversations)
             .whenInvokedWith(any()).thenReturn(Either.Right(Unit))
     }
 

persistence/src/commonMain/db_user/com/wire/kalium/persistence/Members.sq

@@ -62,14 +62,6 @@ UPDATE Member
 SET role = ?
 WHERE user = ? AND conversation = ?;
 
-deleteUserFromGroupConversations:
-DELETE FROM Member
-WHERE conversation IN (
-    SELECT conversation FROM Member
-    JOIN Conversation ON Conversation.qualified_id = Member.conversation
-    WHERE Member.user = ? AND Conversation.type = 'GROUP'
-) AND Member.user = ?;
-
 isUserMember:
 SELECT user FROM Member WHERE conversation = :conversationId AND user = :userId;
 

persistence/src/commonMain/db_user/com/wire/kalium/persistence/Users.sq

@@ -82,9 +82,18 @@ connection_status = excluded.connection_status,
 user_type = excluded.user_type;
 
 markUserAsDeleted:
-UPDATE User
-SET team = NULL , preview_asset_id = NULL, complete_asset_id = NULL, user_type = ?, deleted = 1
-WHERE qualified_id = ?;
+INSERT INTO User(qualified_id, user_type, deleted)
+VALUES (:qualified_id, :user_type, 1)
+ON CONFLICT (qualified_id) DO UPDATE SET
+team = NULL , preview_asset_id = NULL, complete_asset_id = NULL, user_type = excluded.user_type, deleted = 1;
+
+deleteUserFromGroupConversations:
+DELETE FROM Member
+WHERE conversation IN (
+    SELECT conversation FROM Member
+    JOIN Conversation ON Conversation.qualified_id = Member.conversation
+    WHERE Member.user = :qualified_id AND Conversation.type = 'GROUP'
+) AND Member.user = :qualified_id;
 
 markUserAsDefederated:
 UPDATE User

persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAO.kt

@@ -266,7 +266,7 @@ interface UserDAO {
     suspend fun getUsersWithOneOnOneConversation(): List<UserEntity>
 
     suspend fun deleteUserByQualifiedID(qualifiedID: QualifiedIDEntity)
-    suspend fun markUserAsDeleted(qualifiedID: QualifiedIDEntity)
+    suspend fun markUserAsDeletedAndRemoveFromGroupConv(qualifiedID: QualifiedIDEntity)
     suspend fun markUserAsDefederated(qualifiedID: QualifiedIDEntity)
     suspend fun updateUserHandle(qualifiedID: QualifiedIDEntity, handle: String)
     suspend fun updateUserAvailabilityStatus(qualifiedID: QualifiedIDEntity, status: UserAvailabilityStatusEntity)

persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/UserDAOImpl.kt

@@ -208,25 +208,30 @@ class UserDAOImpl internal constructor(
     override suspend fun upsertUsers(users: List<UserEntity>) = withContext(queriesContext) {
         userQueries.transaction {
             for (user: UserEntity in users) {
-                userQueries.insertUser(
-                    qualified_id = user.id,
-                    name = user.name,
-                    handle = user.handle,
-                    email = user.email,
-                    phone = user.phone,
-                    accent_id = user.accentId,
-                    team = user.team,
-                    preview_asset_id = user.previewAssetId,
-                    complete_asset_id = user.completeAssetId,
-                    user_type = user.userType,
-                    bot_service = user.botService,
-                    incomplete_metadata = user.hasIncompleteMetadata,
-                    expires_at = user.expiresAt,
-                    connection_status = user.connectionStatus,
-                    deleted = user.deleted,
-                    supported_protocols = user.supportedProtocols,
-                    active_one_on_one_conversation_id = user.activeOneOnOneConversationId
-                )
+                if (user.deleted) {
+                    // mark as deleted and remove from groups
+                    safeMarkAsDeleted(user.id)
+                } else {
+                    userQueries.insertUser(
+                        qualified_id = user.id,
+                        name = user.name,
+                        handle = user.handle,
+                        email = user.email,
+                        phone = user.phone,
+                        accent_id = user.accentId,
+                        team = user.team,
+                        preview_asset_id = user.previewAssetId,
+                        complete_asset_id = user.completeAssetId,
+                        user_type = user.userType,
+                        bot_service = user.botService,
+                        incomplete_metadata = user.hasIncompleteMetadata,
+                        expires_at = user.expiresAt,
+                        connection_status = user.connectionStatus,
+                        deleted = user.deleted,
+                        supported_protocols = user.supportedProtocols,
+                        active_one_on_one_conversation_id = user.activeOneOnOneConversationId
+                    )
+                }
             }
         }
     }
@@ -299,8 +304,15 @@ class UserDAOImpl internal constructor(
         userQueries.deleteUser(qualifiedID)
     }
 
-    override suspend fun markUserAsDeleted(qualifiedID: QualifiedIDEntity) = withContext(queriesContext) {
-        userQueries.markUserAsDeleted(user_type = UserTypeEntity.NONE, qualified_id = qualifiedID)
+    override suspend fun markUserAsDeletedAndRemoveFromGroupConv(qualifiedID: QualifiedIDEntity) = withContext(queriesContext) {
+        userQueries.transaction {
+            safeMarkAsDeleted(qualifiedID)
+        }
+    }
+
+    private fun safeMarkAsDeleted(qualifiedID: QualifiedIDEntity) {
+        userQueries.markUserAsDeleted(qualifiedID, UserTypeEntity.NONE)
+        userQueries.deleteUserFromGroupConversations(qualifiedID)
     }
 
     override suspend fun markUserAsDefederated(qualifiedID: QualifiedIDEntity) {

persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/conversation/ConversationDAO.kt

@@ -90,7 +90,6 @@ interface ConversationDAO {
     suspend fun updateConversationName(conversationId: QualifiedIDEntity, conversationName: String, timestamp: String)
     suspend fun updateConversationType(conversationID: QualifiedIDEntity, type: ConversationEntity.Type)
     suspend fun updateConversationProtocol(conversationId: QualifiedIDEntity, protocol: ConversationEntity.Protocol): Boolean
-    suspend fun revokeOneOnOneConversationsWithDeletedUser(userId: UserIDEntity)
     suspend fun getConversationsByUserId(userId: UserIDEntity): List<ConversationEntity>
     suspend fun updateConversationReceiptMode(conversationID: QualifiedIDEntity, receiptMode: ConversationEntity.ReceiptMode)
     suspend fun updateGuestRoomLink(

persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/conversation/ConversationDAOImpl.kt

@@ -336,10 +336,6 @@ internal class ConversationDAOImpl internal constructor(
         }
     }
 
-    override suspend fun revokeOneOnOneConversationsWithDeletedUser(userId: UserIDEntity) = withContext(coroutineContext) {
-        memberQueries.deleteUserFromGroupConversations(userId, userId)
-    }
-
     override suspend fun getConversationsByUserId(userId: UserIDEntity): List<ConversationEntity> = withContext(coroutineContext) {
         memberQueries.selectConversationsByMember(userId, conversationMapper::toModel).executeAsList()
     }