Skip to content

Commit

Permalink
add: warning about limitations of updating forks (#135)
Browse files Browse the repository at this point in the history
Forks cannot be updated through the github API. This change provides a warning in this case. Kodiak can still merge the PR, just not update.

Related #104
  • Loading branch information
chdsbd authored and kodiakhq[bot] committed Jul 27, 2019
1 parent 7375b45 commit 4528f02
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 3 deletions.
1 change: 1 addition & 0 deletions kodiak/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def pull_request() -> queries.PullRequest:
mergeStateStatus=queries.MergeStateStatus.BEHIND,
state=queries.PullRequestState.OPEN,
mergeable=queries.MergeableState.MERGEABLE,
isCrossRepository=False,
labels=[],
latest_sha="abcd",
baseRefName="some-branch",
Expand Down
6 changes: 6 additions & 0 deletions kodiak/messages.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FORKS_CANNOT_BE_UPDATED = """\
The [Github merging API](https://developer.github.com/v3/repos/merging/) only supports updating branches within a repository, not across forks. While Kodiak can still merge pull requests created from forked repositories, it cannot automatically update them.
Please see [kodiak#104](https://github.com/chdsbd/kodiak/issues/104) for the most recent information on this limitation.
"""
12 changes: 10 additions & 2 deletions kodiak/pull_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from markdown_html_finder import find_html_positions

import kodiak.app_config as conf
from kodiak import queries
from kodiak import messages, queries
from kodiak.config import V1, BodyText, MergeBodyStyle, MergeTitleStyle
from kodiak.config_utils import get_markdown_for_config
from kodiak.errors import (
Expand Down Expand Up @@ -220,7 +220,9 @@ async def mergeability(
if self.event is None:
self.log.info("no event")
return MergeabilityResponse.NOT_MERGEABLE, None
if not self.event.head_exists:
# PRs from forks will always appear deleted because the v4 api doesn't
# return head information for forks like the v3 api does.
if not self.event.pull_request.isCrossRepository and not self.event.head_exists:
self.log.info("branch deleted")
return MergeabilityResponse.NOT_MERGEABLE, None
if not isinstance(self.event.config, V1):
Expand Down Expand Up @@ -281,6 +283,12 @@ async def mergeability(
)
return MergeabilityResponse.WAIT, self.event
except NeedsBranchUpdate:
if self.event.pull_request.isCrossRepository:
await self.set_status(
summary='🚨 forks cannot updated via the github api. Click "Details" for more info',
markdown_content=messages.FORKS_CANNOT_BE_UPDATED,
)
return MergeabilityResponse.NOT_MERGEABLE, self.event
if merging:
await self.set_status(
summary="⛴ attempting to merge PR", detail="updating branch"
Expand Down
2 changes: 2 additions & 0 deletions kodiak/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class GraphQLResponse(TypedDict):
mergeStateStatus
state
mergeable
isCrossRepository
reviewRequests(first: 100) {
nodes {
requestedReviewer {
Expand Down Expand Up @@ -209,6 +210,7 @@ class PullRequest(BaseModel):
mergeStateStatus: MergeStateStatus
state: PullRequestState
mergeable: MergeableState
isCrossRepository: bool
labels: List[str]
# the SHA of the most recent commit
latest_sha: str
Expand Down
1 change: 1 addition & 0 deletions kodiak/test/fixtures/api/get_event/behind.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"mergeStateStatus": "BEHIND",
"state": "OPEN",
"mergeable": "MERGEABLE",
"isCrossRepository": false,
"reviewRequests": {
"nodes": [
{
Expand Down
1 change: 1 addition & 0 deletions kodiak/test_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def pull_request() -> PullRequest:
mergeStateStatus=MergeStateStatus.CLEAN,
state=PullRequestState.OPEN,
mergeable=MergeableState.MERGEABLE,
isCrossRepository=True,
labels=["bugfix", "automerge"],
latest_sha="f89be6c",
baseRefName="master",
Expand Down
37 changes: 36 additions & 1 deletion kodiak/test_pull_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from pytest_mock import MockFixture
from starlette.testclient import TestClient

from kodiak import queries
from kodiak import messages, queries
from kodiak.config import (
V1,
Merge,
Expand All @@ -19,6 +19,7 @@
get_merge_body,
strip_html_comments_from_markdown,
)
from kodiak.queries import MergeStateStatus
from kodiak.test_utils import wrap_future


Expand Down Expand Up @@ -82,6 +83,40 @@ async def test_deleting_branch_after_merge(
assert delete_branch.called == expected


@pytest.mark.asyncio
async def test_cross_repo_missing_head(
event_response: queries.EventInfoResponse, mocker: MockFixture
) -> None:
"""
if a repository is from a fork (isCrossRepository), we will not be able to
see head information due to a problem with the v4 api failing to return head
information for forks, unlike the v3 api.
"""

event_response.head_exists = False
event_response.pull_request.isCrossRepository = True
assert event_response.pull_request.mergeStateStatus == MergeStateStatus.BEHIND
event_response.pull_request.labels = ["automerge"]
assert event_response.branch_protection is not None
event_response.branch_protection.requiresApprovingReviews = False
event_response.branch_protection.requiresStrictStatusChecks = True
mocker.patch.object(PR, "get_event", return_value=wrap_future(event_response))
set_status = mocker.patch.object(PR, "set_status", return_value=wrap_future(None))
pr = PR(
number=123,
owner="tester",
repo="repo",
installation_id="abc",
client=queries.Client(owner="tester", repo="repo", installation_id="abc"),
)
await pr.mergeability()

assert set_status.call_count == 1
set_status.assert_called_with(
summary=mocker.ANY, markdown_content=messages.FORKS_CANNOT_BE_UPDATED
)


def test_pr(api_client: queries.Client) -> None:
a = PR(
number=123,
Expand Down
1 change: 1 addition & 0 deletions kodiak/test_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ def block_event(config_file_expression: str, config_str: str) -> EventInfoRespon
mergeStateStatus=MergeStateStatus.BEHIND,
state=PullRequestState.OPEN,
mergeable=MergeableState.MERGEABLE,
isCrossRepository=False,
labels=["automerge"],
latest_sha="8d728d017cac4f5ba37533debe65730abe65730a",
baseRefName="master",
Expand Down

0 comments on commit 4528f02

Please sign in to comment.