MM-31711 - Implement cursor paging for threads (#16748)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

api4/user.go

@@ -2855,7 +2855,8 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	options := model.GetUserThreadsOpts{
 		Since:    0,
-		Page:     0,
+		Before:   "",
+		After:    "",
 		PageSize: 30,
 		Unread:   false,
 		Extended: false,
@@ -2872,18 +2873,15 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) {
 		options.Since = since
 	}
 
-	pageString := r.URL.Query().Get("page")
+	options.Before = r.URL.Query().Get("before")
-	if pageString != "" {
+	options.After = r.URL.Query().Get("after")
-		page, parseError := strconv.ParseUint(pageString, 10, 64)
+	// parameters are mutually exclusive
-		if parseError != nil {
+	if options.Before != "" && options.After != "" {
-			c.SetInvalidParam("page")
+		c.Err = model.NewAppError("api.getThreadsForUser", "api.getThreadsForUser.bad_params", nil, "", http.StatusBadRequest)
-			return
+		return
-		}
-		options.Page = page
 	}
-
 	pageSizeString := r.URL.Query().Get("pageSize")
-	if pageString != "" {
+	if pageSizeString != "" {
 		pageSize, parseError := strconv.ParseUint(pageSizeString, 10, 64)
 		if parseError != nil {
 			c.SetInvalidParam("pageSize")

api4/user_test.go

@@ -5261,7 +5261,13 @@ func TestUpdatePassword(t *testing.T) {
 func TestGetThreadsForUser(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
+	os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true")
+	defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS")
 
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.ThreadAutoFollow = true
+		*cfg.ServiceSettings.CollapsedThreads = model.COLLAPSED_THREADS_DEFAULT_ON
+	})
 	t.Run("empty", func(t *testing.T) {
 		Client := th.Client
 
@@ -5271,10 +5277,7 @@ func TestGetThreadsForUser(t *testing.T) {
 
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 
-		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
+		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{})
-			Page:     0,
-			PageSize: 30,
-		})
 		require.Nil(t, resp.Error)
 		require.Len(t, uss.Threads, 0)
 	})
@@ -5291,10 +5294,7 @@ func TestGetThreadsForUser(t *testing.T) {
 
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 
-		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
+		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{})
-			Page:     0,
-			PageSize: 30,
-		})
 		require.Nil(t, resp.Error)
 		require.Len(t, uss.Threads, 1)
 		require.Equal(t, uss.Threads[0].PostId, rpost.Id)
@@ -5314,8 +5314,6 @@ func TestGetThreadsForUser(t *testing.T) {
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 
 		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
-			PageSize: 30,
 			Extended: true,
 		})
 		require.Nil(t, resp.Error)
@@ -5338,9 +5336,7 @@ func TestGetThreadsForUser(t *testing.T) {
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 
 		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		require.Nil(t, resp.Error)
 		require.Len(t, uss.Threads, 1)
@@ -5353,17 +5349,13 @@ func TestGetThreadsForUser(t *testing.T) {
 		require.Nil(t, resp2.Error)
 
 		uss, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		require.Nil(t, resp.Error)
 		require.Len(t, uss.Threads, 0)
 
 		uss, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: true,
-			PageSize: 30,
-			Deleted:  true,
 		})
 		require.Nil(t, resp.Error)
 		require.Len(t, uss.Threads, 1)
@@ -5390,9 +5382,7 @@ func TestGetThreadsForUser(t *testing.T) {
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 
 		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		require.Nil(t, resp.Error)
 		require.Len(t, uss.Threads, 30)
@@ -5401,6 +5391,45 @@ func TestGetThreadsForUser(t *testing.T) {
 		require.Equal(t, uss.Threads[0].ReplyCount, int64(1))
 		require.Equal(t, uss.Threads[0].Participants[0].Id, th.BasicUser.Id)
 	})
+
+	t.Run("paged, 10 threads before/after", func(t *testing.T) {
+		Client := th.Client
+
+		var rootIds []*model.Post
+		for i := 0; i < 30; i++ {
+			time.Sleep(1)
+			rpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: fmt.Sprintf("testMsg-%d", i)})
+			rootIds = append(rootIds, rpost)
+			time.Sleep(1)
+			postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: fmt.Sprintf("testReply-%d", i), RootId: rpost.Id})
+		}
+		rootId := rootIds[15].Id // middle point
+		rootIdBefore := rootIds[14].Id
+		rootIdAfter := rootIds[16].Id
+
+		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
+
+		uss, resp := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
+			Deleted:  false,
+			PageSize: 10,
+			Before:   rootId,
+		})
+
+		require.Nil(t, resp.Error)
+		require.Len(t, uss.Threads, 10)
+		require.Equal(t, uss.Threads[0].PostId, rootIdBefore)
+
+		uss2, resp2 := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
+			Deleted:  false,
+			PageSize: 10,
+			After:    rootId,
+		})
+		require.Nil(t, resp2.Error)
+		require.Len(t, uss2.Threads, 10)
+
+		require.Equal(t, uss2.Threads[0].PostId, rootIdAfter)
+
+	})
 }
 
 func TestThreadSocketEvents(t *testing.T) {
@@ -5515,9 +5544,7 @@ func TestFollowThreads(t *testing.T) {
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 		var uss *model.Threads
 		uss, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		CheckNoError(t, resp)
 		require.Len(t, uss.Threads, 1)
@@ -5527,9 +5554,7 @@ func TestFollowThreads(t *testing.T) {
 		CheckOKStatus(t, resp)
 
 		uss, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		CheckNoError(t, resp)
 		require.Len(t, uss.Threads, 0)
@@ -5539,9 +5564,7 @@ func TestFollowThreads(t *testing.T) {
 		CheckOKStatus(t, resp)
 
 		uss, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		CheckNoError(t, resp)
 		require.Len(t, uss.Threads, 1)
@@ -5676,7 +5699,7 @@ func TestThreadCounts(t *testing.T) {
 	checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 2, 1, &model.GetUserThreadsOpts{
 		Deleted: false,
 	})
-	// with Deleted we should get the same as before deleting, minus the reply in the deleted thread
+	// with Deleted we should get the same as before deleting
 	checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 3, 2, &model.GetUserThreadsOpts{
 		Deleted: true,
 	})
@@ -5733,9 +5756,7 @@ func TestMaintainUnreadMentionsInThread(t *testing.T) {
 	})
 	checkThreadList := func(client *model.Client4, userId string, expectedMentions, expectedThreads int) (*model.Threads, *model.Response) {
 		uss, resp := client.GetUserThreads(userId, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		CheckNoError(t, resp)
 		require.Len(t, uss.Threads, expectedThreads)
@@ -5807,9 +5828,7 @@ func TestReadThreads(t *testing.T) {
 
 		var uss, uss2 *model.Threads
 		uss, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		CheckNoError(t, resp)
 		require.Len(t, uss.Threads, 1)
@@ -5820,9 +5839,7 @@ func TestReadThreads(t *testing.T) {
 		CheckOKStatus(t, resp)
 
 		uss2, resp = th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
-			Page:     0,
+			Deleted: false,
-			PageSize: 30,
-			Deleted:  false,
 		})
 		CheckNoError(t, resp)
 		require.Len(t, uss2.Threads, 1)
@@ -5833,16 +5850,15 @@ func TestReadThreads(t *testing.T) {
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
 		defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.SystemAdminUser.Id)
 
-		rpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"})
+		rpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsgC1"})
-		postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id})
+		postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReplyC1", RootId: rpost.Id})
 
-		rrpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testMsg"})
+		rrpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testMsgC2"})
-		postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testReply", RootId: rrpost.Id})
+		postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testReplyC2", RootId: rrpost.Id})
 
 		uss, _ := checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 2, 2, nil)
 
-		time.Sleep(1)
+		resp := th.Client.UpdateThreadReadForUser(th.BasicUser.Id, th.BasicTeam.Id, rrpost.Id, model.GetMillis()+10)
-		resp := th.Client.UpdateThreadReadForUser(th.BasicUser.Id, th.BasicTeam.Id, rrpost.Id, model.GetMillis())
 		CheckNoError(t, resp)
 		CheckOKStatus(t, resp)
 

i18n/en.json

@@ -1444,6 +1444,10 @@
     "id": "api.file.write_file.app_error",
     "translation": "Unable to write the file."
   },
+  {
+    "id": "api.getThreadsForUser.bad_params",
+    "translation": "Before and After parameters to getThreadsForUser are mutually exclusive"
+  },
   {
     "id": "api.image.get.app_error",
     "translation": "Requested image url cannot be parsed."

model/client4.go

@@ -5794,8 +5794,11 @@ func (c *Client4) GetUserThreads(userId, teamId string, options GetUserThreadsOp
 	if options.Since != 0 {
 		v.Set("since", fmt.Sprintf("%d", options.Since))
 	}
-	if options.Page != 0 {
+	if options.Before != "" {
-		v.Set("page", fmt.Sprintf("%d", options.Page))
+		v.Set("before", options.Before)
+	}
+	if options.After != "" {
+		v.Set("after", options.After)
 	}
 	if options.PageSize != 0 {
 		v.Set("pageSize", fmt.Sprintf("%d", options.PageSize))

model/thread.go

@@ -34,9 +34,6 @@ type Threads struct {
 }
 
 type GetUserThreadsOpts struct {
-	// Page specifies which part of the results to return, by PageSize. Default = 0
-	Page uint64
-
 	// PageSize specifies the size of the returned chunk of results. Default = 30
 	PageSize uint64
 
@@ -49,6 +46,12 @@ type GetUserThreadsOpts struct {
 	// Since filters the threads based on their LastUpdateAt timestamp.
 	Since uint64
 
+	// Before specifies thread id as a cursor for pagination and will return `PageSize` threads before the cursor
+	Before string
+
+	// After specifies thread id as a cursor for pagination and will return `PageSize` threads after the cursor
+	After string
+
 	// Unread will make sure that only threads with unread replies are returned
 	Unread bool
 }

store/sqlstore/thread_store.go

@@ -188,10 +188,23 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 		if opts.Since > 0 {
 			newFetchConditions = sq.And{newFetchConditions, sq.GtOrEq{"ThreadMemberships.LastUpdated": opts.Since}}
 		}
+		order := "DESC"
+		if opts.Before != "" {
+			newFetchConditions = sq.And{
+				newFetchConditions,
+				sq.Expr(`LastReplyAt < (SELECT LastReplyAt FROM Threads WHERE PostId = ?)`, opts.Before),
+			}
+		}
+		if opts.After != "" {
+			order = "ASC"
+			newFetchConditions = sq.And{
+				newFetchConditions,
+				sq.Expr(`LastReplyAt > (SELECT LastReplyAt FROM Threads WHERE PostId = ?)`, opts.After),
+			}
+		}
 		if opts.Unread {
 			newFetchConditions = sq.And{newFetchConditions, sq.Expr("ThreadMemberships.LastViewed < Threads.LastReplyAt")}
 		}
-
 		var threads []*JoinedThread
 		query, args, _ := s.getQueryBuilder().
 			Select("Threads.*, Posts.*, ThreadMemberships.LastViewed as LastViewedAt, ThreadMemberships.UnreadMentions as UnreadMentions").
@@ -201,9 +214,9 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 			LeftJoin("Channels ON Posts.ChannelId = Channels.Id").
 			LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Threads.PostId").
 			Where(newFetchConditions).
-			OrderBy("Threads.LastReplyAt DESC").
+			OrderBy("Threads.LastReplyAt " + order).
-			Offset(pageSize * opts.Page).
 			Limit(pageSize).ToSql()
+
 		_, err := s.GetReplica().Select(&threads, query, args...)
 		threadsChan <- store.StoreResult{Data: threads, NErr: err}
 		close(threadsChan)