MM-49845: fixes acknowledgements in getpostsince (#22124)

* MM-49845: fixes acknowledgements in getpostsince

GetPostsSince returns posts updated after a user requested date.
If a user acknowledges a post the post's update at won't change thus not
getting that post returned from GetPostsSince. This is troublesome
particularly for the mobile client since it relies a lot to
GetPostsSince for keeping the data up to date.

This commit updates the post's update_at when a user
acknowledges/un-acknowledges a post fixing this way the issue above.

* Fixes linter

* Apply suggestions from code review

style changes

Co-authored-by: Jesse Hallam <jesse@thehallams.ca>

* Rename ack to acknowledgement

Co-authored-by: Jesse Hallam <jesse@thehallams.ca>

app/post_acknowledgements.go

@@ -42,6 +42,9 @@ func (a *App) SaveAcknowledgementForPost(c *request.Context, postID, userID stri
 		}
 	}
 
+	// The post is always modified since the UpdateAt always changes
+	a.invalidateCacheForChannelPosts(channel.Id)
+
 	a.Srv().Go(func() {
 		a.sendAcknowledgementEvent(model.WebsocketEventAcknowledgementAdded, acknowledgement, post)
 	})
@@ -85,6 +88,9 @@ func (a *App) DeleteAcknowledgementForPost(c *request.Context, postID, userID st
 		return model.NewAppError("DeleteAcknowledgementForPost", "app.acknowledgement.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
 	}
 
+	// The post is always modified since the UpdateAt always changes
+	a.invalidateCacheForChannelPosts(channel.Id)
+
 	a.Srv().Go(func() {
 		a.sendAcknowledgementEvent(model.WebsocketEventAcknowledgementRemoved, oldAck, post)
 	})

app/post_acknowledgements_test.go

@@ -36,21 +36,41 @@ func testSaveAcknowledgementForPost(t *testing.T) {
 		require.Equal(t, post.Id, acknowledgment.PostId)
 		require.Equal(t, th.BasicUser.Id, acknowledgment.UserId)
 	})
+
+	t.Run("saving acknowledgment should update the post's update_at", func(t *testing.T) {
+		post, err := th.App.CreatePostAsUser(th.Context, &model.Post{
+			UserId:    th.BasicUser.Id,
+			ChannelId: th.BasicChannel.Id,
+			Message:   "message",
+		}, "", true)
+
+		require.Nil(t, err)
+
+		oldUpdateAt := post.UpdateAt
+
+		_, err = th.App.SaveAcknowledgementForPost(th.Context, post.Id, th.BasicUser.Id)
+		require.Nil(t, err)
+
+		post, err = th.App.GetSinglePost(post.Id, false)
+		require.Nil(t, err)
+
+		require.Greater(t, post.UpdateAt, oldUpdateAt)
+	})
 }
 
 func testDeleteAcknowledgementForPost(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
-	post, err := th.App.CreatePostAsUser(th.Context, &model.Post{
+	post, err1 := th.App.CreatePostAsUser(th.Context, &model.Post{
 		UserId:    th.BasicUser.Id,
 		ChannelId: th.BasicChannel.Id,
 		CreateAt:  model.GetMillis(),
 		Message:   "message",
 	}, "", true)
-	require.Nil(t, err)
+	require.Nil(t, err1)
 
 	t.Run("delete acknowledgment for post should delete acknowledgement", func(t *testing.T) {
-		_, err = th.App.SaveAcknowledgementForPost(th.Context, post.Id, th.BasicUser.Id)
+		_, err := th.App.SaveAcknowledgementForPost(th.Context, post.Id, th.BasicUser.Id)
 		require.Nil(t, err)
 
 		acknowledgments, err := th.App.GetAcknowledgementsForPost(post.Id)
@@ -66,6 +86,24 @@ func testDeleteAcknowledgementForPost(t *testing.T) {
 		require.Empty(t, acknowledgments)
 	})
 
+	t.Run("deleting acknowledgment should update the post's update_at", func(t *testing.T) {
+		_, err := th.App.SaveAcknowledgementForPost(th.Context, post.Id, th.BasicUser.Id)
+		require.Nil(t, err)
+
+		post, err = th.App.GetSinglePost(post.Id, false)
+		require.Nil(t, err)
+
+		oldUpdateAt := post.UpdateAt
+
+		err = th.App.DeleteAcknowledgementForPost(th.Context, post.Id, th.BasicUser.Id)
+		require.Nil(t, err)
+
+		post, err = th.App.GetSinglePost(post.Id, false)
+		require.Nil(t, err)
+
+		require.Greater(t, post.UpdateAt, oldUpdateAt)
+	})
+
 	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))
 		require.NoError(t, nErr)

store/sqlstore/post_acknowledgements_store.go

@@ -58,6 +58,12 @@ func (s *SqlPostAcknowledgementStore) Save(postID, userID string, acknowledgedAt
 		return nil, err
 	}
 
+	transaction, err := s.GetMasterX().Beginx()
+	if err != nil {
+		return nil, errors.Wrap(err, "begin_transaction")
+	}
+	defer finalizeTransactionX(transaction, &err)
+
 	query := s.getQueryBuilder().
 		Insert("PostAcknowledgements").
 		Columns("PostId", "UserId", "AcknowledgedAt").
@@ -69,28 +75,54 @@ func (s *SqlPostAcknowledgementStore) Save(postID, userID string, acknowledgedAt
 		query = query.SuffixExpr(sq.Expr("ON CONFLICT (postid, userid) DO UPDATE SET AcknowledgedAt = ?", acknowledgement.AcknowledgedAt))
 	}
 
-	_, err := s.GetMasterX().ExecBuilder(query)
+	_, err = transaction.ExecBuilder(query)
 	if err != nil {
 		return nil, err
 	}
 
+	err = updatePost(transaction, acknowledgement.PostId)
+	if err != nil {
+		return nil, err
+	}
+
+	err = transaction.Commit()
+	if err != nil {
+		return nil, errors.Wrap(err, "commit_transaction")
+	}
+
 	return acknowledgement, nil
 }
 
-func (s *SqlPostAcknowledgementStore) Delete(ack *model.PostAcknowledgement) error {
+func (s *SqlPostAcknowledgementStore) Delete(acknowledgement *model.PostAcknowledgement) error {
+	transaction, err := s.GetMasterX().Beginx()
+	if err != nil {
+		return errors.Wrap(err, "begin_transaction")
+	}
+	defer finalizeTransactionX(transaction, &err)
+
 	query := s.getQueryBuilder().
 		Update("PostAcknowledgements").
 		Set("AcknowledgedAt", 0).
 		Where(sq.And{
-			sq.Eq{"PostId": ack.PostId},
-			sq.Eq{"UserId": ack.UserId},
+			sq.Eq{"PostId": acknowledgement.PostId},
+			sq.Eq{"UserId": acknowledgement.UserId},
 		})
 
-	_, err := s.GetMasterX().ExecBuilder(query)
+	_, err = transaction.ExecBuilder(query)
 	if err != nil {
 		return err
 	}
 
+	err = updatePost(transaction, acknowledgement.PostId)
+	if err != nil {
+		return err
+	}
+
+	err = transaction.Commit()
+	if err != nil {
+		return errors.Wrap(err, "commit_transaction")
+	}
+
 	return nil
 }
 
@@ -142,3 +174,18 @@ func (s *SqlPostAcknowledgementStore) GetForPosts(postIds []string) ([]*model.Po
 
 	return acknowledgements, nil
 }
+
+func updatePost(transaction *sqlxTxWrapper, postId string) error {
+	_, err := transaction.Exec(
+		`UPDATE
+			Posts
+		SET
+			UpdateAt = ?
+		WHERE
+			Id = ?`,
+		model.GetMillis(),
+		postId,
+	)
+
+	return err
+}

store/storetest/post_acknowledgements_store.go

@@ -31,23 +31,33 @@ func testPostAcknowledgementsStoreSave(t *testing.T, ss store.Store) {
 			PersistentNotifications: model.NewBool(false),
 		},
 	}
-	_, err := ss.Post().Save(&p1)
+	post, err := ss.Post().Save(&p1)
 	require.NoError(t, err)
 
 	t.Run("consecutive saves should just update the acknowledged at", func(t *testing.T) {
-		_, err := ss.PostAcknowledgement().Save(p1.Id, userId1, 0)
+		_, err := ss.PostAcknowledgement().Save(post.Id, userId1, 0)
 		require.NoError(t, err)
 
-		_, err = ss.PostAcknowledgement().Save(p1.Id, userId1, 0)
+		_, err = ss.PostAcknowledgement().Save(post.Id, userId1, 0)
 		require.NoError(t, err)
 
-		ack1, err := ss.PostAcknowledgement().Save(p1.Id, userId1, 0)
+		ack1, err := ss.PostAcknowledgement().Save(post.Id, userId1, 0)
 		require.NoError(t, err)
 
-		acknowledgements, err := ss.PostAcknowledgement().GetForPost(p1.Id)
+		acknowledgements, err := ss.PostAcknowledgement().GetForPost(post.Id)
 		require.NoError(t, err)
 		require.ElementsMatch(t, acknowledgements, []*model.PostAcknowledgement{ack1})
 	})
+
+	t.Run("saving should update the update at of the post", func(t *testing.T) {
+		oldUpdateAt := post.UpdateAt
+		_, err := ss.PostAcknowledgement().Save(post.Id, userId1, 0)
+		require.NoError(t, err)
+
+		post, err = ss.Post().GetSingle(post.Id, false)
+		require.NoError(t, err)
+		require.Greater(t, post.UpdateAt, oldUpdateAt)
+	})
 }
 
 func testPostAcknowledgementsStoreGetForPost(t *testing.T, ss store.Store) {