add: code owners support (#509)

With the `reviewDecision` field on the pull request we can fully support code owners. We can rely on this field to determine if a review is needed, which should always cover code ownership.

[`PullRequest` object](https://docs.github.com/en/free-pro-team@latest/graphql/reference/objects#pullrequest), [`PullRequestReviewDecision` enum](https://docs.github.com/en/free-pro-team@latest/graphql/reference/enums#pullrequestreviewdecision)

related #96, #87

bot/kodiak/evaluation.py

@@ -43,6 +43,7 @@
     PRReviewRequest,
     PRReviewState,
     PullRequest,
+    PullRequestReviewDecision,
     PullRequestState,
     PushAllowance,
     RepoInfo,
@@ -795,6 +796,10 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
                 )
                 return
 
+        if pull_request.reviewDecision == PullRequestReviewDecision.REVIEW_REQUIRED:
+            await block_merge(api, pull_request, "missing required reviews")
+            return
+
         required: Set[str] = set()
         passing: Set[str] = set()
 

bot/kodiak/queries.py

@@ -72,6 +72,7 @@ class GraphQLResponse(TypedDict):
         requiresStatusChecks
         requiredStatusCheckContexts
         requiresStrictStatusChecks
+        requiresCodeOwnerReviews
         requiresCommitSignatures
         restrictsPushes
         pushAllowances(first: 100) {
@@ -105,6 +106,7 @@ class GraphQLResponse(TypedDict):
       }
       isDraft
       mergeStateStatus
+      reviewDecision
       state
       mergeable
       isCrossRepository
@@ -257,6 +259,15 @@ class PullRequestAuthor(BaseModel):
     name: Optional[str] = None
 
 
+class PullRequestReviewDecision(Enum):
+    # The pull request has received an approving review.
+    APPROVED = "APPROVED"
+    # Changes have been requested on the pull request.
+    CHANGES_REQUESTED = "CHANGES_REQUESTED"
+    # A review is required before the pull request can be merged.
+    REVIEW_REQUIRED = "REVIEW_REQUIRED"
+
+
 class PullRequest(BaseModel):
     id: str
     number: int
@@ -267,6 +278,9 @@ class PullRequest(BaseModel):
     author: PullRequestAuthor
     isDraft: bool
     mergeStateStatus: MergeStateStatus
+    # null if the pull request does not require a review by default (no branch
+    # protection), and no review was requested or submitted yet.
+    reviewDecision: Optional[PullRequestReviewDecision]
     state: PullRequestState
     mergeable: MergeableState
     isCrossRepository: bool
@@ -346,6 +360,7 @@ class BranchProtectionRule(BaseModel):
     requiresStatusChecks: bool
     requiredStatusCheckContexts: List[str]
     requiresStrictStatusChecks: bool
+    requiresCodeOwnerReviews: bool
     requiresCommitSignatures: bool
     restrictsPushes: bool
     pushAllowances: NodeListPushAllowance

bot/kodiak/test/fixtures/api/get_event/behind.json

@@ -22,6 +22,7 @@
               "WIP (beta)"
             ],
             "requiresStrictStatusChecks": true,
+            "requiresCodeOwnerReviews": false,
             "requiresCommitSignatures": false,
             "restrictsPushes": true,
             "pushAllowances": {

bot/kodiak/test_evaluation.py

@@ -37,6 +37,7 @@
     PRReviewState,
     PullRequest,
     PullRequestAuthor,
+    PullRequestReviewDecision,
     PullRequestState,
     PushAllowance,
     PushAllowanceActor,
@@ -275,6 +276,7 @@ def create_branch_protection() -> BranchProtectionRule:
         requiresStatusChecks=True,
         requiredStatusCheckContexts=["ci/api"],
         requiresStrictStatusChecks=True,
+        requiresCodeOwnerReviews=True,
         requiresCommitSignatures=False,
         restrictsPushes=False,
         pushAllowances=NodeListPushAllowance(nodes=[]),
@@ -1766,6 +1768,47 @@ async def test_mergeable_missing_required_approving_reviews_missing_approving_re
     assert api.queue_for_merge.called is False
 
 
+@pytest.mark.asyncio
+async def test_mergeable_missing_required_approving_reviews_code_owners() -> None:
+    """
+    Don't merge when code owners are required for review.
+    """
+    api = create_api()
+    mergeable = create_mergeable()
+    pull_request = create_pull_request()
+    branch_protection = create_branch_protection()
+    review = create_review()
+
+    pull_request.mergeStateStatus = MergeStateStatus.BLOCKED
+    pull_request.reviewDecision = PullRequestReviewDecision.REVIEW_REQUIRED
+    # previously with code owner blocked PRs Kodiak would update the pull
+    # request, even if the pull request was blocked from merging.
+    pull_request.mergeStateStatus = MergeStateStatus.BEHIND
+
+    branch_protection.requiresApprovingReviews = True
+    branch_protection.requiredApprovingReviewCount = 1
+    branch_protection.requiresCodeOwnerReviews = True
+    # this pull request meets requiredApprovingReviewCount, but is missing a
+    # code owner approval.
+    review.state = PRReviewState.APPROVED
+    review.author.permission = Permission.WRITE
+
+    await mergeable(
+        api=api,
+        pull_request=pull_request,
+        branch_protection=branch_protection,
+        reviews=[review],
+    )
+    assert api.set_status.call_count == 1
+    assert api.dequeue.call_count == 1
+    assert "missing required review" in api.set_status.calls[0]["msg"]
+
+    # verify we haven't tried to update/merge the PR
+    assert api.update_branch.called is False
+    assert api.merge.called is False
+    assert api.queue_for_merge.called is False
+
+
 @pytest.mark.asyncio
 async def test_mergeable_missing_requires_status_checks_failing_status_context() -> None:
     """

bot/kodiak/test_pull_request.py

@@ -61,6 +61,7 @@ def create_event() -> EventInfoResponse:
             "ci/circleci: frontend_test",
         ],
         requiresStrictStatusChecks=True,
+        requiresCodeOwnerReviews=False,
         requiresCommitSignatures=False,
         restrictsPushes=False,
         pushAllowances=NodeListPushAllowance(nodes=[]),

bot/kodiak/test_queries.py

@@ -134,6 +134,7 @@ def block_event() -> EventInfoResponse:
             "WIP (beta)",
         ],
         requiresStrictStatusChecks=True,
+        requiresCodeOwnerReviews=False,
         requiresCommitSignatures=False,
         restrictsPushes=True,
         pushAllowances=NodeListPushAllowance(