Add root id to webhooks (#36415)

* Add root id to webhooks

* Address feedback

* Address coderabbit comment

* Error if root post is not really a root post

* Fix test

* Address nitpick
This commit is contained in:
Daniel Espino García
2026-05-07 14:09:32 +02:00
committed by GitHub
parent c1107620bc
commit 870c2c6b13
5 changed files with 192 additions and 1 deletions
@@ -0,0 +1,77 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
// ***************************************************************
// - [#] indicates a test step (e.g. # Go to a page)
// - [*] indicates an assertion (e.g. * Check the title)
// - Use element ID when selecting an element. Create one if none.
// ***************************************************************
// Group: @channels @incoming_webhook
import {getRandomId} from '@/utils';
describe('Incoming webhook', () => {
let testTeam;
let testChannel;
let incomingWebhook;
before(() => {
cy.apiInitSetup().then(({team, channel}) => {
testTeam = team;
testChannel = channel;
const newIncomingHook = {
channel_id: channel.id,
channel_locked: true,
description: 'Incoming webhook - thread reply',
display_name: 'thread-reply',
};
cy.apiCreateWebhook(newIncomingHook).then((hook) => {
incomingWebhook = hook;
});
});
});
it('posts a webhook message as a reply when root_id references a thread root post', () => {
cy.visit(`/${testTeam.name}/channels/${testChannel.name}`);
const rootMessage = `Root for webhook thread ${getRandomId()}`;
// # Post a root message to open a thread
cy.postMessage(rootMessage);
cy.getLastPostId().then((rootPostId) => {
const webhookReplyText = `Webhook thread reply ${getRandomId()}`;
// # Post to the incoming webhook with root_id set to the thread root
cy.postIncomingWebhook({
url: incomingWebhook.url,
data: {
text: webhookReplyText,
root_id: rootPostId,
},
waitFor: 'text',
});
cy.getLastPostId().then((replyId) => {
// * Reply is stored as part of the thread (author-independent)
cy.request({
headers: {'X-Requested-With': 'XMLHttpRequest'},
url: `/api/v4/posts/${replyId}`,
}).then(({body: replyPost}) => {
expect(replyPost.root_id).to.eq(rootPostId);
});
// * Center-channel reply styling for threaded posts (CommentedOn is only shown when
// isFirstReply is true, which is false when the reply follows its root directly)
cy.get(`#post_${replyId}`).
should('have.class', 'post--comment').
within(() => {
cy.get(`#postMessageText_${replyId}`).should('have.text', webhookReplyText);
});
});
});
});
});
+29 -1
View File
@@ -864,6 +864,34 @@ func (a *App) HandleIncomingWebhook(rctx request.CTX, hookID string, req *model.
return model.NewAppError("HandleIncomingWebhook", "web.incoming_webhook.permissions.app_error", map[string]any{"user": hook.UserId, "channel": channel.Id}, "", http.StatusForbidden)
}
threadRootID := ""
if rootId := req.RootId; rootId != "" {
if !model.IsValidId(rootId) {
return model.NewAppError("HandleIncomingWebhook", "api.context.invalid_param.app_error", map[string]any{"Name": "root_id"}, "", http.StatusBadRequest)
}
rootPost, nErr := a.Srv().Store().Post().GetSingle(rctx, rootId, false)
if nErr != nil {
var nfErr *store.ErrNotFound
switch {
case errors.As(nErr, &nfErr):
return model.NewAppError("HandleIncomingWebhook", "api.post.create_post.root_id.app_error", nil, "", http.StatusBadRequest).Wrap(nErr)
default:
return model.NewAppError("HandleIncomingWebhook", "app.post.get.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
}
}
if rootPost == nil {
return model.NewAppError("HandleIncomingWebhook", "api.post.create_post.root_id.app_error", nil, "", http.StatusBadRequest)
}
if rootPost.ChannelId != channel.Id {
return model.NewAppError("HandleIncomingWebhook", "api.post.create_post.channel_root_id.app_error", nil, "", http.StatusBadRequest)
}
if rootPost.RootId != "" {
return model.NewAppError("HandleIncomingWebhook", "api.post.create_post.root_id.app_error", nil, "", http.StatusBadRequest)
}
threadRootID = rootPost.Id
}
overrideUsername := hook.Username
if req.Username != "" {
overrideUsername = req.Username
@@ -874,7 +902,7 @@ func (a *App) HandleIncomingWebhook(rctx request.CTX, hookID string, req *model.
overrideIconURL = req.IconURL
}
_, err := a.CreateWebhookPost(rctx, hook.UserId, channel, text, overrideUsername, overrideIconURL, req.IconEmoji, req.Props, webhookType, "", req.Priority)
_, err := a.CreateWebhookPost(rctx, hook.UserId, channel, text, overrideUsername, overrideIconURL, req.IconEmoji, req.Props, webhookType, threadRootID, req.Priority)
return err
}
+76
View File
@@ -22,6 +22,82 @@ import (
"github.com/mattermost/mattermost/server/v8/channels/testlib"
)
func TestHandleIncomingWebhookRootId(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = true })
hook, appErr := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id})
require.Nil(t, appErr)
defer func() {
require.Nil(t, th.App.DeleteIncomingWebhook(hook.Id))
}()
root := th.CreatePost(t, th.BasicChannel)
reply := th.CreatePostReply(t, root)
otherChannel := th.CreateChannel(t, th.BasicTeam)
otherPost := th.CreatePost(t, otherChannel)
t.Run("creates reply in thread when root_id is the thread root", func(t *testing.T) {
err := th.App.HandleIncomingWebhook(th.Context, hook.Id, &model.IncomingWebhookRequest{
Text: "webhook thread reply",
RootId: root.Id,
})
require.Nil(t, err)
list, err2 := th.App.GetPosts(th.Context, th.BasicChannel.Id, 0, 5)
require.Nil(t, err2)
var found *model.Post
for _, p := range list.Posts {
if p.Message == "webhook thread reply" {
found = p
break
}
}
require.NotNil(t, found)
assert.Equal(t, root.Id, found.RootId)
})
t.Run("rejects root_id pointing at a reply post", func(t *testing.T) {
err := th.App.HandleIncomingWebhook(th.Context, hook.Id, &model.IncomingWebhookRequest{
Text: "webhook via reply id",
RootId: reply.Id,
})
require.NotNil(t, err)
assert.Equal(t, "api.post.create_post.root_id.app_error", err.Id)
assert.Equal(t, http.StatusBadRequest, err.StatusCode)
})
t.Run("rejects non-existent root_id", func(t *testing.T) {
err := th.App.HandleIncomingWebhook(th.Context, hook.Id, &model.IncomingWebhookRequest{
Text: "missing root",
RootId: model.NewId(),
})
require.NotNil(t, err)
assert.Equal(t, "api.post.create_post.root_id.app_error", err.Id)
assert.Equal(t, http.StatusBadRequest, err.StatusCode)
})
t.Run("rejects root_id in a different channel", func(t *testing.T) {
err := th.App.HandleIncomingWebhook(th.Context, hook.Id, &model.IncomingWebhookRequest{
Text: "wrong channel",
RootId: otherPost.Id,
})
require.NotNil(t, err)
assert.Equal(t, "api.post.create_post.channel_root_id.app_error", err.Id)
assert.Equal(t, http.StatusBadRequest, err.StatusCode)
})
t.Run("rejects invalid root_id", func(t *testing.T) {
err := th.App.HandleIncomingWebhook(th.Context, hook.Id, &model.IncomingWebhookRequest{
Text: "bad id",
RootId: "not-a-valid-id",
})
require.NotNil(t, err)
assert.Equal(t, "api.context.invalid_param.app_error", err.Id)
assert.Equal(t, http.StatusBadRequest, err.StatusCode)
})
}
func TestCreateIncomingWebhookForChannel(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
+1
View File
@@ -52,6 +52,7 @@ type IncomingWebhookRequest struct {
Username string `json:"username"`
IconURL string `json:"icon_url"`
ChannelName string `json:"channel"`
RootId string `json:"root_id"`
Props StringInterface `json:"props"`
Attachments []*MessageAttachment `json:"attachments"`
Type string `json:"type"`
@@ -150,3 +150,12 @@ func TestIncomingWebhookNullArrayItems(t *testing.T) {
require.Len(t, iwr.Attachments, 1)
require.Len(t, iwr.Attachments[0].Fields, 1)
}
func TestIncomingWebhookRequestFromJSONRootId(t *testing.T) {
id := NewId()
payload := `{"text":"hello","root_id":"` + id + `"}`
iwr, err := IncomingWebhookRequestFromJSON(strings.NewReader(payload))
require.Nil(t, err)
require.NotNil(t, iwr)
require.Equal(t, id, iwr.RootId)
}