diff --git a/apps/ios/Shared/Model/AppAPITypes.swift b/apps/ios/Shared/Model/AppAPITypes.swift index aa271ddbd3..13ab083d1f 100644 --- a/apps/ios/Shared/Model/AppAPITypes.swift +++ b/apps/ios/Shared/Model/AppAPITypes.swift @@ -45,8 +45,7 @@ enum ChatCommand: ChatCmdProtocol { case apiGetChat(chatId: ChatId, scope: GroupChatScope?, contentTag: MsgContentTag?, pagination: ChatPagination, search: String, parentItemId: Int64? = nil) case apiGetChatContentTypes(chatId: ChatId, scope: GroupChatScope?) case apiGetChatItemInfo(type: ChatType, id: Int64, scope: GroupChatScope?, itemId: Int64) - case apiSendMessages(type: ChatType, id: Int64, scope: GroupChatScope?, sendAsGroup: Bool, live: Bool, ttl: Int?, composedMessages: [ComposedMessage]) - case apiSendComment(groupId: Int64, parentItemId: Int64, live: Bool, ttl: Int?, composedMessages: [ComposedMessage]) + case apiSendMessages(type: ChatType, id: Int64, scope: GroupChatScope?, parentItemId: Int64?, sendAsGroup: Bool, live: Bool, ttl: Int?, composedMessages: [ComposedMessage]) case apiSetCommentsDisabled(groupId: Int64, parentItemId: Int64, disabled: Bool) case apiCreateChatTag(tag: ChatTagData) case apiSetChatTags(type: ChatType, id: Int64, tagIds: [Int64]) @@ -239,15 +238,12 @@ enum ChatCommand: ChatCmdProtocol { return "/_get chat \(chatId)\(scopeRef(scope))\(parent)\(tag) \(pagination.cmdString)" + (search == "" ? "" : " search=\(search)") case let .apiGetChatContentTypes(chatId, scope): return "/_get content types \(chatId)\(scopeRef(scope))" case let .apiGetChatItemInfo(type, id, scope, itemId): return "/_get item info \(ref(type, id, scope: scope)) \(itemId)" - case let .apiSendMessages(type, id, scope, sendAsGroup, live, ttl, composedMessages): + case let .apiSendMessages(type, id, scope, parentItemId, sendAsGroup, live, ttl, composedMessages): let msgs = encodeJSON(composedMessages) let ttlStr = ttl != nil ? "\(ttl!)" : "default" + let parent = parentItemId != nil ? " parent=\(parentItemId!)" : "" let asGroup = sendAsGroup ? "(as_group=on)" : "" - return "/_send \(ref(type, id, scope: scope))\(asGroup) live=\(onOff(live)) ttl=\(ttlStr) json \(msgs)" - case let .apiSendComment(groupId, parentItemId, live, ttl, composedMessages): - let msgs = encodeJSON(composedMessages) - let ttlStr = ttl != nil ? "\(ttl!)" : "default" - return "/_comment #\(groupId) \(parentItemId) live=\(onOff(live)) ttl=\(ttlStr) json \(msgs)" + return "/_send \(ref(type, id, scope: scope))\(parent)\(asGroup) live=\(onOff(live)) ttl=\(ttlStr) json \(msgs)" case let .apiSetCommentsDisabled(groupId, parentItemId, disabled): return "/_comments_disabled #\(groupId) \(parentItemId) \(onOff(disabled))" case let .apiCreateChatTag(tag): return "/_create tag \(encodeJSON(tag))" @@ -451,7 +447,6 @@ enum ChatCommand: ChatCmdProtocol { case .apiGetChatContentTypes: return "apiGetChatContentTypes" case .apiGetChatItemInfo: return "apiGetChatItemInfo" case .apiSendMessages: return "apiSendMessages" - case .apiSendComment: return "apiSendComment" case .apiSetCommentsDisabled: return "apiSetCommentsDisabled" case .apiCreateChatTag: return "apiCreateChatTag" case .apiSetChatTags: return "apiSetChatTags" diff --git a/apps/ios/Shared/Model/ChatModel.swift b/apps/ios/Shared/Model/ChatModel.swift index 71de43a212..1ee79227f8 100644 --- a/apps/ios/Shared/Model/ChatModel.swift +++ b/apps/ios/Shared/Model/ChatModel.swift @@ -80,6 +80,11 @@ enum SecondaryItemsModelFilter { class ItemsModel: ObservableObject { static let shared = ItemsModel(secondaryIMFilter: nil) public var secondaryIMFilter: SecondaryItemsModelFilter? + // Comments-thread context: holds the local-only ChannelMsgInfo carrier + // (parent item + sharedId) needed by views opening the thread. The wire + // shape has no channelMsgInfo, and the carrier is NOT stamped onto the + // main chats array — only kept here on the secondary model. + public var channelMsgInfo: ChannelMsgInfo? public var preloadState = PreloadState() private let publisher = ObservableObjectPublisher() private var bag = Set() @@ -114,13 +119,15 @@ class ItemsModel: ObservableObject { // Spec: spec/state.md#loadSecondaryChat static func loadSecondaryChat(_ chatId: ChatId, chatFilter: SecondaryItemsModelFilter, willNavigate: @escaping () -> Void = {}) { - // Comments-thread context: fetch with parentItemId, then inject the local-only - // ChannelMsgInfo carrier into the returned ChatInfo (the wire shape has no - // channelMsgInfo field; the carrier is set here so views can read it from cInfo). - // The owner-side parent may briefly lack itemSharedMsgId during send — guard against that. + // Comments-thread context: fetch with parentItemId, then stash the + // local-only ChannelMsgInfo carrier on the secondary ItemsModel itself. + // The main `chats` array stays unmutated — views that need parent/ + // sharedId read it from `secondaryIM.channelMsgInfo` instead. + // The owner-side parent may briefly lack itemSharedMsgId during send. if case let .groupChannelMsgContext(parent) = chatFilter { guard let sharedId = parent.meta.itemSharedMsgId else { return } let im = ItemsModel(secondaryIMFilter: chatFilter) + im.channelMsgInfo = ChannelMsgInfo(channelMsgItem: parent, channelMsgSharedId: sharedId) ChatModel.shared.secondaryIM = im Task { do { @@ -130,15 +137,9 @@ class ItemsModel: ObservableObject { pagination: .last(count: loadItemsPerPage), parentItemId: parent.id ) - let rewritten = injectChannelMsgInfo(chat.chatInfo, parent: parent, sharedId: sharedId) await MainActor.run { im.reversedChatItems = chat.chatItems.reversed() ChatModel.shared.chatId = chatId - // Replace the chat in the model with the rewritten info so views - // querying secondaryIM.cInfo see the injected ChannelMsgInfo. - if let i = ChatModel.shared.getChatIndex(chatId) { - ChatModel.shared.chats[i].chatInfo = rewritten - } willNavigate() } } catch { @@ -152,20 +153,6 @@ class ItemsModel: ObservableObject { im.loadOpenChat(chatId, willNavigate: willNavigate) } - // Rewrites a ChatInfo.group's third associated value to embed the comments-thread - // carrier. Other ChatInfo cases (direct, local, contactRequest, contactConnection) - // are not comment contexts and pass through unchanged. - private static func injectChannelMsgInfo(_ cInfo: ChatInfo, parent: ChatItem, sharedId: String) -> ChatInfo { - if case let .group(groupInfo, groupChatScope, _) = cInfo { - return .group( - groupInfo: groupInfo, - groupChatScope: groupChatScope, - channelMsgInfo: ChannelMsgInfo(channelMsgItem: parent, channelMsgSharedId: sharedId) - ) - } - return cInfo - } - // Spec: spec/state.md#loadOpenChat func loadOpenChat(_ chatId: ChatId, willNavigate: @escaping () -> Void = {}) { navigationTimeoutTask?.cancel() @@ -708,9 +695,12 @@ final class ChatModel: ObservableObject { } updateChatInfo(cInfo) // update chat list + // Comments are routed by ci.parentChatItemId (the wire shape has no + // channelMsgInfo). A non-nil parentChatItemId means this item is a + // comment, which must NOT update the main chat preview/unread count. if let i = getChatIndex(cInfo.id) { // update preview - if (cInfo.groupChatScope() == nil && cInfo.channelMsgInfo() == nil) || cInfo.groupInfo?.membership.memberPending ?? false { + if (cInfo.groupChatScope() == nil && cItem.parentChatItemId == nil) || cInfo.groupInfo?.membership.memberPending ?? false { chats[i].chatItems = switch cInfo { case .group: if let currentPreviewItem = chats[i].chatItems.first { @@ -732,7 +722,7 @@ final class ChatModel: ObservableObject { // pop chat popChatCollector.throttlePopChat(cInfo.id, currentPosition: i) } else { - if cInfo.groupChatScope() == nil && cInfo.channelMsgInfo() == nil { + if cInfo.groupChatScope() == nil && cItem.parentChatItemId == nil { addChat(Chat(chatInfo: cInfo, chatItems: [cItem])) } else { addChat(Chat(chatInfo: cInfo, chatItems: [])) @@ -762,9 +752,10 @@ final class ChatModel: ObservableObject { default: nil } - } else if cInfo.channelMsgInfo() != nil { - // Comments thread open. Inbound items lack channelMsgInfo on the wire (the - // carrier is local-only), so route by ci.parentChatItemId == parent.id. + } else if ci.parentChatItemId != nil { + // Comment item (parent is set). Route into the comments thread + // model if it's open and the parent matches; otherwise drop — + // comments must not leak into the main chat preview. switch secondaryIM?.secondaryIMFilter { case let .some(.groupChannelMsgContext(parent)): (cInfo.id == chatId && ci.parentChatItemId == parent.id) ? secondaryIM : nil @@ -779,7 +770,7 @@ final class ChatModel: ObservableObject { func upsertChatItem(_ cInfo: ChatInfo, _ cItem: ChatItem) -> Bool { // update chat list var itemAdded: Bool = false - if cInfo.groupChatScope() == nil && cInfo.channelMsgInfo() == nil { + if cInfo.groupChatScope() == nil && cItem.parentChatItemId == nil { if let chat = getChat(cInfo.id) { if let pItem = chat.chatItems.last { if pItem.id == cItem.id || (chatId == cInfo.id && im.reversedChatItems.first(where: { $0.id == cItem.id }) == nil) { @@ -850,7 +841,7 @@ final class ChatModel: ObservableObject { func removeChatItem(_ cInfo: ChatInfo, _ cItem: ChatItem) { // update chat list - if cInfo.groupChatScope() == nil && cInfo.channelMsgInfo() == nil { + if cInfo.groupChatScope() == nil && cItem.parentChatItemId == nil { if cItem.isRcvNew { unreadCollector.changeUnreadCounter(cInfo.id, by: -1, unreadMentions: cItem.meta.userMention ? -1 : 0) } diff --git a/apps/ios/Shared/Model/SimpleXAPI.swift b/apps/ios/Shared/Model/SimpleXAPI.swift index 586b2d8370..63c7368a7d 100644 --- a/apps/ios/Shared/Model/SimpleXAPI.swift +++ b/apps/ios/Shared/Model/SimpleXAPI.swift @@ -542,22 +542,18 @@ func apiReorderChatTags(tagIds: [Int64]) async throws { try await sendCommandOkResp(.apiReorderChatTags(tagIds: tagIds)) } -func apiSendMessages(type: ChatType, id: Int64, scope: GroupChatScope?, sendAsGroup: Bool = false, live: Bool = false, ttl: Int? = nil, composedMessages: [ComposedMessage]) async -> [ChatItem]? { - let cmd: ChatCommand = .apiSendMessages(type: type, id: id, scope: scope, sendAsGroup: sendAsGroup, live: live, ttl: ttl, composedMessages: composedMessages) +// Send messages into a chat. For comments on a channel post, pass `parentItemId` +// (the local ChatItemId of the parent post); scope and parentItemId are mutually +// exclusive — the Haskell handler rejects messages that set both. +func apiSendMessages(type: ChatType, id: Int64, scope: GroupChatScope?, parentItemId: Int64? = nil, sendAsGroup: Bool = false, live: Bool = false, ttl: Int? = nil, composedMessages: [ComposedMessage]) async -> [ChatItem]? { + let cmd: ChatCommand = .apiSendMessages(type: type, id: id, scope: scope, parentItemId: parentItemId, sendAsGroup: sendAsGroup, live: live, ttl: ttl, composedMessages: composedMessages) return await processSendMessageCmd(toChatType: type, cmd: cmd) } -// Send a comment on a channel post. The target chat is always a channel (group with useRelays); -// the recipient is the relay, which forwards the comment to other subscribers. -// Returns the locally-created ChatItem(s) for the sender's view; typically a singleton. -func apiSendComment(groupId: Int64, parentItemId: Int64, live: Bool = false, ttl: Int? = nil, composedMessages: [ComposedMessage]) async -> [ChatItem]? { - let cmd: ChatCommand = .apiSendComment(groupId: groupId, parentItemId: parentItemId, live: live, ttl: ttl, composedMessages: composedMessages) - return await processSendMessageCmd(toChatType: .group, cmd: cmd) -} - // Toggle commentsDisabled on a channel post via XMsgUpdate.prefs. The Haskell handler -// returns CRCmdOk; the local parent ChatItem update arrives via the owner's own echo -// of XMsgUpdate.prefs through the standard receive pipeline. No item returned here. +// returns CRCmdOk; the owner's UI reconciles via the CEvtChatItemUpdated event emitted +// by the command handler after the broadcast (the owner does not receive their own +// broadcast back). No item returned here. func apiSetCommentsDisabled(groupId: Int64, parentItemId: Int64, disabled: Bool) async throws { try await sendCommandOkResp(.apiSetCommentsDisabled(groupId: groupId, parentItemId: parentItemId, disabled: disabled)) } diff --git a/apps/ios/spec/api.md b/apps/ios/spec/api.md index 62a8a532d5..5c226f6e60 100644 --- a/apps/ios/spec/api.md +++ b/apps/ios/spec/api.md @@ -84,9 +84,8 @@ The `ChatCommand` enum ([`AppAPITypes.swift` L15](../Shared/Model/AppAPITypes.sw | `apiGetChat` | `chatId, scope, contentTag, pagination, search, parentItemId` | Get messages for a chat. When `parentItemId` is non-nil, returns the comments thread under that channel post (mutually exclusive with `scope`). | [L45](../Shared/Model/AppAPITypes.swift#L45) | | `apiGetChatContentTypes` | `chatId, scope` | Get content type counts for a chat | [L46](../Shared/Model/AppAPITypes.swift#L46) | | `apiGetChatItemInfo` | `type, id, scope, itemId` | Get detailed info for a message | [L47](../Shared/Model/AppAPITypes.swift#L47) | -| `apiSendMessages` | `type, id, scope, sendAsGroup, live, ttl, composedMessages` | Send one or more messages; `sendAsGroup` sends as channel owner | [L48](../Shared/Model/AppAPITypes.swift#L48) | -| `apiSendComment` | `groupId, parentItemId, live, ttl, composedMessages` | Send one or more comments under a channel post. Maps to Haskell `/_comment # `; requires `useRelays' gInfo`. Outbound: only structural and content validation runs locally (quoted items must be in the same comment section). Authorization (role gate, `commentsDisabled` gate) is enforced at the receiver via the same XMsgNew/XMsgUpdate inbound gates that protect channel posts. | [L49](../Shared/Model/AppAPITypes.swift#L49) | -| `apiSetCommentsDisabled` | `groupId, parentItemId, disabled` | Per-post comments override. Maps to Haskell `/_comments_disabled # on|off`; requires `useRelays' gInfo` and caller role at or above `GRModerator`. Returns `CRCmdOk` (the Swift wrapper is `async throws -> Void`). | [L50](../Shared/Model/AppAPITypes.swift#L50) | +| `apiSendMessages` | `type, id, scope, parentItemId, sendAsGroup, live, ttl, composedMessages` | Send one or more messages. `sendAsGroup` sends as channel owner. `parentItemId` (mutually exclusive with `scope`) makes the messages comments under the channel post with that id; requires `useRelays' gInfo`. Quoted items must be in the same comment section. Role and `commentsDisabled` gates are enforced on the wire path. | [L48](../Shared/Model/AppAPITypes.swift#L48) | +| `apiSetCommentsDisabled` | `groupId, parentItemId, disabled` | Per-post comments override. Maps to Haskell `/_comments_disabled # on|off`; requires `useRelays' gInfo` and caller role at or above `GRModerator`. Returns `CRCmdOk` (the Swift wrapper is `async throws -> Void`); the owner's UI reconciles via the `CEvtChatItemUpdated` event emitted by the command handler after the broadcast. | [L49](../Shared/Model/AppAPITypes.swift#L49) | | `apiCreateChatItems` | `noteFolderId, composedMessages` | Create items in notes folder | [L56](../Shared/Model/AppAPITypes.swift#L56) | | `apiUpdateChatItem` | `type, id, scope, itemId, updatedMessage, live` | Edit a sent message | [L58](../Shared/Model/AppAPITypes.swift#L58) | | `apiDeleteChatItem` | `type, id, scope, itemIds, mode` | Delete messages | [L59](../Shared/Model/AppAPITypes.swift#L59) | diff --git a/apps/ios/spec/state.md b/apps/ios/spec/state.md index 6c0bf41089..e1e87c6249 100644 --- a/apps/ios/spec/state.md +++ b/apps/ios/spec/state.md @@ -249,13 +249,13 @@ enum SecondaryItemsModelFilter { Opening a comments thread bypasses the standard `loadOpenChat` flow because the comments scope is not represented on the wire as a `GroupChatScope`. Instead: -1. The caller invokes [`ItemsModel.loadSecondaryChat`](../Shared/Model/ChatModel.swift#L116-L153) with `chatFilter: .groupChannelMsgContext(parent:)`. +1. The caller invokes `ItemsModel.loadSecondaryChat` with `chatFilter: .groupChannelMsgContext(parent:)`. 2. Items are fetched via `apiGetChat(..., parentItemId: parent.id)` (see `apiGetChat` in [spec/api.md §2.3](api.md#23-chat--message-operations)). -3. The returned `ChatInfo.group` has its third associated value rewritten by [`injectChannelMsgInfo`](../Shared/Model/ChatModel.swift#L158-L167) to embed a local-only [`ChannelMsgInfo`](../SimpleXChat/ChatTypes.swift#L2028-L2036) carrier so toolbar and routing logic can read the parent without a second lookup. -4. [`getCIItemsModel`](../Shared/Model/ChatModel.swift#L747-L777) gains a new branch (`cInfo.channelMsgInfo() != nil`) that routes events into the secondary `ItemsModel` when `ci.parentChatItemId == parent.id`. -5. Gating sites that previously checked `cInfo.groupChatScope() == nil` to decide whether to update the main chat preview now also check `cInfo.channelMsgInfo() == nil`, so comment items do not bubble into the main chat list. +3. `loadSecondaryChat` stashes a local-only `ChannelMsgInfo` (parent + sharedId) on `ItemsModel.channelMsgInfo` of the secondary model so toolbar and routing logic can read the parent without a second lookup. The main `chats` array is left unmutated — comments-thread context lives entirely on the secondary `ItemsModel`. +4. `getCIItemsModel` routes inbound items into the secondary `ItemsModel` when `ci.parentChatItemId != nil` matches `parent.id`. The wire shape has no `channelMsgInfo` field, so routing must key on `ci.parentChatItemId`. +5. Gating sites that previously also checked `cInfo.channelMsgInfo() == nil` to decide whether to update the main chat preview now check `cItem.parentChatItemId == nil`, so comment items do not bubble into the main chat list, the unread counter, or the deletion preview cleanup. -The carrier is **not** serialized: `ChannelMsgInfo` is `Decodable` only so it nests cleanly into `ChatInfo.group`, but the server never sends this field — it is injected exclusively by `loadSecondaryChat`. Inbound `XMsgNew` items with a `parent` come back over the regular event stream; they are routed by `ci.parentChatItemId` alone. +`ChannelMsgInfo` is a local-only carrier — the server never sends it. Inbound `XMsgNew` items with a `parent` arrive on the regular event stream and are routed entirely by `ci.parentChatItemId`. --- diff --git a/apps/simplex-directory-service/src/Directory/Service.hs b/apps/simplex-directory-service/src/Directory/Service.hs index 6e414ef011..f5741f7e66 100644 --- a/apps/simplex-directory-service/src/Directory/Service.hs +++ b/apps/simplex-directory-service/src/Directory/Service.hs @@ -665,7 +665,7 @@ directoryServiceEvent st opts@DirectoryOpts {adminUsers, superUsers, serviceName mc <- getCaptchaContent s sendComposedMessages_ cc sendRef [(quotedId, MCText noticeText), (Nothing, mc)] where - sendRef = SRGroup groupId (Just $ GCSMemberSupport (Just gmId)) False + sendRef = SRGroup groupId (Just $ GCSMemberSupport (Just gmId)) Nothing False gmId = groupMemberId' m sendVoiceCaptcha :: SendRef -> String -> IO () @@ -715,7 +715,7 @@ directoryServiceEvent st opts@DirectoryOpts {adminUsers, superUsers, serviceName dePendingMemberMsg g@GroupInfo {groupId, groupProfile = GroupProfile {displayName = n}} m@GroupMember {memberProfile = LocalProfile {displayName}} ciId msgText | memberRequiresCaptcha a m = do let gmId = groupMemberId' m - sendRef = SRGroup groupId (Just $ GCSMemberSupport (Just gmId)) False + sendRef = SRGroup groupId (Just $ GCSMemberSupport (Just gmId)) Nothing False -- /audio is matched as text, not as DirectoryCmd, because it is only valid -- in group context at captcha stage, while DirectoryCmd is for DM commands. isAudioCmd = T.strip msgText == "/audio" @@ -751,7 +751,7 @@ directoryServiceEvent st opts@DirectoryOpts {adminUsers, superUsers, serviceName a = groupMemberAcceptance g rejectPendingMember rjctNotice = do let gmId = groupMemberId' m - sendComposedMessages cc (SRGroup groupId (Just $ GCSMemberSupport (Just gmId)) False) [MCText rjctNotice] + sendComposedMessages cc (SRGroup groupId (Just $ GCSMemberSupport (Just gmId)) Nothing False) [MCText rjctNotice] sendChatCmd cc (APIRemoveMembers groupId [gmId] False) >>= \case Right (CRUserDeletedMembers _ _ (_ : _) _ _) -> do atomically $ TM.delete gmId $ pendingCaptchas env diff --git a/simplex-chat.cabal b/simplex-chat.cabal index 4ecd726394..928b2a30fb 100644 --- a/simplex-chat.cabal +++ b/simplex-chat.cabal @@ -130,10 +130,10 @@ library Simplex.Chat.Store.Postgres.Migrations.M20260122_has_link Simplex.Chat.Store.Postgres.Migrations.M20260222_chat_relays Simplex.Chat.Store.Postgres.Migrations.M20260403_item_viewed - Simplex.Chat.Store.Postgres.Migrations.M20260407_channel_comments Simplex.Chat.Store.Postgres.Migrations.M20260429_relay_request_retries Simplex.Chat.Store.Postgres.Migrations.M20260507_relay_inactive_at Simplex.Chat.Store.Postgres.Migrations.M20260514_relay_request_group_link_index + Simplex.Chat.Store.Postgres.Migrations.M20260516_channel_comments else exposed-modules: Simplex.Chat.Archive @@ -286,10 +286,10 @@ library Simplex.Chat.Store.SQLite.Migrations.M20260122_has_link Simplex.Chat.Store.SQLite.Migrations.M20260222_chat_relays Simplex.Chat.Store.SQLite.Migrations.M20260403_item_viewed - Simplex.Chat.Store.SQLite.Migrations.M20260407_channel_comments Simplex.Chat.Store.SQLite.Migrations.M20260429_relay_request_retries Simplex.Chat.Store.SQLite.Migrations.M20260507_relay_inactive_at Simplex.Chat.Store.SQLite.Migrations.M20260514_relay_request_group_link_index + Simplex.Chat.Store.SQLite.Migrations.M20260516_channel_comments other-modules: Paths_simplex_chat hs-source-dirs: diff --git a/src/Simplex/Chat/Controller.hs b/src/Simplex/Chat/Controller.hs index 994abafbdf..5ff49f5580 100644 --- a/src/Simplex/Chat/Controller.hs +++ b/src/Simplex/Chat/Controller.hs @@ -329,7 +329,6 @@ data ChatCommand | APIGetChatItems {chatPagination :: ChatPagination, search :: Maybe Text} | APIGetChatItemInfo {chatRef :: ChatRef, chatItemId :: ChatItemId} | APISendMessages {sendRef :: SendRef, liveMessage :: Bool, ttl :: Maybe Int, composedMessages :: NonEmpty ComposedMessage} - | APISendComment {groupId :: GroupId, parentChatItemId :: ChatItemId, liveMessage :: Bool, ttl :: Maybe Int, composedMessages :: NonEmpty ComposedMessage} | APISetCommentsDisabled {groupId :: GroupId, parentChatItemId :: ChatItemId, disabled :: Bool} | APICreateChatTag ChatTagData | APISetChatTags ChatRef (Maybe (NonEmpty ChatTagId)) @@ -992,7 +991,10 @@ logEventToFile = \case data SendRef = SRDirect ContactId - | SRGroup GroupId (Maybe GroupChatScope) ShowGroupAsSender + | -- | Send into a group. `parentChatItemId` (when set) makes this a comment + -- on a channel post and is mutually exclusive with `scope`. `asGroup` + -- (showGroupAsSender) is for channel-post sends by the owner. + SRGroup GroupId (Maybe GroupChatScope) (Maybe ChatItemId) ShowGroupAsSender deriving (Eq, Show) data ChatPagination diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 10fd768502..30502de904 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -625,7 +625,9 @@ processChatCommand vr nm = \case mapM_ assertNoMentions cms withContactLock "sendMessage" chatId $ sendContactContentMessages user chatId live itemTTL (L.map composedMessageReq cms) - SRGroup chatId gsScope asGroup -> do + SRGroup chatId gsScope parentItemId_ asGroup -> do + when (isJust gsScope && isJust parentItemId_) $ + throwCmdError "scope and parent are mutually exclusive" case gsScope of Just (GCSMemberSupport _) -> when asGroup $ throwCmdError "cannot send as group in support scope" Nothing -> pure () @@ -633,24 +635,17 @@ processChatCommand vr nm = \case (gInfo, cmrs) <- withFastStore $ \db -> do g <- getGroupInfo db vr user chatId (g,) <$> mapM (composedMessageReqMentions db user g) cms - sendGroupContentMessages user gInfo gsScope asGroup live itemTTL cmrs - APISendComment groupId parentItemId live itemTTL cms -> withUser $ \user -> do - mapM_ assertAllowedContent' cms - withGroupLock "sendComment" groupId $ do - gInfo <- withFastStore $ \db -> getGroupInfo db vr user groupId - unless (useRelays' gInfo) $ throwCmdError "comments are only supported in channel groups" - (channelMsgInfo, cmrs) <- withFastStore $ \db -> do - cmi <- getChannelMsgInfo db user groupId parentItemId - cmrs' <- mapM (composedMessageReqMentions db user gInfo) cms - pure (cmi, cmrs') - -- Validate that quoted items belong to the same comment section. - forM_ cms $ \ComposedMessage {quotedItemId} -> - forM_ quotedItemId $ \qId -> - unlessM (withFastStore' $ \db -> quotedItemInCommentSection db parentItemId qId) $ - throwCmdError "quoted item does not belong to the same comment section" - assertMultiSendable live cmrs - recipients <- getGroupRecipients vr user gInfo Nothing groupKnockingVersion - sendGroupContentMessages_ user gInfo Nothing False Nothing (Just channelMsgInfo) recipients live itemTTL cmrs + case parentItemId_ of + Nothing -> sendGroupContentMessages user gInfo gsScope asGroup Nothing live itemTTL cmrs + Just parentItemId -> do + unless (useRelays' gInfo) $ throwCmdError "comments are only supported in channel groups" + channelMsgInfo <- withFastStore $ \db -> getChannelMsgInfo db user chatId parentItemId + -- Validate that quoted items belong to the same comment section. + forM_ cms $ \ComposedMessage {quotedItemId} -> + forM_ quotedItemId $ \qId -> + unlessM (withFastStore' $ \db -> quotedItemInCommentSection db parentItemId qId) $ + throwCmdError "quoted item does not belong to the same comment section" + sendGroupContentMessages user gInfo Nothing False (Just channelMsgInfo) live itemTTL cmrs APISetCommentsDisabled groupId parentItemId disabled -> withUser $ \user -> withGroupLock "setCommentsDisabled" groupId $ do gInfo <- withFastStore $ \db -> getGroupInfo db vr user groupId @@ -662,10 +657,15 @@ processChatCommand vr nm = \case case channelMsgInfo of ChannelMsgInfo {channelMsgItem = CChatItem _ ChatItem {content = ciContent}, channelMsgSharedId} -> forM_ (ciMsgContent ciContent) $ \mc -> do - let prefs = MsgPrefs {commentsDisabled = disabled} + let prefs = MsgPrefs {commentsDisabled = Just disabled, commentsTotal = Nothing} chatMsgEvent = XMsgUpdate channelMsgSharedId mc M.empty Nothing Nothing Nothing (Just True) (Just prefs) recipients <- getGroupRecipients vr user gInfo Nothing groupKnockingVersion void $ sendGroupMessages user gInfo Nothing False recipients (chatMsgEvent :| []) + -- Owner's UI reconciles via this event. The owner does not receive their + -- own broadcast back, so the standard receive pipeline would not emit + -- CEvtChatItemUpdated here. Re-read the parent and emit directly. + CChatItem md updatedCi <- withFastStore $ \db -> getGroupChatItem db user groupId parentItemId + toView $ CEvtChatItemUpdated user (AChatItem SCTGroup md (GroupChat gInfo Nothing) updatedCi) ok user APICreateChatTag (ChatTagData emoji text) -> withUser $ \user -> withFastStore' $ \db -> do _ <- createChatTag db user emoji text @@ -695,7 +695,7 @@ processChatCommand vr nm = \case gInfo <- withFastStore $ \db -> getGroupInfo db vr user gId let mc = MCReport reportText reportReason cm = ComposedMessage {fileSource = Nothing, quotedItemId = Just reportedItemId, msgContent = mc, mentions = M.empty} - sendGroupContentMessages user gInfo (Just $ GCSMemberSupport Nothing) False False Nothing [composedMessageReq cm] + sendGroupContentMessages user gInfo (Just $ GCSMemberSupport Nothing) False Nothing False Nothing [composedMessageReq cm] ReportMessage {groupName, contactName_, reportReason, reportedMessage} -> withUser $ \user -> do gId <- withFastStore $ \db -> getGroupIdByName db user groupName reportedItemId <- withFastStore $ \db -> getGroupChatItemIdByText db user gId contactName_ reportedMessage @@ -991,7 +991,7 @@ processChatCommand vr nm = \case Just cmrs' -> withGroupLock "forwardChatItem, to group" toChatId $ do gInfo <- withFastStore $ \db -> getGroupInfo db vr user toChatId - sendGroupContentMessages user gInfo toScope sendAsGroup False itemTTL cmrs' + sendGroupContentMessages user gInfo toScope sendAsGroup Nothing False itemTTL cmrs' Nothing -> pure $ CRNewChatItems user [] CTLocal -> do cmrs <- prepareForward user @@ -1149,7 +1149,7 @@ processChatCommand vr nm = \case ct <- withFastStore $ \db -> getContact db vr u contactId forM (contactConn ct) $ \conn -> (CBDirect,) <$> withAgent (`getConnectionRatchetAdHash` aConnId conn) - SRGroup toGroupId _ asGroup -> do + SRGroup toGroupId _ _ asGroup -> do GroupInfo {groupProfile = GroupProfile {publicGroup}, membership = m} <- withFastStore $ \db -> getGroupInfo db vr u toGroupId pure $ mkBinding m <$> publicGroup where @@ -2378,7 +2378,7 @@ processChatCommand vr nm = \case ChatRef CTDirect ctId _ -> pure $ SRDirect ctId ChatRef CTGroup gId scope_ -> do gInfo <- withFastStore $ \db -> getGroupInfo db vr user gId - pure $ SRGroup gId scope_ (useRelays' gInfo) + pure $ SRGroup gId scope_ Nothing (useRelays' gInfo) _ -> throwCmdError "unsupported share target" processChatCommand vr nm (APIShareChatMsgContent (ChatRef CTGroup groupId Nothing) sendRef) >>= \case CRChatMsgContent _ mc -> @@ -2410,7 +2410,7 @@ processChatCommand vr nm = \case forM scope_ $ \(GSNMemberSupport mName_) -> GCSMemberSupport <$> mapM (getGroupMemberIdByName db user gId) mName_ (gInfo, cScope_,) <$> liftIO (getMessageMentions db user gId msg) - let sendRef = SRGroup (groupId' gInfo) cScope_ (sendAsGroup' gInfo cScope_) + let sendRef = SRGroup (groupId' gInfo) cScope_ Nothing (sendAsGroup' gInfo cScope_) processChatCommand vr nm $ APISendMessages sendRef False Nothing [ComposedMessage Nothing Nothing mc mentions] SNLocal -> do folderId <- withFastStore (`getUserNoteFolderId` user) @@ -3210,7 +3210,7 @@ processChatCommand vr nm = \case qiId <- getGroupChatItemIdByText db user gId cName quotedMsg (gInfo, qiId,) <$> liftIO (getMessageMentions db user gId msg) let mc = MCText msg - processChatCommand vr nm $ APISendMessages (SRGroup (groupId' gInfo) Nothing (sendAsGroup' gInfo Nothing)) False Nothing [ComposedMessage Nothing (Just quotedItemId) mc mentions] + processChatCommand vr nm $ APISendMessages (SRGroup (groupId' gInfo) Nothing Nothing (sendAsGroup' gInfo Nothing)) False Nothing [ComposedMessage Nothing (Just quotedItemId) mc mentions] ClearNoteFolder -> withUser $ \user -> do folderId <- withFastStore (`getUserNoteFolderId` user) processChatCommand vr nm $ APIClearChat (ChatRef CTLocal folderId Nothing) @@ -4389,12 +4389,12 @@ processChatCommand vr nm = \case quoteData ChatItem {content = CISndMsgContent qmc} = pure (qmc, CIQDirectSnd, True) quoteData ChatItem {content = CIRcvMsgContent qmc} = pure (qmc, CIQDirectRcv, False) quoteData _ = throwError SEInvalidQuote - sendGroupContentMessages :: User -> GroupInfo -> Maybe GroupChatScope -> ShowGroupAsSender -> Bool -> Maybe Int -> NonEmpty ComposedMessageReq -> CM ChatResponse - sendGroupContentMessages user gInfo scope showGroupAsSender live itemTTL cmrs = do + sendGroupContentMessages :: User -> GroupInfo -> Maybe GroupChatScope -> ShowGroupAsSender -> Maybe ChannelMsgInfo -> Bool -> Maybe Int -> NonEmpty ComposedMessageReq -> CM ChatResponse + sendGroupContentMessages user gInfo scope showGroupAsSender channelMsgInfo_ live itemTTL cmrs = do assertMultiSendable live cmrs chatScopeInfo <- mapM (getChatScopeInfo vr user) scope recipients <- getGroupRecipients vr user gInfo chatScopeInfo modsCompatVersion - sendGroupContentMessages_ user gInfo scope showGroupAsSender chatScopeInfo Nothing recipients live itemTTL cmrs + sendGroupContentMessages_ user gInfo scope showGroupAsSender chatScopeInfo channelMsgInfo_ recipients live itemTTL cmrs where hasReport = any (\(ComposedMessage {msgContent}, _, _, _) -> isReport msgContent) cmrs modsCompatVersion = if hasReport then contentReportsVersion else groupKnockingVersion @@ -4462,12 +4462,20 @@ processChatCommand vr nm = \case (fInv, ciFile) <- xftpSndFileTransfer user file fileSize n $ CGGroup gInfo recipients pure (Just fInv, Just ciFile) Nothing -> pure (Nothing, Nothing) + -- Channel posts (showGroupAsSender = True, no comment parent) carry + -- an initial MsgPrefs so new joiners receiving the original XMsgNew + -- learn the starting state (commentsDisabled = False, commentsTotal = 0). + -- Comments and regular messages carry no prefs. + prefs_ :: Maybe MsgPrefs + prefs_ = case (showGroupAsSender, channelMsgInfo_) of + (True, Nothing) -> Just MsgPrefs {commentsDisabled = Just False, commentsTotal = Just 0} + _ -> Nothing prepareMsgs :: NonEmpty (ComposedMessageReq, Maybe FileInvitation) -> Maybe CITimed -> CM (NonEmpty (ChatMsgEvent 'Json, Maybe (CIQuote 'CTGroup))) prepareMsgs cmsFileInvs timed_ = withFastStore $ \db -> forM cmsFileInvs $ \((ComposedMessage {quotedItemId, msgContent = mc}, itemForwarded, _, ciMentions), fInv_) -> let msgScope = toMsgScope gInfo <$> chatScopeInfo mentions = M.map (\CIMention {memberId} -> MsgMention {memberId}) ciMentions - in prepareGroupMsg db user gInfo msgScope parentRef_ showGroupAsSender mc mentions quotedItemId itemForwarded fInv_ timed_ live + in prepareGroupMsg db user gInfo msgScope parentRef_ prefs_ showGroupAsSender mc mentions quotedItemId itemForwarded fInv_ timed_ live createMemberSndStatuses :: [Either ChatError (ChatItem 'CTGroup 'MDSnd)] -> NonEmpty (Either ChatError SndMessage) -> @@ -4621,7 +4629,7 @@ processChatCommand vr nm = \case ChatRef CTDirect cId _ -> a $ SRDirect cId ChatRef CTGroup gId scope -> do gInfo <- withFastStore $ \db -> getGroupInfo db vr user gId - a $ SRGroup gId scope (sendAsGroup' gInfo scope) + a $ SRGroup gId scope Nothing (sendAsGroup' gInfo scope) _ -> throwCmdError "not supported" getSharedMsgId :: CM SharedMsgId getSharedMsgId = do @@ -5034,7 +5042,6 @@ chatCommandP = "/_get items " *> (APIGetChatItems <$> chatPaginationP <*> optional (" search=" *> textP)), "/_get item info " *> (APIGetChatItemInfo <$> chatRefP <* A.space <*> A.decimal), "/_send " *> (APISendMessages <$> sendRefP <*> liveMessageP <*> sendMessageTTLP <*> (" json " *> jsonP <|> " text " *> composedMessagesTextP)), - "/_comment #" *> (APISendComment <$> A.decimal <* A.space <*> A.decimal <*> liveMessageP <*> sendMessageTTLP <*> (" json " *> jsonP <|> " text " *> composedMessagesTextP)), "/_comments_disabled #" *> (APISetCommentsDisabled <$> A.decimal <* A.space <*> A.decimal <* A.space <*> onOffP), "/_create tag " *> (APICreateChatTag <$> jsonP), "/_tags " *> (APISetChatTags <$> chatRefP <*> optional _strP), @@ -5496,7 +5503,8 @@ chatCommandP = cType -> (\chatId -> ChatRef cType chatId Nothing) <$> A.decimal sendRefP = (A.char '@' $> SRDirect <*> A.decimal) - <|> (A.char '#' $> SRGroup <*> A.decimal <*> optional gcScopeP <*> asGroupP) + <|> (A.char '#' $> SRGroup <*> A.decimal <*> optional gcScopeP <*> optional parentItemIdP <*> asGroupP) + parentItemIdP = " parent=" *> A.decimal asGroupP = ("(as_group=" *> onOffP <* A.char ')') <|> pure False gcScopeP = "(_support" *> (GCSMemberSupport <$> optional (A.char ':' *> A.decimal)) <* A.char ')' sendNameP = diff --git a/src/Simplex/Chat/Library/Internal.hs b/src/Simplex/Chat/Library/Internal.hs index 39a21251e5..d3e619d97e 100644 --- a/src/Simplex/Chat/Library/Internal.hs +++ b/src/Simplex/Chat/Library/Internal.hs @@ -201,8 +201,8 @@ toggleNtf m ntfOn = forM_ (memberConnId m) $ \connId -> withAgent (\a -> toggleConnectionNtfs a connId ntfOn) `catchAllErrors` eToView -prepareGroupMsg :: DB.Connection -> User -> GroupInfo -> Maybe MsgScope -> Maybe MsgRef -> ShowGroupAsSender -> MsgContent -> Map MemberName MsgMention -> Maybe ChatItemId -> Maybe CIForwardedFrom -> Maybe FileInvitation -> Maybe CITimed -> Bool -> ExceptT StoreError IO (ChatMsgEvent 'Json, Maybe (CIQuote 'CTGroup)) -prepareGroupMsg db user g@GroupInfo {membership} msgScope parentRef_ showGroupAsSender mc mentions quotedItemId_ itemForwarded fInv_ timed_ live = do +prepareGroupMsg :: DB.Connection -> User -> GroupInfo -> Maybe MsgScope -> Maybe MsgRef -> Maybe MsgPrefs -> ShowGroupAsSender -> MsgContent -> Map MemberName MsgMention -> Maybe ChatItemId -> Maybe CIForwardedFrom -> Maybe FileInvitation -> Maybe CITimed -> Bool -> ExceptT StoreError IO (ChatMsgEvent 'Json, Maybe (CIQuote 'CTGroup)) +prepareGroupMsg db user g@GroupInfo {membership} msgScope parentRef_ prefs_ showGroupAsSender mc mentions quotedItemId_ itemForwarded fInv_ timed_ live = do (mc', quotedItem_) <- case (quotedItemId_, itemForwarded) of (Nothing, Nothing) -> pure (mcSimple mc, Nothing) (Nothing, Just _) -> pure (mcForward mc, Nothing) @@ -216,7 +216,7 @@ prepareGroupMsg db user g@GroupInfo {membership} msgScope parentRef_ showGroupAs quotedItem = CIQuote {chatDir = qd, itemId = Just quotedItemId, sharedMsgId = itemSharedMsgId, sentAt = itemTs, content = qmc', formattedText = ft'} pure (mcQuote QuotedMsg {msgRef, content = qmc'} mc, Just quotedItem) (Just _, Just _) -> throwError SEInvalidQuote - let mc'' = mc' {mentions = MsgMentions mentions, file = fInv_, ttl = ttl' <$> timed_, live = justTrue live, scope = msgScope, asGroup = justTrue showGroupAsSender, parent = parentRef_} + let mc'' = mc' {mentions = MsgMentions mentions, file = fInv_, ttl = ttl' <$> timed_, live = justTrue live, scope = msgScope, asGroup = justTrue showGroupAsSender, parent = parentRef_, prefs = prefs_} pure (XMsgNew mc'', quotedItem_) where quoteData :: ChatItem c d -> GroupMember -> ExceptT StoreError IO (MsgContent, CIQDirection 'CTGroup, Bool, Maybe GroupMember) @@ -348,7 +348,7 @@ prohibitedGroupContent gInfo@GroupInfo {membership = mem@GroupMember {memberRole | not (useRelays' gInfo) -> Just GFComments | not (groupFeatureAllowed SGFComments gInfo) -> Just GFComments | isJust itemDeleted -> Just GFComments - | commentsDisabled -> Just GFComments + | isTrue commentsDisabled -> Just GFComments | otherwise -> Nothing Nothing -> Nothing where @@ -373,6 +373,14 @@ prohibitedGroupContent gInfo@GroupInfo {membership = mem@GroupMember {memberRole -- True iff the channel post's commenting window has expired. -- The group preference `comments.closeAfter` is the duration in seconds -- since post creation; `Nothing` means the window never closes. +-- +-- TODO: itemTs here is the receiver's local broker timestamp. Different +-- receivers see different itemTs for the same channel post, so the close +-- window starts at slightly different absolute times per recipient. Drift +-- is usually small but is unbounded for late joiners. A future fix would +-- broadcast a canonical createdAt on the wire (e.g. as a field on MsgPrefs +-- alongside commentsDisabled / commentsTotal) and measure the window +-- against that single owner-set time. commentsClosed :: GroupInfo -> Maybe ChannelMsgInfo -> UTCTime -> Bool commentsClosed GroupInfo {fullGroupPreferences = FullGroupPreferences {comments = CommentsGroupPreference {closeAfter}}} @@ -1299,14 +1307,21 @@ sendHistory user gInfo@GroupInfo {membership} m@GroupMember {activeConn = Just c if isNothing fInvDescr_ && not (msgContentHasText mc) then pure [] else do - let CIMeta {itemTs, itemSharedMsgId, itemTimed} = meta + let CIMeta {itemTs, itemSharedMsgId, itemTimed, commentsTotal, commentsDisabled} = meta quotedItemId_ = quoteItemId =<< quotedItem fInv_ = fst <$> fInvDescr_ (mc', _, mentions') = updatedMentionNames mc formattedText mentions mentions'' = M.map (\CIMention {memberId} -> MsgMention {memberId}) mentions' asGroup = isNothing sender_ + -- Channel posts in history replay carry the parent's current + -- comments state so new joiners see the right count and lock + -- status. Comments themselves are filtered out of history by + -- getGroupHistoryItems, so this only fires for parent posts. + prefs_ + | asGroup = Just MsgPrefs {commentsDisabled = Just (isTrue commentsDisabled), commentsTotal = Just (unIntDef0 commentsTotal)} + | otherwise = Nothing -- TODO [knocking] send history to other scopes too? - (chatMsgEvent, _) <- withStore $ \db -> prepareGroupMsg db user gInfo Nothing Nothing asGroup mc' mentions'' quotedItemId_ Nothing fInv_ itemTimed False + (chatMsgEvent, _) <- withStore $ \db -> prepareGroupMsg db user gInfo Nothing Nothing prefs_ asGroup mc' mentions'' quotedItemId_ Nothing fInv_ itemTimed False -- for channel messages default chat version range to membership range let senderVRange = maybe (memberChatVRange' membership) memberChatVRange' sender_ xMsgNewChatMsg = ChatMessage {chatVRange = senderVRange, msgId = itemSharedMsgId, chatMsgEvent} diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index c2c141f2de..c2f1100e7f 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -2075,10 +2075,16 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = newGroupContentMessage :: GroupInfo -> Maybe GroupMember -> MsgContainer -> RcvMessage -> UTCTime -> Bool -> CM (Maybe DeliveryTaskContext) newGroupContentMessage gInfo m_ mc msg@RcvMessage {sharedMsgId_} brokerTs forwarded = case m_ of - Nothing -> do - createContentItem gInfo Nothing Nothing Nothing - -- no delivery task - message already forwarded by relay - pure Nothing + Nothing + -- Comments must have an author (a member, not the channel-as-sender). + -- A relay-forwarded message without member identity but carrying + -- mc.parent is a protocol violation — drop it rather than silently + -- discarding the parent reference and storing it as a channel post. + | isJust parent_ -> messageError "channel comment without author (FwdChannel)" $> Nothing + | otherwise -> do + createContentItem gInfo Nothing Nothing Nothing + -- no delivery task - message already forwarded by relay + pure Nothing Just m@GroupMember {memberId} -> do (gInfo', m', scopeInfo) <- mkGetMessageChatScope vr user gInfo m content msgScope_ channelMsgInfo_ <- resolveCommentParent gInfo' parent_ @@ -2109,7 +2115,7 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = rejected gInfo' m' scopeInfo f = newChatItem gInfo' m' scopeInfo Nothing (ciContentNoParse $ CIRcvGroupFeatureRejected f) Nothing Nothing False timed_ gInfo' = if forwarded then rcvCITimed_ (Just Nothing) itemTTL else rcvGroupCITimed gInfo' itemTTL live' = fromMaybe False live_ - MsgContainer {content = c, mentions = MsgMentions mentions, file = fInv_, ttl = itemTTL, live = live_, scope = msgScope_, asGroup = asGroup_, parent = parent_} = mc + MsgContainer {content = c, mentions = MsgMentions mentions, file = fInv_, ttl = itemTTL, live = live_, scope = msgScope_, asGroup = asGroup_, parent = parent_, prefs = prefs_} = mc content = case c of MCChat {text, chatLink, ownerSig = Just LinkOwnerSig {chatBinding = B64UrlByteString binding}} -> case publicGroup of Just pgp | maybe False (binding ==) (expectedBinding pgp) -> c @@ -2173,10 +2179,34 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = newChatItem gInfo' m' scopeInfo channelMsgInfo'_ ciContent ciFile_ timed live = do let mentions' = if maybe False memberBlocked m' then M.empty else mentions (ci, cInfo) <- saveRcvCI gInfo' m' scopeInfo channelMsgInfo'_ ciContent ciFile_ timed live mentions' - ci' <- maybe (pure ci) (\m -> blockedMemberCI gInfo' m ci) m' + -- Channel posts (no parent ref, sent as group) carrying MsgPrefs on + -- XMsgNew: apply the per-post initial state to the just-inserted row + -- so new joiners see the correct commentsDisabled / commentsTotal + -- without needing to wait for an XMsgUpdate or comment-arrival. + ci'' <- case prefs_ of + Just MsgPrefs {commentsDisabled = mDisabled, commentsTotal = mTotal} + | isNothing channelMsgInfo'_ && sentAsGroup -> do + withStore' $ \db -> setChannelMsgInitialPrefs db (chatItemId' ci) mDisabled mTotal + pure $ patchInitialPrefs ci mDisabled mTotal + _ -> pure ci + ci' <- maybe (pure ci'') (\m -> blockedMemberCI gInfo' m ci'') m' let memberId_ = memberId' <$> m' reactions <- maybe (pure []) (\sharedMsgId -> withStore' $ \db -> getGroupCIReactions db gInfo' memberId_ sharedMsgId) sharedMsgId_ groupMsgToView cInfo ci' {reactions} + where + -- Mirror the DB writes in-memory so the CEvtChatItemNew event + -- delivered to the UI reflects the per-post prefs the receiver + -- just persisted. commentsTotal uses max to match the DB clamp. + patchInitialPrefs :: forall d'. ChatItem 'CTGroup d' -> Maybe Bool -> Maybe Int -> ChatItem 'CTGroup d' + patchInitialPrefs ci@ChatItem {meta = m0} mDisabled mTotal = + let CIMeta {commentsTotal = curTotal} = m0 + m1 = case mDisabled of + Just b -> (m0 :: CIMeta 'CTGroup d') {commentsDisabled = BoolDef b} + Nothing -> m0 + m2 = case mTotal of + Just n -> (m1 :: CIMeta 'CTGroup d') {commentsTotal = IntDef0 (max (unIntDef0 curTotal) n)} + Nothing -> m1 + in ci {meta = m2} groupMessageUpdate :: GroupInfo -> Maybe GroupMember -> SharedMsgId -> MsgContent -> Map MemberName MsgMention -> Maybe MsgScope -> RcvMessage -> UTCTime -> Maybe Int -> Maybe Bool -> Maybe Bool -> Maybe MsgPrefs -> CM (Maybe DeliveryTaskContext) groupMessageUpdate gInfo@GroupInfo {groupId} m_ sharedMsgId mc mentions msgScope_ msg@RcvMessage {msgId} brokerTs ttl_ live_ asGroup_ prefs_ @@ -2236,10 +2266,15 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = | otherwise -> messageError "x.msg.update: group member attempted to update a message of another member" $> Nothing CChatItem SMDRcv ci@ChatItem {chatDir = CIChannelRcv, meta = CIMeta {itemLive}, content = CIRcvMsgContent oldMC} | maybe True (\m -> memberRole' m == GROwner) m_ -> updateCI True ci scopeInfo oldMC itemLive Nothing - | isJust prefs_ && maybe False (\m -> memberRole' m >= GRModerator) m_ -> do - applied <- applyMsgPrefs ci - pure $ if applied then Just (infoToDeliveryContext gInfo scopeInfo True) else Nothing + | isJust prefs_ && maybe False (\m -> memberRole' m >= GRModerator) m_ -> applyModeratorPrefs SMDRcv ci scopeInfo | otherwise -> messageError "x.msg.update: member attempted to update channel message" $> Nothing + -- Owner's local copy of their own channel post is stored as SMDSnd / CIGroupSnd + -- with showGroupAsSender = True. A moderator's broadcast of XMsgUpdate {prefs} + -- must apply on the owner's side too, so this mirrors the SMDRcv/CIChannelRcv + -- moderator branch above. + CChatItem SMDSnd ci@ChatItem {chatDir = CIGroupSnd, meta = CIMeta {showGroupAsSender = True}, content = CISndMsgContent _} + | isJust prefs_ && maybe False (\m -> memberRole' m >= GRModerator) m_ -> applyModeratorPrefs SMDSnd ci scopeInfo + | otherwise -> messageError "x.msg.update: only moderators may update channel-post prefs" $> Nothing _ -> messageError "x.msg.update: invalid message update" $> Nothing where isSender m' = maybe False (\m -> sameMemberId (memberId' m) m') m_ @@ -2256,23 +2291,36 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = ciMentions <- getRcvCIMentions db user gInfo ft_ mentions ci' <- updateGroupChatItem db user groupId ci {reactions} content edited live $ Just msgId updateGroupCIMentions db gInfo ci' ciMentions - applyMsgPrefs ci' - toView $ CEvtChatItemUpdated user (AChatItem SCTGroup SMDRcv (GroupChat gInfo scopeInfo) ci') - startUpdatedTimedItemThread user (ChatRef CTGroup groupId $ toChatScope <$> scopeInfo) ci ci' + ci'' <- fromMaybe ci' <$> applyMsgPrefs ci' + toView $ CEvtChatItemUpdated user (AChatItem SCTGroup SMDRcv (GroupChat gInfo scopeInfo) ci'') + startUpdatedTimedItemThread user (ChatRef CTGroup groupId $ toChatScope <$> scopeInfo) ci ci'' pure $ Just $ infoToDeliveryContext gInfo scopeInfo showGroupAsSender - else do - prefsApplied <- applyMsgPrefs ci - if prefsApplied - then pure $ Just $ infoToDeliveryContext gInfo scopeInfo showGroupAsSender - else do + else + applyMsgPrefs ci >>= \case + Just ci' -> do + toView $ CEvtChatItemUpdated user (AChatItem SCTGroup SMDRcv (GroupChat gInfo scopeInfo) ci') + pure $ Just $ infoToDeliveryContext gInfo scopeInfo showGroupAsSender + Nothing -> do toView $ CEvtChatItemNotChanged user (AChatItem SCTGroup SMDRcv (GroupChat gInfo scopeInfo) ci) pure Nothing - applyMsgPrefs :: ChatItem 'CTGroup 'MDRcv -> CM Bool - applyMsgPrefs ci = case prefs_ of - Just MsgPrefs {commentsDisabled} -> do - withStore' $ \db -> setChannelMsgCommentsDisabled db (chatItemId' ci) commentsDisabled - pure True - Nothing -> pure False + applyModeratorPrefs :: MsgDirectionI d => SMsgDirection d -> ChatItem 'CTGroup d -> Maybe GroupChatScopeInfo -> CM (Maybe DeliveryTaskContext) + applyModeratorPrefs md ci scopeInfo = + applyMsgPrefs ci >>= \case + Just ci' -> do + toView $ CEvtChatItemUpdated user (AChatItem SCTGroup md (GroupChat gInfo scopeInfo) ci') + pure $ Just (infoToDeliveryContext gInfo scopeInfo True) + Nothing -> pure Nothing + -- Applies the per-message prefs carried in XMsgUpdate.prefs to the local + -- chat item, returning the in-memory updated CI when a change was made + -- (so callers can emit CEvtChatItemUpdated with current state). Only + -- `commentsDisabled` is acted on; `commentsTotal` arriving via XMsgUpdate + -- is ignored — it is meaningful only at first-receipt on XMsgNew. + applyMsgPrefs :: ChatItem 'CTGroup d -> CM (Maybe (ChatItem 'CTGroup d)) + applyMsgPrefs ci@ChatItem {meta = m} = case prefs_ of + Just MsgPrefs {commentsDisabled = Just b} -> do + withStore' $ \db -> setChannelMsgCommentsDisabled db (chatItemId' ci) b + pure $ Just ci {meta = m {commentsDisabled = BoolDef b}} + _ -> pure Nothing groupMessageDelete :: GroupInfo -> Maybe GroupMember -> SharedMsgId -> Maybe MemberId -> Maybe MsgScope -> Bool -> RcvMessage -> UTCTime -> CM (Maybe DeliveryTaskContext) groupMessageDelete gInfo@GroupInfo {membership} m_ sharedMsgId sndMemberId_ scope_ onlyHistory rcvMsg brokerTs = diff --git a/src/Simplex/Chat/Messages.hs b/src/Simplex/Chat/Messages.hs index ee6f8d57c7..28483c66e8 100644 --- a/src/Simplex/Chat/Messages.hs +++ b/src/Simplex/Chat/Messages.hs @@ -360,6 +360,12 @@ data ChannelMsgInfo = ChannelMsgInfo -- of a comment message. Channel posts have no member identity, so memberId -- is Nothing for both subscriber-received (CIChannelRcv) and owner-sent -- (CIGroupSnd with showGroupAsSender = True) cases. +-- +-- NOTE: only `msgId` is consumed by the comment-parent path +-- (resolveCommentParent / getChannelMsgInfoBySharedMsgId). `sentAt`, `sent`, +-- and `memberId` are dead bytes on the wire for this use of MsgRef. The type +-- is shared with quotes (which DO read all fields) so the structural fix +-- would be a ParentRef sum type — deferred to a future PR. channelMsgRef :: ChannelMsgInfo -> MsgRef channelMsgRef ChannelMsgInfo {channelMsgItem = CChatItem _ ChatItem {chatDir, meta = CIMeta {itemTs}}, channelMsgSharedId} = MsgRef {msgId = Just channelMsgSharedId, sentAt = itemTs, sent = isSnd, memberId = Nothing} @@ -547,9 +553,13 @@ data CIMeta (c :: ChatType) (d :: MsgDirection) = CIMeta -- Set on a comment row; references the parent channel post. parentChatItemId :: Maybe ChatItemId, -- Set on a parent channel post; running total of non-deleted comments. - commentsTotal :: Int, + -- IntDef0 so an older remote host that omits this field doesn't trigger + -- CInfoInvalidJSON in a newer remote controller. + commentsTotal :: IntDef0, -- Set on a parent channel post; True locks the post against new comments. - commentsDisabled :: Bool, + -- BoolDef so an older remote host that omits this field doesn't trigger + -- CInfoInvalidJSON in a newer remote controller. + commentsDisabled :: BoolDef, createdAt :: UTCTime, updatedAt :: UTCTime } @@ -558,10 +568,12 @@ data CIMeta (c :: ChatType) (d :: MsgDirection) = CIMeta type ShowGroupAsSender = Bool mkCIMeta :: forall c d. ChatTypeI c => ChatItemId -> CIContent d -> Text -> CIStatus d -> Maybe Bool -> Maybe SharedMsgId -> Maybe CIForwardedFrom -> Maybe (CIDeleted c) -> Bool -> Maybe CITimed -> Maybe Bool -> Bool -> Bool -> UTCTime -> ChatItemTs -> Maybe GroupMemberId -> Bool -> Maybe MsgSigStatus -> Maybe ChatItemId -> Int -> Bool -> UTCTime -> UTCTime -> CIMeta c d -mkCIMeta itemId itemContent itemText itemStatus sentViaProxy itemSharedMsgId itemForwarded itemDeleted itemEdited itemTimed itemLive userMention hasLink_ currentTs itemTs forwardedByMember showGroupAsSender msgSigned parentChatItemId commentsTotal commentsDisabled createdAt updatedAt = +mkCIMeta itemId itemContent itemText itemStatus sentViaProxy itemSharedMsgId itemForwarded itemDeleted itemEdited itemTimed itemLive userMention hasLink_ currentTs itemTs forwardedByMember showGroupAsSender msgSigned parentChatItemId commentsTotal_ commentsDisabled_ createdAt updatedAt = let deletable = deletable' itemContent itemDeleted itemTs nominalDay currentTs editable = deletable && isNothing itemForwarded hasLink = BoolDef hasLink_ + commentsTotal = IntDef0 commentsTotal_ + commentsDisabled = BoolDef commentsDisabled_ in CIMeta {itemId, itemTs, itemText, itemStatus, sentViaProxy, itemSharedMsgId, itemForwarded, itemDeleted, itemEdited, itemTimed, itemLive, userMention, hasLink, deletable, editable, forwardedByMember, showGroupAsSender, msgSigned, parentChatItemId, commentsTotal, commentsDisabled, createdAt, updatedAt} deletable' :: forall c d. ChatTypeI c => CIContent d -> Maybe (CIDeleted c) -> UTCTime -> NominalDiffTime -> UTCTime -> Bool @@ -595,8 +607,8 @@ dummyMeta itemId ts itemText = showGroupAsSender = False, msgSigned = Nothing, parentChatItemId = Nothing, - commentsTotal = 0, - commentsDisabled = False, + commentsTotal = IntDef0 0, + commentsDisabled = BoolDef False, createdAt = ts, updatedAt = ts } diff --git a/src/Simplex/Chat/Protocol.hs b/src/Simplex/Chat/Protocol.hs index 5644c4b5fd..4c18aaa5bf 100644 --- a/src/Simplex/Chat/Protocol.hs +++ b/src/Simplex/Chat/Protocol.hs @@ -258,10 +258,25 @@ data MsgRef = MsgRef $(JQ.deriveJSON defaultJSON ''MsgRef) --- Per-message preferences carried in XMsgUpdate. --- `commentsDisabled = True` locks commenting on the addressed channel post. +-- Per-message preferences carried in MsgContainer (XMsgNew, so new joiners +-- learn the parent post's state at first delivery) and in XMsgUpdate (so +-- moderators can toggle commentsDisabled later). +-- +-- Both fields are Maybe so the wire shape encodes "no change": +-- - XMsgNew on a channel post: emit Just (MsgPrefs commentsDisabled commentsTotal) +-- when either differs from defaults; otherwise omit `prefs` entirely. +-- - XMsgUpdate toggling commentsDisabled: emit Just (MsgPrefs (Just newVal) Nothing). +-- - XMsgUpdate content-only edit: emit Nothing. +-- - Comments (i.e. parent is set on the container): always Nothing. +-- +-- `commentsDisabled` locks commenting on the addressed channel post. +-- `commentsTotal` is the canonical comment count at the moment of +-- emission; receivers only ever apply it at the INSERT of a new parent post +-- (never overwrite a row's existing comments_total), so the count cannot be +-- silently lowered by a stale broadcast. data MsgPrefs = MsgPrefs - { commentsDisabled :: Bool + { commentsDisabled :: Maybe Bool, + commentsTotal :: Maybe Int } deriving (Eq, Show) @@ -671,7 +686,11 @@ data MsgContainer = MsgContainer asGroup :: Maybe Bool, quote :: Maybe QuotedMsg, parent :: Maybe MsgRef, - forward :: Maybe Bool + forward :: Maybe Bool, + -- Per-message preferences for channel posts. Always Nothing on comments + -- and on non-channel messages; populated on channel-post XMsgNew so new + -- joiners learn the current commentsDisabled / commentsTotal state. + prefs :: Maybe MsgPrefs } deriving (Eq, Show) @@ -690,7 +709,8 @@ mcEmpty = asGroup = Nothing, quote = Nothing, parent = Nothing, - forward = Nothing + forward = Nothing, + prefs = Nothing } mcSimple :: MsgContent -> MsgContainer @@ -981,7 +1001,8 @@ parseMsgContainer v = do quote <- v .:? "quote" parent <- v .:? "parent" forward <- (v .:? "forward") >>= parseForward - pure MsgContainer {content, mentions, file, ttl, live, scope, asGroup, quote, parent, forward} + prefs <- v .:? "prefs" + pure MsgContainer {content, mentions, file, ttl, live, scope, asGroup, quote, parent, forward, prefs} where -- Backward compatibility: legacy clients encode forward either as a Bool or as an -- object (the latter is used by public group links). Any present form → Just True. @@ -996,10 +1017,10 @@ justTrue True = Just True justTrue False = Nothing msgContainerJSON :: MsgContainer -> J.Object -msgContainerJSON MsgContainer {content, mentions = MsgMentions mentions, file, ttl, live, scope, asGroup, quote, parent, forward} = +msgContainerJSON MsgContainer {content, mentions = MsgMentions mentions, file, ttl, live, scope, asGroup, quote, parent, forward, prefs} = JM.fromList $ discriminators - <> ("file" .=? file) (("ttl" .=? ttl) (("live" .=? live) (("mentions" .=? nonEmptyMap mentions) (("scope" .=? scope) (("asGroup" .=? asGroup) ["content" .= content]))))) + <> ("file" .=? file) (("ttl" .=? ttl) (("live" .=? live) (("mentions" .=? nonEmptyMap mentions) (("scope" .=? scope) (("asGroup" .=? asGroup) (("prefs" .=? prefs) ["content" .= content])))))) where discriminators = ["quote" .= q | Just q <- [quote]] diff --git a/src/Simplex/Chat/Store/Messages.hs b/src/Simplex/Chat/Store/Messages.hs index 0c15253956..86d9498918 100644 --- a/src/Simplex/Chat/Store/Messages.hs +++ b/src/Simplex/Chat/Store/Messages.hs @@ -52,6 +52,7 @@ module Simplex.Chat.Store.Messages getChannelMsgInfoBySharedMsgId, adjustChannelMsgCommentCount, setChannelMsgCommentsDisabled, + setChannelMsgInitialPrefs, quotedItemInCommentSection, getGroupChatScopeInfoForItem, getLocalChat, @@ -1461,10 +1462,18 @@ getGroupChat db vr user groupId scope_ parentChatItemId_ contentFilter paginatio -- | Resolve a channel post by its local ChatItemId, returning both the post -- itself and its SharedMsgId. Used by the send and receive paths to derive -- the wire-side MsgRef while keeping the DB-side parent_chat_item_id linkage. +-- +-- Validates that the loaded item is a valid channel post: it must not itself +-- be a comment (parentChatItemId IS NULL), and its direction must be either +-- CIChannelRcv (subscriber side) or CIGroupSnd with showGroupAsSender = True +-- (owner side). Otherwise the resolved id cannot become a comment parent, +-- and the request is treated as not-found to avoid leaking more detail to +-- the caller and to keep the flat-thread invariant structural. getChannelMsgInfo :: DB.Connection -> User -> GroupId -> ChatItemId -> ExceptT StoreError IO ChannelMsgInfo getChannelMsgInfo db user groupId parentChatItemId = do - parent@(CChatItem _ ChatItem {meta = CIMeta {itemSharedMsgId}}) <- - getGroupChatItem db user groupId parentChatItemId + parent <- getGroupChatItem db user groupId parentChatItemId + assertChannelPost parentChatItemId parent + let CChatItem _ ChatItem {meta = CIMeta {itemSharedMsgId}} = parent case itemSharedMsgId of Just sId -> pure ChannelMsgInfo {channelMsgItem = parent, channelMsgSharedId = sId} Nothing -> throwError $ SEChatItemNotFound parentChatItemId @@ -1472,11 +1481,26 @@ getChannelMsgInfo db user groupId parentChatItemId = do -- | Resolve a channel post by its wire SharedMsgId, returning both the post -- itself and its SharedMsgId. Used by the receive path to look up the parent -- post referenced by an incoming comment's MsgContainer.parent. +-- +-- Validates the loaded item is a channel post (see getChannelMsgInfo). getChannelMsgInfoBySharedMsgId :: DB.Connection -> User -> GroupInfo -> SharedMsgId -> ExceptT StoreError IO ChannelMsgInfo getChannelMsgInfoBySharedMsgId db user g sharedMsgId = do parent <- getGroupChatItemBySharedMsgId db user g Nothing sharedMsgId + assertChannelPost (cChatItemId parent) parent pure ChannelMsgInfo {channelMsgItem = parent, channelMsgSharedId = sharedMsgId} +-- | Verify the loaded item is a valid channel post (top-level, sent as the +-- channel rather than as a member). Reject with SEChatItemNotFound on any +-- mismatch so the failure mode at the caller is "not a valid channel post" +-- without leaking more detail. +assertChannelPost :: ChatItemId -> CChatItem 'CTGroup -> ExceptT StoreError IO () +assertChannelPost itemId (CChatItem _ ChatItem {chatDir, meta = CIMeta {parentChatItemId, showGroupAsSender}}) = do + when (isJust parentChatItemId) $ throwError $ SEChatItemNotFound itemId + case chatDir of + CIChannelRcv -> pure () + CIGroupSnd | showGroupAsSender -> pure () + _ -> throwError $ SEChatItemNotFound itemId + -- | Increment or decrement the live comment count of a channel post. -- Clamped at 0 to guard against transient negative counts under concurrent deletes. adjustChannelMsgCommentCount :: DB.Connection -> ChatItemId -> Int -> IO () @@ -1494,6 +1518,25 @@ setChannelMsgCommentsDisabled db parentChatItemId disabled = "UPDATE chat_items SET comments_disabled = ? WHERE chat_item_id = ?" (BI disabled, parentChatItemId) +-- | Apply per-post initial prefs received on a channel post's XMsgNew +-- (the owner's first send, a relay forward, or a history replay to a new +-- joiner). Only the fields that are present in MsgPrefs are written. +-- comments_total uses MAX(current, new) so a late or stale broadcast can +-- never lower an established count built up by local comment arrivals. +setChannelMsgInitialPrefs :: DB.Connection -> ChatItemId -> Maybe Bool -> Maybe Int -> IO () +setChannelMsgInitialPrefs db itemId mDisabled mTotal = do + forM_ mDisabled $ \b -> + DB.execute db "UPDATE chat_items SET comments_disabled = ? WHERE chat_item_id = ?" (BI b, itemId) + forM_ mTotal $ \n -> + DB.execute + db +#if defined(dbPostgres) + "UPDATE chat_items SET comments_total = GREATEST(comments_total, ?) WHERE chat_item_id = ?" +#else + "UPDATE chat_items SET comments_total = MAX(comments_total, ?) WHERE chat_item_id = ?" +#endif + (n, itemId) + -- | Check that a quoted item either IS the parent post or belongs to the same -- comment section. Returns True if the quote is valid for this parent. quotedItemInCommentSection :: DB.Connection -> ChatItemId -> ChatItemId -> IO Bool @@ -3927,6 +3970,7 @@ getGroupHistoryItems db user@User {userId} g@GroupInfo {groupId} m count = do ciIds <- getLastItemIds_ reverse <$> mapM (runExceptT . getGroupCIWithReactions db user g) ciIds where + -- TODO option B/C: bounded comment replay (per-post cap M, post-window K) — see scratch/channel-comments-history-replay-strategy.md getLastItemIds_ :: IO [ChatItemId] getLastItemIds_ = map fromOnly @@ -3940,6 +3984,7 @@ getGroupHistoryItems db user@User {userId} g@GroupInfo {groupId} m count = do AND i.user_id = ? AND i.group_id = ? AND i.include_in_history = 1 AND i.item_deleted = 0 + AND i.parent_chat_item_id IS NULL ORDER BY i.item_ts DESC, i.chat_item_id DESC LIMIT ? |] diff --git a/src/Simplex/Chat/Store/Postgres/Migrations.hs b/src/Simplex/Chat/Store/Postgres/Migrations.hs index 0ccfebe362..3e70230d03 100644 --- a/src/Simplex/Chat/Store/Postgres/Migrations.hs +++ b/src/Simplex/Chat/Store/Postgres/Migrations.hs @@ -28,10 +28,10 @@ import Simplex.Chat.Store.Postgres.Migrations.M20260108_chat_indices import Simplex.Chat.Store.Postgres.Migrations.M20260122_has_link import Simplex.Chat.Store.Postgres.Migrations.M20260222_chat_relays import Simplex.Chat.Store.Postgres.Migrations.M20260403_item_viewed -import Simplex.Chat.Store.Postgres.Migrations.M20260407_channel_comments import Simplex.Chat.Store.Postgres.Migrations.M20260429_relay_request_retries import Simplex.Chat.Store.Postgres.Migrations.M20260507_relay_inactive_at import Simplex.Chat.Store.Postgres.Migrations.M20260514_relay_request_group_link_index +import Simplex.Chat.Store.Postgres.Migrations.M20260516_channel_comments import Simplex.Messaging.Agent.Store.Shared (Migration (..)) schemaMigrations :: [(String, Text, Maybe Text)] @@ -60,10 +60,10 @@ schemaMigrations = ("20260122_has_link", m20260122_has_link, Just down_m20260122_has_link), ("20260222_chat_relays", m20260222_chat_relays, Just down_m20260222_chat_relays), ("20260403_item_viewed", m20260403_item_viewed, Just down_m20260403_item_viewed), - ("20260407_channel_comments", m20260407_channel_comments, Just down_m20260407_channel_comments), ("20260429_relay_request_retries", m20260429_relay_request_retries, Just down_m20260429_relay_request_retries), ("20260507_relay_inactive_at", m20260507_relay_inactive_at, Just down_m20260507_relay_inactive_at), - ("20260514_relay_request_group_link_index", m20260514_relay_request_group_link_index, Just down_m20260514_relay_request_group_link_index) + ("20260514_relay_request_group_link_index", m20260514_relay_request_group_link_index, Just down_m20260514_relay_request_group_link_index), + ("20260516_channel_comments", m20260516_channel_comments, Just down_m20260516_channel_comments) ] -- | The list of migrations in ascending order by date diff --git a/src/Simplex/Chat/Store/Postgres/Migrations/M20260407_channel_comments.hs b/src/Simplex/Chat/Store/Postgres/Migrations/M20260516_channel_comments.hs similarity index 82% rename from src/Simplex/Chat/Store/Postgres/Migrations/M20260407_channel_comments.hs rename to src/Simplex/Chat/Store/Postgres/Migrations/M20260516_channel_comments.hs index 93ffd400f3..4d07deb119 100644 --- a/src/Simplex/Chat/Store/Postgres/Migrations/M20260407_channel_comments.hs +++ b/src/Simplex/Chat/Store/Postgres/Migrations/M20260516_channel_comments.hs @@ -1,13 +1,13 @@ {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE QuasiQuotes #-} -module Simplex.Chat.Store.Postgres.Migrations.M20260407_channel_comments where +module Simplex.Chat.Store.Postgres.Migrations.M20260516_channel_comments where import Data.Text (Text) import Text.RawString.QQ (r) -m20260407_channel_comments :: Text -m20260407_channel_comments = +m20260516_channel_comments :: Text +m20260516_channel_comments = [r| ALTER TABLE chat_items ADD COLUMN parent_chat_item_id BIGINT REFERENCES chat_items ON DELETE CASCADE; ALTER TABLE chat_items ADD COLUMN comments_total INTEGER NOT NULL DEFAULT 0; @@ -17,8 +17,8 @@ CREATE INDEX idx_chat_items_parent_chat_item_id ON chat_items(parent_chat_item_i CREATE INDEX idx_chat_items_parent_item_ts ON chat_items(user_id, group_id, parent_chat_item_id, item_ts); |] -down_m20260407_channel_comments :: Text -down_m20260407_channel_comments = +down_m20260516_channel_comments :: Text +down_m20260516_channel_comments = [r| DROP INDEX idx_chat_items_parent_chat_item_id; DROP INDEX idx_chat_items_parent_item_ts; diff --git a/src/Simplex/Chat/Store/SQLite/Migrations.hs b/src/Simplex/Chat/Store/SQLite/Migrations.hs index 4c81e7595f..778261fda0 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations.hs +++ b/src/Simplex/Chat/Store/SQLite/Migrations.hs @@ -151,10 +151,10 @@ import Simplex.Chat.Store.SQLite.Migrations.M20260108_chat_indices import Simplex.Chat.Store.SQLite.Migrations.M20260122_has_link import Simplex.Chat.Store.SQLite.Migrations.M20260222_chat_relays import Simplex.Chat.Store.SQLite.Migrations.M20260403_item_viewed -import Simplex.Chat.Store.SQLite.Migrations.M20260407_channel_comments import Simplex.Chat.Store.SQLite.Migrations.M20260429_relay_request_retries import Simplex.Chat.Store.SQLite.Migrations.M20260507_relay_inactive_at import Simplex.Chat.Store.SQLite.Migrations.M20260514_relay_request_group_link_index +import Simplex.Chat.Store.SQLite.Migrations.M20260516_channel_comments import Simplex.Messaging.Agent.Store.Shared (Migration (..)) schemaMigrations :: [(String, Query, Maybe Query)] @@ -306,10 +306,10 @@ schemaMigrations = ("20260122_has_link", m20260122_has_link, Just down_m20260122_has_link), ("20260222_chat_relays", m20260222_chat_relays, Just down_m20260222_chat_relays), ("20260403_item_viewed", m20260403_item_viewed, Just down_m20260403_item_viewed), - ("20260407_channel_comments", m20260407_channel_comments, Just down_m20260407_channel_comments), ("20260429_relay_request_retries", m20260429_relay_request_retries, Just down_m20260429_relay_request_retries), ("20260507_relay_inactive_at", m20260507_relay_inactive_at, Just down_m20260507_relay_inactive_at), - ("20260514_relay_request_group_link_index", m20260514_relay_request_group_link_index, Just down_m20260514_relay_request_group_link_index) + ("20260514_relay_request_group_link_index", m20260514_relay_request_group_link_index, Just down_m20260514_relay_request_group_link_index), + ("20260516_channel_comments", m20260516_channel_comments, Just down_m20260516_channel_comments) ] -- | The list of migrations in ascending order by date diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/M20260407_channel_comments.hs b/src/Simplex/Chat/Store/SQLite/Migrations/M20260516_channel_comments.hs similarity index 82% rename from src/Simplex/Chat/Store/SQLite/Migrations/M20260407_channel_comments.hs rename to src/Simplex/Chat/Store/SQLite/Migrations/M20260516_channel_comments.hs index eb22f5ee70..b8d3dacf1c 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/M20260407_channel_comments.hs +++ b/src/Simplex/Chat/Store/SQLite/Migrations/M20260516_channel_comments.hs @@ -1,12 +1,12 @@ {-# LANGUAGE QuasiQuotes #-} -module Simplex.Chat.Store.SQLite.Migrations.M20260407_channel_comments where +module Simplex.Chat.Store.SQLite.Migrations.M20260516_channel_comments where import Database.SQLite.Simple (Query) import Database.SQLite.Simple.QQ (sql) -m20260407_channel_comments :: Query -m20260407_channel_comments = +m20260516_channel_comments :: Query +m20260516_channel_comments = [sql| ALTER TABLE chat_items ADD COLUMN parent_chat_item_id INTEGER REFERENCES chat_items ON DELETE CASCADE; ALTER TABLE chat_items ADD COLUMN comments_total INTEGER NOT NULL DEFAULT 0; @@ -16,8 +16,8 @@ CREATE INDEX idx_chat_items_parent_chat_item_id ON chat_items(parent_chat_item_i CREATE INDEX idx_chat_items_parent_item_ts ON chat_items(user_id, group_id, parent_chat_item_id, item_ts); |] -down_m20260407_channel_comments :: Query -down_m20260407_channel_comments = +down_m20260516_channel_comments :: Query +down_m20260516_channel_comments = [sql| DROP INDEX idx_chat_items_parent_chat_item_id; DROP INDEX idx_chat_items_parent_item_ts; diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql b/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql index 86c198670c..f12a2670a7 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_schema.sql @@ -469,7 +469,10 @@ CREATE TABLE chat_items( show_group_as_sender INTEGER NOT NULL DEFAULT 0, has_link INTEGER NOT NULL DEFAULT 0, msg_signed TEXT, - item_viewed INTEGER NOT NULL DEFAULT 0 + item_viewed INTEGER NOT NULL DEFAULT 0, + parent_chat_item_id INTEGER REFERENCES chat_items ON DELETE CASCADE, + comments_total INTEGER NOT NULL DEFAULT 0, + comments_disabled INTEGER NOT NULL DEFAULT 0 ) STRICT; CREATE TABLE sqlite_sequence(name,seq); CREATE TABLE chat_item_messages( @@ -1301,6 +1304,15 @@ ON groups( relay_request_group_link ) WHERE relay_request_group_link IS NOT NULL; +CREATE INDEX idx_chat_items_parent_chat_item_id ON chat_items( + parent_chat_item_id +); +CREATE INDEX idx_chat_items_parent_item_ts ON chat_items( + user_id, + group_id, + parent_chat_item_id, + item_ts +); CREATE TRIGGER on_group_members_insert_update_summary AFTER INSERT ON group_members FOR EACH ROW diff --git a/src/Simplex/Chat/Types.hs b/src/Simplex/Chat/Types.hs index f2892898c4..c5a3b970ed 100644 --- a/src/Simplex/Chat/Types.hs +++ b/src/Simplex/Chat/Types.hs @@ -2059,6 +2059,17 @@ instance FromJSON BoolDef where parseJSON v = BoolDef <$> parseJSON v omittedField = Just (BoolDef False) +-- | An Int field that defaults to 0 when absent in JSON. Like BoolDef, this +-- exists so a newer remote controller can parse CIMeta JSON emitted by an +-- older remote host that doesn't include the field, avoiding a fallback to +-- CInfoInvalidJSON. See docs/CONTRIBUTING.md. +newtype IntDef0 = IntDef0 {unIntDef0 :: Int} + deriving newtype (Eq, Show, Num, Ord, ToJSON) + +instance FromJSON IntDef0 where + parseJSON v = IntDef0 <$> parseJSON v + omittedField = Just (IntDef0 0) + $(JQ.deriveJSON defaultJSON ''UserContact) $(JQ.deriveJSON defaultJSON ''Profile) diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index a205cb075b..9cc3d56830 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -296,19 +296,6 @@ chatGroupTests = do it "should compute sendAsGroup in CLI forward" testForwardCLISendAsGroup it "should update member message in channel" testChannelMemberMessageUpdate it "should delete member message in channel" testChannelMemberMessageDelete - describe "channel comments" $ do - it "subscriber should comment on channel post" testChannelCommentSubscriberCanComment - it "should reject comment in non-channel group" testChannelCommentNotInRegularGroup - it "should reject comment when comments disabled on post" testChannelCommentDisabledRejected - it "subscriber should edit and delete own comment" testChannelCommentEditDelete - it "comments_total should increment on insert and decrement on delete" testChannelCommentCountIncrement - it "observer should not be able to comment" testChannelCommentObserverRejected - it "comments should not appear in main channel pagination" testChannelCommentMainChatExclusion - it "subscriber should quote comment on channel post" testChannelCommentQuote - it "subscriber should receive comment from another subscriber via relay" testChannelCommentRcvFromAnotherSubscriber - it "owner should moderate-delete subscriber comment and decrement count" testChannelCommentModerationDelete - it "content edit should preserve commentsDisabled flag" testChannelCommentDisabledViaPrefs - it "subscriber should react to a channel comment" testChannelCommentReact testGroupCheckMessages :: HasCallStack => TestParams -> IO () testGroupCheckMessages = @@ -10861,6 +10848,8 @@ testChannelMemberMessageDelete ps = eve <# "#team cath> [marked deleted] hello" ] +<<<<<<< Updated upstream +======= testChannelCommentSubscriberCanComment :: HasCallStack => TestParams -> IO () testChannelCommentSubscriberCanComment ps = withNewTestChat ps "alice" aliceProfile $ \alice -> @@ -10877,7 +10866,7 @@ testChannelCommentSubscriberCanComment ps = -- subscriber comments on the post parentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> parentId <> " text reply") + cath ##> ("/_send #1 parent=" <> parentId <> " text reply") cath <# "#team reply" bob <# "#team cath> reply" concurrentlyN_ @@ -10898,7 +10887,7 @@ testChannelCommentNotInRegularGroup = alice #> "#team hello" [bob, cath] *<# "#team alice> hello" parentId <- lastGroupItemId bob 1 - bob ##> ("/_comment #1 " <> parentId <> " text reply") + bob ##> ("/_send #1 parent=" <> parentId <> " text reply") bob <## "bad chat command: comments are only supported in channel groups" testChannelCommentDisabledRejected :: HasCallStack => TestParams -> IO () @@ -10918,17 +10907,25 @@ testChannelCommentDisabledRejected ps = aliceParentId <- lastGroupItemId alice 1 alice ##> ("/_comments_disabled #1 " <> aliceParentId <> " on") alice <## "ok" + -- handler emits CEvtChatItemUpdated for alice (re-rendered post) + -- via localQ; the response "ok" is printed synchronously, so the + -- async event arrives after + alice <# "#team hello" -- owner's own comment attempt is rejected (local state updated) - alice ##> ("/_comment #1 " <> aliceParentId <> " text reply") + alice ##> ("/_send #1 parent=" <> aliceParentId <> " text reply") alice <## "bad chat command: feature not allowed Comments" -- wait for XMsgUpdate with prefs to propagate through relay to subscribers threadDelay 1000000 + -- all subscribers (including the relay bob) apply prefs locally and + -- emit CEvtChatItemUpdated (the post re-rendered with updated + -- commentsDisabled); drain before next assertion + [bob, cath, dan, eve] *<# "#team> hello" -- subscriber's comment attempt is also rejected (relay forwarded XMsgUpdate with prefs) cathParentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> cathParentId <> " text reply") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text reply") cath <## "bad chat command: feature not allowed Comments" testChannelCommentEditDelete :: HasCallStack => TestParams -> IO () @@ -10946,7 +10943,7 @@ testChannelCommentEditDelete ps = -- cath comments on the post cathParentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> cathParentId <> " text reply") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text reply") cath <# "#team reply" bob <# "#team cath> reply" concurrentlyN_ @@ -11007,7 +11004,7 @@ testChannelCommentCountIncrement ps = checkAllCounts 0 -- cath comments (capture cath's comment id immediately for later delete) - cath ##> ("/_comment #1 " <> cathParentId <> " text reply one") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text reply one") cath <# "#team reply one" cathCommentId <- lastGroupItemId cath 1 bob <# "#team cath> reply one" @@ -11023,7 +11020,7 @@ testChannelCommentCountIncrement ps = checkAllCounts 1 -- dan comments on the parent (danParentId captured before cath's comment) - dan ##> ("/_comment #1 " <> danParentId <> " text reply two") + dan ##> ("/_send #1 parent=" <> danParentId <> " text reply two") dan <# "#team reply two" bob <# "#team dan> reply two" concurrentlyN_ @@ -11068,6 +11065,11 @@ testChannelCommentObserverRejected ps = bob <# "#team> hello" [cath, dan, eve] *<# "#team> hello [>>]" + -- capture cath's view of the channel post BEFORE any intervening + -- items (cath's own message, role-change event) so the parent we + -- pass to /_comment is the channel post itself, not a later item + cathParentId <- lastGroupItemId cath 1 + -- make cath known to other subscribers cath #> "#team hi from cath" bob <# "#team cath> hi from cath" @@ -11093,8 +11095,7 @@ testChannelCommentObserverRejected ps = ] -- cath's comment attempt is rejected by local role check - cathParentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> cathParentId <> " text reply") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text reply") cath <## "#team: you have insufficient permissions for this action, the required role is commenter" testChannelCommentMainChatExclusion :: HasCallStack => TestParams -> IO () @@ -11112,7 +11113,7 @@ testChannelCommentMainChatExclusion ps = aliceParentId <- lastGroupItemId alice 1 cathParentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> cathParentId <> " text reply") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text reply") cath <# "#team reply" bob <# "#team cath> reply" concurrentlyN_ @@ -11165,7 +11166,7 @@ testChannelCommentQuote ps = danParentId <- lastGroupItemId dan 1 -- cath comments on the post - cath ##> ("/_comment #1 " <> cathParentId <> " text first comment") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text first comment") cath <# "#team first comment" cathCommentId <- lastGroupItemId cath 1 bob <# "#team cath> first comment" @@ -11182,7 +11183,7 @@ testChannelCommentQuote ps = -- dan quotes cath's comment (another comment in the same section) danQuotedId <- lastGroupItemId dan 1 -- cath's comment let cm1 = "{\"quotedItemId\": " <> danQuotedId <> ", \"msgContent\": {\"type\": \"text\", \"text\": \"quoting comment\"}}" - dan ##> ("/_comment #1 " <> danParentId <> " json [" <> cm1 <> "]") + dan ##> ("/_send #1 parent=" <> danParentId <> " json [" <> cm1 <> "]") dan <# "#team > cath first comment" dan <## " quoting comment" bob <# "#team dan> > cath first comment" @@ -11203,7 +11204,7 @@ testChannelCommentQuote ps = -- cath quotes the parent post itself in a comment let cm2 = "{\"quotedItemId\": " <> cathParentId <> ", \"msgContent\": {\"type\": \"text\", \"text\": \"quoting parent\"}}" - cath ##> ("/_comment #1 " <> cathParentId <> " json [" <> cm2 <> "]") + cath ##> ("/_send #1 parent=" <> cathParentId <> " json [" <> cm2 <> "]") cath <# "#team > hello" cath <## " quoting parent" bob <# "#team cath> > hello" @@ -11230,7 +11231,7 @@ testChannelCommentQuote ps = -- try to quote it while commenting on the second post — should fail cathSecondParentId <- lastGroupItemId cath 1 let cm3 = "{\"quotedItemId\": " <> cathCommentId <> ", \"msgContent\": {\"type\": \"text\", \"text\": \"cross-section quote\"}}" - cath ##> ("/_comment #1 " <> cathSecondParentId <> " json [" <> cm3 <> "]") + cath ##> ("/_send #1 parent=" <> cathSecondParentId <> " json [" <> cm3 <> "]") cath <## "bad chat command: quoted item does not belong to the same comment section" testChannelCommentRcvFromAnotherSubscriber :: HasCallStack => TestParams -> IO () @@ -11252,7 +11253,7 @@ testChannelCommentRcvFromAnotherSubscriber ps = eveParentId <- lastGroupItemId eve 1 -- cath comments via the relay; dan and eve receive via the unknown-member path - cath ##> ("/_comment #1 " <> cathParentId <> " text reply") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text reply") cath <# "#team reply" bob <# "#team cath> reply" concurrentlyN_ @@ -11285,7 +11286,7 @@ testChannelCommentModerationDelete ps = aliceParentId <- lastGroupItemId alice 1 cathParentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> cathParentId <> " text moderate-me") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text moderate-me") cath <# "#team moderate-me" bob <# "#team cath> moderate-me" concurrentlyN_ @@ -11341,12 +11342,17 @@ testChannelCommentDisabledViaPrefs ps = -- step 1: alice disables comments on the post via XMsgUpdate.prefs alice ##> ("/_comments_disabled #1 " <> aliceParentId <> " on") alice <## "ok" + -- handler emits CEvtChatItemUpdated for alice via localQ; the + -- response "ok" is printed synchronously and arrives first + alice <# "#team hello" -- wait for XMsgUpdate with prefs to propagate to subscribers via the relay threadDelay 1000000 + -- all subscribers (including the relay bob) apply prefs locally and emit CEvtChatItemUpdated; drain + [bob, cath, dan, eve] *<# "#team> hello" -- cath's local commentsDisabled is now True; attempted comment is rejected - cath ##> ("/_comment #1 " <> cathParentId <> " text after-disable") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text after-disable") cath <## "bad chat command: feature not allowed Comments" -- step 2: alice edits the POST CONTENT (no prefs in the update — prefs defaults to Nothing). @@ -11360,7 +11366,7 @@ testChannelCommentDisabledViaPrefs ps = threadDelay 1000000 -- commentsDisabled is still True after the content edit; cath remains rejected - cath ##> ("/_comment #1 " <> cathParentId <> " text after-content-edit") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text after-content-edit") cath <## "bad chat command: feature not allowed Comments" testChannelCommentReact :: HasCallStack => TestParams -> IO () @@ -11378,7 +11384,7 @@ testChannelCommentReact ps = -- cath comments on alice's post cathParentId <- lastGroupItemId cath 1 - cath ##> ("/_comment #1 " <> cathParentId <> " text react-target") + cath ##> ("/_send #1 parent=" <> cathParentId <> " text react-target") cath <# "#team react-target" bob <# "#team cath> react-target" concurrentlyN_ @@ -11414,6 +11420,7 @@ testChannelCommentReact ps = eve <## " + 👍" ] +>>>>>>> Stashed changes testGroupLinkContentFilter :: HasCallStack => TestParams -> IO () testGroupLinkContentFilter = testChat3 aliceProfile bobProfile cathProfile $ diff --git a/tests/ProtocolTests.hs b/tests/ProtocolTests.hs index ce1328a3c9..6346e36e18 100644 --- a/tests/ProtocolTests.hs +++ b/tests/ProtocolTests.hs @@ -203,12 +203,18 @@ decodeChatMessageTest = describe "Chat message encoding/decoding" $ do chatInitialVRange (Just $ SharedMsgId "\1\2\3\4") (XMsgNew (mcEmpty {content = MCText "reply", quote = Just quotedMsg, parent = Just parentMsgRef})) + it "x.msg.new channel post with prefs" $ + "{\"v\":\"1\",\"msgId\":\"AQIDBA==\",\"event\":\"x.msg.new\",\"params\":{\"content\":{\"text\":\"hello\",\"type\":\"text\"},\"prefs\":{\"commentsDisabled\":true,\"commentsTotal\":42}}}" + ##==## ChatMessage + chatInitialVRange + (Just $ SharedMsgId "\1\2\3\4") + (XMsgNew (mcEmpty {content = MCText "hello", prefs = Just MsgPrefs {commentsDisabled = Just True, commentsTotal = Just 42}})) it "x.msg.update" $ "{\"v\":\"1\",\"event\":\"x.msg.update\",\"params\":{\"msgId\":\"AQIDBA==\", \"content\":{\"text\":\"hello\",\"type\":\"text\"}}}" #==# XMsgUpdate (SharedMsgId "\1\2\3\4") (MCText "hello") [] Nothing Nothing Nothing Nothing Nothing it "x.msg.update with prefs (comments disabled)" $ "{\"v\":\"1\",\"event\":\"x.msg.update\",\"params\":{\"msgId\":\"AQIDBA==\",\"content\":{\"text\":\"hello\",\"type\":\"text\"},\"prefs\":{\"commentsDisabled\":true}}}" - #==# XMsgUpdate (SharedMsgId "\1\2\3\4") (MCText "hello") [] Nothing Nothing Nothing Nothing (Just MsgPrefs {commentsDisabled = True}) + #==# XMsgUpdate (SharedMsgId "\1\2\3\4") (MCText "hello") [] Nothing Nothing Nothing Nothing (Just MsgPrefs {commentsDisabled = Just True, commentsTotal = Nothing}) it "x.msg.del" $ "{\"v\":\"1\",\"event\":\"x.msg.del\",\"params\":{\"msgId\":\"AQIDBA==\"}}" #==# XMsgDel (SharedMsgId "\1\2\3\4") Nothing Nothing False