Bug Fixes (#470)

* fix: Restore buffer local settings outside reviewer (#446)
* fix: do not show healthcheck alert for warnings (#468)
* feat: Add MR URL to the summary details (#467)
* fix: make cycling reviewed files faster (#474)
* feat(pipeline): display trigger jobs for a pipeline in the pipelines popup  (#465)
* fix: Jumping to renamed files (#484)

---------

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

Author: 3 people

.github/CONTRIBUTING.md

@@ -10,7 +10,7 @@ It's possible that the feature you want is already implemented, or does not belo
 
 If you are using Lazy as a plugin manager, the easiest way to work on changes is by setting a specific path for the plugin that points to your repository locally. This is what I do:
 
-```lua 
+```lua
 {
   "harrisoncramer/gitlab.nvim",
   dependencies = {
@@ -54,8 +54,8 @@ $ luacheck --globals vim busted --no-max-line-length -- .
 
 4. Make the merge request to the `develop` branch of `.gitlab.nvim`
 
-Please provide a description of the feature, and links to any relevant issues. 
+Please provide a description of the feature, and links to any relevant issues.
 
-That's it! I'll try to respond to any incoming merge request in a few days. Once we've reviewed it, it will be merged into the develop branch. 
+That's it! I'll try to respond to any incoming merge request in a few days. Once we've reviewed it, it will be merged into the develop branch.
 
 After some time, if the develop branch is found to be stable, that branch will be merged into `main` and released. When merged into `main` the pipeline will detect whether we're merging in a patch, minor, or major change, and create a new tag (e.g. 1.0.12) and release.

cmd/app/comment_helpers.go

@@ -42,19 +42,13 @@ 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:      &oldFileName,
+		OldPath:      &positionData.OldFileName,
 		NewLine:      positionData.NewLine,
 		OldLine:      positionData.OldLine,
 	}

cmd/app/pipeline.go

@@ -20,16 +20,18 @@ type RetriggerPipelineResponse struct {
 type PipelineWithJobs struct {
 	Jobs           []*gitlab.Job        `json:"jobs"`
 	LatestPipeline *gitlab.PipelineInfo `json:"latest_pipeline"`
+	Name           string               `json:"name"`
 }
 
 type GetPipelineAndJobsResponse struct {
 	SuccessResponse
-	Pipeline PipelineWithJobs `json:"latest_pipeline"`
+	Pipelines []PipelineWithJobs `json:"latest_pipeline"`
 }
 
 type PipelineManager interface {
 	ListProjectPipelines(pid interface{}, opt *gitlab.ListProjectPipelinesOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.PipelineInfo, *gitlab.Response, error)
 	ListPipelineJobs(pid interface{}, pipelineID int, opts *gitlab.ListJobsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Job, *gitlab.Response, error)
+	ListPipelineBridges(pid interface{}, pipelineID int, opts *gitlab.ListJobsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Bridge, *gitlab.Response, error)
 	RetryPipelineBuild(pid interface{}, pipeline int, options ...gitlab.RequestOptionFunc) (*gitlab.Pipeline, *gitlab.Response, error)
 }
 
@@ -101,7 +103,6 @@ func (a pipelineService) GetPipelineAndJobs(w http.ResponseWriter, r *http.Reque
 	}
 
 	jobs, res, err := a.client.ListPipelineJobs(a.projectInfo.ProjectId, pipeline.ID, &gitlab.ListJobsOptions{})
-
 	if err != nil {
 		handleError(w, err, "Could not get pipeline jobs", http.StatusInternalServerError)
 		return
@@ -112,13 +113,51 @@ func (a pipelineService) GetPipelineAndJobs(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	pipelines := []PipelineWithJobs{}
+	pipelines = append(pipelines, PipelineWithJobs{
+		Jobs:           jobs,
+		LatestPipeline: pipeline,
+		Name:           "root",
+	})
+
+	bridges, res, err := a.client.ListPipelineBridges(a.projectInfo.ProjectId, pipeline.ID, &gitlab.ListJobsOptions{})
+
+	if err != nil {
+		handleError(w, err, "Could not get pipeline trigger jobs", http.StatusInternalServerError)
+		return
+	}
+	if res.StatusCode >= 300 {
+		handleError(w, GenericError{r.URL.Path}, "Could not get pipeline trigger jobs", res.StatusCode)
+		return
+	}
+
+	for _, bridge := range bridges {
+		if bridge.DownstreamPipeline == nil {
+			continue
+		}
+
+		pipelineIdInBridge := bridge.DownstreamPipeline.ID
+		bridgePipelineJobs, res, err := a.client.ListPipelineJobs(bridge.DownstreamPipeline.ProjectID, pipelineIdInBridge, &gitlab.ListJobsOptions{})
+		if err != nil {
+			handleError(w, err, "Could not get jobs for a pipeline from a trigger job", http.StatusInternalServerError)
+			return
+		}
+		if res.StatusCode >= 300 {
+			handleError(w, GenericError{r.URL.Path}, "Could not get jobs for a pipeline from a trigger job", res.StatusCode)
+			return
+		}
+
+		pipelines = append(pipelines, PipelineWithJobs{
+			Jobs:           bridgePipelineJobs,
+			LatestPipeline: bridge.DownstreamPipeline,
+			Name:           bridge.Name,
+		})
+	}
+
 	w.WriteHeader(http.StatusOK)
 	response := GetPipelineAndJobsResponse{
 		SuccessResponse: SuccessResponse{Message: "Pipeline retrieved"},
-		Pipeline: PipelineWithJobs{
-			LatestPipeline: pipeline,
-			Jobs:           jobs,
-		},
+		Pipelines:       pipelines,
 	}
 
 	err = json.NewEncoder(w).Encode(response)

cmd/app/pipeline_test.go

@@ -27,6 +27,14 @@ func (f fakePipelineManager) ListPipelineJobs(pid interface{}, pipelineID int, o
 	return []*gitlab.Job{}, resp, err
 }
 
+func (f fakePipelineManager) ListPipelineBridges(pid interface{}, pipelineID int, opts *gitlab.ListJobsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Bridge, *gitlab.Response, error) {
+	resp, err := f.handleGitlabError()
+	if err != nil {
+		return nil, nil, err
+	}
+	return []*gitlab.Bridge{}, resp, err
+}
+
 func (f fakePipelineManager) RetryPipelineBuild(pid interface{}, pipeline int, options ...gitlab.RequestOptionFunc) (*gitlab.Pipeline, *gitlab.Response, error) {
 	resp, err := f.handleGitlabError()
 	if err != nil {

doc/gitlab.nvim.txt

@@ -302,6 +302,7 @@ you call this function with no values the defaults will be used:
           "delete_branch",
           "squash",
           "labels",
+          "web_url",
         },
       },
       discussion_signs = {
@@ -315,6 +316,7 @@ you call this function with no values the defaults will be used:
           comment = "→|",
           range = " |",
         },
+        skip_old_revision_discussion = false, -- Don't show diagnostics for discussions that were created for earlier MR revisions
       },
       pipeline = {
         created = "",

lua/gitlab/actions/comment.lua

@@ -15,7 +15,6 @@ local reviewer = require("gitlab.reviewer")
 local Location = require("gitlab.reviewer.location")
 
 local M = {
-  current_win = nil,
   start_line = nil,
   end_line = nil,
   draft_popup = nil,
@@ -25,10 +24,9 @@ local M = {
 ---Fires the API that sends the comment data to the Go server, called when you "confirm" creation
 ---via the M.settings.keymaps.popup.perform_action keybinding
 ---@param text string comment text
----@param visual_range LineRange | nil range of visual selection or nil
 ---@param unlinked boolean if true, the comment is not linked to a line
 ---@param discussion_id string | nil The ID of the discussion to which the reply is responding, nil if not a reply
-local confirm_create_comment = function(text, visual_range, unlinked, discussion_id)
+local confirm_create_comment = function(text, unlinked, discussion_id)
   if text == nil then
     u.notify("Reviewer did not provide text of change", vim.log.levels.ERROR)
     return
@@ -75,30 +73,16 @@ local confirm_create_comment = function(text, visual_range, unlinked, discussion
     return
   end
 
-  local reviewer_data = reviewer.get_reviewer_data()
-  if reviewer_data == nil then
-    u.notify("Error getting reviewer data", vim.log.levels.ERROR)
-    return
-  end
-
-  local location = Location.new(reviewer_data, visual_range)
-  location:build_location_data()
-  local location_data = location.location_data
-  if location_data == nil then
-    u.notify("Error getting location information", vim.log.levels.ERROR)
-    return
-  end
-
   local revision = state.MR_REVISIONS[1]
   local position_data = {
-    file_name = reviewer_data.file_name,
-    old_file_name = reviewer_data.old_file_name,
+    file_name = M.location.reviewer_data.file_name,
+    old_file_name = M.location.reviewer_data.old_file_name,
     base_commit_sha = revision.base_commit_sha,
     start_commit_sha = revision.start_commit_sha,
     head_commit_sha = revision.head_commit_sha,
-    old_line = location_data.old_line,
-    new_line = location_data.new_line,
-    line_range = location_data.line_range,
+    old_line = M.location.location_data.old_line,
+    new_line = M.location.location_data.new_line,
+    line_range = M.location.location_data.line_range,
   }
 
   -- Creating a new comment (linked to specific changes)
@@ -148,10 +132,10 @@ M.confirm_edit_comment = function(discussion_id, note_id, unlinked)
 end
 
 ---@class LayoutOpts
----@field ranged boolean
 ---@field unlinked boolean
 ---@field discussion_id string|nil
 ---@field reply boolean|nil
+---@field file_name string|nil
 
 ---This function sets up the layout and popups needed to create a comment, note and
 ---multi-line comment. It also sets up the basic keybindings for switching between
@@ -163,21 +147,24 @@ M.create_comment_layout = function(opts)
   local title
   local user_settings
   if opts.discussion_id ~= nil then
-    title = "Reply"
+    title = "Reply" .. (opts.file_name and string.format(" [%s]", opts.file_name) or "")
     user_settings = popup_settings.reply
   elseif opts.unlinked then
     title = "Note"
     user_settings = popup_settings.note
   else
-    title = "Comment"
+    -- 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
+    title =
+      popup.create_title("Comment", file_name, M.location.visual_range.start_line, M.location.visual_range.end_line)
     user_settings = popup_settings.comment
   end
   local settings = u.merge(popup_settings, user_settings or {})
 
-  M.current_win = vim.api.nvim_get_current_win()
+  local current_win = vim.api.nvim_get_current_win()
   M.comment_popup = Popup(popup.create_popup_state(title, settings))
   M.draft_popup = Popup(popup.create_box_popup_state("Draft", false, settings))
-  M.start_line, M.end_line = u.get_visual_selection_boundaries()
 
   local internal_layout = Layout.Box({
     Layout.Box(M.comment_popup, { grow = 1 }),
@@ -194,22 +181,21 @@ M.create_comment_layout = function(opts)
   }, internal_layout)
 
   popup.set_cycle_popups_keymaps({ M.comment_popup, M.draft_popup })
-  popup.set_up_autocommands(M.comment_popup, layout, M.current_win)
+  popup.set_up_autocommands(M.comment_popup, layout, current_win)
 
-  local range = opts.ranged and { start_line = M.start_line, end_line = M.end_line } or nil
   local unlinked = opts.unlinked or false
 
   ---Keybinding for focus on draft section
   popup.set_popup_keymaps(M.draft_popup, function()
     local text = u.get_buffer_text(M.comment_popup.bufnr)
-    confirm_create_comment(text, range, unlinked, opts.discussion_id)
-    vim.api.nvim_set_current_win(M.current_win)
+    confirm_create_comment(text, unlinked, opts.discussion_id)
+    vim.api.nvim_set_current_win(current_win)
   end, miscellaneous.toggle_bool, popup.non_editable_popup_opts)
 
   ---Keybinding for focus on text section
   popup.set_popup_keymaps(M.comment_popup, function(text)
-    confirm_create_comment(text, range, unlinked, opts.discussion_id)
-    vim.api.nvim_set_current_win(M.current_win)
+    confirm_create_comment(text, unlinked, opts.discussion_id)
+    vim.api.nvim_set_current_win(current_win)
   end, miscellaneous.attach_file, popup.editable_popup_opts)
 
   vim.schedule(function()
@@ -223,44 +209,43 @@ end
 --- This function will open a comment popup in order to create a comment on the changed/updated
 --- line in the current MR
 M.create_comment = function()
+  M.location = Location.new()
   if not M.can_create_comment(false) then
     return
   end
 
-  local layout = M.create_comment_layout({ ranged = false, unlinked = false })
+  local layout = M.create_comment_layout({ unlinked = false })
   layout:mount()
 end
 
 --- This function will open a multi-line comment popup in order to create a multi-line comment
 --- on the changed/updated line in the current MR
 M.create_multiline_comment = function()
+  M.location = Location.new()
   if not M.can_create_comment(true) then
     u.press_escape()
     return
   end
 
-  local layout = M.create_comment_layout({ ranged = true, unlinked = false })
+  local layout = M.create_comment_layout({ unlinked = false })
   layout:mount()
 end
 
 --- This function will open a a popup to create a "note" (e.g. unlinked comment)
 --- on the changed/updated line in the current MR
 M.create_note = function()
-  local layout = M.create_comment_layout({ ranged = false, unlinked = true })
+  local layout = M.create_comment_layout({ unlinked = true })
   layout:mount()
 end
 
 ---Given the current visually selected area of text, builds text to fill in the
 ---comment popup with a suggested change
 ---@return LineRange|nil
----@return integer
 local build_suggestion = function()
   local current_line = vim.api.nvim_win_get_cursor(0)[1]
-  M.start_line, M.end_line = u.get_visual_selection_boundaries()
-
-  local range_length = M.end_line - M.start_line
+  local range_length = M.location.visual_range.end_line - M.location.visual_range.start_line
   local backticks = "```"
-  local selected_lines = u.get_lines(M.start_line, M.end_line)
+  local selected_lines = u.get_lines(M.location.visual_range.start_line, M.location.visual_range.end_line)
 
   for _, line in ipairs(selected_lines) do
     if string.match(line, "^```%S*$") then
@@ -270,36 +255,37 @@ local build_suggestion = function()
   end
 
   local suggestion_start
-  if M.start_line == current_line then
+  if M.location.visual_range.start_line == current_line then
     suggestion_start = backticks .. "suggestion:-0+" .. range_length
-  elseif M.end_line == current_line then
+  elseif M.location.visual_range.end_line == current_line then
     suggestion_start = backticks .. "suggestion:-" .. range_length .. "+0"
   else
     --- This should never happen afaik
     u.notify("Unexpected suggestion position", vim.log.levels.ERROR)
-    return nil, 0
+    return nil
   end
   suggestion_start = suggestion_start
   local suggestion_lines = {}
   table.insert(suggestion_lines, suggestion_start)
   vim.list_extend(suggestion_lines, selected_lines)
   table.insert(suggestion_lines, backticks)
 
-  return suggestion_lines, range_length
+  return suggestion_lines
 end
 
 --- This function will open a a popup to create a suggestion comment
 --- on the changed/updated line in the current MR
 --- See: https://docs.gitlab.com/ee/user/project/merge_requests/reviews/suggestions.html
 M.create_comment_suggestion = function()
+  M.location = Location.new()
   if not M.can_create_comment(true) then
     u.press_escape()
     return
   end
 
-  local suggestion_lines, range_length = build_suggestion()
+  local suggestion_lines = build_suggestion()
 
-  local layout = M.create_comment_layout({ ranged = range_length > 0, unlinked = false })
+  local layout = M.create_comment_layout({ unlinked = false })
   layout:mount()
 
   vim.schedule(function()
@@ -368,6 +354,11 @@ M.can_create_comment = function(must_be_visual)
     return false
   end
 
+  if M.location == nil or M.location.location_data == nil then
+    u.notify("Error getting location information", vim.log.levels.ERROR)
+    return false
+  end
+
   return true
 end