MM-57326: [Shared Channels] Message priority, acknowledgement and persistent notifications need to be synced (#30736)

2025-06-16 02:30:21 +02:00
--- a/server/channels/app/platform/shared_channel_notifier.go
+++ b/server/channels/app/platform/shared_channel_notifier.go
@@ -21,6 +21,8 @@ var sharedChannelEventsForSync = []model.WebsocketEventType{
 	model.WebsocketEventPostDeleted,
 	model.WebsocketEventReactionAdded,
 	model.WebsocketEventReactionRemoved,
+	model.WebsocketEventAcknowledgementAdded,
+	model.WebsocketEventAcknowledgementRemoved,
 }

 var sharedChannelEventsForInvitation = []model.WebsocketEventType{
--- a/server/channels/app/post.go
+++ b/server/channels/app/post.go
@@ -773,9 +773,14 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, updateP
 	if newPost == nil {
 		return nil, model.NewAppError("UpdatePost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest)
 	}
-	// Restore the post metadata that was stripped by the plugin. Set it to
-	// the last known good.
-	newPost.Metadata = oldPost.Metadata
+	// Always use incoming metadata when provided, otherwise retain existing
+	if receivedUpdatedPost.Metadata != nil {
+		newPost.Metadata = receivedUpdatedPost.Metadata.Copy()
+	} else {
+		// Restore the post metadata that was stripped by the plugin. Set it to
+		// the last known good.
+		newPost.Metadata = oldPost.Metadata
+	}

 	rpost, nErr := a.Srv().Store().Post().Update(c, newPost, oldPost)
 	if nErr != nil {
--- a/server/channels/app/post_acknowledgements.go
+++ b/server/channels/app/post_acknowledgements.go
@@ -15,9 +15,19 @@ import (
 )

 func (a *App) SaveAcknowledgementForPost(c request.CTX, postID, userID string) (*model.PostAcknowledgement, *model.AppError) {
-	post, err := a.GetSinglePost(c, postID, false)
-	if err != nil {
-		return nil, err
+	return a.saveAcknowledgementForPostWithPost(c, nil, userID, postID)
+}
+
+func (a *App) saveAcknowledgementForPostWithPost(c request.CTX, post *model.Post, userID string, postID ...string) (*model.PostAcknowledgement, *model.AppError) {
+	if post == nil {
+		if len(postID) == 0 {
+			return nil, model.NewAppError("SaveAcknowledgementForPost", "app.acknowledgement.save.missing_post.app_error", nil, "", http.StatusBadRequest)
+		}
+		var err *model.AppError
+		post, err = a.GetSinglePost(c, postID[0], false)
+		if err != nil {
+			return nil, err
+		}
 	}

 	channel, err := a.GetChannel(c, post.ChannelId)
@@ -29,8 +39,14 @@ func (a *App) SaveAcknowledgementForPost(c request.CTX, postID, userID string) (
 		return nil, model.NewAppError("SaveAcknowledgementForPost", "api.acknowledgement.save.archived_channel.app_error", nil, "", http.StatusForbidden)
 	}

-	acknowledgedAt := model.GetMillis()
-	acknowledgement, nErr := a.Srv().Store().PostAcknowledgement().Save(postID, userID, acknowledgedAt)
+	// Pre-populate the ChannelId to save a DB call in store
+	acknowledgement := &model.PostAcknowledgement{
+		PostId:    post.Id,
+		UserId:    userID,
+		ChannelId: post.ChannelId,
+	}
+
+	savedAck, nErr := a.Srv().Store().PostAcknowledgement().SaveWithModel(acknowledgement)
 	if nErr != nil {
 		var appErr *model.AppError
 		switch {
@@ -56,15 +72,28 @@ func (a *App) SaveAcknowledgementForPost(c request.CTX, postID, userID string) (
 	// The post is always modified since the UpdateAt always changes
 	a.Srv().Store().Post().InvalidateLastPostTimeCache(channel.Id)

-	a.sendAcknowledgementEvent(c, model.WebsocketEventAcknowledgementAdded, acknowledgement, post)
+	a.sendAcknowledgementEvent(c, model.WebsocketEventAcknowledgementAdded, savedAck, post)

-	return acknowledgement, nil
+	// Trigger post updated event to ensure shared channel sync
+	a.sendPostUpdateEvent(c, post)
+
+	return savedAck, nil
 }

 func (a *App) DeleteAcknowledgementForPost(c request.CTX, postID, userID string) *model.AppError {
-	post, err := a.GetSinglePost(c, postID, false)
-	if err != nil {
-		return err
+	return a.deleteAcknowledgementForPostWithPost(c, nil, userID, postID)
+}
+
+func (a *App) deleteAcknowledgementForPostWithPost(c request.CTX, post *model.Post, userID string, postID ...string) *model.AppError {
+	if post == nil {
+		if len(postID) == 0 {
+			return model.NewAppError("DeleteAcknowledgementForPost", "app.acknowledgement.delete.missing_post.app_error", nil, "", http.StatusBadRequest)
+		}
+		var err *model.AppError
+		post, err = a.GetSinglePost(c, postID[0], false)
+		if err != nil {
+			return err
+		}
 	}

 	channel, err := a.GetChannel(c, post.ChannelId)
@@ -76,7 +105,7 @@ func (a *App) DeleteAcknowledgementForPost(c request.CTX, postID, userID string)
 		return model.NewAppError("DeleteAcknowledgementForPost", "api.acknowledgement.delete.archived_channel.app_error", nil, "", http.StatusForbidden)
 	}

-	oldAck, nErr := a.Srv().Store().PostAcknowledgement().Get(postID, userID)
+	oldAck, nErr := a.Srv().Store().PostAcknowledgement().Get(post.Id, userID)

 	if nErr != nil {
 		var nfErr *store.ErrNotFound
@@ -102,6 +131,9 @@ func (a *App) DeleteAcknowledgementForPost(c request.CTX, postID, userID string)

 	a.sendAcknowledgementEvent(c, model.WebsocketEventAcknowledgementRemoved, oldAck, post)

+	// Trigger post updated event to ensure shared channel sync
+	a.sendPostUpdateEvent(c, post)
+
 	return nil
 }

@@ -130,6 +162,80 @@ func (a *App) GetAcknowledgementsForPostList(postList *model.PostList) (map[stri
 	return acknowledgementsMap, nil
 }

+// SaveAcknowledgementsForPost saves multiple acknowledgements for a post in a single operation.
+func (a *App) SaveAcknowledgementsForPost(c request.CTX, postID string, userIDs []string) ([]*model.PostAcknowledgement, *model.AppError) {
+	if len(userIDs) == 0 {
+		return []*model.PostAcknowledgement{}, nil
+	}
+
+	post, err := a.GetSinglePost(c, postID, false)
+	if err != nil {
+		return nil, err
+	}
+
+	channel, err := a.GetChannel(c, post.ChannelId)
+	if err != nil {
+		return nil, err
+	}
+
+	if channel.DeleteAt > 0 {
+		return nil, model.NewAppError("SaveAcknowledgementsForPost", "api.acknowledgement.save.archived_channel.app_error", nil, "", http.StatusForbidden)
+	}
+
+	// Create acknowledgements with current timestamp
+	acknowledgedAt := model.GetMillis()
+	var acknowledgements []*model.PostAcknowledgement
+
+	for _, userID := range userIDs {
+		acknowledgements = append(acknowledgements, &model.PostAcknowledgement{
+			PostId:         post.Id,
+			UserId:         userID,
+			ChannelId:      post.ChannelId,
+			AcknowledgedAt: acknowledgedAt,
+		})
+	}
+
+	// Save all acknowledgements
+	savedAcks, nErr := a.Srv().Store().PostAcknowledgement().BatchSave(acknowledgements)
+	if nErr != nil {
+		var appErr *model.AppError
+		switch {
+		case errors.As(nErr, &appErr):
+			return nil, appErr
+		default:
+			return nil, model.NewAppError("SaveAcknowledgementsForPost", "app.acknowledgement.batch_save.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
+		}
+	}
+
+	// Resolve persistent notifications for each user
+	for _, userID := range userIDs {
+		if appErr := a.ResolvePersistentNotification(c, post, userID); appErr != nil {
+			a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonResolvePersistentNotificationError, model.NotificationNoPlatform)
+			a.NotificationsLog().Error("Error resolving persistent notification",
+				mlog.String("sender_id", userID),
+				mlog.String("post_id", post.RootId),
+				mlog.String("status", model.NotificationStatusError),
+				mlog.String("reason", model.NotificationReasonResolvePersistentNotificationError),
+				mlog.Err(appErr),
+			)
+			// We continue processing other acknowledgements even if one fails
+		}
+	}
+
+	// The post is always modified since the UpdateAt always changes
+	a.Srv().Store().Post().InvalidateLastPostTimeCache(channel.Id)
+
+	// Send WebSocket events for each acknowledgement
+	for _, ack := range savedAcks {
+		a.sendAcknowledgementEvent(c, model.WebsocketEventAcknowledgementAdded, ack, post)
+	}
+
+	// Trigger post updated event to ensure shared channel sync
+	a.sendPostUpdateEvent(c, post)
+
+	return savedAcks, nil
+}
+
 func (a *App) sendAcknowledgementEvent(rctx request.CTX, event model.WebsocketEventType, acknowledgement *model.PostAcknowledgement, post *model.Post) {
 	// send out that a acknowledgement has been added/removed
 	message := model.NewWebSocketEvent(event, "", post.ChannelId, "", nil, "")
@@ -141,3 +247,107 @@ func (a *App) sendAcknowledgementEvent(rctx request.CTX, event model.WebsocketEv
 	message.Add("acknowledgement", string(acknowledgementJSON))
 	a.Publish(message)
 }
+
+func (a *App) SaveAcknowledgementForPostWithModel(c request.CTX, acknowledgement *model.PostAcknowledgement) (*model.PostAcknowledgement, *model.AppError) {
+	// Get the post to verify it exists and get the channel
+	post, err := a.GetSinglePost(c, acknowledgement.PostId, false)
+	if err != nil {
+		return nil, err
+	}
+
+	channel, err := a.GetChannel(c, post.ChannelId)
+	if err != nil {
+		return nil, err
+	}
+
+	if channel.DeleteAt > 0 {
+		return nil, model.NewAppError("SaveAcknowledgementForPostWithModel", "api.acknowledgement.save.archived_channel.app_error", nil, "", http.StatusForbidden)
+	}
+
+	// Make sure ChannelId is set
+	if acknowledgement.ChannelId == "" {
+		acknowledgement.ChannelId = post.ChannelId
+	}
+
+	savedAck, nErr := a.Srv().Store().PostAcknowledgement().SaveWithModel(acknowledgement)
+	if nErr != nil {
+		var appErr *model.AppError
+		switch {
+		case errors.As(nErr, &appErr):
+			return nil, appErr
+		default:
+			return nil, model.NewAppError("SaveAcknowledgementForPostWithModel", "app.acknowledgement.save.save.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
+		}
+	}
+
+	if appErr := a.ResolvePersistentNotification(c, post, acknowledgement.UserId); appErr != nil {
+		a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonResolvePersistentNotificationError, model.NotificationNoPlatform)
+		a.NotificationsLog().Error("Error resolving persistent notification",
+			mlog.String("sender_id", acknowledgement.UserId),
+			mlog.String("post_id", post.RootId),
+			mlog.String("status", model.NotificationStatusError),
+			mlog.String("reason", model.NotificationReasonResolvePersistentNotificationError),
+			mlog.Err(appErr),
+		)
+		return nil, appErr
+	}
+
+	// The post is always modified since the UpdateAt always changes
+	a.Srv().Store().Post().InvalidateLastPostTimeCache(channel.Id)
+
+	a.sendAcknowledgementEvent(c, model.WebsocketEventAcknowledgementAdded, savedAck, post)
+
+	// Trigger post updated event to ensure shared channel sync
+	a.sendPostUpdateEvent(c, post)
+
+	return savedAck, nil
+}
+
+func (a *App) DeleteAcknowledgementForPostWithModel(c request.CTX, acknowledgement *model.PostAcknowledgement) *model.AppError {
+	// Get the post to verify it exists and get the channel
+	post, err := a.GetSinglePost(c, acknowledgement.PostId, false)
+	if err != nil {
+		return err
+	}
+
+	channel, err := a.GetChannel(c, post.ChannelId)
+	if err != nil {
+		return err
+	}
+
+	if channel.DeleteAt > 0 {
+		return model.NewAppError("DeleteAcknowledgementForPostWithModel", "api.acknowledgement.delete.archived_channel.app_error", nil, "", http.StatusForbidden)
+	}
+
+	nErr := a.Srv().Store().PostAcknowledgement().Delete(acknowledgement)
+	if nErr != nil {
+		return model.NewAppError("DeleteAcknowledgementForPostWithModel", "app.acknowledgement.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
+	}
+
+	// The post is always modified since the UpdateAt always changes
+	a.Srv().Store().Post().InvalidateLastPostTimeCache(channel.Id)
+
+	a.sendAcknowledgementEvent(c, model.WebsocketEventAcknowledgementRemoved, acknowledgement, post)
+
+	// Trigger post updated event to ensure shared channel sync
+	a.sendPostUpdateEvent(c, post)
+
+	return nil
+}
+
+func (a *App) sendPostUpdateEvent(c request.CTX, post *model.Post) {
+	if post == nil {
+		c.Logger().Warn("sendPostUpdateEvent called with nil post")
+		return
+	}
+
+	// Send a post edited event to trigger shared channel sync
+	message := model.NewWebSocketEvent(model.WebsocketEventPostEdited, "", post.ChannelId, "", nil, "")
+
+	// Prepare the post with metadata for the event
+	preparedPost := a.PreparePostForClient(c, post, false, true, true)
+
+	if appErr := a.publishWebsocketEventForPost(c, preparedPost, message); appErr != nil {
+		c.Logger().Warn("Failed to send post update event for acknowledgement sync", mlog.String("post_id", post.Id), mlog.Err(appErr))
+	}
+}
--- a/server/channels/app/post_acknowledgements_test.go
+++ b/server/channels/app/post_acknowledgements_test.go
@@ -109,7 +109,13 @@ func testDeleteAcknowledgementForPost(t *testing.T) {
 	})

 	t.Run("delete acknowledgment for post after 5 min after acknowledged should not delete", func(t *testing.T) {
-		_, nErr := th.App.Srv().Store().PostAcknowledgement().Save(post.Id, th.BasicUser.Id, model.GetMillis()-int64(6*60*1000))
+		acknowledgement := &model.PostAcknowledgement{
+			PostId:         post.Id,
+			UserId:         th.BasicUser.Id,
+			AcknowledgedAt: model.GetMillis() - int64(6*60*1000),
+			ChannelId:      post.ChannelId,
+		}
+		_, nErr := th.App.Srv().Store().PostAcknowledgement().SaveWithModel(acknowledgement)
 		require.NoError(t, nErr)

 		acknowledgments, err := th.App.GetAcknowledgementsForPost(post.Id)
@@ -180,13 +186,13 @@ func testGetAcknowledgementsForPostList(t *testing.T) {
 		acknowledgementsMap, err := th.App.GetAcknowledgementsForPostList(postList)
 		require.Nil(t, err)

-		expected := map[string][]*model.PostAcknowledgement{
-			p1.Id: acks1,
-			p2.Id: acks2,
-		}
-		require.Equal(t, expected, acknowledgementsMap)
+		// Verify p1 acknowledgements (order-agnostic)
 		require.Len(t, acknowledgementsMap[p1.Id], 2)
+		require.ElementsMatch(t, acks1, acknowledgementsMap[p1.Id])
+
+		// Verify p2 acknowledgements (order-agnostic)
 		require.Len(t, acknowledgementsMap[p2.Id], 1)
+		require.ElementsMatch(t, acks2, acknowledgementsMap[p2.Id])
 		require.Nil(t, acknowledgementsMap[p3.Id])
 	})
 }