Feat: Enable sorting discussions by original comment (#422)

Author: jakubbortlik

cmd/app/list_discussions.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"sort"
 	"sync"
+	"time"
 
 	"encoding/json"
 
@@ -19,8 +20,16 @@ func Contains[T comparable](elems []T, v T) bool {
 	return false
 }
 
+type SortBy string
+
+const (
+	SortByLatestReply     SortBy = "latest_reply"
+	SortByOriginalComment SortBy = "original_comment"
+)
+
 type DiscussionsRequest struct {
 	Blacklist []string `json:"blacklist" validate:"required"`
+	SortBy    SortBy   `json:"sort_by"`
 }
 
 type DiscussionsResponse struct {
@@ -30,20 +39,30 @@ type DiscussionsResponse struct {
 	Emojis              map[int][]*gitlab.AwardEmoji `json:"emojis"`
 }
 
-type SortableDiscussions []*gitlab.Discussion
+type SortableDiscussions struct {
+	Discussions []*gitlab.Discussion
+	SortBy      SortBy
+}
 
-func (n SortableDiscussions) Len() int {
-	return len(n)
+func (d SortableDiscussions) Len() int {
+	return len(d.Discussions)
 }
 
-func (d SortableDiscussions) Less(i int, j int) bool {
-	iTime := d[i].Notes[len(d[i].Notes)-1].CreatedAt
-	jTime := d[j].Notes[len(d[j].Notes)-1].CreatedAt
-	return iTime.After(*jTime)
+func (d SortableDiscussions) Less(i, j int) bool {
+	var iTime, jTime *time.Time
+	if d.SortBy == SortByOriginalComment {
+		iTime = d.Discussions[i].Notes[0].CreatedAt
+		jTime = d.Discussions[j].Notes[0].CreatedAt
+		return iTime.Before(*jTime)
+	} else { // SortByLatestReply
+		iTime = d.Discussions[i].Notes[len(d.Discussions[i].Notes)-1].CreatedAt
+		jTime = d.Discussions[j].Notes[len(d.Discussions[j].Notes)-1].CreatedAt
+		return iTime.After(*jTime)
+	}
 }
 
-func (n SortableDiscussions) Swap(i, j int) {
-	n[i], n[j] = n[j], n[i]
+func (d SortableDiscussions) Swap(i, j int) {
+	d.Discussions[i], d.Discussions[j] = d.Discussions[j], d.Discussions[i]
 }
 
 type DiscussionsLister interface {
@@ -115,8 +134,14 @@ func (a discussionsListerService) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	sortedLinkedDiscussions := SortableDiscussions(linkedDiscussions)
-	sortedUnlinkedDiscussions := SortableDiscussions(unlinkedDiscussions)
+	sortedLinkedDiscussions := SortableDiscussions{
+		Discussions: linkedDiscussions,
+		SortBy:      request.SortBy,
+	}
+	sortedUnlinkedDiscussions := SortableDiscussions{
+		Discussions: unlinkedDiscussions,
+		SortBy:      request.SortBy,
+	}
 
 	sort.Sort(sortedLinkedDiscussions)
 	sort.Sort(sortedUnlinkedDiscussions)

cmd/app/list_discussions_test.go

@@ -21,8 +21,14 @@ func (f fakeDiscussionsLister) ListMergeRequestDiscussions(pid interface{}, merg
 	if err != nil {
 		return nil, nil, err
 	}
-	now := time.Now()
-	newer := now.Add(time.Second * 100)
+
+	timePointers := make([]*time.Time, 6)
+	timePointers[0] = new(time.Time)
+	*timePointers[0] = time.Now()
+	for i := 1; i < len(timePointers); i++ {
+		timePointers[i] = new(time.Time)
+		*timePointers[i] = timePointers[i-1].Add(time.Second * 100)
+	}
 
 	type Author struct {
 		ID        int    `json:"id"`
@@ -35,8 +41,18 @@ func (f fakeDiscussionsLister) ListMergeRequestDiscussions(pid interface{}, merg
 	}
 
 	testListDiscussionsResponse := []*gitlab.Discussion{
-		{Notes: []*gitlab.Note{{CreatedAt: &now, Type: "DiffNote", Author: Author{Username: "hcramer"}}}},
-		{Notes: []*gitlab.Note{{CreatedAt: &newer, Type: "DiffNote", Author: Author{Username: "hcramer2"}}}},
+		{Notes: []*gitlab.Note{
+			{CreatedAt: timePointers[0], Type: "DiffNote", Author: Author{Username: "hcramer0"}},
+			{CreatedAt: timePointers[4], Type: "DiffNote", Author: Author{Username: "hcramer1"}},
+		}},
+		{Notes: []*gitlab.Note{
+			{CreatedAt: timePointers[2], Type: "DiffNote", Author: Author{Username: "hcramer2"}},
+			{CreatedAt: timePointers[3], Type: "DiffNote", Author: Author{Username: "hcramer3"}},
+		}},
+		{Notes: []*gitlab.Note{
+			{CreatedAt: timePointers[1], Type: "DiffNote", Author: Author{Username: "hcramer4"}},
+			{CreatedAt: timePointers[5], Type: "DiffNote", Author: Author{Username: "hcramer5"}},
+		}},
 	}
 	return testListDiscussionsResponse, resp, err
 }
@@ -66,8 +82,23 @@ func getDiscussionsList(t *testing.T, svc http.Handler, request *http.Request) D
 }
 
 func TestListDiscussions(t *testing.T) {
-	t.Run("Returns sorted discussions", func(t *testing.T) {
-		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}})
+	t.Run("Returns discussions sorted by latest reply", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}, SortBy: "latest_reply"})
+		svc := middleware(
+			discussionsListerService{testProjectData, fakeDiscussionsLister{}},
+			withMr(testProjectData, fakeMergeRequestLister{}),
+			withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}),
+			withMethodCheck(http.MethodPost),
+		)
+		data := getDiscussionsList(t, svc, request)
+		assert(t, data.Message, "Discussions retrieved")
+		assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer4") /* Sorting applied */
+		assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer0")
+		assert(t, data.Discussions[2].Notes[0].Author.Username, "hcramer2")
+	})
+
+	t.Run("Returns discussions sorted by original comment", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}, SortBy: "original_comment"})
 		svc := middleware(
 			discussionsListerService{testProjectData, fakeDiscussionsLister{}},
 			withMr(testProjectData, fakeMergeRequestLister{}),
@@ -76,12 +107,13 @@ func TestListDiscussions(t *testing.T) {
 		)
 		data := getDiscussionsList(t, svc, request)
 		assert(t, data.Message, "Discussions retrieved")
-		assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer2") /* Sorting applied */
-		assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer")
+		assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer0") /* Sorting applied */
+		assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer4")
+		assert(t, data.Discussions[2].Notes[0].Author.Username, "hcramer2")
 	})
 
 	t.Run("Uses blacklist to filter unwanted authors", func(t *testing.T) {
-		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{"hcramer"}})
+		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{"hcramer0"}, SortBy: "latest_reply"})
 		svc := middleware(
 			discussionsListerService{testProjectData, fakeDiscussionsLister{}},
 			withMr(testProjectData, fakeMergeRequestLister{}),
@@ -90,8 +122,9 @@ func TestListDiscussions(t *testing.T) {
 		)
 		data := getDiscussionsList(t, svc, request)
 		assert(t, data.SuccessResponse.Message, "Discussions retrieved")
-		assert(t, len(data.Discussions), 1)
-		assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer2")
+		assert(t, len(data.Discussions), 2)
+		assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer4")
+		assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer2")
 	})
 	t.Run("Handles errors from Gitlab client", func(t *testing.T) {
 		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}})

doc/gitlab.nvim.txt

@@ -219,6 +219,7 @@ you call this function with no values the defaults will be used:
           toggle_tree_type = "i", -- Toggle type of discussion tree - "simple", or "by_file_name"
           publish_draft = "P", -- Publish the currently focused note/comment
           toggle_draft_mode = "D", -- Toggle between draft mode (comments posted as drafts) and live mode (comments are posted immediately)
+          toggle_sort_method = "st", -- Toggle whether discussions are sorted by the "latest_reply", or by "original_comment", see `:h gitlab.nvim.toggle_sort_method`
           toggle_node = "t", -- Open or close the discussion
           toggle_all_discussions = "T", -- Open or close separately both resolved and unresolved discussions
           toggle_resolved_discussions = "R", -- Open or close all resolved discussions
@@ -255,6 +256,7 @@ you call this function with no values the defaults will be used:
         auto_open = true, -- Automatically open when the reviewer is opened
         default_view = "discussions" -- Show "discussions" or "notes" by default
         blacklist = {}, -- List of usernames to remove from tree (bots, CI, etc)
+        sort_by = "latest_reply" -- Sort discussion tree by the "latest_reply", or by "original_comment", see `:h gitlab.nvim.toggle_sort_method`
         keep_current_open = false, -- If true, current discussion stays open even if it should otherwise be closed when toggling
         position = "left", -- "top", "right", "bottom" or "left"
         size = "20%", -- Size of split
@@ -349,6 +351,9 @@ you call this function with no values the defaults will be used:
           resolved = "DiagnosticSignOk",
           unresolved = "DiagnosticSignWarn",
           draft = "DiffviewNonText",
+          draft_mode = "DiagnosticWarn",
+          live_mode = "DiagnosticOk",
+          sort_method = "Keyword",
         }
       }
     })
@@ -929,6 +934,13 @@ gitlab.toggle_draft_mode() ~
 Toggles between draft mode, where comments and notes are added to a review as
 drafts, and regular (or live) mode, where comments are posted immediately.
 
+                                                                *gitlab.nvim.toggle_sort_method*
+gitlab.toggle_sort_method() ~
+
+Toggles whether the discussion tree is sorted by the "latest_reply", with
+threads with the most recent activity on top (the default), or by
+"original_comment", with the oldest threads on top.
+
                                                                 *gitlab.nvim.add_assignee*
 gitlab.add_assignee() ~
 

lua/gitlab/actions/discussions/init.lua

@@ -646,6 +646,16 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
     })
   end
 
+  if keymaps.discussion_tree.toggle_sort_method then
+    vim.keymap.set("n", keymaps.discussion_tree.toggle_sort_method, function()
+      M.toggle_sort_method()
+    end, {
+      buffer = bufnr,
+      desc = "Toggle sort method: by 'latest reply' or by 'original comment'",
+      nowait = keymaps.discussion_tree.toggle_sort_method_nowait,
+    })
+  end
+
   if keymaps.discussion_tree.toggle_resolved then
     vim.keymap.set("n", keymaps.discussion_tree.toggle_resolved, function()
       if M.is_current_node_note(tree) and not M.is_draft_note(tree) then
@@ -795,6 +805,18 @@ M.toggle_draft_mode = function()
   winbar.update_winbar()
 end
 
+---Toggle between sorting by "original comment" (oldest at the top) or "latest reply" (newest at the
+---top).
+M.toggle_sort_method = function()
+  if state.settings.discussion_tree.sort_by == "original_comment" then
+    state.settings.discussion_tree.sort_by = "latest_reply"
+  else
+    state.settings.discussion_tree.sort_by = "original_comment"
+  end
+  winbar.update_winbar()
+  M.rebuild_view(false, true)
+end
+
 ---Indicates whether the node under the cursor is a draft note or not
 ---@param tree NuiTree
 ---@return boolean

lua/gitlab/actions/discussions/winbar.lua

@@ -169,31 +169,41 @@ M.make_winbar = function(t)
     notes_title = "%#Text#" .. notes_title
   end
 
+  local sort_method = M.get_sort_method()
   local mode = M.get_mode()
 
   -- Join everything together and return it
   local separator = "%#Comment#|"
   local end_section = "%="
   local help = "%#Comment#Help: " .. (t.help_keymap and t.help_keymap:gsub(" ", "<space>") .. " " or "unmapped")
   return string.format(
-    " %s %s %s %s %s %s %s",
+    " %s %s %s %s %s %s %s %s %s",
     discussion_title,
     separator,
     notes_title,
     end_section,
+    sort_method,
+    separator,
     mode,
     separator,
     help
   )
 end
 
+---Returns a string for the winbar indicating the sort method
+---@return string
+M.get_sort_method = function()
+  local sort_method = state.settings.discussion_tree.sort_by == "original_comment" and "↓ by thread" or "↑ by reply"
+  return "%#GitlabSortMethod#" .. sort_method
+end
+
 ---Returns a string for the winbar indicating the mode type, live or draft
 ---@return string
 M.get_mode = function()
   if state.settings.discussion_tree.draft_mode then
-    return "%#DiagnosticWarn#Draft Mode"
+    return "%#GitlabDraftMode#Draft Mode"
   else
-    return "%#DiagnosticOK#Live Mode"
+    return "%#GitlabLiveMode#Live Mode"
   end
 end
 

lua/gitlab/colors.lua

@@ -30,5 +30,8 @@ vim.api.nvim_create_autocmd("VimEnter", {
     vim.api.nvim_set_hl(0, "GitlabResolved", get_colors_for_group(discussion.resolved))
     vim.api.nvim_set_hl(0, "GitlabUnresolved", get_colors_for_group(discussion.unresolved))
     vim.api.nvim_set_hl(0, "GitlabDraft", get_colors_for_group(discussion.draft))
+    vim.api.nvim_set_hl(0, "GitlabDraftMode", get_colors_for_group(discussion.draft_mode))
+    vim.api.nvim_set_hl(0, "GitlabLiveMode", get_colors_for_group(discussion.live_mode))
+    vim.api.nvim_set_hl(0, "GitlabSortMethod", get_colors_for_group(discussion.sort_method))
   end,
 })

lua/gitlab/init.lua

@@ -92,6 +92,7 @@ return {
     end
   end,
   toggle_draft_mode = discussions.toggle_draft_mode,
+  toggle_sort_method = discussions.toggle_sort_method,
   publish_all_drafts = draft_notes.publish_all_drafts,
   refresh_data = function()
     -- This also rebuilds the regular views

lua/gitlab/state.lua

@@ -116,6 +116,7 @@ M.settings = {
       toggle_tree_type = "i",
       publish_draft = "P",
       toggle_draft_mode = "D",
+      toggle_sort_method = "st",
       toggle_node = "t",
       toggle_all_discussions = "T",
       toggle_resolved_discussions = "R",
@@ -153,6 +154,7 @@ M.settings = {
     auto_open = true,
     default_view = "discussions",
     blacklist = {},
+    sort_by = "latest_reply",
     keep_current_open = false,
     position = "left",
     size = "20%",
@@ -240,6 +242,9 @@ M.settings = {
       resolved = "DiagnosticSignOk",
       unresolved = "DiagnosticSignWarn",
       draft = "DiffviewNonText",
+      draft_mode = "DiagnosticWarn",
+      live_mode = "DiagnosticOk",
+      sort_method = "Keyword",
     },
   },
 }
@@ -606,6 +611,7 @@ M.dependencies = {
     body = function()
       return {
         blacklist = M.settings.discussion_tree.blacklist,
+        sort_by = M.settings.discussion_tree.sort_by,
       }
     end,
   },