diff --git a/server/channels/api4/websocket.go b/server/channels/api4/websocket.go index 428d8e5e70..c15c96aa1f 100644 --- a/server/channels/api4/websocket.go +++ b/server/channels/api4/websocket.go @@ -17,6 +17,7 @@ import ( const ( connectionIDParam = "connection_id" sequenceNumberParam = "sequence_number" + postedAckParam = "posted_ack" ) func (api *API) InitWebSocket() { @@ -48,6 +49,7 @@ func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) { TFunc: c.AppContext.T, Locale: "", Active: true, + PostedAck: r.URL.Query().Get(postedAckParam) == "true", } // The WebSocket upgrade request coming from mobile is missing the // user agent so we need to fallback on the session's metadata. diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 9f1b7793dc..6e8a8551dc 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -740,15 +740,19 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea // A user following a thread but had left the channel won't get a notification // https://mattermost.atlassian.net/browse/MM-36769 if profileMap[uid] == nil { - a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonMissingProfile) - a.NotificationsLog().Error("Missing profile", - mlog.String("type", model.NotificationTypeWebsocket), - mlog.String("post_id", post.Id), - mlog.String("status", model.NotificationStatusError), - mlog.String("reason", model.NotificationReasonMissingProfile), - mlog.String("sender_id", sender.Id), - mlog.String("receiver_id", uid), - ) + // This also sometimes happens when bots, which will never show up in the map, reply to threads + // Their own post goes through this and they get "notified", which we don't need to count as an error if they can't + if uid != post.UserId { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonMissingProfile) + a.NotificationsLog().Error("Missing profile", + mlog.String("type", model.NotificationTypeWebsocket), + mlog.String("post_id", post.Id), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonMissingProfile), + mlog.String("sender_id", sender.Id), + mlog.String("receiver_id", uid), + ) + } continue } if a.IsCRTEnabledForUser(c, uid) { diff --git a/server/channels/app/platform/web_conn.go b/server/channels/app/platform/web_conn.go index d44a9b3b23..bcaac22d46 100644 --- a/server/channels/app/platform/web_conn.go +++ b/server/channels/app/platform/web_conn.go @@ -68,6 +68,7 @@ type WebConnConfig struct { Active bool ReuseCount int OriginClient string + PostedAck bool // These aren't necessary to be exported to api layer. sequence int @@ -89,6 +90,7 @@ type WebConn struct { Locale string Sequence int64 UserId string + PostedAck bool allChannelMembers map[string]string lastAllChannelMembersTime int64 @@ -234,6 +236,7 @@ func (ps *PlatformService) NewWebConn(cfg *WebConnConfig, suite SuiteIFace, runn UserId: cfg.Session.UserId, T: cfg.TFunc, Locale: cfg.Locale, + PostedAck: cfg.PostedAck, reuseCount: cfg.ReuseCount, endWritePump: make(chan struct{}), pumpFinished: make(chan struct{}), diff --git a/server/channels/app/web_broadcast_hooks.go b/server/channels/app/web_broadcast_hooks.go index 6b08942586..fbabcc9dcf 100644 --- a/server/channels/app/web_broadcast_hooks.go +++ b/server/channels/app/web_broadcast_hooks.go @@ -82,11 +82,9 @@ func usePostedAckHook(message *model.WebSocketEvent, postedUserId string, channe } func (h *postedAckBroadcastHook) Process(msg *platform.HookedWebSocketEvent, webConn *platform.WebConn, args map[string]any) error { - // Don't ACK mobile app websocket events at this time, we may revisit this when we can add this to mobile app - if session := webConn.GetSession(); session != nil { - if session.IsMobile() { - return nil - } + // Don't ACK unless we say to explicitly + if !webConn.PostedAck { + return nil } postedUserId, err := getTypedArg[string](args, "posted_user_id") diff --git a/server/channels/app/web_broadcast_hooks_test.go b/server/channels/app/web_broadcast_hooks_test.go index e199c3ca2c..3b1f82bc49 100644 --- a/server/channels/app/web_broadcast_hooks_test.go +++ b/server/channels/app/web_broadcast_hooks_test.go @@ -87,8 +87,9 @@ func TestPostedAckHook_Process(t *testing.T) { hook := &postedAckBroadcastHook{} userID := model.NewId() webConn := &platform.WebConn{ - UserId: userID, - Platform: &platform.PlatformService{}, + UserId: userID, + Platform: &platform.PlatformService{}, + PostedAck: true, } webConn.SetSession(&model.Session{}) @@ -140,12 +141,12 @@ func TestPostedAckHook_Process(t *testing.T) { assert.True(t, msg.Event().GetData()["should_ack"].(bool)) }) - t.Run("should not ack for mobile app", func(t *testing.T) { + t.Run("should not ack if posted ack is false", func(t *testing.T) { mobileWebConn := &platform.WebConn{ - UserId: userID, - Platform: &platform.PlatformService{}, + UserId: userID, + Platform: &platform.PlatformService{}, + PostedAck: false, } - mobileWebConn.SetSession(&model.Session{Props: map[string]string{"isMobile": "true"}}) msg := platform.MakeHookedWebSocketEvent(model.NewWebSocketEvent(model.WebsocketEventPosted, "", "", "", nil, "")) hook.Process(msg, mobileWebConn, map[string]any{ diff --git a/webapp/channels/src/actions/websocket_actions.jsx b/webapp/channels/src/actions/websocket_actions.jsx index 05e37e6f18..05ff6bb751 100644 --- a/webapp/channels/src/actions/websocket_actions.jsx +++ b/webapp/channels/src/actions/websocket_actions.jsx @@ -179,7 +179,7 @@ export function initialize() { WebSocketClient.addMissedMessageListener(restart); WebSocketClient.addCloseListener(handleClose); - WebSocketClient.initialize(connUrl); + WebSocketClient.initialize(connUrl, undefined, true); } export function close() { diff --git a/webapp/channels/src/components/post_view/post_body_additional_content/__snapshots__/post_body_additional_content.test.tsx.snap b/webapp/channels/src/components/post_view/post_body_additional_content/__snapshots__/post_body_additional_content.test.tsx.snap index 63ed6ad116..7afaaccb56 100644 --- a/webapp/channels/src/components/post_view/post_body_additional_content/__snapshots__/post_body_additional_content.test.tsx.snap +++ b/webapp/channels/src/components/post_view/post_body_additional_content/__snapshots__/post_body_additional_content.test.tsx.snap @@ -111,6 +111,7 @@ exports[`PostBodyAdditionalContent with a normal link Should render the plugin c "messageListeners": Set {}, "missedEventCallback": null, "missedMessageListeners": Set {}, + "postedAck": false, "reconnectCallback": null, "reconnectListeners": Set {}, "responseCallbacks": Object {}, @@ -157,6 +158,7 @@ exports[`PostBodyAdditionalContent with a normal link Should render the plugin c "messageListeners": Set {}, "missedEventCallback": null, "missedMessageListeners": Set {}, + "postedAck": false, "reconnectCallback": null, "reconnectListeners": Set {}, "responseCallbacks": Object {}, diff --git a/webapp/channels/src/plugins/pluggable/__snapshots__/pluggable.test.tsx.snap b/webapp/channels/src/plugins/pluggable/__snapshots__/pluggable.test.tsx.snap index 70c57ab5fb..e65b9a34f7 100644 --- a/webapp/channels/src/plugins/pluggable/__snapshots__/pluggable.test.tsx.snap +++ b/webapp/channels/src/plugins/pluggable/__snapshots__/pluggable.test.tsx.snap @@ -115,6 +115,7 @@ exports[`plugins/Pluggable should match snapshot with extended component 1`] = ` "messageListeners": Set {}, "missedEventCallback": null, "missedMessageListeners": Set {}, + "postedAck": false, "reconnectCallback": null, "reconnectListeners": Set {}, "responseCallbacks": Object {}, @@ -248,6 +249,7 @@ exports[`plugins/Pluggable should match snapshot with extended component with pl "messageListeners": Set {}, "missedEventCallback": null, "missedMessageListeners": Set {}, + "postedAck": false, "reconnectCallback": null, "reconnectListeners": Set {}, "responseCallbacks": Object {}, @@ -507,6 +509,7 @@ exports[`plugins/Pluggable should match snapshot with null pluggableId 1`] = ` "messageListeners": Set {}, "missedEventCallback": null, "missedMessageListeners": Set {}, + "postedAck": false, "reconnectCallback": null, "reconnectListeners": Set {}, "responseCallbacks": Object {}, @@ -641,6 +644,7 @@ exports[`plugins/Pluggable should match snapshot with valid pluggableId 1`] = ` "messageListeners": Set {}, "missedEventCallback": null, "missedMessageListeners": Set {}, + "postedAck": false, "reconnectCallback": null, "reconnectListeners": Set {}, "responseCallbacks": Object {}, diff --git a/webapp/platform/client/src/websocket.ts b/webapp/platform/client/src/websocket.ts index 73e1507292..8b97f7abe8 100644 --- a/webapp/platform/client/src/websocket.ts +++ b/webapp/platform/client/src/websocket.ts @@ -68,6 +68,7 @@ export default class WebSocketClient { private closeListeners = new Set(); private connectionId: string | null; + private postedAck: boolean; constructor() { this.conn = null; @@ -77,12 +78,13 @@ export default class WebSocketClient { this.connectFailCount = 0; this.responseCallbacks = {}; this.connectionId = ''; + this.postedAck = false; } // on connect, only send auth cookie and blank state. // on hello, get the connectionID and store it. // on reconnect, send cookie, connectionID, sequence number. - initialize(connectionUrl = this.connectionUrl, token?: string) { + initialize(connectionUrl = this.connectionUrl, token?: string, postedAck?: boolean) { if (this.conn) { return; } @@ -96,10 +98,14 @@ export default class WebSocketClient { console.log('websocket connecting to ' + connectionUrl); //eslint-disable-line no-console } + if (typeof postedAck != 'undefined') { + this.postedAck = postedAck; + } + // Add connection id, and last_sequence_number to the query param. // We cannot use a cookie because it will bleed across tabs. // We cannot also send it as part of the auth_challenge, because the session cookie is already sent with the request. - this.conn = new WebSocket(`${connectionUrl}?connection_id=${this.connectionId}&sequence_number=${this.serverSequence}`); + this.conn = new WebSocket(`${connectionUrl}?connection_id=${this.connectionId}&sequence_number=${this.serverSequence}${this.postedAck ? '&posted_ack=true' : ''}`); this.connectionUrl = connectionUrl; this.conn.onopen = () => { @@ -148,7 +154,7 @@ export default class WebSocketClient { setTimeout( () => { - this.initialize(connectionUrl, token); + this.initialize(connectionUrl, token, postedAck); }, retryTime, );