From 25df6d91864297241876ab55c6a90689a2e37c8d Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Tue, 8 Sep 2026 13:24:01 +0000 Subject: [PATCH] core: archive reports filed by the removed member Removing a member archived the reports they filed via markMemberCIsDeleted without emitting an event, so the reports counter stayed high while the list emptied. Match those reports in the same query and emit their ids, which requires running before the member's own items are deleted. --- ...09-08-archive-reports-on-member-removal.md | 4 +++- src/Simplex/Chat/Library/Commands.hs | 4 ++-- src/Simplex/Chat/Library/Subscriber.hs | 4 ++-- src/Simplex/Chat/Store/Messages.hs | 5 +++-- .../SQLite/Migrations/chat_query_plans.txt | 19 ++++++++++--------- tests/ChatTests/Groups.hs | 13 +++++++++++-- 6 files changed, 31 insertions(+), 18 deletions(-) diff --git a/plans/2026-09-08-archive-reports-on-member-removal.md b/plans/2026-09-08-archive-reports-on-member-removal.md index 62ae5616b7..21274c32e9 100644 --- a/plans/2026-09-08-archive-reports-on-member-removal.md +++ b/plans/2026-09-08-archive-reports-on-member-removal.md @@ -73,7 +73,9 @@ Both call sites inline the two lines rather than sharing a new function. This ma ## Testing -`testGroupMemberReportsRemoveMember` (`tests/ChatTests/Groups.hs`) covers both defects in one scenario: cath reports bob's message, bob reports cath's message, then alice removes bob with messages. It asserts that **two** reports are archived (`#jokes: 2 messages deleted by user`) — the one about bob and the one filed by him — and that the reporter's own copy is archived on her device too. +`testGroupMemberReportsRemoveMember` (`tests/ChatTests/Groups.hs`) covers both defects in one scenario: cath reports bob's message, bob reports cath's message, then alice removes bob with messages. It asserts that **two** reports are archived (`#jokes: 2 messages deleted by user`) — the one about bob and the one filed by him — which is the assertion that fails without the `OR group_member_id = ?` branch, since only one id would be emitted and the badge would stick. + +Note that bob's own report row does not survive as marked-deleted: reports live in a member-support scope, and `chat_items.group_scope_group_member_id` is `ON DELETE CASCADE`, so removing bob's member record deletes the row in his scope. Cath's report, in her own scope, remains archived. Both outcomes leave zero active reports, which is what the badge must agree with. Regression coverage relied on: `remove member with messages (full deletion is enabled)`, `remove member with messages mark deleted`, `remove member - delete messages of left/removed members`, and `should send report to group owner, admins and moderators, but not other users`. diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 49f410f627..44d691f5ec 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -3172,11 +3172,11 @@ processChatCommand cxt nm = \case else void $ deleteOrUpdateMemberRecordIO db user gInfo m pure m {memberStatus = GSMemRemoved} deleteMessages user gInfo@GroupInfo {membership} ms = do + ciIds <- concat <$> withStore' (\db -> forM ms $ \m -> markMemberReportsDeleted db user gInfo m membership) + unless (null ciIds) $ toView $ CEvtGroupChatItemsDeleted user gInfo ciIds True (Just membership) if groupFeatureUserAllowed SGFFullDelete gInfo then deleteGroupMembersCIs user gInfo ms else markGroupMembersCIsDeleted user gInfo ms membership - ciIds <- concat <$> withStore' (\db -> forM ms $ \m -> markMemberReportsDeleted db user gInfo m membership) - unless (null ciIds) $ toView $ CEvtGroupChatItemsDeleted user gInfo ciIds True (Just membership) APILeaveGroup groupId -> withUser $ \user@User {userId} -> do gInfo@GroupInfo {membership} <- withFastStore $ \db -> getGroupInfo db cxt user groupId filesInfo <- withFastStore' $ \db -> getGroupFileInfo db user gInfo diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 1b0d379710..8700e482e8 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3655,11 +3655,11 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = groupMsgToView cInfo ci deleteMessages :: GroupInfo -> GroupMember -> CM () deleteMessages gInfo' delMem = do + ciIds <- withStore' $ \db -> markMemberReportsDeleted db user gInfo' delMem m + unless (null ciIds) $ toView $ CEvtGroupChatItemsDeleted user gInfo' ciIds False (Just m) if groupFeatureMemberAllowed SGFFullDelete m gInfo' then deleteGroupMemberCIs user gInfo' delMem else markGroupMemberCIsDeleted user gInfo' delMem m - ciIds <- withStore' $ \db -> markMemberReportsDeleted db user gInfo' delMem m - unless (null ciIds) $ toView $ CEvtGroupChatItemsDeleted user gInfo' ciIds False (Just m) forwardToMember :: GroupMember -> CM () forwardToMember member = let fwd = GrpMsgForward {fwdSender = FwdMember (memberId' m) (memberShortenedName m), fwdBrokerTs = brokerTs} diff --git a/src/Simplex/Chat/Store/Messages.hs b/src/Simplex/Chat/Store/Messages.hs index ae04d9c82e..db615d6026 100644 --- a/src/Simplex/Chat/Store/Messages.hs +++ b/src/Simplex/Chat/Store/Messages.hs @@ -3034,10 +3034,11 @@ markMemberReportsDeleted db User {userId} GroupInfo {groupId} reportedMember byG [sql| UPDATE chat_items SET item_deleted = ?, item_deleted_ts = ?, item_deleted_by_group_member_id = ?, updated_at = ? - WHERE user_id = ? AND group_id = ? AND msg_content_tag = ? AND quoted_member_id = ? AND item_deleted = ? + WHERE user_id = ? AND group_id = ? AND msg_content_tag = ? AND item_deleted = ? + AND (quoted_member_id = ? OR group_member_id = ?) RETURNING chat_item_id; |] - (DBCIDeleted, deletedTs, groupMemberId' byGroupMember, deletedTs, userId, groupId, MCReport_, memberId' reportedMember, DBCINotDeleted) + ((DBCIDeleted, deletedTs, groupMemberId' byGroupMember, deletedTs) :. (userId, groupId, MCReport_, DBCINotDeleted, memberId' reportedMember, groupMemberId' reportedMember)) markReceivedGroupReportsDeleted :: DB.Connection -> User -> GroupInfo -> UTCTime -> IO [ChatItemId] markReceivedGroupReportsDeleted db User {userId} GroupInfo {groupId, membership} deletedTs = do 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 5df99106bb..81eaf74616 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt @@ -4151,6 +4151,16 @@ Query: Plan: SEARCH chat_items USING INDEX idx_chat_items_group_shared_msg_id (user_id=? AND group_id=? AND group_member_id=?) +Query: + UPDATE chat_items + SET item_deleted = ?, item_deleted_ts = ?, item_deleted_by_group_member_id = ?, updated_at = ? + WHERE user_id = ? AND group_id = ? AND msg_content_tag = ? AND item_deleted = ? + AND (quoted_member_id = ? OR group_member_id = ?) + RETURNING chat_item_id; + +Plan: +SEARCH chat_items USING INDEX idx_chat_items_groups_msg_content_tag_deleted (user_id=? AND group_id=? AND msg_content_tag=? AND item_deleted=?) + Query: UPDATE chat_items SET item_deleted = ?, item_deleted_ts = ?, item_deleted_by_group_member_id = ?, updated_at = ? @@ -4160,15 +4170,6 @@ Query: Plan: SEARCH chat_items USING COVERING INDEX idx_chat_items_groups_msg_content_tag_deleted (user_id=? AND group_id=? AND msg_content_tag=? AND item_deleted=? AND item_sent=?) -Query: - UPDATE chat_items - SET item_deleted = ?, item_deleted_ts = ?, item_deleted_by_group_member_id = ?, updated_at = ? - WHERE user_id = ? AND group_id = ? AND msg_content_tag = ? AND quoted_member_id = ? AND item_deleted = ? - RETURNING chat_item_id; - -Plan: -SEARCH chat_items USING INDEX idx_chat_items_groups_msg_content_tag_deleted (user_id=? AND group_id=? AND msg_content_tag=? AND item_deleted=?) - Query: UPDATE chat_items SET item_deleted = ?, item_deleted_ts = ?, item_deleted_by_group_member_id = ?, updated_at = ? diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index 4de3e4f294..aae749d82d 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -7401,16 +7401,25 @@ testGroupMemberReportsRemoveMember = concurrently_ (alice <# "#jokes bob> inappropriate joke") (cath <# "#jokes bob> inappropriate joke") + cath #> "#jokes another joke" + concurrently_ + (alice <# "#jokes cath> another joke") + (bob <# "#jokes cath> another joke") cath ##> "/report #jokes content inappropriate joke" cath <# "#jokes (support) > bob inappropriate joke" cath <## " report content" alice <# "#jokes (support: cath) cath> > bob inappropriate joke" alice <## " report content" - alice #$> ("/_get chat #1 content=report count=100", chat, [(0, "report content")]) + bob ##> "/report #jokes content another joke" + bob <# "#jokes (support) > cath another joke" + bob <## " report content" + alice <# "#jokes (support: bob) bob> > cath another joke" + alice <## " report content" + alice #$> ("/_get chat #1 content=report count=100", chat, [(0, "report content"), (0, "report content")]) cath #$> ("/_get chat #1 content=report count=100", chat, [(1, "report content")]) threadDelay 1000000 alice ##> "/rm #jokes bob messages=on" - alice <## "#jokes: 1 messages deleted by user" + alice <## "#jokes: 2 messages deleted by user" alice <## "#jokes: you removed bob from the group with all messages" bob <## "#jokes: alice removed you from the group with all messages" bob <## "use /d #jokes to delete the group"