Bugfix: Updates Diff Hashes (#106)

Rather than using branch names we are using the hashes provided directly in the Gitlab API response, to compare the point at which the branch diverged from the target to the head commit. We are additionally flashing a warning if the MR contains a merge conflict.

cmd/git.go

@@ -27,8 +27,15 @@ Extracts information about the current repository and returns
 it to the client for initialization. The current directory must be a valid
 Gitlab project and the branch must be a feature branch
 */
-func ExtractGitInfo(getProjectRemoteUrl func() (string, error), getCurrentBranchName func() (string, error)) (GitProjectInfo, error) {
+func ExtractGitInfo(refreshGitInfo func() error, getProjectRemoteUrl func() (string, error), getCurrentBranchName func() (string, error)) (GitProjectInfo, error) {
+
+	err := refreshGitInfo()
+	if err != nil {
+		return GitProjectInfo{}, fmt.Errorf("Could not get latest information from remote: %v", err)
+	}
+
 	url, err := getProjectRemoteUrl()
+
 	if err != nil {
 		return GitProjectInfo{}, fmt.Errorf("Could not get project Url: %v", err)
 	}
@@ -97,3 +104,14 @@ func GetProjectUrlFromNativeGitCmd() (string, error) {
 
 	return strings.TrimSpace(string(url)), nil
 }
+
+/* Pulls down latest commit information from Gitlab */
+func RefreshProjectInfo() error {
+	cmd := exec.Command("git", "fetch", "origin")
+	_, err := cmd.Output()
+	if err != nil {
+		return fmt.Errorf("Failed to run `git fetch origin`: %v", err)
+	}
+
+	return nil
+}

cmd/git_test.go

@@ -10,6 +10,9 @@ func TestExtractGitInfo_Success(t *testing.T) {
 	getCurrentBranchName := func() (string, error) {
 		return "feature/abc", nil
 	}
+	refreshGitInfo := func() error {
+		return nil
+	}
 	testCases := []struct {
 		getProjectRemoteUrl func() (string, error)
 		expected            GitProjectInfo
@@ -150,7 +153,7 @@ func TestExtractGitInfo_Success(t *testing.T) {
 	}
 	for _, tC := range testCases {
 		t.Run(tC.desc, func(t *testing.T) {
-			actual, err := ExtractGitInfo(tC.getProjectRemoteUrl, getCurrentBranchName)
+			actual, err := ExtractGitInfo(refreshGitInfo, tC.getProjectRemoteUrl, getCurrentBranchName)
 			if err != nil {
 				t.Errorf("No error was expected, got %s", err)
 			}
@@ -165,7 +168,9 @@ func TestExtractGitInfo_FailToGetProjectRemoteUrl(t *testing.T) {
 	getCurrentBranchName := func() (string, error) {
 		return "feature/abc", nil
 	}
-
+	refreshGitInfo := func() error {
+		return nil
+	}
 	testCases := []struct {
 		getProjectRemoteUrl  func() (string, error)
 		expectedErrorMessage string
@@ -188,7 +193,7 @@ func TestExtractGitInfo_FailToGetProjectRemoteUrl(t *testing.T) {
 	}
 	for _, tC := range testCases {
 		t.Run(tC.desc, func(t *testing.T) {
-			_, actualErr := ExtractGitInfo(tC.getProjectRemoteUrl, getCurrentBranchName)
+			_, actualErr := ExtractGitInfo(refreshGitInfo, tC.getProjectRemoteUrl, getCurrentBranchName)
 			if actualErr == nil {
 				t.Errorf("Expected an error, got none")
 			}
@@ -201,9 +206,15 @@ func TestExtractGitInfo_FailToGetProjectRemoteUrl(t *testing.T) {
 
 func TestExtractGitInfo_FailToGetCurrentBranchName(t *testing.T) {
 	expectedErrNestedMsg := "error when getting current branch name"
-	_, actualErr := ExtractGitInfo(func() (string, error) {
+
+	refreshGitInfo := func() error {
+		return nil
+	}
+	_, actualErr := ExtractGitInfo(refreshGitInfo,
+		func() (string, error) {
 			return "git@custom-gitlab.com:namespace/project.git", nil
-	}, func() (string, error) {
+		},
+		func() (string, error) {
 			return "", errors.New(expectedErrNestedMsg)
 		})
 
@@ -215,3 +226,26 @@ func TestExtractGitInfo_FailToGetCurrentBranchName(t *testing.T) {
 		t.Errorf("\nExpected: %s\nActual:   %s", expectedErr, actualErr)
 	}
 }
+
+func TestRefreshGitRemote_FailToRefreshRemote(t *testing.T) {
+	expectedErrNestedMsg := "error when fetching origin commits"
+	_, actualErr := ExtractGitInfo(
+		func() error {
+			return errors.New(expectedErrNestedMsg)
+		},
+		func() (string, error) {
+			return "git@custom-gitlab.com:namespace/project.git", nil
+		},
+		func() (string, error) {
+			return "feature/abc", nil
+		},
+	)
+
+	if actualErr == nil {
+		t.Errorf("Expected an error, got none")
+	}
+	expectedErr := fmt.Errorf("Could not get latest information from remote: %s", expectedErrNestedMsg)
+	if actualErr.Error() != expectedErr.Error() {
+		t.Errorf("\nExpected: %s\nActual:   %s", expectedErr, actualErr)
+	}
+}

cmd/main.go

@@ -11,9 +11,9 @@ import (
 )
 
 func main() {
-	g, err := ExtractGitInfo(GetProjectUrlFromNativeGitCmd, GetCurrentBranchNameFromNativeGitCmd)
+	g, err := ExtractGitInfo(RefreshProjectInfo, GetProjectUrlFromNativeGitCmd, GetCurrentBranchNameFromNativeGitCmd)
 	if err != nil {
-		log.Fatalf("Failed to get git namespace, project, branch, or url: %v", err)
+		log.Fatalf("Failure initializing plugin with `git` commands: %v", err)
 	}
 
 	var c Client

lua/gitlab/job.lua

@@ -22,6 +22,9 @@ M.run_job = function(endpoint, method, body, callback)
     args = args,
     on_stdout = function(_, output)
       vim.defer_fn(function()
+        if output == nil then
+          return
+        end
         local data_ok, data = pcall(vim.json.decode, output)
         if not data_ok then
           local msg = string.format("Failed to parse JSON from %s endpoint", endpoint)

lua/gitlab/reviewer/diffview.lua

@@ -10,8 +10,19 @@ local M = {
 }
 
 M.open = function()
-  vim.api.nvim_command(string.format("DiffviewOpen %s", state.INFO.target_branch))
+  local diff_refs = state.INFO.diff_refs
+  if diff_refs == nil then
+    u.notify("Gitlab did not provide diff refs required to review this MR", vim.log.levels.ERROR)
+    return
+  end
+
+  vim.api.nvim_command(string.format("DiffviewOpen %s..%s", diff_refs.base_sha, diff_refs.head_sha))
   M.tabnr = vim.api.nvim_get_current_tabpage()
+
+  if state.INFO.has_conflicts then
+    u.notify("This merge request has conflicts!", vim.log.levels.WARN)
+  end
+
   local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.close", {})
   vim.api.nvim_create_autocmd("User", {
     pattern = { "DiffviewViewClosed" },