Draft: fix: improve discussions panel behaviour

README.md

@@ -2,7 +2,7 @@
 
 This Neovim plugin is designed to make it easy to review Gitlab MRs from within the editor. This means you can do things like:
 
-- Create, approve, and merge MRs for the current branch
+- Create, approve, rebase, and merge MRs for the current branch
 - Read and edit an MR description
 - Add or remove reviewers and assignees
 - Resolve, reply to, and unresolve discussion threads
@@ -143,9 +143,13 @@ glA	Approve MR
 glR	Revoke MR approval
 glM	Merge the feature branch to the target branch and close MR
 glm	Set MR to merge automatically when the pipeline succeeds
+glrr	Rebase the feature branch of the MR on the server (if not already rebased) and pull the new state
+glrs	Same as `glrr`, but skip the CI pipeline
+glrf	Same as `glrr`, but rebase even if MR already is rebased
 glC	Create a new MR for currently checked-out feature branch
 glc	Chose MR for review
 glS	Start review for the currently checked-out branch
+gl<C-R>	Load new MR state from Gitlab and apply new diff refs to the diff view
 gls	Show the editable summary of the MR
 glu	Copy the URL of the MR to the system clipboard
 glo	Open the URL of the MR in the default Internet browser

cmd/app/rebase_mr.go

@@ -0,0 +1,80 @@
+package app
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"time"
+
+	gitlab "gitlab.com/gitlab-org/api/client-go"
+)
+
+type RebaseMrRequest struct {
+	SkipCI bool `json:"skip_ci,omitempty"`
+}
+
+type MergeRequestRebaser interface {
+	RebaseMergeRequest(pid interface{}, mergeRequest int64, opt *gitlab.RebaseMergeRequestOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Response, error)
+}
+
+type MergeRequestRebaserClient interface {
+	MergeRequestRebaser
+	MergeRequestGetter
+}
+
+type mergeRequestRebaserService struct {
+	data
+	client MergeRequestRebaserClient
+}
+
+/* Rebases a merge request on the server */
+func (a mergeRequestRebaserService) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	payload := r.Context().Value(payload("payload")).(*RebaseMrRequest)
+
+	opts := gitlab.RebaseMergeRequestOptions{
+		SkipCI: &payload.SkipCI,
+	}
+
+	res, err := a.client.RebaseMergeRequest(a.projectInfo.ProjectId, a.projectInfo.MergeId, &opts)
+	if err != nil {
+		handleError(w, err, "Could not rebase MR", http.StatusInternalServerError)
+		return
+	}
+
+	if res.StatusCode >= 300 {
+		handleError(w, GenericError{r.URL.Path}, "Could not rebase MR", res.StatusCode)
+		return
+	}
+
+	// Poll until rebase completes (GitLab rebase is async)
+	for {
+		mr, _, getMrErr := a.client.GetMergeRequest(
+			a.projectInfo.ProjectId,
+			a.projectInfo.MergeId,
+			&gitlab.GetMergeRequestsOptions{
+				IncludeRebaseInProgress: gitlab.Ptr(true),
+			},
+		)
+		if getMrErr != nil {
+			handleError(w, getMrErr, "Could not check rebase status", http.StatusInternalServerError)
+			return
+		}
+		if !mr.RebaseInProgress {
+			break
+		}
+		time.Sleep(1 * time.Second)
+	}
+
+	skippingCI := ""
+	if payload.SkipCI {
+		skippingCI = " (skipping CI)"
+	}
+	response := SuccessResponse{Message: fmt.Sprintf("MR rebased on server%s", skippingCI)}
+
+	w.WriteHeader(http.StatusOK)
+
+	err = json.NewEncoder(w).Encode(response)
+	if err != nil {
+		handleError(w, err, "Could not encode response", http.StatusInternalServerError)
+	}
+}

cmd/app/rebase_mr_test.go

@@ -0,0 +1,110 @@
+package app
+
+import (
+	"net/http"
+	"testing"
+
+	gitlab "gitlab.com/gitlab-org/api/client-go"
+)
+
+type fakeMergeRequestRebaserClient struct {
+	testBase
+	rebaseInProgressCount int // number of times to return RebaseInProgress: true
+	getMergeRequestCalls  int // tracks how many times GetMergeRequest was called
+}
+
+func (f *fakeMergeRequestRebaserClient) RebaseMergeRequest(pid interface{}, mergeRequest int64, opt *gitlab.RebaseMergeRequestOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Response, error) {
+	resp, err := f.handleGitlabError()
+	if err != nil {
+		return nil, err
+	}
+
+	return resp, err
+}
+
+func (f *fakeMergeRequestRebaserClient) GetMergeRequest(pid interface{}, mergeRequest int64, opt *gitlab.GetMergeRequestsOptions, options ...gitlab.RequestOptionFunc) (*gitlab.MergeRequest, *gitlab.Response, error) {
+	resp, err := f.handleGitlabError()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	f.getMergeRequestCalls++
+	rebaseInProgress := f.getMergeRequestCalls <= f.rebaseInProgressCount
+
+	return &gitlab.MergeRequest{RebaseInProgress: rebaseInProgress}, resp, err
+}
+
+func TestRebaseHandler(t *testing.T) {
+	var testRebaseMrPayload = RebaseMrRequest{SkipCI: false}
+	t.Run("Rebases merge request when rebase completes immediately", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/rebase", testRebaseMrPayload)
+		fakeClient := &fakeMergeRequestRebaserClient{rebaseInProgressCount: 0}
+		svc := middleware(
+			mergeRequestRebaserService{testProjectData, fakeClient},
+			withMr(testProjectData, fakeMergeRequestLister{}),
+			withPayloadValidation(methodToPayload{
+				http.MethodPost: newPayload[RebaseMrRequest],
+			}),
+			withMethodCheck(http.MethodPost),
+		)
+		data := getSuccessData(t, svc, request)
+		assert(t, data.Message, "MR rebased on server")
+		assert(t, fakeClient.getMergeRequestCalls, 1)
+	})
+	t.Run("Rebases merge request and polls until rebase completes", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/rebase", testRebaseMrPayload)
+		fakeClient := &fakeMergeRequestRebaserClient{rebaseInProgressCount: 1}
+		svc := middleware(
+			mergeRequestRebaserService{testProjectData, fakeClient},
+			withMr(testProjectData, fakeMergeRequestLister{}),
+			withPayloadValidation(methodToPayload{
+				http.MethodPost: newPayload[RebaseMrRequest],
+			}),
+			withMethodCheck(http.MethodPost),
+		)
+		data := getSuccessData(t, svc, request)
+		assert(t, data.Message, "MR rebased on server")
+		assert(t, fakeClient.getMergeRequestCalls, 2)
+	})
+	var testRebaseMrPayloadSkipCI = RebaseMrRequest{SkipCI: true}
+	t.Run("Rebases merge request and skips CI", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/rebase", testRebaseMrPayloadSkipCI)
+		fakeClient := &fakeMergeRequestRebaserClient{}
+		svc := middleware(
+			mergeRequestRebaserService{testProjectData, fakeClient},
+			withMr(testProjectData, fakeMergeRequestLister{}),
+			withPayloadValidation(methodToPayload{
+				http.MethodPost: newPayload[RebaseMrRequest],
+			}),
+			withMethodCheck(http.MethodPost),
+		)
+		data := getSuccessData(t, svc, request)
+		assert(t, data.Message, "MR rebased on server (skipping CI)")
+	})
+	t.Run("Handles errors from Gitlab client", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/rebase", testRebaseMrPayload)
+		svc := middleware(
+			mergeRequestRebaserService{testProjectData, &fakeMergeRequestRebaserClient{testBase: testBase{errFromGitlab: true}}},
+			withMr(testProjectData, fakeMergeRequestLister{}),
+			withPayloadValidation(methodToPayload{
+				http.MethodPost: newPayload[RebaseMrRequest],
+			}),
+			withMethodCheck(http.MethodPost),
+		)
+		data, _ := getFailData(t, svc, request)
+		checkErrorFromGitlab(t, data, "Could not rebase MR")
+	})
+	t.Run("Handles non-200s from Gitlab", func(t *testing.T) {
+		request := makeRequest(t, http.MethodPost, "/mr/rebase", testRebaseMrPayload)
+		svc := middleware(
+			mergeRequestRebaserService{testProjectData, &fakeMergeRequestRebaserClient{testBase: testBase{status: http.StatusSeeOther}}},
+			withMr(testProjectData, fakeMergeRequestLister{}),
+			withPayloadValidation(methodToPayload{
+				http.MethodPost: newPayload[RebaseMrRequest],
+			}),
+			withMethodCheck(http.MethodPost),
+		)
+		data, _ := getFailData(t, svc, request)
+		checkNon200(t, data, "Could not rebase MR", "/mr/rebase")
+	})
+}

cmd/app/server.go

@@ -117,6 +117,12 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer
 		withPayloadValidation(methodToPayload{http.MethodPost: newPayload[AcceptMergeRequestRequest]}),
 		withMethodCheck(http.MethodPost),
 	))
+	m.HandleFunc("/mr/rebase", middleware(
+		mergeRequestRebaserService{d, gitlabClient},
+		withMr(d, gitlabClient),
+		withPayloadValidation(methodToPayload{http.MethodPost: newPayload[RebaseMrRequest]}),
+		withMethodCheck(http.MethodPost),
+	))
 	m.HandleFunc("/mr/discussions/list", middleware(
 		discussionsListerService{d, gitlabClient},
 		withMr(d, gitlabClient),

doc/gitlab.nvim.txt

@@ -33,7 +33,7 @@ OVERVIEW                                                *gitlab.nvim.overview*
 This Neovim plugin is designed to make it easy to review Gitlab MRs from within
 the editor. This means you can do things like:
 
-- Create, approve, and merge MRs for the current branch
+- Create, approve, rebase, and merge MRs for the current branch
 - Read and edit an MR description
 - Add or remove reviewers and assignees
 - Resolve, reply to, and unresolve discussion threads
@@ -183,9 +183,13 @@ you call this function with no values the defaults will be used:
           revoke = "glR", -- Revoke MR approval
           merge = "glM", -- Merge the feature branch to the target branch and close MR
           set_auto_merge = "glm", -- Set MR to merge automatically when the pipeline succeeds
+          rebase = "glrr", -- Rebase the feature branch of the MR on the server and pull the new state
+          rebase_skip_ci = "glrs", -- Same as `rebase`, but skip the CI pipeline
+          rebase_force = "glrf", -- Same as `rebase`, but rebase even if MR already is rebased
           create_mr = "glC", -- Create a new MR for currently checked-out feature branch
           choose_merge_request = "glc", -- Chose MR for review (if necessary check out the feature branch)
           start_review = "glS", -- Start review for the currently checked-out branch
+          reload_review = "gl<C-R>", -- Load new MR state from Gitlab and apply new diff refs to the diff view
           summary = "gls", -- Show the editable summary of the MR
           copy_mr_url = "glu", -- Copy the URL of the MR to the system clipboard
           open_in_browser = "glo", -- Openthe URL of the MR in the default Internet browser
@@ -260,6 +264,7 @@ you call this function with no values the defaults will be used:
         },
         spinner_chars = { "/", "|", "\\", "-" }, -- Characters for the refresh animation
         auto_open = true, -- Automatically open when the reviewer is opened
+        enter_on_open = true, -- Automatically enter the discussion tree when it 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`
@@ -391,6 +396,10 @@ you call this function with no values the defaults will be used:
           border = "rounded",
         },
       },
+      rebase_mr = {
+        skip_ci = false, -- If true, a CI pipeline is not created. Can be overridden in gitlab.rebase call.
+        force = false, --  If true, MR is rebased even if MR already is rebased. Can be overridden in gitlab.rebase call.
+      },
       colors = {
         discussion_tree = {
           username = "Keyword",
@@ -614,6 +623,19 @@ this command to work.
 See |gitlab.nvim.merge| for more help on this function.
 
 
+REBASING AN MR                                      *gitlab.nvim.rebasing-an-mr*
+
+The `rebase` action will rebase an MR on the server, wait for the rebase to
+take effect and then update the local reviewer state. The MR must be in a
+"rebasable" state for this command to work, i.e., the worktree must be clean
+and there must be no conflicts.
+>lua
+    require("gitlab").rebase()
+    require("gitlab").rebase({ skip_ci = true, force = true })
+<
+See |gitlab.nvim.rebase| for more help on this function.
+
+
 CREATING AN MR                                    *gitlab.nvim.creating-an-mr*
 
 To create an MR for the current branch, make sure you have the branch checked
@@ -838,6 +860,25 @@ Opens the reviewer pane. Can be used from anywhere within Neovim after the
 plugin is loaded. If run twice, will open a second reviewer pane.
 >lua
   require("gitlab").review()
+<
+                                                                *gitlab.nvim.reload_review*
+gitlab.reload_review() ~
+
+Loads new MR state from Gitlab. Then if diffview.api is available (with the
+https://github.com/dlyongemallo/diffview.nvim fork) applies the new diff refs
+to the existing diffview, otherwise (with
+https://github.com/sindrets/diffview.nvim) closes and re-opens the reviewer.
+
+>lua
+  require("gitlab").reload_review()
+<
+                                                                *gitlab.nvim.close_review*
+gitlab.close_review() ~
+
+Closes the reviewer tab and discussion tree and cleans up (e.g., removes
+winbar timer).
+>lua
+  require("gitlab").close_review()
 <
                                                                 *gitlab.nvim.summary*
 gitlab.summary() ~
@@ -1091,6 +1132,23 @@ request" page). You can see the current settings in the Summary view, see
               Use the `keymaps.popup.perform_action` to merge the MR
               with your message.
 
+                                                                *gitlab.nvim.rebase*
+gitlab.rebase({opts}) ~
+
+Rebases the feature branch of the MR on the server and pulls the new state of
+the target branch.
+>lua
+  require("gitlab").rebase()
+  require("gitlab").rebase({ skip_ci = true, force = true })
+<
+    Parameters: ~
+        • {opts}: (table|nil) Keyword arguments that can be used to override
+          default behavior.
+            • {skip_ci}: (bool) If true, a CI pipeline is not created (this
+              may lead to a failed Mergeability Check (CI_MUST_PASS).
+            • {force}: (bool) If true, MR is rebased even if MR already is
+              rebased (this may run an unnecessary CI pipeline).
+
                                                                 *gitlab.nvim.data*
 gitlab.data({resources}, {cb}) ~
 

lua/gitlab/actions/comment.lua

@@ -300,14 +300,14 @@ end
 ---@return boolean
 M.can_create_comment = function(must_be_visual)
   -- Check that diffview is initialized
-  if reviewer.tabnr == nil then
+  if reviewer.tabid == nil then
     u.notify("Reviewer must be initialized first", vim.log.levels.ERROR)
     return false
   end
 
   -- Check that we are in the Diffview tab
-  local tabnr = vim.api.nvim_get_current_tabpage()
-  if tabnr ~= reviewer.tabnr then
+  local tabid = vim.api.nvim_get_current_tabpage()
+  if tabid ~= reviewer.tabid then
     u.notify("Comments can only be left in the reviewer pane", vim.log.levels.ERROR)
     return false
   end

lua/gitlab/actions/common.lua

@@ -127,7 +127,7 @@ M.print_node = function(tree)
 end
 
 ---Check if type of node is note or note body
----@param node NuiTree.Node?
+---@param node? NuiTree.Node
 ---@return boolean
 M.is_node_note = function(node)
   if node and (node.type == "note_body" or node.type == "note") then
@@ -139,7 +139,7 @@ end
 
 ---Get root node
 ---@param tree NuiTree
----@param node NuiTree.Node?
+---@param node? NuiTree.Node
 ---@return NuiTree.Node?
 M.get_root_node = function(tree, node)
   if not node then
@@ -155,7 +155,7 @@ end
 
 ---Get note node
 ---@param tree NuiTree
----@param node NuiTree.Node?
+---@param node? NuiTree.Node
 ---@return NuiTree.Node?
 M.get_note_node = function(tree, node)
   if not node then