Release (#490)

Fix: Jumping to renamed files (#484)
Fix: Store reviewer data before creating comment popup (#476)
Fix: Make publishing drafts more robust (#483)
Fix: Swap file_name and old_file_name in reviewer data (#485)

---------

Co-authored-by: Jakub F. Bortlík <[email protected]>

Author: harrisoncramer

cmd/app/client.go

@@ -68,8 +68,8 @@ func NewClient() (*Client, error) {
 
 	retryClient := retryablehttp.NewClient()
 	retryClient.HTTPClient.Transport = tr
-	retryClient.RetryMax = 0
 	gitlabOptions = append(gitlabOptions, gitlab.WithHTTPClient(retryClient.HTTPClient))
+	gitlabOptions = append(gitlabOptions, gitlab.WithoutRetries())
 
 	client, err := gitlab.NewClient(pluginOptions.AuthToken, gitlabOptions...)
 

cmd/app/comment_helpers.go

@@ -42,13 +42,19 @@ type RequestWithPosition interface {
 func buildCommentPosition(commentWithPositionData RequestWithPosition) *gitlab.PositionOptions {
 	positionData := commentWithPositionData.GetPositionData()
 
+	// If the file has been renamed, then this is a relevant part of the payload
+	oldFileName := positionData.OldFileName
+	if oldFileName == "" {
+		oldFileName = positionData.FileName
+	}
+
 	opt := &gitlab.PositionOptions{
 		PositionType: &positionData.Type,
 		StartSHA:     &positionData.StartCommitSHA,
 		HeadSHA:      &positionData.HeadCommitSHA,
 		BaseSHA:      &positionData.BaseCommitSHA,
 		NewPath:      &positionData.FileName,
-		OldPath:      &positionData.OldFileName,
+		OldPath:      &oldFileName,
 		NewLine:      positionData.NewLine,
 		OldLine:      positionData.OldLine,
 	}

lua/gitlab/actions/comment.lua

@@ -153,9 +153,9 @@ M.create_comment_layout = function(opts)
     title = "Note"
     user_settings = popup_settings.note
   else
-    -- TODO: investigate why `old_file_name` is in fact the new name for renamed files!
-    local file_name = M.location.reviewer_data.old_file_name ~= "" and M.location.reviewer_data.old_file_name
-      or M.location.reviewer_data.file_name
+    local file_name = (M.location.reviewer_data.new_sha_focused or M.location.reviewer_data.old_file_name == "")
+        and M.location.reviewer_data.file_name
+      or M.location.reviewer_data.old_file_name
     title =
       popup.create_title("Comment", file_name, M.location.visual_range.start_line, M.location.visual_range.end_line)
     user_settings = popup_settings.comment

lua/gitlab/actions/draft_notes/init.lua

@@ -4,6 +4,7 @@
 -- under lua/gitlab/actions/discussions/init.lua
 local common = require("gitlab.actions.common")
 local discussion_tree = require("gitlab.actions.discussions.tree")
+local git = require("gitlab.git")
 local job = require("gitlab.job")
 local NuiTree = require("nui.tree")
 local List = require("gitlab.utils.list")
@@ -85,12 +86,20 @@ end
 
 ---Publishes all draft notes and comments. Re-renders all discussion views.
 M.confirm_publish_all_drafts = function()
+  if not git.check_current_branch_up_to_date_on_remote(vim.log.levels.ERROR) then
+    return
+  end
   local body = { publish_all = true }
   job.run_job("/mr/draft_notes/publish", "POST", body, function(data)
     u.notify(data.message, vim.log.levels.INFO)
     state.DRAFT_NOTES = {}
-    local discussions = require("gitlab.actions.discussions")
-    discussions.rebuild_view(false, true)
+    require("gitlab.actions.discussions").rebuild_view(false, true)
+  end, function()
+    require("gitlab.actions.discussions").rebuild_view(false, true)
+    u.notify(
+      "Draft(s) may have been published despite the error. Check the discussion tree. Try publishing drafts individually.",
+      vim.log.levels.WARN
+    )
   end)
 end
 
@@ -99,6 +108,9 @@ end
 ---and re-render it.
 ---@param tree NuiTree
 M.confirm_publish_draft = function(tree)
+  if not git.check_current_branch_up_to_date_on_remote(vim.log.levels.ERROR) then
+    return
+  end
   local current_node = tree:get_node()
   local note_node = common.get_note_node(tree, current_node)
   local root_node = common.get_root_node(tree, current_node)
@@ -111,12 +123,13 @@ M.confirm_publish_draft = function(tree)
   ---@type integer
   local note_id = note_node.is_root and root_node.id or note_node.id
   local body = { note = note_id }
+  local unlinked = tree.bufnr == require("gitlab.actions.discussions").unlinked_bufnr
   job.run_job("/mr/draft_notes/publish", "POST", body, function(data)
     u.notify(data.message, vim.log.levels.INFO)
-
-    local discussions = require("gitlab.actions.discussions")
-    local unlinked = tree.bufnr == discussions.unlinked_bufnr
     M.rebuild_view(unlinked)
+  end, function()
+    M.rebuild_view(unlinked)
+    u.notify("Draft may have been published despite the error. Check the discussion tree.", vim.log.levels.WARN)
   end)
 end
 

lua/gitlab/git.lua

@@ -6,11 +6,9 @@ local M = {}
 ---@param command table
 ---@return string|nil, string|nil
 local run_system = function(command)
-  -- Load here to prevent loop
-  local u = require("gitlab.utils")
   local result = vim.fn.trim(vim.fn.system(command))
   if vim.v.shell_error ~= 0 then
-    u.notify(result, vim.log.levels.ERROR)
+    require("gitlab.utils").notify(result, vim.log.levels.ERROR)
     return nil, result
   end
   return result, nil
@@ -52,10 +50,28 @@ M.switch_branch = function(branch)
   return run_system({ "git", "checkout", "-q", branch })
 end
 
----Fetches the name of the remote tracking branch for the current branch
----@return string|nil, string|nil
+---Returns the name of the remote-tracking branch for the current branch or nil if it can't be found
+---@return string|nil
 M.get_remote_branch = function()
-  return run_system({ "git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}" })
+  local remote_branch, err = run_system({ "git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}" })
+  if err or remote_branch == "" then
+    require("gitlab.utils").notify("Could not get remote branch: " .. err, vim.log.levels.ERROR)
+    return nil
+  end
+  return remote_branch
+end
+
+---Fetch the remote branch
+---@param remote_branch string The name of the repo and branch to fetch (e.g., "origin/some_branch")
+---@return boolean fetch_successfull False if an error occurred while fetching, true otherwise.
+M.fetch_remote_branch = function(remote_branch)
+  local remote, branch = string.match(remote_branch, "([^/]+)/(.*)")
+  local _, fetch_err = run_system({ "git", "fetch", remote, branch })
+  if fetch_err ~= nil then
+    require("gitlab.utils").notify("Error fetching remote-tracking branch: " .. fetch_err, vim.log.levels.ERROR)
+    return false
+  end
+  return true
 end
 
 ---Determines whether the tracking branch is ahead of or behind the current branch, and warns the user if so
@@ -64,6 +80,10 @@ end
 ---@param log_level number
 ---@return boolean
 M.get_ahead_behind = function(current_branch, remote_branch, log_level)
+  if not M.fetch_remote_branch(remote_branch) then
+    return false
+  end
+
   local u = require("gitlab.utils")
   local result, err =
     run_system({ "git", "rev-list", "--left-right", "--count", current_branch .. "..." .. remote_branch })
@@ -104,17 +124,22 @@ M.get_ahead_behind = function(current_branch, remote_branch, log_level)
   return true -- Checks passed, branch is up-to-date
 end
 
----Return the name of the current branch
----@return string|nil, string|nil
+---Return the name of the current branch or nil if it can't be retrieved
+---@return string|nil
 M.get_current_branch = function()
-  return run_system({ "git", "branch", "--show-current" })
+  local current_branch, err = run_system({ "git", "branch", "--show-current" })
+  if err or current_branch == "" then
+    require("gitlab.utils").notify("Could not get current branch: " .. err, vim.log.levels.ERROR)
+    return nil
+  end
+  return current_branch
 end
 
 ---Return the list of possible merge targets.
 ---@return table|nil
 M.get_all_merge_targets = function()
-  local current_branch, err = M.get_current_branch()
-  if not current_branch or err ~= nil then
+  local current_branch = M.get_current_branch()
+  if current_branch == nil then
     return
   end
   return List.new(M.get_all_remote_branches()):filter(function(branch)
@@ -158,19 +183,13 @@ end
 ---@param log_level integer
 ---@return boolean
 M.check_current_branch_up_to_date_on_remote = function(log_level)
-  local u = require("gitlab.utils")
-
-  -- Get current branch
-  local current_branch, err_current_branch = M.get_current_branch()
-  if err_current_branch or not current_branch then
-    u.notify("Could not get current branch: " .. err_current_branch, vim.log.levels.ERROR)
+  local current_branch = M.get_current_branch()
+  if current_branch == nil then
     return false
   end
 
-  -- Get remote tracking branch
-  local remote_branch, err_remote_branch = M.get_remote_branch()
-  if err_remote_branch or not remote_branch then
-    u.notify("Could not get remote branch: " .. err_remote_branch, vim.log.levels.ERROR)
+  local remote_branch = M.get_remote_branch()
+  if remote_branch == nil then
     return false
   end
 

lua/gitlab/job.lua

@@ -4,7 +4,7 @@ local Job = require("plenary.job")
 local u = require("gitlab.utils")
 local M = {}
 
-M.run_job = function(endpoint, method, body, callback)
+M.run_job = function(endpoint, method, body, callback, on_error_callback)
   local state = require("gitlab.state")
   local args = { "-s", "-X", (method or "POST"), string.format("localhost:%s", state.settings.port) .. endpoint }
 
@@ -16,7 +16,8 @@ M.run_job = function(endpoint, method, body, callback)
 
   -- This handler will handle all responses from the Go server. Anything with a successful
   -- status will call the callback (if it is supplied for the job). Otherwise, it will print out the
-  -- success message or error message and details from the Go server.
+  -- success message or error message and details from the Go server and run the on_error_callback
+  -- (if supplied for the job).
   local stderr = {}
   Job:new({
     command = "curl",
@@ -53,6 +54,9 @@ M.run_job = function(endpoint, method, body, callback)
           -- Handle error case
           local message = string.format("%s: %s", data.message, data.details)
           u.notify(message, vim.log.levels.ERROR)
+          if on_error_callback then
+            on_error_callback(data)
+          end
         end
       end, 0)
     end,

lua/gitlab/reviewer/init.lua

@@ -123,7 +123,8 @@ M.jump = function(file_name, old_file_name, line_number, new_buffer)
 
   local files = view.panel:ordered_file_list()
   local file = List.new(files):find(function(f)
-    return new_buffer and f.path == file_name or f.oldpath == old_file_name
+    local oldpath = f.oldpath ~= nil and f.oldpath or f.path
+    return new_buffer and f.path == file_name or oldpath == old_file_name
   end)
   if file == nil then
     u.notify(
@@ -195,10 +196,8 @@ M.get_reviewer_data = function(current_win)
   local opposite_bufnr = new_sha_focused and layout.a.file.bufnr or layout.b.file.bufnr
 
   return {
-    -- TODO: swap 'a' and 'b' to fix lua/gitlab/actions/comment.lua:158, and hopefully also
-    -- lua/gitlab/indicators/diagnostics.lua:129.
-    file_name = layout.a.file.path,
-    old_file_name = M.is_file_renamed() and layout.b.file.path or "",
+    old_file_name = M.is_file_renamed() and layout.a.file.path or "",
+    file_name = layout.b.file.path,
     old_line_from_buf = old_line,
     new_line_from_buf = new_line,
     modification_type = modification_type,