forum: fix reply_to_top_id for replies inside a forum
resolveChannelReply applied discussion-thread logic (reply_to_top_id = the replied-to message's own id) to forum replies. Replying to a General message produced reply_to_top_id = <that id>, a topic no client can resolve: the reply vanished from every topic view and reply-jump on strict clients said "message doesn't exist". Forum replies now inherit the target's topic via domain.ForumReplyTopicID (target's topic, or its own id if it's a topic-create, else General), and General (topic 1) is accepted as a valid virtual topic everywhere, so sends carrying top_msg_id: 1 are no longer rejected. Non-forum discussion threads are unchanged.
This commit is contained in:
parent
f0bf315bf3
commit
c7a77c23c8
4 changed files with 213 additions and 37 deletions
|
|
@ -735,6 +735,23 @@ type ChannelMessage struct {
|
||||||
Deleted bool
|
Deleted bool
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ForumReplyTopicID resolves the topic a reply to target belongs to inside a
|
||||||
|
// forum. Every forum message lives in exactly one topic, and a reply inherits
|
||||||
|
// the target's topic - never the target's own id. Using target.ID is
|
||||||
|
// discussion-thread logic (comment threads on a broadcast post) and does not
|
||||||
|
// apply to forums: it manufactures a topic reference that no channel_forum_topics
|
||||||
|
// row backs, which strict clients cannot place. A target with no recorded topic
|
||||||
|
// is in General.
|
||||||
|
func ForumReplyTopicID(target ChannelMessage) int {
|
||||||
|
if target.Action != nil && target.Action.Type == ChannelActionTopicCreate {
|
||||||
|
return target.ID // the target itself is a topic root
|
||||||
|
}
|
||||||
|
if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 {
|
||||||
|
return target.ReplyTo.TopMessageID
|
||||||
|
}
|
||||||
|
return ForumGeneralTopicID
|
||||||
|
}
|
||||||
|
|
||||||
// ProjectChannelHistoryClearMessage returns the owner-local service-message
|
// ProjectChannelHistoryClearMessage returns the owner-local service-message
|
||||||
// projection for one channel history boundary. Identity fields from the shared
|
// projection for one channel history boundary. Identity fields from the shared
|
||||||
// source are retained when available, while all user payload, media, reply,
|
// source are retained when available, while all user payload, media, reply,
|
||||||
|
|
|
||||||
120
internal/rpc/forum_reply_topic_rpc_test.go
Normal file
120
internal/rpc/forum_reply_topic_rpc_test.go
Normal file
|
|
@ -0,0 +1,120 @@
|
||||||
|
package rpc
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/iamxvbaba/td/clock"
|
||||||
|
"github.com/iamxvbaba/td/tg"
|
||||||
|
"go.uber.org/zap/zaptest"
|
||||||
|
|
||||||
|
appchannels "telesrv/internal/app/channels"
|
||||||
|
appusers "telesrv/internal/app/users"
|
||||||
|
"telesrv/internal/domain"
|
||||||
|
"telesrv/internal/store/memory"
|
||||||
|
)
|
||||||
|
|
||||||
|
// A reply inside a forum must inherit the *target's* topic, never the target's
|
||||||
|
// own message id. Regression: replying to a General message produced
|
||||||
|
// reply_to_top_id = <that message's id>, a topic that no client can resolve, so
|
||||||
|
// the reply vanished from every topic view and reply-jump said "doesn't exist".
|
||||||
|
func TestForumReplyInheritsTargetTopic(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
userStore := memory.NewUserStore()
|
||||||
|
owner, _ := userStore.Create(ctx, domain.User{AccessHash: 91, Phone: "15550009101", FirstName: "Owner"})
|
||||||
|
channelStore := memory.NewChannelStore()
|
||||||
|
r := New(Config{}, Deps{
|
||||||
|
Users: appusers.NewService(userStore),
|
||||||
|
Channels: appchannels.NewService(channelStore),
|
||||||
|
}, zaptest.NewLogger(t), clock.System)
|
||||||
|
ownerCtx := WithUserID(ctx, owner.ID)
|
||||||
|
|
||||||
|
created, err := r.onChannelsCreateChannel(ownerCtx, &tg.ChannelsCreateChannelRequest{Title: "Forum", Megagroup: true})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create channel: %v", err)
|
||||||
|
}
|
||||||
|
channel := created.(*tg.Updates).Chats[0].(*tg.Channel)
|
||||||
|
input := &tg.InputChannel{ChannelID: channel.ID, AccessHash: channel.AccessHash}
|
||||||
|
peer := &tg.InputPeerChannel{ChannelID: channel.ID, AccessHash: channel.AccessHash}
|
||||||
|
if _, err := r.onChannelsToggleForum(ownerCtx, &tg.ChannelsToggleForumRequest{Channel: input, Enabled: true, Tabs: true}); err != nil {
|
||||||
|
t.Fatalf("toggle forum: %v", err)
|
||||||
|
}
|
||||||
|
topicUpd, err := r.onMessagesCreateForumTopic(ownerCtx, &tg.MessagesCreateForumTopicRequest{
|
||||||
|
Peer: peer, Title: "Test", IconColor: domain.DefaultForumTopicIconColor, RandomID: 9101001,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create topic: %v", err)
|
||||||
|
}
|
||||||
|
testTopicID := forumTopicRootMessageID(t, topicUpd, "Test")
|
||||||
|
|
||||||
|
send := func(text string, randomID int64, reply *tg.InputReplyToMessage) *tg.Message {
|
||||||
|
req := &tg.MessagesSendMessageRequest{Peer: peer, Message: text, RandomID: randomID}
|
||||||
|
if reply != nil {
|
||||||
|
req.SetReplyTo(reply)
|
||||||
|
}
|
||||||
|
upd, err := r.onMessagesSendMessage(ownerCtx, req)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("send %q: %v", text, err)
|
||||||
|
}
|
||||||
|
for _, u := range upd.(*tg.Updates).Updates {
|
||||||
|
if nm, ok := u.(*tg.UpdateNewChannelMessage); ok {
|
||||||
|
if m, ok := nm.Message.(*tg.Message); ok && m.Message == text {
|
||||||
|
return m
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
t.Fatalf("no new message for %q in %+v", text, upd)
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
topID := func(m *tg.Message) int {
|
||||||
|
h, ok := m.ReplyTo.(*tg.MessageReplyHeader)
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("message %d has reply header %T, want *MessageReplyHeader", m.ID, m.ReplyTo)
|
||||||
|
}
|
||||||
|
id, _ := h.GetReplyToTopID()
|
||||||
|
if !h.ForumTopic {
|
||||||
|
t.Fatalf("message %d reply header missing forum_topic flag: %+v", m.ID, h)
|
||||||
|
}
|
||||||
|
return id
|
||||||
|
}
|
||||||
|
|
||||||
|
// A plain General message (no reply header).
|
||||||
|
g1 := send("g1", 9101002, nil)
|
||||||
|
|
||||||
|
// Reply to it -> topic must be General (1), not g1.ID.
|
||||||
|
r1 := send("r1", 9101003, &tg.InputReplyToMessage{ReplyToMsgID: g1.ID})
|
||||||
|
if got := topID(r1); got != domain.ForumGeneralTopicID {
|
||||||
|
t.Fatalf("reply to a General message: reply_to_top_id = %d, want %d (General), not the target id %d",
|
||||||
|
got, domain.ForumGeneralTopicID, g1.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reply again, this time the client also passes top_msg_id: 1 (General).
|
||||||
|
// Previously this was rejected because General has no channel_forum_topics row.
|
||||||
|
replyWithTop := &tg.InputReplyToMessage{ReplyToMsgID: g1.ID}
|
||||||
|
replyWithTop.SetTopMsgID(domain.ForumGeneralTopicID)
|
||||||
|
r2 := send("r2", 9101004, replyWithTop)
|
||||||
|
if got := topID(r2); got != domain.ForumGeneralTopicID {
|
||||||
|
t.Fatalf("reply with top_msg_id=1: reply_to_top_id = %d, want %d", got, domain.ForumGeneralTopicID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Post directly into the "Test" topic, then reply to a plain message there.
|
||||||
|
tInTopic := &tg.InputReplyToMessage{ReplyToMsgID: 0}
|
||||||
|
tInTopic.SetTopMsgID(testTopicID)
|
||||||
|
m1 := send("t1", 9101005, tInTopic)
|
||||||
|
if got := topID(m1); got != testTopicID {
|
||||||
|
t.Fatalf("message in Test topic: reply_to_top_id = %d, want %d", got, testTopicID)
|
||||||
|
}
|
||||||
|
rt := send("rt", 9101006, &tg.InputReplyToMessage{ReplyToMsgID: m1.ID})
|
||||||
|
if got := topID(rt); got != testTopicID {
|
||||||
|
t.Fatalf("reply inside Test topic: reply_to_top_id = %d, want %d (topic), not %d", got, testTopicID, m1.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Replying to a General message while claiming a mismatched topic is rejected.
|
||||||
|
bad := &tg.InputReplyToMessage{ReplyToMsgID: g1.ID}
|
||||||
|
bad.SetTopMsgID(testTopicID)
|
||||||
|
req := &tg.MessagesSendMessageRequest{Peer: peer, Message: "bad", RandomID: 9101007}
|
||||||
|
req.SetReplyTo(bad)
|
||||||
|
if _, err := r.onMessagesSendMessage(ownerCtx, req); err == nil {
|
||||||
|
t.Fatal("reply with a topic id that doesn't match the target's topic was accepted")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -540,18 +540,14 @@ func (s *ChannelStore) resolveChannelReplyLocked(req domain.SendChannelMessageRe
|
||||||
if req.ReplyTo.TopMessageID <= 0 || !channel.Forum {
|
if req.ReplyTo.TopMessageID <= 0 || !channel.Forum {
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
return nil, domain.ErrReplyMessageIDInvalid
|
||||||
}
|
}
|
||||||
topic, ok := s.topics[req.ChannelID][req.ReplyTo.TopMessageID]
|
|
||||||
if !ok || topic.Hidden {
|
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
|
||||||
}
|
|
||||||
if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) {
|
|
||||||
return nil, domain.ErrChannelWriteForbidden
|
|
||||||
}
|
|
||||||
reply := cloneMessageReply(req.ReplyTo)
|
reply := cloneMessageReply(req.ReplyTo)
|
||||||
reply.MessageID = 0
|
reply.MessageID = 0
|
||||||
reply.Peer = channelPeer
|
reply.Peer = channelPeer
|
||||||
reply.TopMessageID = topic.TopicID
|
|
||||||
reply.ForumTopic = true
|
reply.ForumTopic = true
|
||||||
|
if err := s.validateForumReplyTopicLocked(channel, member, req.ReplyTo.TopMessageID, req.UserID, selfBoostsApplied); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
reply.TopMessageID = req.ReplyTo.TopMessageID
|
||||||
return reply, nil
|
return reply, nil
|
||||||
}
|
}
|
||||||
target, ok := s.findMessageLocked(req.ChannelID, req.ReplyTo.MessageID)
|
target, ok := s.findMessageLocked(req.ChannelID, req.ReplyTo.MessageID)
|
||||||
|
|
@ -561,6 +557,22 @@ func (s *ChannelStore) resolveChannelReplyLocked(req domain.SendChannelMessageRe
|
||||||
reply := cloneMessageReply(req.ReplyTo)
|
reply := cloneMessageReply(req.ReplyTo)
|
||||||
reply.MessageID = target.ID
|
reply.MessageID = target.ID
|
||||||
reply.Peer = channelPeer
|
reply.Peer = channelPeer
|
||||||
|
|
||||||
|
if channel.Forum {
|
||||||
|
// A forum reply belongs to the TARGET's topic, never the target's own id.
|
||||||
|
topicID := domain.ForumReplyTopicID(target)
|
||||||
|
if req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != topicID {
|
||||||
|
return nil, domain.ErrReplyMessageIDInvalid
|
||||||
|
}
|
||||||
|
if err := s.validateForumReplyTopicLocked(channel, member, topicID, req.UserID, selfBoostsApplied); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
reply.TopMessageID = topicID
|
||||||
|
reply.ForumTopic = true
|
||||||
|
return reply, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Non-forum discussion thread: reply_to_top_id is the comment-thread root.
|
||||||
reply.TopMessageID = target.ID
|
reply.TopMessageID = target.ID
|
||||||
if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 {
|
if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 {
|
||||||
reply.TopMessageID = target.ReplyTo.TopMessageID
|
reply.TopMessageID = target.ReplyTo.TopMessageID
|
||||||
|
|
@ -571,17 +583,25 @@ func (s *ChannelStore) resolveChannelReplyLocked(req domain.SendChannelMessageRe
|
||||||
if channel.Forum && req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != reply.TopMessageID {
|
if channel.Forum && req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != reply.TopMessageID {
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
return nil, domain.ErrReplyMessageIDInvalid
|
||||||
}
|
}
|
||||||
if channel.Forum && reply.TopMessageID > 0 {
|
|
||||||
if topic, ok := s.topics[req.ChannelID][reply.TopMessageID]; ok && !topic.Hidden {
|
|
||||||
if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) {
|
|
||||||
return nil, domain.ErrChannelWriteForbidden
|
|
||||||
}
|
|
||||||
reply.ForumTopic = true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return reply, nil
|
return reply, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// validateForumReplyTopicLocked mirrors the postgres store: General
|
||||||
|
// (ForumGeneralTopicID) is a virtual topic with no row and is always valid.
|
||||||
|
func (s *ChannelStore) validateForumReplyTopicLocked(channel domain.Channel, member domain.ChannelMember, topicID int, userID int64, selfBoostsApplied int) error {
|
||||||
|
if topicID == domain.ForumGeneralTopicID {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
topic, ok := s.topics[channel.ID][topicID]
|
||||||
|
if !ok || topic.Hidden {
|
||||||
|
return domain.ErrReplyMessageIDInvalid
|
||||||
|
}
|
||||||
|
if topic.Closed && !canManageForumTopic(channel, member, topic, userID, selfBoostsApplied) {
|
||||||
|
return domain.ErrChannelWriteForbidden
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func inactiveChannelDate(dialog domain.Dialog, channel domain.Channel, member domain.ChannelMember) int {
|
func inactiveChannelDate(dialog domain.Dialog, channel domain.Channel, member domain.ChannelMember) int {
|
||||||
if dialog.TopMessageDate > 0 {
|
if dialog.TopMessageDate > 0 {
|
||||||
return dialog.TopMessageDate
|
return dialog.TopMessageDate
|
||||||
|
|
|
||||||
|
|
@ -850,21 +850,14 @@ WHERE owner_user_id=$1 AND peer_type='user' AND peer_id=$2 AND box_id=$3 AND NOT
|
||||||
if req.ReplyTo.TopMessageID <= 0 || !channel.Forum {
|
if req.ReplyTo.TopMessageID <= 0 || !channel.Forum {
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
return nil, domain.ErrReplyMessageIDInvalid
|
||||||
}
|
}
|
||||||
topic, err := s.getForumTopic(ctx, db, req.ChannelID, req.ReplyTo.TopMessageID)
|
|
||||||
if err != nil {
|
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
|
||||||
}
|
|
||||||
if topic.Hidden {
|
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
|
||||||
}
|
|
||||||
if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) {
|
|
||||||
return nil, domain.ErrChannelWriteForbidden
|
|
||||||
}
|
|
||||||
reply := cloneMessageReply(req.ReplyTo)
|
reply := cloneMessageReply(req.ReplyTo)
|
||||||
reply.MessageID = 0
|
reply.MessageID = 0
|
||||||
reply.Peer = channelPeer
|
reply.Peer = channelPeer
|
||||||
reply.TopMessageID = topic.TopicID
|
|
||||||
reply.ForumTopic = true
|
reply.ForumTopic = true
|
||||||
|
if err := s.validateForumReplyTopic(ctx, db, channel, member, req.ReplyTo.TopMessageID, req.UserID, selfBoostsApplied); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
reply.TopMessageID = req.ReplyTo.TopMessageID
|
||||||
return reply, nil
|
return reply, nil
|
||||||
}
|
}
|
||||||
target, err := s.getChannelMessage(ctx, db, req.ChannelID, req.ReplyTo.MessageID)
|
target, err := s.getChannelMessage(ctx, db, req.ChannelID, req.ReplyTo.MessageID)
|
||||||
|
|
@ -880,6 +873,22 @@ WHERE owner_user_id=$1 AND peer_type='user' AND peer_id=$2 AND box_id=$3 AND NOT
|
||||||
reply := cloneMessageReply(req.ReplyTo)
|
reply := cloneMessageReply(req.ReplyTo)
|
||||||
reply.MessageID = target.ID
|
reply.MessageID = target.ID
|
||||||
reply.Peer = channelPeer
|
reply.Peer = channelPeer
|
||||||
|
|
||||||
|
if channel.Forum {
|
||||||
|
// A forum reply belongs to the TARGET's topic, never the target's own id.
|
||||||
|
topicID := domain.ForumReplyTopicID(target)
|
||||||
|
if req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != topicID {
|
||||||
|
return nil, domain.ErrReplyMessageIDInvalid
|
||||||
|
}
|
||||||
|
if err := s.validateForumReplyTopic(ctx, db, channel, member, topicID, req.UserID, selfBoostsApplied); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
reply.TopMessageID = topicID
|
||||||
|
reply.ForumTopic = true
|
||||||
|
return reply, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Non-forum discussion thread: reply_to_top_id is the comment-thread root.
|
||||||
reply.TopMessageID = target.ID
|
reply.TopMessageID = target.ID
|
||||||
if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 {
|
if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 {
|
||||||
reply.TopMessageID = target.ReplyTo.TopMessageID
|
reply.TopMessageID = target.ReplyTo.TopMessageID
|
||||||
|
|
@ -896,19 +905,29 @@ WHERE owner_user_id=$1 AND peer_type='user' AND peer_id=$2 AND box_id=$3 AND NOT
|
||||||
if channel.Forum && req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != reply.TopMessageID {
|
if channel.Forum && req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != reply.TopMessageID {
|
||||||
return nil, domain.ErrReplyMessageIDInvalid
|
return nil, domain.ErrReplyMessageIDInvalid
|
||||||
}
|
}
|
||||||
if channel.Forum && reply.TopMessageID > 0 {
|
|
||||||
if topic, err := s.getForumTopic(ctx, db, req.ChannelID, reply.TopMessageID); err == nil && !topic.Hidden {
|
|
||||||
if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) {
|
|
||||||
return nil, domain.ErrChannelWriteForbidden
|
|
||||||
}
|
|
||||||
reply.ForumTopic = true
|
|
||||||
} else if err != nil && !errors.Is(err, domain.ErrMessageIDInvalid) {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return reply, nil
|
return reply, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// validateForumReplyTopic checks that topicID is a topic the caller may post
|
||||||
|
// into. General (ForumGeneralTopicID) is a virtual topic with no
|
||||||
|
// channel_forum_topics row and is always valid.
|
||||||
|
func (s *ChannelStore) validateForumReplyTopic(ctx context.Context, db sqlcgen.DBTX, channel domain.Channel, member domain.ChannelMember, topicID int, userID int64, selfBoostsApplied int) error {
|
||||||
|
if topicID == domain.ForumGeneralTopicID {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
topic, err := s.getForumTopic(ctx, db, channel.ID, topicID)
|
||||||
|
if err != nil {
|
||||||
|
return domain.ErrReplyMessageIDInvalid
|
||||||
|
}
|
||||||
|
if topic.Hidden {
|
||||||
|
return domain.ErrReplyMessageIDInvalid
|
||||||
|
}
|
||||||
|
if topic.Closed && !canManageForumTopic(channel, member, topic, userID, selfBoostsApplied) {
|
||||||
|
return domain.ErrChannelWriteForbidden
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func visibleChannelTopAfter(ctx context.Context, db sqlcgen.DBTX, channelID int64, availableMinID int, fallbackDate int) (int, int, error) {
|
func visibleChannelTopAfter(ctx context.Context, db sqlcgen.DBTX, channelID int64, availableMinID int, fallbackDate int) (int, int, error) {
|
||||||
var id, date int
|
var id, date int
|
||||||
err := db.QueryRow(ctx, `
|
err := db.QueryRow(ctx, `
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue