Merge pull request #401 from diggerhq/fix-custom-workflow-issue

Fix custom workflow issue

Author: veziak

pkg/azure/azure.go

@@ -295,7 +295,18 @@ func (a *AzureReposService) IsClosed(prNumber int) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	return *pullRequest.Status == git.PullRequestStatusValues.Completed || *pullRequest.Status == git.PullRequestStatusValues.Abandoned, nil
+	return *pullRequest.Status == git.PullRequestStatusValues.Abandoned, nil
+}
+
+func (a *AzureReposService) IsMerged(prNumber int) (bool, error) {
+	pullRequest, err := a.Client.GetPullRequestById(context.Background(), git.GetPullRequestByIdArgs{
+		Project:       &a.ProjectName,
+		PullRequestId: &prNumber,
+	})
+	if err != nil {
+		return false, err
+	}
+	return *pullRequest.Status == git.PullRequestStatusValues.Completed, nil
 }
 
 func ProcessAzureReposEvent(azureEvent interface{}, diggerConfig *configuration.DiggerConfig, ciService ci.CIService) ([]configuration.Project, *configuration.Project, int, error) {

pkg/ci/ci.go

@@ -7,6 +7,10 @@ type CIService interface {
 	SetStatus(prNumber int, status string, statusContext string) error
 	GetCombinedPullRequestStatus(prNumber int) (string, error)
 	MergePullRequest(prNumber int) error
+	// IsMergeable is still open and ready to be merged
 	IsMergeable(prNumber int) (bool, error)
+	// IsMerged merged and closed
+	IsMerged(prNumber int) (bool, error)
+	// IsClosed closed without merging
 	IsClosed(prNumber int) (bool, error)
 }

pkg/digger/digger.go

@@ -76,6 +76,8 @@ func RunCommandsPerProject(
 	plansToPublish := make([]string, 0)
 	for _, projectCommands := range commandsPerProject {
 		for _, command := range projectCommands.Commands {
+			fmt.Printf("Running '%s' for project '%s'\n", command, projectCommands.ProjectName)
+
 			policyInput := map[string]interface{}{"user": requestedBy, "action": command}
 
 			allowedToPerformCommand, err := policyChecker.Check(projectNamespace, projectCommands.ProjectName, policyInput)
@@ -124,14 +126,24 @@ func RunCommandsPerProject(
 				projectLock,
 				planStorage,
 			}
+
 			switch command {
 			case "digger plan":
-				usage.SendUsageRecord(requestedBy, eventName, "plan")
-				ciService.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/plan")
+				err := usage.SendUsageRecord(requestedBy, eventName, "plan")
+				if err != nil {
+					return false, false, fmt.Errorf("failed to send usage report. %v", err)
+				}
+				err = ciService.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/plan")
+				if err != nil {
+					return false, false, fmt.Errorf("failed to set PR status. %v", err)
+				}
 				planPerformed, plan, err := diggerExecutor.Plan()
 				if err != nil {
 					log.Printf("Failed to run digger plan command. %v", err)
-					ciService.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/plan")
+					err := ciService.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/plan")
+					if err != nil {
+						return false, false, fmt.Errorf("failed to set PR status. %v", err)
+					}
 					return false, false, fmt.Errorf("failed to run digger plan command. %v", err)
 				} else if planPerformed {
 					if plan != "" {
@@ -145,45 +157,73 @@ func RunCommandsPerProject(
 							}
 						}
 					}
-					ciService.SetStatus(prNumber, "success", projectCommands.ProjectName+"/plan")
+					err := ciService.SetStatus(prNumber, "success", projectCommands.ProjectName+"/plan")
+					if err != nil {
+						return false, false, fmt.Errorf("failed to set PR status. %v", err)
+					}
 				}
 			case "digger apply":
 				appliesPerProject[projectCommands.ProjectName] = false
-				usage.SendUsageRecord(requestedBy, eventName, "apply")
-				ciService.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/apply")
+				err := usage.SendUsageRecord(requestedBy, eventName, "apply")
+				if err != nil {
+					return false, false, fmt.Errorf("failed to send usage report. %v", err)
+				}
+				err = ciService.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/apply")
+				if err != nil {
+					return false, false, fmt.Errorf("failed to set PR status. %v", err)
+				}
+
+				isMerged, err := ciService.IsMerged(prNumber)
+				if err != nil {
+					return false, false, fmt.Errorf("error checking if PR is merged: %v", err)
+				}
 
 				// this might go into some sort of "appliability" plugin later
 				isMergeable, err := ciService.IsMergeable(prNumber)
 				if err != nil {
 					return false, false, fmt.Errorf("error validating is PR is mergeable: %v", err)
 				}
-				if !isMergeable {
+				fmt.Printf("PR status, mergeable: %v, merged: %v\n", isMergeable, isMerged)
+				if !isMergeable && !isMerged {
 					comment := "Cannot perform Apply since the PR is not currently mergeable."
+					fmt.Println(comment)
 					err = ciService.PublishComment(prNumber, comment)
 					if err != nil {
-						fmt.Printf("error publishing comment: %v", err)
+						fmt.Printf("error publishing comment: %v\n", err)
 					}
 					return false, false, nil
 				} else {
 					applyPerformed, err := diggerExecutor.Apply()
 					if err != nil {
 						log.Printf("Failed to run digger apply command. %v", err)
-						ciService.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/apply")
+						err := ciService.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/apply")
+						if err != nil {
+							return false, false, fmt.Errorf("failed to set PR status. %v", err)
+						}
 						return false, false, fmt.Errorf("failed to run digger apply command. %v", err)
 					} else if applyPerformed {
-						ciService.SetStatus(prNumber, "success", projectCommands.ProjectName+"/apply")
+						err := ciService.SetStatus(prNumber, "success", projectCommands.ProjectName+"/apply")
+						if err != nil {
+							return false, false, fmt.Errorf("failed to set PR status. %v", err)
+						}
 						appliesPerProject[projectCommands.ProjectName] = true
 					}
 				}
 			case "digger unlock":
-				usage.SendUsageRecord(requestedBy, eventName, "unlock")
-				err := diggerExecutor.Unlock()
+				err := usage.SendUsageRecord(requestedBy, eventName, "unlock")
+				if err != nil {
+					return false, false, fmt.Errorf("failed to send usage report. %v", err)
+				}
+				err = diggerExecutor.Unlock()
 				if err != nil {
 					return false, false, fmt.Errorf("failed to unlock project. %v", err)
 				}
 			case "digger lock":
-				usage.SendUsageRecord(requestedBy, eventName, "lock")
-				err := diggerExecutor.Lock()
+				err := usage.SendUsageRecord(requestedBy, eventName, "lock")
+				if err != nil {
+					return false, false, fmt.Errorf("failed to send usage report. %v", err)
+				}
+				err = diggerExecutor.Lock()
 				if err != nil {
 					return false, false, fmt.Errorf("failed to lock project. %v", err)
 				}

pkg/digger/digger_test.go

@@ -90,6 +90,11 @@ func (m *MockPRManager) DownloadLatestPlans(prNumber int) (string, error) {
 	return "plan", nil
 }
 
+func (m *MockPRManager) IsMerged(prNumber int) (bool, error) {
+	m.Commands = append(m.Commands, RunInfo{"IsClosed", strconv.Itoa(prNumber), time.Now()})
+	return false, nil
+}
+
 func (m *MockPRManager) IsClosed(prNumber int) (bool, error) {
 	m.Commands = append(m.Commands, RunInfo{"IsClosed", strconv.Itoa(prNumber), time.Now()})
 	return false, nil

pkg/github/github.go

@@ -109,15 +109,26 @@ func (svc *GithubService) IsMergeable(prNumber int) (bool, error) {
 	pr, _, err := svc.Client.PullRequests.Get(context.Background(), svc.Owner, svc.RepoName, prNumber)
 	if err != nil {
 		log.Fatalf("error getting pull request: %v", err)
+		return false, err
 	}
 
 	return pr.GetMergeable() && isMergeableState(pr.GetMergeableState()), nil
 }
 
+func (svc *GithubService) IsMerged(prNumber int) (bool, error) {
+	pr, _, err := svc.Client.PullRequests.Get(context.Background(), svc.Owner, svc.RepoName, prNumber)
+	if err != nil {
+		log.Fatalf("error getting pull request: %v", err)
+		return false, err
+	}
+	return *pr.Merged, nil
+}
+
 func (svc *GithubService) IsClosed(prNumber int) (bool, error) {
 	pr, _, err := svc.Client.PullRequests.Get(context.Background(), svc.Owner, svc.RepoName, prNumber)
 	if err != nil {
 		log.Fatalf("error getting pull request: %v", err)
+		return false, err
 	}
 
 	return pr.GetState() == "closed", nil

pkg/gitlab/gitlab.go

@@ -208,23 +208,32 @@ func (gitlabService GitLabService) IsMergeable(mergeRequestID int) (bool, error)
 }
 
 func (gitlabService GitLabService) IsClosed(mergeRequestID int) (bool, error) {
+	mergeRequest := getMergeRequest(gitlabService)
+	if mergeRequest.State == "closed" {
+		return true, nil
+	}
+	return false, nil
+}
+
+func (gitlabService GitLabService) IsMerged(mergeRequestID int) (bool, error) {
+	mergeRequest := getMergeRequest(gitlabService)
+	if mergeRequest.State == "merged" {
+		return true, nil
+	}
+	return false, nil
+}
+
+func getMergeRequest(gitlabService GitLabService) *go_gitlab.MergeRequest {
 	projectId := *gitlabService.Context.ProjectId
 	mergeRequestIID := *gitlabService.Context.MergeRequestIId
-
-	fmt.Printf("IsClosed mergeRequestIID : %d, projectId: %d \n", mergeRequestIID, projectId)
+	fmt.Printf("getMergeRequest mergeRequestIID : %d, projectId: %d \n", mergeRequestIID, projectId)
 	opt := &go_gitlab.GetMergeRequestsOptions{}
-
 	mergeRequest, _, err := gitlabService.Client.MergeRequests.GetMergeRequest(projectId, mergeRequestIID, opt)
-
 	if err != nil {
 		fmt.Printf("Failed to get a MergeRequest: %d, %v \n", mergeRequestIID, err)
 		print(err.Error())
 	}
-
-	if mergeRequest.State == "closed" || mergeRequest.State == "merged" {
-		return true, nil
-	}
-	return false, nil
+	return mergeRequest
 }
 
 type GitLabEvent struct {

pkg/utils/mocks.go

@@ -73,6 +73,10 @@ func (t MockPullRequestManager) IsMergeable(prNumber int) (bool, error) {
 	return true, nil
 }
 
+func (t MockPullRequestManager) IsMerged(prNumber int) (bool, error) {
+	return false, nil
+}
+
 func (t MockPullRequestManager) DownloadLatestPlans(prNumber int) (string, error) {
 	return "", nil
 }

pkg/utils/pullrequestmanager_mock.go

@@ -43,3 +43,8 @@ func (mockGithubPullrequestManager *MockGithubPullrequestManager) IsClosed(prNum
 	mockGithubPullrequestManager.commands = append(mockGithubPullrequestManager.commands, "IsClosed")
 	return false, nil
 }
+
+func (mockGithubPullrequestManager *MockGithubPullrequestManager) IsMerged(prNumber int) (bool, error) {
+	mockGithubPullrequestManager.commands = append(mockGithubPullrequestManager.commands, "IsClosed")
+	return false, nil
+}