diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 7f4ee238b1..f7891cae3a 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -2347,7 +2347,13 @@ processChatCommand vr nm = \case (gInfo, m) <- withFastStore $ \db -> (,) <$> getGroupInfo db vr user groupId <*> getGroupMemberById db vr user gmId when (isNothing $ supportChat m) $ throwCmdError "member has no support chat" when (memberPending m) $ throwCmdError "member is pending" - (gInfo', m') <- withFastStore' $ \db -> deleteGroupMemberSupportChat db user gInfo m + (gInfo', m') <- withFastStore' $ \db -> do + gInfo' <- + if gmRequiresAttention m + then decreaseGroupMembersRequireAttention db user gInfo + else pure gInfo + m' <- deleteGroupMemberSupportChat db m + pure (gInfo', m') pure $ CRMemberSupportChatDeleted user gInfo' m' APIMembersRole groupId memberIds newRole -> withUser $ \user -> withGroupLock "memberRole" groupId $ do diff --git a/src/Simplex/Chat/Library/Internal.hs b/src/Simplex/Chat/Library/Internal.hs index 523224c76d..9ee00b0dc5 100644 --- a/src/Simplex/Chat/Library/Internal.hs +++ b/src/Simplex/Chat/Library/Internal.hs @@ -1677,19 +1677,35 @@ deleteMemberConnection' GroupMember {activeConn} waitDelivery = do withStore' $ \db -> updateConnectionStatus db conn ConnDeleted deleteOrUpdateMemberRecord :: User -> GroupInfo -> GroupMember -> CM GroupInfo -deleteOrUpdateMemberRecord user gInfo member = - withStore' $ \db -> deleteOrUpdateMemberRecordIO db user gInfo member +deleteOrUpdateMemberRecord user gInfo m = + withStore' $ \db -> deleteOrUpdateMemberRecordIO db user gInfo m deleteOrUpdateMemberRecordIO :: DB.Connection -> User -> GroupInfo -> GroupMember -> IO GroupInfo -deleteOrUpdateMemberRecordIO db user@User {userId} gInfo member = do +deleteOrUpdateMemberRecordIO db user@User {userId} gInfo m = do + (gInfo', m') <- deleteSupportChatIfExists db user gInfo m + checkGroupMemberHasItems db user m' >>= \case + Just _ -> updateGroupMemberStatus db userId m' GSMemRemoved + Nothing -> deleteGroupMember db user m' + pure gInfo' + +updateMemberRecordDeleted :: User -> GroupInfo -> GroupMember -> GroupMemberStatus -> CM GroupInfo +updateMemberRecordDeleted user@User {userId} gInfo m newStatus = + withStore' $ \db -> do + (gInfo', m') <- deleteSupportChatIfExists db user gInfo m + updateGroupMemberStatus db userId m' newStatus + pure gInfo' + +deleteSupportChatIfExists :: DB.Connection -> User -> GroupInfo -> GroupMember -> IO (GroupInfo, GroupMember) +deleteSupportChatIfExists db user gInfo m = do gInfo' <- - if gmRequiresAttention member + if gmRequiresAttention m then decreaseGroupMembersRequireAttention db user gInfo else pure gInfo - checkGroupMemberHasItems db user member >>= \case - Just _ -> updateGroupMemberStatus db userId member GSMemRemoved - Nothing -> deleteGroupMember db user member - pure gInfo' + m' <- + if isJust (supportChat m) + then deleteGroupMemberSupportChat db m + else pure m + pure (gInfo', m') sendDirectContactMessages :: MsgEncodingI e => User -> Contact -> NonEmpty (ChatMsgEvent e) -> CM [Either ChatError SndMessage] sendDirectContactMessages user ct events = do diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 27f3a4f61c..c1fc1e917f 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -2735,7 +2735,7 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = withStore' $ \db -> updateGroupMemberStatus db userId membership GSMemRemoved let membership' = membership {memberStatus = GSMemRemoved} when withMessages $ deleteMessages gInfo membership' SMDSnd - deleteMemberItem RGEUserDeleted + deleteMemberItem gInfo RGEUserDeleted toView $ CEvtDeletedMemberUser user gInfo {membership = membership'} m withMessages pure $ Just DJSGroup {jobSpec = DJRelayRemoved} else @@ -2746,29 +2746,33 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = Right deletedMember@GroupMember {groupMemberId, memberProfile, memberStatus} -> checkRole deletedMember $ do -- ? prohibit deleting member if it's the sender - sender should use x.grp.leave - if isUserGrpFwdRelay gInfo && not forwarded + let shouldForward = isUserGrpFwdRelay gInfo && not forwarded + if shouldForward then do -- Special case: forward before deleting connection. - -- It allows us to avoid adding logic in forwardMsgs to circumvent member filtering. forwardToMember deletedMember deleteMemberConnection' deletedMember True else deleteMemberConnection deletedMember - -- undeleted "member connected" chat item will prevent deletion of member record - gInfo' <- deleteOrUpdateMemberRecord user gInfo deletedMember + let deliveryScope = memberEventDeliveryScope deletedMember + gInfo' <- case deliveryScope of + -- Keep member record if it's support scope - it will be required for forwarding inside that scope. + Just (DJSMemberSupport _) | shouldForward -> updateMemberRecordDeleted user gInfo deletedMember GSMemRemoved + -- Undeleted "member connected" chat item will prevent deletion of member record. + _ -> deleteOrUpdateMemberRecord user gInfo deletedMember let wasDeleted = memberStatus == GSMemRemoved || memberStatus == GSMemLeft deletedMember' = deletedMember {memberStatus = GSMemRemoved} when withMessages $ deleteMessages gInfo' deletedMember' SMDRcv - unless wasDeleted $ deleteMemberItem $ RGEMemberDeleted groupMemberId (fromLocalProfile memberProfile) + unless wasDeleted $ deleteMemberItem gInfo' $ RGEMemberDeleted groupMemberId (fromLocalProfile memberProfile) toView $ CEvtDeletedMember user gInfo' m deletedMember' withMessages - pure $ memberEventDeliveryScope deletedMember + pure deliveryScope where checkRole GroupMember {memberRole} a | senderRole < GRAdmin || senderRole < memberRole = messageError "x.grp.mem.del with insufficient member permissions" $> Nothing | otherwise = a - deleteMemberItem gEvent = do - (gInfo', m', scopeInfo) <- mkGroupChatScope gInfo m - (ci, cInfo) <- saveRcvChatItemNoParse user (CDGroupRcv gInfo' scopeInfo m') msg brokerTs (CIRcvGroupEvent gEvent) + deleteMemberItem gi gEvent = do + (gi', m', scopeInfo) <- mkGroupChatScope gi m + (ci, cInfo) <- saveRcvChatItemNoParse user (CDGroupRcv gi' scopeInfo m') msg brokerTs (CIRcvGroupEvent gEvent) groupMsgToView cInfo ci deleteMessages :: MsgDirectionI d => GroupInfo -> GroupMember -> SMsgDirection d -> CM () deleteMessages gInfo' delMem msgDir @@ -2791,11 +2795,7 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = xGrpLeave gInfo m msg brokerTs = do deleteMemberConnection m -- member record is not deleted to allow creation of "member left" chat item - gInfo' <- withStore' $ \db -> do - updateGroupMemberStatus db userId m GSMemLeft - if gmRequiresAttention m - then decreaseGroupMembersRequireAttention db user gInfo - else pure gInfo + gInfo' <- updateMemberRecordDeleted user gInfo m GSMemLeft (gInfo'', m', scopeInfo) <- mkGroupChatScope gInfo' m (ci, cInfo) <- saveRcvChatItemNoParse user (CDGroupRcv gInfo'' scopeInfo m') msg brokerTs (CIRcvGroupEvent RGEMemberLeft) groupMsgToView cInfo ci diff --git a/src/Simplex/Chat/Store/Groups.hs b/src/Simplex/Chat/Store/Groups.hs index 44dcd3c7f9..9219094cbc 100644 --- a/src/Simplex/Chat/Store/Groups.hs +++ b/src/Simplex/Chat/Store/Groups.hs @@ -1415,9 +1415,8 @@ updateGroupMemberAccepted db User {userId} m@GroupMember {groupMemberId} status (status, role, currentTs, userId, groupMemberId) pure m {memberStatus = status, memberRole = role, updatedAt = currentTs} -deleteGroupMemberSupportChat :: DB.Connection -> User -> GroupInfo -> GroupMember -> IO (GroupInfo, GroupMember) -deleteGroupMemberSupportChat db user g m@GroupMember {groupMemberId} = do - let requiredAttention = gmRequiresAttention m +deleteGroupMemberSupportChat :: DB.Connection -> GroupMember -> IO GroupMember +deleteGroupMemberSupportChat db m@GroupMember {groupMemberId} = do currentTs <- getCurrentTime DB.execute db @@ -1439,11 +1438,7 @@ deleteGroupMemberSupportChat db user g m@GroupMember {groupMemberId} = do WHERE group_member_id = ? |] (currentTs, groupMemberId) - let m' = m {supportChat = Nothing, updatedAt = currentTs} - g' <- if requiredAttention - then decreaseGroupMembersRequireAttention db user g - else pure g - pure (g', m') + pure m {supportChat = Nothing, updatedAt = currentTs} updateGroupMembersRequireAttention :: DB.Connection -> User -> GroupInfo -> GroupMember -> GroupMember -> IO GroupInfo updateGroupMembersRequireAttention db user g member member' diff --git a/src/Simplex/Chat/Store/Messages.hs b/src/Simplex/Chat/Store/Messages.hs index d7dd0fde01..72101c37e3 100644 --- a/src/Simplex/Chat/Store/Messages.hs +++ b/src/Simplex/Chat/Store/Messages.hs @@ -1645,7 +1645,7 @@ getGroupUnreadCount_ :: DB.Connection -> User -> GroupInfo -> Maybe GroupChatSco getGroupUnreadCount_ db user g scopeInfo_ contentFilter = head <$> queryUnreadGroupItems db user g scopeInfo_ contentFilter baseQuery "" where - baseQuery = "SELECT COUNT(1), COALESCE(SUM(user_mention), 0) FROM chat_items WHERE user_id = ? AND group_id = ? AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL " + baseQuery = "SELECT COUNT(1), COALESCE(SUM(user_mention), 0) FROM chat_items WHERE user_id = ? AND group_id = ? " getGroupReportsCount_ :: DB.Connection -> User -> GroupInfo -> Bool -> IO Int getGroupReportsCount_ db User {userId} GroupInfo {groupId} archived = diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt index c46f6a5e7d..91f2ea3899 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt @@ -6113,7 +6113,7 @@ Query: SELECT COUNT(1) FROM groups WHERE user_id = ? AND chat_item_ttl > 0 Plan: SEARCH groups USING INDEX idx_groups_chat_ts (user_id=?) -Query: SELECT COUNT(1), COALESCE(SUM(user_mention), 0) FROM chat_items WHERE user_id = ? AND group_id = ? AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND item_status = ? +Query: SELECT COUNT(1), COALESCE(SUM(user_mention), 0) FROM chat_items WHERE user_id = ? AND group_id = ? AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND item_status = ? Plan: SEARCH chat_items USING COVERING INDEX idx_chat_items_group_scope_stats_all (user_id=? AND group_id=? AND group_scope_tag=? AND group_scope_group_member_id=? AND item_status=?) diff --git a/tests/ChatClient.hs b/tests/ChatClient.hs index 8fd8a5976d..d0b186dd94 100644 --- a/tests/ChatClient.hs +++ b/tests/ChatClient.hs @@ -479,6 +479,9 @@ testChat3 = testChatCfgOpts3 testCfg testOpts testChatCfg3 :: HasCallStack => ChatConfig -> Profile -> Profile -> Profile -> (HasCallStack => TestCC -> TestCC -> TestCC -> IO ()) -> TestParams -> IO () testChatCfg3 cfg = testChatCfgOpts3 cfg testOpts +testChatOpts3 :: HasCallStack => ChatOpts -> Profile -> Profile -> Profile -> (HasCallStack => TestCC -> TestCC -> TestCC -> IO ()) -> TestParams -> IO () +testChatOpts3 = testChatCfgOpts3 testCfg + testChatCfgOpts3 :: HasCallStack => ChatConfig -> ChatOpts -> Profile -> Profile -> Profile -> (HasCallStack => TestCC -> TestCC -> TestCC -> IO ()) -> TestParams -> IO () testChatCfgOpts3 cfg opts p1 p2 p3 test = testChatN cfg opts [p1, p2, p3] test_ where diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index 42562d8e16..b0f4fc99dc 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -224,11 +224,15 @@ chatGroupTests = do it "should forward file inside support scope" testScopedSupportForwardFile it "should forward member removal in support scope in review (x.grp.mem.del)" testScopedSupportForwardMemberRemoval it "should forward admin removal in support scope in review (x.grp.mem.del, relay forwards it was removed)" testScopedSupportForwardAdminRemoval + it "should forward pending member leaving in support scope in review (x.grp.leave)" testScopedSupportForwardLeave it "should forward group deletion in support scope in review (x.grp.del)" testScopedSupportForwardGroupDeletion it "should send messages to admins and members" testSupportCLISendCommand it "should correctly maintain unread stats for support chats on reading chat items" testScopedSupportUnreadStatsOnRead it "should correctly maintain unread stats for support chats on deleting chat items" testScopedSupportUnreadStatsOnDelete it "should correct member attention stat for support chat on opening it" testScopedSupportUnreadStatsCorrectOnOpen + it "should remove support chat with member when member is removed" testScopedSupportMemberRemoved + it "should remove support chat with member when user removes member" testScopedSupportUserRemovesMember + it "should remove support chat with member when member leaves" testScopedSupportMemberLeaves -- TODO [channels fwd] enable tests (requires communicating useRelays to members) -- TODO [channels fwd] add tests for channels -- TODO - tests with multiple relays (all relays should deliver messages, members should deduplicate) @@ -7864,9 +7868,9 @@ testScopedSupportForwardMemberRemoval = alice ##> "#team (support: eve) hi" alice <## "bad chat command: support member not current or pending" bob ##> "#team (support: eve) hi" - bob <## "bad chat command: support member not current or pending" + bob <##. "chat db error: SEGroupMemberNameNotFound" dan ##> "#team (support: eve) hi" - dan <## "bad chat command: support member not current or pending" + dan <##. "chat db error: SEGroupMemberNameNotFound" eve ##> "/groups" eve <## "#team (you are removed, delete local copy: /d #team)" @@ -7972,6 +7976,30 @@ testScopedSupportForwardAdminRemoval = alice ##> "/groups" alice <## "#team (you are removed, delete local copy: /d #team)" +testScopedSupportForwardLeave :: HasCallStack => TestParams -> IO () +testScopedSupportForwardLeave = + testChat5 aliceProfile bobProfile cathProfile danProfile eveProfile $ + \alice bob cath dan eve -> do + createGroup4 "team" alice (bob, GRAdmin) (cath, GRMember) (dan, GRModerator) + setupReviewForward alice bob cath dan eve + + -- eve leaves group, bob and dan receive member leave message + eve ##> "/leave #team" + eve <## "#team: you left the group" + eve <## "use /d #team to delete the group" + alice <## "#team: eve left the group" + bob <## "#team: eve left the group" + dan <## "#team: eve left the group" + + alice ##> "#team (support: eve) hi" + alice <## "bad chat command: support member not current or pending" + bob ##> "#team (support: eve) hi" + bob <##. "bad chat command: support member not current or pending" + dan ##> "#team (support: eve) hi" + dan <##. "bad chat command: support member not current or pending" + eve ##> "/groups" + eve <## "#team (you left, delete local copy: /d #team)" + testScopedSupportForwardGroupDeletion :: HasCallStack => TestParams -> IO () testScopedSupportForwardGroupDeletion = testChat5 aliceProfile bobProfile cathProfile danProfile eveProfile $ @@ -8300,6 +8328,99 @@ testScopedSupportUnreadStatsCorrectOnOpen = { markRead = False } +testScopedSupportMemberRemoved :: HasCallStack => TestParams -> IO () +testScopedSupportMemberRemoved = + testChatOpts3 opts aliceProfile bobProfile cathProfile $ \alice bob cath -> do + createGroup3' "team" alice (bob, GRMember) (cath, GRAdmin) + + bob #> "#team (support) 1" + [alice, cath] *<# "#team (support: bob) bob> 1" + + bob #> "#team (support) 2" + [alice, cath] *<# "#team (support: bob) bob> 2" + + alice ##> "/member support chats #team" + alice <## "members require attention: 1" + alice <## "bob (Bob) (id 2): unread: 2, require attention: 2, mentions: 0" + + cath ##> "/rm team bob" + concurrentlyN_ + [ cath <## "#team: you removed bob from the group", + do + bob <## "#team: cath removed you from the group" + bob <## "use /d #team to delete the group", + alice <## "#team: cath removed bob from the group" + ] + + alice ##> "/member support chats #team" + alice <## "members require attention: 0" + where + opts = + testOpts + { markRead = False + } + +testScopedSupportUserRemovesMember :: HasCallStack => TestParams -> IO () +testScopedSupportUserRemovesMember = + testChatOpts2 opts aliceProfile bobProfile $ \alice bob -> do + createGroup2' "team" alice (bob, GRMember) True + + bob #> "#team (support) 1" + alice <# "#team (support: bob) bob> 1" + + bob #> "#team (support) 2" + alice <# "#team (support: bob) bob> 2" + + alice ##> "/member support chats #team" + alice <## "members require attention: 1" + alice <## "bob (Bob) (id 2): unread: 2, require attention: 2, mentions: 0" + + alice ##> "/rm team bob" + concurrentlyN_ + [ alice <## "#team: you removed bob from the group", + do + bob <## "#team: alice removed you from the group" + bob <## "use /d #team to delete the group" + ] + + alice ##> "/member support chats #team" + alice <## "members require attention: 0" + where + opts = + testOpts + { markRead = False + } + +testScopedSupportMemberLeaves :: HasCallStack => TestParams -> IO () +testScopedSupportMemberLeaves = + testChatOpts2 opts aliceProfile bobProfile $ \alice bob -> do + createGroup2' "team" alice (bob, GRMember) True + + bob #> "#team (support) 1" + alice <# "#team (support: bob) bob> 1" + + bob #> "#team (support) 2" + alice <# "#team (support: bob) bob> 2" + + alice ##> "/member support chats #team" + alice <## "members require attention: 1" + alice <## "bob (Bob) (id 2): unread: 2, require attention: 2, mentions: 0" + + bob ##> "/l team" + concurrentlyN_ + [ do + bob <## "#team: you left the group" + bob <## "use /d #team to delete the group", + alice <## "#team: bob left the group" + ] + + alice ##> "/member support chats #team" + alice <## "members require attention: 0" + where + opts = + testOpts + { markRead = False + } testChannelsRelayDeliver :: HasCallStack => TestParams -> IO () testChannelsRelayDeliver = testChat5 aliceProfile bobProfile cathProfile danProfile eveProfile $ \alice bob cath dan eve -> do