fix: Set mergeable correctly when branch protection doesn't require r…

…eviewers (#2470)

* Allow no required reviewers

* Fix comment in tests

Co-authored-by: Stas Ostrovskyi <stasostrovskyi@users.noreply.github.com>

server/events/vcs/fixtures/github-branch-protection-required-checks.json

@@ -0,0 +1,17 @@
+{
+    "url": "https://api.github.com/repos/octocat/Hello-World/branches/master/protection",
+    "required_status_checks": {
+        "url": "https://api.github.com/repos/octocat/Hello-World/branches/master/protection/required_status_checks",
+        "strict": true,
+        "contexts": [
+            "atlantis/apply"
+        ],
+        "contexts_url": "https://api.github.com/repos/octocat/Hello-World/branches/master/protection/required_status_checks/contexts",
+        "checks": [
+            {
+                "context": "atlantis/apply",
+                "app_id": 123456
+            }
+        ]
+    }
+}

server/events/vcs/fixtures/github-commit-check-suites.json

@@ -0,0 +1,169 @@
+{
+    "total_count": 1,
+    "check_suites": [
+        {
+            "id": 1234567890,
+            "node_id": "CS_kwDOHE7PYM8AAAAB2iIZfQ",
+            "head_branch": "atlantis-patch-2",
+            "head_sha": "4273e07c528292222f119a040079093bf1f11232",
+            "status": "queued",
+            "conclusion": null,
+            "url": "https://api.github.com/repos/octocat/Hello-World/check-suites/1234567890",
+            "before": "0000000000000000000000000000000000000000",
+            "after": "4273e07c528292222f119a040079093bf1f11232",
+            "pull_requests": [
+                {
+                    "url": "https://api.github.com/repos/octocat/Hello-World/pulls/1",
+                    "id": 1035065545,
+                    "number": 1,
+                    "head": {
+                        "ref": "atlantis-patch-2",
+                        "sha": "4273e07c528292222f119a040079093bf1f11232",
+                        "repo": {
+                            "id": 474926944,
+                            "url": "https://api.github.com/repos/octocat/Hello-World",
+                            "name": "Hello-World"
+                        }
+                    },
+                    "base": {
+                        "ref": "master",
+                        "sha": "6f5744874b33ceb6a5c91edc91085991dbd1f61a",
+                        "repo": {
+                            "id": 474926944,
+                            "url": "https://api.github.com/repos/octocat/Hello-World",
+                            "name": "Hello-World"
+                        }
+                    }
+                }
+            ],
+            "app": {
+                "id": 184783,
+                "slug": "atlantis",
+                "node_id": "A_kwHOBbMkBs4AAtHP",
+                "owner": {
+                    "login": "octocat",
+                    "id": 95626246,
+                    "node_id": "O_kgDOBbMkBg",
+                    "avatar_url": "https://avatars.githubusercontent.com/u/95626246?v=4",
+                    "gravatar_id": "",
+                    "url": "https://api.github.com/users/octocat",
+                    "html_url": "https://github.com/octocat",
+                    "followers_url": "https://api.github.com/users/octocat/followers",
+                    "following_url": "https://api.github.com/users/octocat/following{/other_user}",
+                    "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
+                    "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
+                    "subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
+                    "organizations_url": "https://api.github.com/users/octocat/orgs",
+                    "repos_url": "https://api.github.com/users/octocat/repos",
+                    "events_url": "https://api.github.com/users/octocat/events{/privacy}",
+                    "received_events_url": "https://api.github.com/users/octocat/received_events",
+                    "type": "Organization",
+                    "site_admin": false
+                },
+                "name": "atlantis",
+                "description": "",
+                "external_url": "https://atlantis.localhost/",
+                "html_url": "https://github.com/apps/atlantis",
+                "created_at": "2022-03-29T08:51:26Z",
+                "updated_at": "2022-07-01T11:35:37Z",
+                "permissions": {
+                    "administration": "read",
+                    "checks": "write",
+                    "contents": "write",
+                    "issues": "write",
+                    "metadata": "read",
+                    "pull_requests": "write",
+                    "statuses": "write"
+                },
+                "events": []
+            },
+            "created_at": "2022-08-24T07:06:21Z",
+            "updated_at": "2022-08-24T07:06:21Z",
+            "rerequestable": true,
+            "runs_rerequestable": true,
+            "latest_check_runs_count": 0,
+            "check_runs_url": "https://api.github.com/repos/octocat/Hello-World/check-suites/1234567890/check-runs",
+            "head_commit": {
+                "id": "4273e07c528292222f119a040079093bf1f11232",
+                "tree_id": "56781332464aabdfae51b7f37f72ffc6ce8ce54e",
+                "message": "test atlantis",
+                "timestamp": "2022-08-24T07:06:21Z",
+                "author": {
+                    "name": "octocat",
+                    "email": "octocat@noreply.github.com"
+                },
+                "committer": {
+                    "name": "GitHub",
+                    "email": "noreply@github.com"
+                }
+            },
+            "repository": {
+                "id": 474926944,
+                "node_id": "R_kgDOHE7PYA",
+                "name": "Hello-World",
+                "full_name": "octocat/Hello-World",
+                "private": true,
+                "owner": {
+                    "login": "octocat",
+                    "id": 95626246,
+                    "node_id": "O_kgDOBbMkBg",
+                    "avatar_url": "https://avatars.githubusercontent.com/u/95626246?v=4",
+                    "gravatar_id": "",
+                    "url": "https://api.github.com/users/octocat",
+                    "html_url": "https://github.com/octocat",
+                    "followers_url": "https://api.github.com/users/octocat/followers",
+                    "following_url": "https://api.github.com/users/octocat/following{/other_user}",
+                    "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
+                    "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
+                    "subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
+                    "organizations_url": "https://api.github.com/users/octocat/orgs",
+                    "repos_url": "https://api.github.com/users/octocat/repos",
+                    "events_url": "https://api.github.com/users/octocat/events{/privacy}",
+                    "received_events_url": "https://api.github.com/users/octocat/received_events",
+                    "type": "Organization",
+                    "site_admin": false
+                },
+                "html_url": "https://github.com/octocat/Hello-World",
+                "description": null,
+                "fork": false,
+                "url": "https://api.github.com/repos/octocat/Hello-World",
+                "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks",
+                "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}",
+                "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}",
+                "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams",
+                "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks",
+                "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}",
+                "events_url": "https://api.github.com/repos/octocat/Hello-World/events",
+                "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}",
+                "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}",
+                "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags",
+                "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}",
+                "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}",
+                "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}",
+                "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}",
+                "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}",
+                "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages",
+                "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers",
+                "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors",
+                "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers",
+                "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription",
+                "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}",
+                "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}",
+                "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}",
+                "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}",
+                "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}",
+                "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}",
+                "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges",
+                "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}",
+                "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads",
+                "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}",
+                "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}",
+                "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}",
+                "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}",
+                "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}",
+                "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}",
+                "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments"
+            }
+        }
+    ]
+}

server/events/vcs/github_client.go

@@ -365,7 +365,7 @@ func (g *GithubClient) GetPullReviewDecision(repo models.Repo, pull models.PullR
 		return approvalStatus, errors.Wrap(err, "getting reviewDecision")
 	}
 
-	if query.Repository.PullRequest.ReviewDecision == "APPROVED" {
+	if query.Repository.PullRequest.ReviewDecision == "APPROVED" || len(query.Repository.PullRequest.ReviewDecision) == 0 {
 		return true, nil
 	}
 

server/events/vcs/github_client_test.go

@@ -586,6 +586,157 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
 	}
 }
 
+func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T) {
+	vcsStatusName := "atlantis"
+	cases := []struct {
+		state          string
+		reviewDecision string
+		expMergeable   bool
+	}{
+		{
+			"dirty",
+			`"REVIEW_REQUIRED"`,
+			false,
+		},
+		{
+			"unknown",
+			`"REVIEW_REQUIRED"`,
+			false,
+		},
+		{
+			"blocked",
+			`"REVIEW_REQUIRED"`,
+			false,
+		},
+		{
+			"blocked",
+			`"APPROVED"`,
+			true,
+		},
+		{
+			"blocked",
+			"null",
+			true,
+		},
+		{
+			"behind",
+			`"REVIEW_REQUIRED"`,
+			false,
+		},
+		{
+			"random",
+			`"REVIEW_REQUIRED"`,
+			false,
+		},
+		{
+			"unstable",
+			`"REVIEW_REQUIRED"`,
+			true,
+		},
+		{
+			"has_hooks",
+			`"APPROVED"`,
+			true,
+		},
+		{
+			"clean",
+			`"APPROVED"`,
+			true,
+		},
+		{
+			"",
+			`"APPROVED"`,
+			false,
+		},
+	}
+
+	// Use a real GitHub json response and edit the mergeable_state field.
+	jsBytes, err := os.ReadFile("fixtures/github-pull-request.json")
+	Ok(t, err)
+	prJSON := string(jsBytes)
+
+	// Status Check Response
+	jsBytes, err = os.ReadFile("fixtures/github-commit-status-full.json")
+	Ok(t, err)
+	commitJSON := string(jsBytes)
+
+	// Branch protection Response
+	jsBytes, err = os.ReadFile("fixtures/github-branch-protection-required-checks.json")
+	Ok(t, err)
+	branchProtectionJSON := string(jsBytes)
+
+	// List check suites Response
+	jsBytes, err = os.ReadFile("fixtures/github-commit-check-suites.json")
+	Ok(t, err)
+	checkSuites := string(jsBytes)
+
+	for _, c := range cases {
+		t.Run(c.state, func(t *testing.T) {
+			response := strings.Replace(prJSON,
+				`"mergeable_state": "clean"`,
+				fmt.Sprintf(`"mergeable_state": "%s"`, c.state),
+				1,
+			)
+
+			// reviewDecision Response
+			reviewDecision := fmt.Sprintf(`{
+				"data": {
+					"repository": {
+						"pullRequest": {
+							"reviewDecision": %s
+						}
+					}
+				}
+			}`, c.reviewDecision)
+
+			testServer := httptest.NewTLSServer(
+				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					switch r.RequestURI {
+					case "/api/v3/repos/owner/repo/pulls/1":
+						w.Write([]byte(response)) // nolint: errcheck
+						return
+					case "/api/v3/repos/owner/repo/pulls/1/reviews?per_page=300":
+						w.Write([]byte("[]")) // nolint: errcheck
+						return
+					case "/api/v3/repos/owner/repo/commits/new-topic/status":
+						w.Write([]byte(commitJSON)) // nolint: errcheck
+					case "/api/graphql":
+						w.Write([]byte(reviewDecision)) // nolint: errcheck
+					case "/api/v3/repos/owner/repo/branches/master/protection":
+						w.Write([]byte(branchProtectionJSON)) // nolint: errcheck
+					case "/api/v3/repos/owner/repo/commits/new-topic/check-suites":
+						w.Write([]byte(checkSuites)) // nolint: errcheck
+					default:
+						t.Errorf("got unexpected request at %q", r.RequestURI)
+						http.Error(w, "not found", http.StatusNotFound)
+						return
+					}
+				}))
+			testServerURL, err := url.Parse(testServer.URL)
+			Ok(t, err)
+			client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{AllowMergeableBypassApply: true}, logging.NewNoopLogger(t))
+			Ok(t, err)
+			defer disableSSLVerification()()
+
+			actMergeable, err := client.PullIsMergeable(models.Repo{
+				FullName:          "owner/repo",
+				Owner:             "owner",
+				Name:              "repo",
+				CloneURL:          "",
+				SanitizedCloneURL: "",
+				VCSHost: models.VCSHost{
+					Type:     models.Github,
+					Hostname: "github.com",
+				},
+			}, models.PullRequest{
+				Num: 1,
+			}, vcsStatusName)
+			Ok(t, err)
+			Equals(t, c.expMergeable, actMergeable)
+		})
+	}
+}
+
 func TestGithubClient_MergePullHandlesError(t *testing.T) {
 	cases := []struct {
 		code    int