Comment on pull request when lock discarded.

When the lock is discarded from the Atlantis UI, comment back on pull request so users know the lock was discarded. - Adds the BaseRepo field to the PullRequest model. - This change is backwards compatible with previous installations where the DB will have a Project model with a PullRequest without the BaseRepo field.
runatlantis · May 30, 2018 · a57d4c6 · a57d4c6
1 parent 9df3609
commit a57d4c6
Show file tree

Hide file tree

Showing 9 changed files with 164 additions and 117 deletions.
diff --git a/server/events/command_handler.go b/server/events/command_handler.go
@@ -86,7 +86,7 @@ func (c *CommandHandler) ExecuteCommand(baseRepo models.Repo, headRepo models.Re
 	case models.Github:
 		pull, headRepo, err = c.getGithubData(baseRepo, pullNum)
 	case models.Gitlab:
-		pull, err = c.getGitlabData(baseRepo.FullName, pullNum)
+		pull, err = c.getGitlabData(baseRepo, pullNum)
 	default:
 		err = errors.New("Unknown VCS type, this is a bug!")
 	}
@@ -120,15 +120,15 @@ func (c *CommandHandler) getGithubData(baseRepo models.Repo, pullNum int) (model
 	return pull, repo, nil
 }
 
-func (c *CommandHandler) getGitlabData(repoFullName string, pullNum int) (models.PullRequest, error) {
+func (c *CommandHandler) getGitlabData(baseRepo models.Repo, pullNum int) (models.PullRequest, error) {
 	if c.GitlabMergeRequestGetter == nil {
 		return models.PullRequest{}, errors.New("Atlantis not configured to support GitLab")
 	}
-	mr, err := c.GitlabMergeRequestGetter.GetMergeRequest(repoFullName, pullNum)
+	mr, err := c.GitlabMergeRequestGetter.GetMergeRequest(baseRepo.FullName, pullNum)
 	if err != nil {
 		return models.PullRequest{}, errors.Wrap(err, "making merge request API call to GitLab")
 	}
-	pull := c.EventParser.ParseGitlabMergeRequest(mr)
+	pull := c.EventParser.ParseGitlabMergeRequest(mr, baseRepo)
 	return pull, nil
 }
 

diff --git a/server/events/event_parser.go b/server/events/event_parser.go
@@ -48,7 +48,7 @@ type EventParsing interface {
 	ParseGithubRepo(ghRepo *github.Repository) (models.Repo, error)
 	ParseGitlabMergeEvent(event gitlab.MergeEvent) (models.PullRequest, models.Repo, error)
 	ParseGitlabMergeCommentEvent(event gitlab.MergeCommentEvent) (baseRepo models.Repo, headRepo models.Repo, user models.User, err error)
-	ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.PullRequest
+	ParseGitlabMergeRequest(mr *gitlab.MergeRequest, baseRepo models.Repo) models.PullRequest
 }
 
 type EventParser struct {
@@ -104,7 +104,11 @@ func (e *EventParser) ParseGithubPull(pull *github.PullRequest) (models.PullRequ
 		return pullModel, headRepoModel, errors.New("number is null")
 	}
 
-	headRepoModel, err := e.ParseGithubRepo(pull.Head.Repo)
+	baseRepoModel, err := e.ParseGithubRepo(pull.Base.Repo)
+	if err != nil {
+		return pullModel, headRepoModel, err
+	}
+	headRepoModel, err = e.ParseGithubRepo(pull.Head.Repo)
 	if err != nil {
 		return pullModel, headRepoModel, err
 	}
@@ -121,7 +125,7 @@ func (e *EventParser) ParseGithubPull(pull *github.PullRequest) (models.PullRequ
 		URL:        url,
 		Num:        num,
 		State:      pullState,
-		HeadRepo:   headRepoModel,
+		BaseRepo:   baseRepoModel,
 	}, headRepoModel, nil
 }
 
@@ -137,16 +141,17 @@ func (e *EventParser) ParseGitlabMergeEvent(event gitlab.MergeEvent) (models.Pul
 	// GitLab also has a "merged" state, but we map that to Closed so we don't
 	// need to check for it.
 
+	repo, err := models.NewRepo(models.Gitlab, event.Project.PathWithNamespace, event.Project.GitHTTPURL, e.GitlabUser, e.GitlabToken)
 	pull := models.PullRequest{
 		URL:        event.ObjectAttributes.URL,
 		Author:     event.User.Username,
 		Num:        event.ObjectAttributes.IID,
 		HeadCommit: event.ObjectAttributes.LastCommit.ID,
 		Branch:     event.ObjectAttributes.SourceBranch,
 		State:      modelState,
+		BaseRepo:   repo,
 	}
 
-	repo, err := models.NewRepo(models.Gitlab, event.Project.PathWithNamespace, event.Project.GitHTTPURL, e.GitlabUser, e.GitlabToken)
 	return pull, repo, err
 }
 
@@ -170,7 +175,11 @@ func (e *EventParser) ParseGitlabMergeCommentEvent(event gitlab.MergeCommentEven
 	return
 }
 
-func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.PullRequest {
+// ParseGitlabMergeRequest parses the merge requests and returns a pull request
+// model. We require passing in baseRepo because although can't get this information
+// from the merge request, the only caller of this function already has that
+// data. This means we can construct the pull request object correctly.
+func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest, baseRepo models.Repo) models.PullRequest {
 	pullState := models.Closed
 	if mr.State == gitlabPullOpened {
 		pullState = models.Open
@@ -185,5 +194,6 @@ func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.Pu
 		HeadCommit: mr.SHA,
 		Branch:     mr.SourceBranch,
 		State:      pullState,
+		BaseRepo:   baseRepo,
 	}
 }
diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go
@@ -137,7 +137,7 @@ func TestParseGithubPull(t *testing.T) {
 		HeadCommit: Pull.Head.GetSHA(),
 		Num:        Pull.GetNumber(),
 		State:      models.Open,
-		HeadRepo: models.Repo{
+		BaseRepo: models.Repo{
 			Owner:             "owner",
 			FullName:          "owner/repo",
 			CloneURL:          "https://github-user:github-token@github.com/owner/repo.git",
@@ -158,16 +158,8 @@ func TestParseGitlabMergeEvent(t *testing.T) {
 	Ok(t, err)
 	pull, repo, err := parser.ParseGitlabMergeEvent(*event)
 	Ok(t, err)
-	Equals(t, models.PullRequest{
-		URL:        "http://example.com/diaspora/merge_requests/1",
-		Author:     "root",
-		Num:        1,
-		HeadCommit: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
-		Branch:     "ms-viewport",
-		State:      models.Open,
-	}, pull)
 
-	Equals(t, models.Repo{
+	expRepo := models.Repo{
 		FullName:          "gitlabhq/gitlab-test",
 		Name:              "gitlab-test",
 		SanitizedCloneURL: "https://example.com/gitlabhq/gitlab-test.git",
@@ -177,7 +169,19 @@ func TestParseGitlabMergeEvent(t *testing.T) {
 			Hostname: "example.com",
 			Type:     models.Gitlab,
 		},
-	}, repo)
+	}
+
+	Equals(t, models.PullRequest{
+		URL:        "http://example.com/diaspora/merge_requests/1",
+		Author:     "root",
+		Num:        1,
+		HeadCommit: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+		Branch:     "ms-viewport",
+		State:      models.Open,
+		BaseRepo:   expRepo,
+	}, pull)
+
+	Equals(t, expRepo, repo)
 
 	t.Log("If the state is closed, should set field correctly.")
 	event.ObjectAttributes.State = "closed"
@@ -191,19 +195,31 @@ func TestParseGitlabMergeRequest(t *testing.T) {
 	var event *gitlab.MergeRequest
 	err := json.Unmarshal([]byte(mergeRequestJSON), &event)
 	Ok(t, err)
-	pull := parser.ParseGitlabMergeRequest(event)
+	repo := models.Repo{
+		FullName:          "gitlabhq/gitlab-test",
+		Name:              "gitlab-test",
+		SanitizedCloneURL: "https://example.com/gitlabhq/gitlab-test.git",
+		Owner:             "gitlabhq",
+		CloneURL:          "https://gitlab-user:gitlab-token@example.com/gitlabhq/gitlab-test.git",
+		VCSHost: models.VCSHost{
+			Hostname: "example.com",
+			Type:     models.Gitlab,
+		},
+	}
+	pull := parser.ParseGitlabMergeRequest(event, repo)
 	Equals(t, models.PullRequest{
 		URL:        "https://gitlab.com/lkysow/atlantis-example/merge_requests/8",
 		Author:     "lkysow",
 		Num:        8,
 		HeadCommit: "0b4ac85ea3063ad5f2974d10cd68dd1f937aaac2",
 		Branch:     "abc",
 		State:      models.Open,
+		BaseRepo:   repo,
 	}, pull)
 
 	t.Log("If the state is closed, should set field correctly.")
 	event.State = "closed"
-	pull = parser.ParseGitlabMergeRequest(event)
+	pull = parser.ParseGitlabMergeRequest(event, repo)
 	Equals(t, models.Closed, pull.State)
 }
 

diff --git a/server/events/mocks/mock_event_parsing.go b/server/events/mocks/mock_event_parsing.go
@@ -124,8 +124,8 @@ func (mock *MockEventParsing) ParseGitlabMergeCommentEvent(event go_gitlab.Merge
 	return ret0, ret1, ret2, ret3
 }
 
-func (mock *MockEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest) models.PullRequest {
-	params := []pegomock.Param{mr}
+func (mock *MockEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest, baseRepo models.Repo) models.PullRequest {
+	params := []pegomock.Param{mr, baseRepo}
 	result := pegomock.GetGenericMockFrom(mock).Invoke("ParseGitlabMergeRequest", params, []reflect.Type{reflect.TypeOf((*models.PullRequest)(nil)).Elem()})
 	var ret0 models.PullRequest
 	if len(result) != 0 {
@@ -289,8 +289,8 @@ func (c *EventParsing_ParseGitlabMergeCommentEvent_OngoingVerification) GetAllCa
 	return
 }
 
-func (verifier *VerifierEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest) *EventParsing_ParseGitlabMergeRequest_OngoingVerification {
-	params := []pegomock.Param{mr}
+func (verifier *VerifierEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest, baseRepo models.Repo) *EventParsing_ParseGitlabMergeRequest_OngoingVerification {
+	params := []pegomock.Param{mr, baseRepo}
 	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ParseGitlabMergeRequest", params)
 	return &EventParsing_ParseGitlabMergeRequest_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
 }
@@ -300,18 +300,22 @@ type EventParsing_ParseGitlabMergeRequest_OngoingVerification struct {
 	methodInvocations []pegomock.MethodInvocation
 }
 
-func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetCapturedArguments() *go_gitlab.MergeRequest {
-	mr := c.GetAllCapturedArguments()
-	return mr[len(mr)-1]
+func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetCapturedArguments() (*go_gitlab.MergeRequest, models.Repo) {
+	mr, baseRepo := c.GetAllCapturedArguments()
+	return mr[len(mr)-1], baseRepo[len(baseRepo)-1]
 }
 
-func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetAllCapturedArguments() (_param0 []*go_gitlab.MergeRequest) {
+func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetAllCapturedArguments() (_param0 []*go_gitlab.MergeRequest, _param1 []models.Repo) {
 	params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
 	if len(params) > 0 {
 		_param0 = make([]*go_gitlab.MergeRequest, len(params[0]))
 		for u, param := range params[0] {
 			_param0[u] = param.(*go_gitlab.MergeRequest)
 		}
+		_param1 = make([]models.Repo, len(params[1]))
+		for u, param := range params[1] {
+			_param1[u] = param.(models.Repo)
+		}
 	}
 	return
 }
diff --git a/server/events/models/models.go b/server/events/models/models.go
@@ -113,11 +113,6 @@ type PullRequest struct {
 	State PullRequestState
 	// BaseRepo is the repository that the pull request will be merged into.
 	BaseRepo Repo
-	// HeadRepo is the repository that is getting merged into the BaseRepo.
-	// If the pull request branch is from the same repository then HeadRepo will
-	// be the same as BaseRepo.
-	// See https://help.github.com/articles/about-pull-request-merges/.
-	HeadRepo Repo
 }
 
 type PullRequestState int

diff --git a/server/events/vcs/fixtures/fixtures.go b/server/events/vcs/fixtures/fixtures.go
@@ -22,7 +22,8 @@ var Pull = github.PullRequest{
 		Repo: &Repo,
 	},
 	Base: &github.PullRequestBranch{
-		SHA: github.String("sha256"),
+		SHA:  github.String("sha256"),
+		Repo: &Repo,
 	},
 	HTMLURL: github.String("html-url"),
 	User: &github.User{