MM-11434 Only call PreparePostForClient once when creating a post (#9868) (#9876)

* MM-11434 Only call PreparePostForClient once when creating a post

* Have PreparePostForClient provide new metadata when a post already has it and update tests

api4/post.go

@@ -67,7 +67,9 @@ func createPost(c *Context, w http.ResponseWriter, r *http.Request) {
 	c.App.UpdateLastActivityAtIfNeeded(c.Session)
 
 	w.WriteHeader(http.StatusCreated)
-	w.Write([]byte(c.App.PreparePostForClient(rp).ToJson()))
+
+	// Note that rp has already had PreparePostForClient called on it by App.CreatePost
+	w.Write([]byte(rp.ToJson()))
 }
 
 func createEphemeralPost(c *Context, w http.ResponseWriter, r *http.Request) {

app/notification.go

@@ -318,7 +318,10 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	}
 
 	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_POSTED, "", post.ChannelId, "", nil)
-	message.Add("post", a.PreparePostForClient(post).ToJson())
+
+	// Note that PreparePostForClient should've already been called by this point
+	message.Add("post", post.ToJson())
+
 	message.Add("channel_type", channel.Type)
 	message.Add("channel_display_name", notification.GetChannelName(model.SHOW_USERNAME, ""))
 	message.Add("channel_name", channel.Name)

app/post.go

@@ -201,6 +201,10 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo
 		}
 	}
 
+	// Normally, we would let the API layer call PreparePostForClient, but we do it here since it also needs
+	// to be done when we send the post over the websocket in handlePostEvents
+	rpost = a.PreparePostForClient(rpost)
+
 	if err := a.handlePostEvents(rpost, user, channel, triggerWebhooks, parentPostList); err != nil {
 		return nil, err
 	}

app/post_metadata.go

@@ -52,38 +52,36 @@ func (a *App) PreparePostForClient(originalPost *model.Post) *model.Post {
 	// Proxy image links before constructing metadata so that requests go through the proxy
 	post = a.PostWithProxyAddedToImageURLs(post)
 
-	if post.Metadata == nil {
+	post.Metadata = &model.PostMetadata{}
-		post.Metadata = &model.PostMetadata{}
 
-		// Emojis and reaction counts
+	// Emojis and reaction counts
-		if emojis, reactions, err := a.getEmojisAndReactionsForPost(post); err != nil {
+	if emojis, reactions, err := a.getEmojisAndReactionsForPost(post); err != nil {
-			mlog.Warn("Failed to get emojis and reactions for a post", mlog.String("post_id", post.Id), mlog.Any("err", err))
+		mlog.Warn("Failed to get emojis and reactions for a post", mlog.String("post_id", post.Id), mlog.Any("err", err))
-		} else {
+	} else {
-			post.Metadata.Emojis = emojis
+		post.Metadata.Emojis = emojis
-			post.Metadata.Reactions = reactions
+		post.Metadata.Reactions = reactions
-		}
-
-		// Files
-		if fileInfos, err := a.getFileMetadataForPost(post); err != nil {
-			mlog.Warn("Failed to get files for a post", mlog.String("post_id", post.Id), mlog.Any("err", err))
-		} else {
-			post.Metadata.Files = fileInfos
-		}
-
-		// Embeds and image dimensions
-		firstLink, images := getFirstLinkAndImages(post.Message)
-
-		if embed, err := a.getEmbedForPost(post, firstLink); err != nil {
-			mlog.Warn("Failed to get embedded content for a post", mlog.String("post_id", post.Id), mlog.Any("err", err))
-		} else if embed == nil {
-			post.Metadata.Embeds = []*model.PostEmbed{}
-		} else {
-			post.Metadata.Embeds = []*model.PostEmbed{embed}
-		}
-
-		post.Metadata.Images = a.getImagesForPost(post, images)
 	}
 
+	// Files
+	if fileInfos, err := a.getFileMetadataForPost(post); err != nil {
+		mlog.Warn("Failed to get files for a post", mlog.String("post_id", post.Id), mlog.Any("err", err))
+	} else {
+		post.Metadata.Files = fileInfos
+	}
+
+	// Embeds and image dimensions
+	firstLink, images := getFirstLinkAndImages(post.Message)
+
+	if embed, err := a.getEmbedForPost(post, firstLink); err != nil {
+		mlog.Warn("Failed to get embedded content for a post", mlog.String("post_id", post.Id), mlog.Any("err", err))
+	} else if embed == nil {
+		post.Metadata.Embeds = []*model.PostEmbed{}
+	} else {
+		post.Metadata.Embeds = []*model.PostEmbed{embed}
+	}
+
+	post.Metadata.Images = a.getImagesForPost(post, images)
+
 	return post
 }
 

app/post_metadata_test.go

@@ -26,7 +26,7 @@ func TestPreparePostListForClient(t *testing.T) {
 
 	postList := model.NewPostList()
 	for i := 0; i < 5; i++ {
-		postList.AddPost(th.CreatePost(th.BasicChannel))
+		postList.AddPost(&model.Post{})
 	}
 
 	clientPostList := th.App.PreparePostListForClient(postList)
@@ -66,8 +66,10 @@ func TestPreparePostForClient(t *testing.T) {
 		th := setup()
 		defer th.TearDown()
 
-		post := th.CreatePost(th.BasicChannel)
+		message := model.NewId()
-		message := post.Message
+		post := &model.Post{
+			Message: message,
+		}
 
 		clientPost := th.App.PreparePostForClient(post)
 
@@ -93,7 +95,7 @@ func TestPreparePostForClient(t *testing.T) {
 		th := setup()
 		defer th.TearDown()
 
-		post := th.App.PreparePostForClient(th.CreatePost(th.BasicChannel))
+		post := th.CreatePost(th.BasicChannel)
 
 		clientPost := th.App.PreparePostForClient(post)
 
@@ -424,10 +426,6 @@ func testProxyLinkedImage(t *testing.T, th *TestHelper, shouldProxy bool) {
 		Message:   fmt.Sprintf(postTemplate, imageURL),
 	}
 
-	var err *model.AppError
-	post, err = th.App.CreatePost(post, th.BasicChannel, false)
-	require.Nil(t, err)
-
 	clientPost := th.App.PreparePostForClient(post)
 
 	if shouldProxy {