Skip to content

Commit 5f44961

Browse files
committed
feat: enable sorting discussions by original comment
1 parent d8d289d commit 5f44961

File tree

6 files changed

+114
-22
lines changed

6 files changed

+114
-22
lines changed

cmd/app/list_discussions.go

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"net/http"
55
"sort"
66
"sync"
7+
"time"
78

89
"encoding/json"
910

@@ -19,8 +20,16 @@ func Contains[T comparable](elems []T, v T) bool {
1920
return false
2021
}
2122

23+
type SortBy string
24+
25+
const (
26+
SortByLatestReply SortBy = "latest_reply"
27+
SortByOriginalComment SortBy = "original_comment"
28+
)
29+
2230
type DiscussionsRequest struct {
2331
Blacklist []string `json:"blacklist" validate:"required"`
32+
SortBy SortBy `json:"sort_by"`
2433
}
2534

2635
type DiscussionsResponse struct {
@@ -30,20 +39,30 @@ type DiscussionsResponse struct {
3039
Emojis map[int][]*gitlab.AwardEmoji `json:"emojis"`
3140
}
3241

33-
type SortableDiscussions []*gitlab.Discussion
42+
type SortableDiscussions struct {
43+
Discussions []*gitlab.Discussion
44+
SortBy SortBy
45+
}
3446

35-
func (n SortableDiscussions) Len() int {
36-
return len(n)
47+
func (d SortableDiscussions) Len() int {
48+
return len(d.Discussions)
3749
}
3850

39-
func (d SortableDiscussions) Less(i int, j int) bool {
40-
iTime := d[i].Notes[len(d[i].Notes)-1].CreatedAt
41-
jTime := d[j].Notes[len(d[j].Notes)-1].CreatedAt
42-
return iTime.After(*jTime)
51+
func (d SortableDiscussions) Less(i, j int) bool {
52+
var iTime, jTime *time.Time
53+
if d.SortBy == SortByOriginalComment {
54+
iTime = d.Discussions[i].Notes[0].CreatedAt
55+
jTime = d.Discussions[j].Notes[0].CreatedAt
56+
return iTime.Before(*jTime)
57+
} else { // SortByLatestReply
58+
iTime = d.Discussions[i].Notes[len(d.Discussions[i].Notes)-1].CreatedAt
59+
jTime = d.Discussions[j].Notes[len(d.Discussions[j].Notes)-1].CreatedAt
60+
return iTime.After(*jTime)
61+
}
4362
}
4463

45-
func (n SortableDiscussions) Swap(i, j int) {
46-
n[i], n[j] = n[j], n[i]
64+
func (d SortableDiscussions) Swap(i, j int) {
65+
d.Discussions[i], d.Discussions[j] = d.Discussions[j], d.Discussions[i]
4766
}
4867

4968
type DiscussionsLister interface {
@@ -115,8 +134,14 @@ func (a discussionsListerService) ServeHTTP(w http.ResponseWriter, r *http.Reque
115134
return
116135
}
117136

118-
sortedLinkedDiscussions := SortableDiscussions(linkedDiscussions)
119-
sortedUnlinkedDiscussions := SortableDiscussions(unlinkedDiscussions)
137+
sortedLinkedDiscussions := SortableDiscussions{
138+
Discussions: linkedDiscussions,
139+
SortBy: request.SortBy,
140+
}
141+
sortedUnlinkedDiscussions := SortableDiscussions{
142+
Discussions: unlinkedDiscussions,
143+
SortBy: request.SortBy,
144+
}
120145

121146
sort.Sort(sortedLinkedDiscussions)
122147
sort.Sort(sortedUnlinkedDiscussions)

cmd/app/list_discussions_test.go

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@ func (f fakeDiscussionsLister) ListMergeRequestDiscussions(pid interface{}, merg
2121
if err != nil {
2222
return nil, nil, err
2323
}
24-
now := time.Now()
25-
newer := now.Add(time.Second * 100)
24+
25+
timePointers := make([]*time.Time, 6)
26+
timePointers[0] = new(time.Time)
27+
*timePointers[0] = time.Now()
28+
for i := 1; i < len(timePointers); i++ {
29+
timePointers[i] = new(time.Time)
30+
*timePointers[i] = timePointers[i-1].Add(time.Second * 100)
31+
}
2632

2733
type Author struct {
2834
ID int `json:"id"`
@@ -35,8 +41,18 @@ func (f fakeDiscussionsLister) ListMergeRequestDiscussions(pid interface{}, merg
3541
}
3642

3743
testListDiscussionsResponse := []*gitlab.Discussion{
38-
{Notes: []*gitlab.Note{{CreatedAt: &now, Type: "DiffNote", Author: Author{Username: "hcramer"}}}},
39-
{Notes: []*gitlab.Note{{CreatedAt: &newer, Type: "DiffNote", Author: Author{Username: "hcramer2"}}}},
44+
{Notes: []*gitlab.Note{
45+
{CreatedAt: timePointers[0], Type: "DiffNote", Author: Author{Username: "hcramer0"}},
46+
{CreatedAt: timePointers[4], Type: "DiffNote", Author: Author{Username: "hcramer1"}},
47+
}},
48+
{Notes: []*gitlab.Note{
49+
{CreatedAt: timePointers[2], Type: "DiffNote", Author: Author{Username: "hcramer2"}},
50+
{CreatedAt: timePointers[3], Type: "DiffNote", Author: Author{Username: "hcramer3"}},
51+
}},
52+
{Notes: []*gitlab.Note{
53+
{CreatedAt: timePointers[1], Type: "DiffNote", Author: Author{Username: "hcramer4"}},
54+
{CreatedAt: timePointers[5], Type: "DiffNote", Author: Author{Username: "hcramer5"}},
55+
}},
4056
}
4157
return testListDiscussionsResponse, resp, err
4258
}
@@ -66,8 +82,23 @@ func getDiscussionsList(t *testing.T, svc http.Handler, request *http.Request) D
6682
}
6783

6884
func TestListDiscussions(t *testing.T) {
69-
t.Run("Returns sorted discussions", func(t *testing.T) {
70-
request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}})
85+
t.Run("Returns discussions sorted by latest reply", func(t *testing.T) {
86+
request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}, SortBy: "latest_reply"})
87+
svc := middleware(
88+
discussionsListerService{testProjectData, fakeDiscussionsLister{}},
89+
withMr(testProjectData, fakeMergeRequestLister{}),
90+
withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}),
91+
withMethodCheck(http.MethodPost),
92+
)
93+
data := getDiscussionsList(t, svc, request)
94+
assert(t, data.Message, "Discussions retrieved")
95+
assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer4") /* Sorting applied */
96+
assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer0")
97+
assert(t, data.Discussions[2].Notes[0].Author.Username, "hcramer2")
98+
})
99+
100+
t.Run("Returns discussions sorted by original comment", func(t *testing.T) {
101+
request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}, SortBy: "original_comment"})
71102
svc := middleware(
72103
discussionsListerService{testProjectData, fakeDiscussionsLister{}},
73104
withMr(testProjectData, fakeMergeRequestLister{}),
@@ -76,12 +107,13 @@ func TestListDiscussions(t *testing.T) {
76107
)
77108
data := getDiscussionsList(t, svc, request)
78109
assert(t, data.Message, "Discussions retrieved")
79-
assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer2") /* Sorting applied */
80-
assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer")
110+
assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer0") /* Sorting applied */
111+
assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer4")
112+
assert(t, data.Discussions[2].Notes[0].Author.Username, "hcramer2")
81113
})
82114

83115
t.Run("Uses blacklist to filter unwanted authors", func(t *testing.T) {
84-
request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{"hcramer"}})
116+
request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{"hcramer0"}, SortBy: "latest_reply"})
85117
svc := middleware(
86118
discussionsListerService{testProjectData, fakeDiscussionsLister{}},
87119
withMr(testProjectData, fakeMergeRequestLister{}),
@@ -90,8 +122,9 @@ func TestListDiscussions(t *testing.T) {
90122
)
91123
data := getDiscussionsList(t, svc, request)
92124
assert(t, data.SuccessResponse.Message, "Discussions retrieved")
93-
assert(t, len(data.Discussions), 1)
94-
assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer2")
125+
assert(t, len(data.Discussions), 2)
126+
assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer4")
127+
assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer2")
95128
})
96129
t.Run("Handles errors from Gitlab client", func(t *testing.T) {
97130
request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}})

doc/gitlab.nvim.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ you call this function with no values the defaults will be used:
219219
toggle_tree_type = "i", -- Toggle type of discussion tree - "simple", or "by_file_name"
220220
publish_draft = "P", -- Publish the currently focused note/comment
221221
toggle_draft_mode = "D", -- Toggle between draft mode (comments posted as drafts) and live mode (comments are posted immediately)
222+
toggle_sort_by = "st", -- Toggle whether discussions are sorted by the "latest_reply", or by "original_comment", see `:h gitlab.nvim.toggle_sort_by`
222223
toggle_node = "t", -- Open or close the discussion
223224
toggle_all_discussions = "T", -- Open or close separately both resolved and unresolved discussions
224225
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:
255256
auto_open = true, -- Automatically open when the reviewer is opened
256257
default_view = "discussions" -- Show "discussions" or "notes" by default
257258
blacklist = {}, -- List of usernames to remove from tree (bots, CI, etc)
259+
sort_by = "latest_reply" -- Sort discussion tree by the "latest_reply", or by "original_comment", see `:h gitlab.nvim.toggle_sort_by`
258260
keep_current_open = false, -- If true, current discussion stays open even if it should otherwise be closed when toggling
259261
position = "left", -- "top", "right", "bottom" or "left"
260262
size = "20%", -- Size of split
@@ -929,6 +931,13 @@ gitlab.toggle_draft_mode() ~
929931
Toggles between draft mode, where comments and notes are added to a review as
930932
drafts, and regular (or live) mode, where comments are posted immediately.
931933

934+
*gitlab.nvim.toggle_sort_by*
935+
gitlab.toggle_sort_by() ~
936+
937+
Toggles whether the discussion tree is sorted by the "latest_reply", with
938+
threads with the most recent activity on top, or by "original_comment", with
939+
the oldest threads on top.
940+
932941
*gitlab.nvim.add_assignee*
933942
gitlab.add_assignee() ~
934943

lua/gitlab/actions/discussions/init.lua

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,16 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
646646
})
647647
end
648648

649+
if keymaps.discussion_tree.toggle_sort_by then
650+
vim.keymap.set("n", keymaps.discussion_tree.toggle_sort_by, function()
651+
M.toggle_sort_by()
652+
end, {
653+
buffer = bufnr,
654+
desc = "Toggle sort order: by 'latest reply' or by 'original comment'",
655+
nowait = keymaps.discussion_tree.toggle_sort_by_nowait,
656+
})
657+
end
658+
649659
if keymaps.discussion_tree.toggle_resolved then
650660
vim.keymap.set("n", keymaps.discussion_tree.toggle_resolved, function()
651661
if M.is_current_node_note(tree) and not M.is_draft_note(tree) then
@@ -795,6 +805,17 @@ M.toggle_draft_mode = function()
795805
winbar.update_winbar()
796806
end
797807

808+
---Toggle between draft mode (comments posted as drafts) and live mode (comments are posted immediately)
809+
M.toggle_sort_by = function()
810+
if state.settings.discussion_tree.sort_by == "original_comment" then
811+
state.settings.discussion_tree.sort_by = "latest_reply"
812+
else
813+
state.settings.discussion_tree.sort_by = "original_comment"
814+
end
815+
u.notify("Sort discussion tree by '" .. state.settings.discussion_tree.sort_by .. "'", vim.log.levels.INFO)
816+
M.rebuild_view(false, true)
817+
end
818+
798819
---Indicates whether the node under the cursor is a draft note or not
799820
---@param tree NuiTree
800821
---@return boolean

lua/gitlab/init.lua

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ return {
9292
end
9393
end,
9494
toggle_draft_mode = discussions.toggle_draft_mode,
95+
toggle_sort_by = discussions.toggle_sort_by,
9596
publish_all_drafts = draft_notes.publish_all_drafts,
9697
refresh_data = function()
9798
-- This also rebuilds the regular views

lua/gitlab/state.lua

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ M.settings = {
116116
toggle_tree_type = "i",
117117
publish_draft = "P",
118118
toggle_draft_mode = "D",
119+
toggle_sort_by = "st",
119120
toggle_node = "t",
120121
toggle_all_discussions = "T",
121122
toggle_resolved_discussions = "R",
@@ -153,6 +154,7 @@ M.settings = {
153154
auto_open = true,
154155
default_view = "discussions",
155156
blacklist = {},
157+
sort_by = "latest_reply",
156158
keep_current_open = false,
157159
position = "left",
158160
size = "20%",
@@ -606,6 +608,7 @@ M.dependencies = {
606608
body = function()
607609
return {
608610
blacklist = M.settings.discussion_tree.blacklist,
611+
sort_by = M.settings.discussion_tree.sort_by,
609612
}
610613
end,
611614
},

0 commit comments

Comments
 (0)