Migrated comment.go to new approach (three API calls)

cmd/comment.go

@@ -31,20 +31,15 @@ type MRVersion struct {
 }
 
 func (c *Client) Comment() error {
-	if len(os.Args) < 7 {
+	if len(os.Args) < 6 {
 		c.Usage("comment")
 	}
 
-	lineNumber, fileName, comment, sha := os.Args[3], os.Args[4], os.Args[5], os.Args[6]
+	lineNumber, fileName, comment := os.Args[3], os.Args[4], os.Args[5]
 	if lineNumber == "" || fileName == "" || comment == "" {
 		c.Usage("comment")
 	}
 
-	lineNumberInt, err := strconv.Atoi(lineNumber)
-	if err != nil {
-		return fmt.Errorf("Not a valid line number: %w", err)
-	}
-
 	err, response := getMRVersions(c.projectId, c.mergeId)
 	if err != nil {
 		log.Fatalf("Error making diff thread: %s", err)
@@ -59,71 +54,45 @@ func (c *Client) Comment() error {
 		return fmt.Errorf("Error unmarshalling version info JSON: %w", err)
 	}
 
-	time := time.Now()
+	/* This is necessary since we do not know whether the comment is on a line that
+	has been changed or not. Making all three of these API calls will let us leave
+	the comment regardless. See the Gitlab documentation: https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff */
+	wg := sync.WaitGroup{}
+	wg.Add(3)
 
-	if sha == "" {
+	resultChannel := make(chan *http.Response, 3)
-		/* This is necessary since we do not know whether the comment is on a line that
-		has been changed or not. Making all three of these API calls will let us leave
-		the comment regardless. See the Gitlab documentation: https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff */
-		wg := sync.WaitGroup{}
-		wg.Add(2)
-
-		resultChannel := make(chan *gitlab.Discussion, 2)
-
-		for i := 0; i < 2; i++ {
-			ii := i
-			go func() {
-				defer wg.Done()
-				options := gitlab.CreateMergeRequestDiscussionOptions{
-					Body:      &comment,
-					CreatedAt: &time,
-					Position: &gitlab.NotePosition{
-						PositionType: "text",
-						BaseSHA:      diffVersionInfo[0].BaseCommitSHA,
-						HeadSHA:      diffVersionInfo[0].HeadCommitSHA,
-						StartSHA:     diffVersionInfo[0].StartCommitSHA,
-						NewPath:      fileName,
-						OldPath:      fileName,
-					},
-				}
-
-				if ii == 0 {
-					options.Position.NewLine = lineNumberInt
-				} else {
-					options.Position.NewLine = lineNumberInt
-					options.Position.OldLine = lineNumberInt
-				}
-
-				discussion, _, _ := c.git.Discussions.CreateMergeRequestDiscussion(c.projectId, c.mergeId, &options)
-				resultChannel <- discussion
-
-			}()
-		}
 
+	for i := 0; i < 3; i++ {
+		ii := i
 		go func() {
-			wg.Wait()
+			defer wg.Done()
-			close(resultChannel)
+			response, err := c.CommentOnDeletion(lineNumber, fileName, comment, diffVersionInfo[0], ii)
-		}()
+			if err != nil {
-
+				resultChannel <- nil
-		var createdDiscussion *gitlab.Discussion
+			} else {
-		for discussion := range resultChannel {
+				resultChannel <- response
-			if discussion != nil {
-				createdDiscussion = discussion
 			}
-		}
+		}()
+	}
 
-		if createdDiscussion == nil {
+	go func() {
-			return fmt.Errorf("Could not leave comment")
+		wg.Wait()
-		}
+		close(resultChannel)
-	} else {
+	}()
-		err := c.CommentOnDeletion(lineNumber, fileName, comment, diffVersionInfo[0])
+
-		if err != nil {
+	var commentResponse *http.Response
-			return err
+	for res := range resultChannel {
+		if res != nil {
+			commentResponse = res
 		}
 	}
 
+	if commentResponse == nil {
+		return fmt.Errorf("Could not leave comment")
+	}
 	fmt.Println("Left Comment: " + comment[0:min(len(comment), 25)] + "...")
 	return nil
+
 }
 
 func min(a int, b int) int {
@@ -161,6 +130,56 @@ func getMRVersions(projectId string, mergeId int) (e error, response *http.Respo
 	return nil, response
 }
 
+/* Creates a new merge request discussion https://docs.gitlab.com/ee/api/discussions.html#create-new-merge-request-thread */
+func (c *Client) CommentOnDeletion(lineNumber string, fileName string, comment string, diffVersionInfo MRVersion, i int) (*http.Response, error) {
+
+	deletionDiscussionUrl := fmt.Sprintf("https://gitlab.com/api/v4/projects/%s/merge_requests/%d/discussions", c.projectId, c.mergeId)
+
+	payload := &bytes.Buffer{}
+	writer := multipart.NewWriter(payload)
+	_ = writer.WriteField("body", comment)
+	_ = writer.WriteField("position[base_sha]", diffVersionInfo.BaseCommitSHA)
+	_ = writer.WriteField("position[start_sha]", diffVersionInfo.StartCommitSHA)
+	_ = writer.WriteField("position[head_sha]", diffVersionInfo.HeadCommitSHA)
+	_ = writer.WriteField("position[position_type]", "text")
+
+	/* We need to set these properties differently depending on whether we're commenting on a deleted line,
+	a modified line, an added line, or an unmodified line */
+
+	_ = writer.WriteField("position[old_path]", fileName)
+	_ = writer.WriteField("position[new_path]", fileName)
+	if i == 0 {
+		_ = writer.WriteField("position[old_line]", lineNumber)
+	} else if i == 1 {
+		_ = writer.WriteField("position[new_line]", lineNumber)
+	} else {
+		_ = writer.WriteField("position[old_line]", lineNumber)
+		_ = writer.WriteField("position[new_line]", lineNumber)
+	}
+
+	err := writer.Close()
+	if err != nil {
+		return nil, fmt.Errorf("Error making form data: %w", err)
+	}
+
+	client := &http.Client{}
+	req, err := http.NewRequest(http.MethodPost, deletionDiscussionUrl, payload)
+
+	if err != nil {
+		return nil, fmt.Errorf("Error building request: %w", err)
+	}
+	req.Header.Add("PRIVATE-TOKEN", os.Getenv("GITLAB_TOKEN"))
+
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+	res, err := client.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("Error making request: %w", err)
+	}
+
+	defer res.Body.Close()
+	return res, nil
+}
+
 func (c *Client) OverviewComment() error {
 	lineNumber, fileName, comment, sha := os.Args[3], os.Args[4], os.Args[5], os.Args[6]
 	if lineNumber == "" || fileName == "" || comment == "" {
@@ -186,40 +205,3 @@ func (c *Client) OverviewComment() error {
 	fmt.Println("Left Overview Comment: " + comment[0:min(len(comment), 25)] + "...")
 	return nil
 }
-
-func (c *Client) CommentOnDeletion(lineNumber string, fileName string, comment string, diffVersionInfo MRVersion) error {
-
-	deletionDiscussionUrl := fmt.Sprintf("https://gitlab.com/api/v4/projects/%s/merge_requests/%d/discussions", c.projectId, c.mergeId)
-
-	payload := &bytes.Buffer{}
-	writer := multipart.NewWriter(payload)
-	_ = writer.WriteField("body", comment)
-	_ = writer.WriteField("position[base_sha]", diffVersionInfo.BaseCommitSHA)
-	_ = writer.WriteField("position[start_sha]", diffVersionInfo.StartCommitSHA)
-	_ = writer.WriteField("position[head_sha]", diffVersionInfo.HeadCommitSHA)
-	_ = writer.WriteField("position[position_type]", "text")
-	_ = writer.WriteField("position[old_path]", fileName)
-	_ = writer.WriteField("position[new_path]", fileName)
-	_ = writer.WriteField("position[old_line]", lineNumber)
-	err := writer.Close()
-	if err != nil {
-		return fmt.Errorf("Error making form data: %w", err)
-	}
-
-	client := &http.Client{}
-	req, err := http.NewRequest(http.MethodPost, deletionDiscussionUrl, payload)
-
-	if err != nil {
-		return fmt.Errorf("Error building request: %w", err)
-	}
-	req.Header.Add("PRIVATE-TOKEN", os.Getenv("GITLAB_TOKEN"))
-
-	req.Header.Set("Content-Type", writer.FormDataContentType())
-	res, err := client.Do(req)
-	if err != nil {
-		return fmt.Errorf("Error making request: %w", err)
-	}
-
-	defer res.Body.Close()
-	return nil
-}

lua/gitlab/comment.lua

@@ -56,8 +56,6 @@ M.confirm_create_comment = function(text)
     end
   end
 
-  print(relative_file_path)
-
   Job:new({
     command = state.BIN,
     args = {

lua/gitlab/utils/init.lua

@@ -153,15 +153,17 @@ local function darken_metadata(bufnr, regex)
 end
 
 local function print_success(_, line)
-  if line ~= nil and line ~= "" then
+  print(line)
-    notify(line, "info")
+  -- if line ~= nil and line ~= "" then
-  end
+  --   notify(line, "info")
+  -- end
 end
 
 local function print_error(_, line)
-  if line ~= nil and line ~= "" then
+  print(line)
-    notify(line, "error")
+  -- if line ~= nil and line ~= "" then
-  end
+  --   notify(line, "error")
+  -- end
 end
 
 local function exit(popup)