From b8c386ac6ba824323dc2eb5f115144bdc0760964 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Mon, 20 Nov 2023 07:19:20 -0500 Subject: [PATCH] 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 | 20 ++++++++++++- cmd/git_test.go | 50 +++++++++++++++++++++++++++----- cmd/main.go | 4 +-- lua/gitlab/job.lua | 3 ++ lua/gitlab/reviewer/diffview.lua | 13 ++++++++- 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/cmd/git.go b/cmd/git.go index c7065bd..e5caf2a 100644 --- a/cmd/git.go +++ b/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 +} diff --git a/cmd/git_test.go b/cmd/git_test.go index 70cd661..475de96 100644 --- a/cmd/git_test.go +++ b/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,11 +206,17 @@ func TestExtractGitInfo_FailToGetProjectRemoteUrl(t *testing.T) { func TestExtractGitInfo_FailToGetCurrentBranchName(t *testing.T) { expectedErrNestedMsg := "error when getting current branch name" - _, actualErr := ExtractGitInfo(func() (string, error) { - return "git@custom-gitlab.com:namespace/project.git", nil - }, func() (string, error) { - return "", errors.New(expectedErrNestedMsg) - }) + + refreshGitInfo := func() error { + return nil + } + _, actualErr := ExtractGitInfo(refreshGitInfo, + func() (string, error) { + return "git@custom-gitlab.com:namespace/project.git", nil + }, + func() (string, error) { + return "", errors.New(expectedErrNestedMsg) + }) if actualErr == nil { t.Errorf("Expected an error, got none") @@ -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) + } +} diff --git a/cmd/main.go b/cmd/main.go index bd930ae..f0b4757 100644 --- a/cmd/main.go +++ b/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 diff --git a/lua/gitlab/job.lua b/lua/gitlab/job.lua index 499c574..2ca4e52 100644 --- a/lua/gitlab/job.lua +++ b/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) diff --git a/lua/gitlab/reviewer/diffview.lua b/lua/gitlab/reviewer/diffview.lua index d89aece..26e29a3 100644 --- a/lua/gitlab/reviewer/diffview.lua +++ b/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" },