From 816451a53413f22549578443cc7730952f955ea6 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 1 Jul 2024 13:28:21 -0700 Subject: [PATCH 1/7] dont delete main branches --- src/github/logic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/github/logic.py b/src/github/logic.py index 29d41486..21e46f98 100644 --- a/src/github/logic.py +++ b/src/github/logic.py @@ -301,7 +301,8 @@ def maybe_delete_branch_if_merged(pull_request: PullRequest): owner = pull_request.repository_owner_handle() repo_name = pull_request.repository_name() branch_name = pull_request.head_ref_name() - github_client.delete_branch_if_exists(owner, repo_name, branch_name) + if branch_name not in ["master", "main"]: + github_client.delete_branch_if_exists(owner, repo_name, branch_name) # ---------------------------------------------------------------------------------- From 8e47641723af1763212b1f2fc49475d0d1445fce Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 1 Jul 2024 13:35:27 -0700 Subject: [PATCH 2/7] add test --- test/github/test_logic.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/github/test_logic.py b/test/github/test_logic.py index 19c4bdf2..616fbe95 100644 --- a/test/github/test_logic.py +++ b/test/github/test_logic.py @@ -622,6 +622,13 @@ def test_do_not_delete_branch_if_closed(self, mock_delete_branch): github_logic.maybe_delete_branch_if_merged(pull_request) mock_delete_branch.assert_not_called() + def test_do_not_delete_main_branch(self, mock_delete_branch): + pull_request = build( + builder.pull_request().merged(True).head_ref_name("main") + ) + github_logic.maybe_delete_branch_if_merged(pull_request) + mock_delete_branch.assert_not_called() + if __name__ == "__main__": from unittest import main as run_tests From 7ddbc14642d8d2887c7835c90bc17af5c597c981 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Mon, 1 Jul 2024 13:40:26 -0700 Subject: [PATCH 3/7] lint --- test/github/test_logic.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/github/test_logic.py b/test/github/test_logic.py index 616fbe95..38b40483 100644 --- a/test/github/test_logic.py +++ b/test/github/test_logic.py @@ -623,9 +623,7 @@ def test_do_not_delete_branch_if_closed(self, mock_delete_branch): mock_delete_branch.assert_not_called() def test_do_not_delete_main_branch(self, mock_delete_branch): - pull_request = build( - builder.pull_request().merged(True).head_ref_name("main") - ) + pull_request = build(builder.pull_request().merged(True).head_ref_name("main")) github_logic.maybe_delete_branch_if_merged(pull_request) mock_delete_branch.assert_not_called() From 238a755849431a2790b7e83bd892db1ccd01419a Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Tue, 2 Jul 2024 11:18:48 -0700 Subject: [PATCH 4/7] check against repo settings --- src/github/graphql/client.py | 6 ++++++ src/github/graphql/queries/GetRepository.py | 17 +++++++++++++++++ src/github/logic.py | 4 ++-- src/github/webhook.py | 3 ++- test/github/test_logic.py | 16 ++++++++++++++-- 5 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 src/github/graphql/queries/GetRepository.py diff --git a/src/github/graphql/client.py b/src/github/graphql/client.py index 95aad185..3fe5d08f 100644 --- a/src/github/graphql/client.py +++ b/src/github/graphql/client.py @@ -7,6 +7,7 @@ GetPullRequestByRepositoryAndNumber, GetPullRequestAndComment, GetPullRequestAndReview, + GetRepository, IteratePullRequestIdsForCommitId, IterateReviewsForPullRequestId, ) @@ -146,3 +147,8 @@ def is_review(review_edge): }, )["node"]["reviews"]["edges"] return None + + +def get_repository(repository_id: str): + data = _execute_graphql_query(GetRepository, {"repositoryId": repository_id}) + return data["repository"] diff --git a/src/github/graphql/queries/GetRepository.py b/src/github/graphql/queries/GetRepository.py new file mode 100644 index 00000000..ca6b79cd --- /dev/null +++ b/src/github/graphql/queries/GetRepository.py @@ -0,0 +1,17 @@ +from typing import FrozenSet + +# @GraphqlInPython +_get_repository = """ +query GetRepository($repositoryId: ID!) { + repository: node($repositoryId: ID!) { + ... on Repository { + deleteBranchOnMerge + defaultBranchRef{ + name + } + } + } +} +""" + +GetRepository: FrozenSet[str] = frozenset([_get_repository]) diff --git a/src/github/logic.py b/src/github/logic.py index 21e46f98..fa349904 100644 --- a/src/github/logic.py +++ b/src/github/logic.py @@ -296,12 +296,12 @@ def maybe_automerge_pull_request(pull_request: PullRequest) -> bool: return False -def maybe_delete_branch_if_merged(pull_request: PullRequest): +def maybe_delete_branch_if_merged(pull_request: PullRequest, repository): if pull_request.merged(): owner = pull_request.repository_owner_handle() repo_name = pull_request.repository_name() branch_name = pull_request.head_ref_name() - if branch_name not in ["master", "main"]: + if branch_name == repository.default_branch and repository.deleteBranchOnMerge: github_client.delete_branch_if_exists(owner, repo_name, branch_name) diff --git a/src/github/webhook.py b/src/github/webhook.py index c7191cbd..af6850dc 100644 --- a/src/github/webhook.py +++ b/src/github/webhook.py @@ -19,7 +19,8 @@ def _handle_pull_request_webhook(payload: dict) -> HttpResponse: # a label change will trigger this webhook, so it may trigger automerge github_logic.maybe_automerge_pull_request(pull_request) if payload["action"] == "closed": - github_logic.maybe_delete_branch_if_merged(pull_request) + repository = graphql_client.get_repository(pull_request.repository_id()) + github_logic.maybe_delete_branch_if_merged(pull_request, repository) github_logic.maybe_add_automerge_warning_comment(pull_request) github_controller.upsert_pull_request(pull_request) return HttpResponse("200") diff --git a/test/github/test_logic.py b/test/github/test_logic.py index 38b40483..abbf58cf 100644 --- a/test/github/test_logic.py +++ b/test/github/test_logic.py @@ -622,9 +622,21 @@ def test_do_not_delete_branch_if_closed(self, mock_delete_branch): github_logic.maybe_delete_branch_if_merged(pull_request) mock_delete_branch.assert_not_called() - def test_do_not_delete_main_branch(self, mock_delete_branch): + def test_do_not_delete_default_branch(self, mock_delete_branch): pull_request = build(builder.pull_request().merged(True).head_ref_name("main")) - github_logic.maybe_delete_branch_if_merged(pull_request) + repository = {"deleteBranchOnMerge": True, "defaultBranchRef": {"name": "main"}} + github_logic.maybe_delete_branch_if_merged(pull_request, repository) + mock_delete_branch.assert_not_called() + + def test_do_not_delete_default_branch(self, mock_delete_branch): + pull_request = build( + builder.pull_request().merged(True).head_ref_name("feature-branch") + ) + repository = { + "deleteBranchOnMerge": False, + "defaultBranchRef": {"name": "main"}, + } + github_logic.maybe_delete_branch_if_merged(pull_request, repository) mock_delete_branch.assert_not_called() From 622b9ef4e20e706dd2a7a53b4ffd691134fd3807 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Tue, 2 Jul 2024 11:20:13 -0700 Subject: [PATCH 5/7] neq --- src/github/logic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/github/logic.py b/src/github/logic.py index fa349904..da3acd38 100644 --- a/src/github/logic.py +++ b/src/github/logic.py @@ -301,7 +301,7 @@ def maybe_delete_branch_if_merged(pull_request: PullRequest, repository): owner = pull_request.repository_owner_handle() repo_name = pull_request.repository_name() branch_name = pull_request.head_ref_name() - if branch_name == repository.default_branch and repository.deleteBranchOnMerge: + if branch_name != repository.default_branch and repository.deleteBranchOnMerge: github_client.delete_branch_if_exists(owner, repo_name, branch_name) From 12d122fe2e524cb2c1d02de9a80bee883d190bc4 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Tue, 2 Jul 2024 11:21:32 -0700 Subject: [PATCH 6/7] test name --- test/github/test_logic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/github/test_logic.py b/test/github/test_logic.py index abbf58cf..bbb15086 100644 --- a/test/github/test_logic.py +++ b/test/github/test_logic.py @@ -628,7 +628,7 @@ def test_do_not_delete_default_branch(self, mock_delete_branch): github_logic.maybe_delete_branch_if_merged(pull_request, repository) mock_delete_branch.assert_not_called() - def test_do_not_delete_default_branch(self, mock_delete_branch): + def test_do_not_delete_if_repo_setting(self, mock_delete_branch): pull_request = build( builder.pull_request().merged(True).head_ref_name("feature-branch") ) From 55651fe863e237a83b2714e69d0c8e1cccc7d789 Mon Sep 17 00:00:00 2001 From: Michael Huang Date: Tue, 2 Jul 2024 11:22:49 -0700 Subject: [PATCH 7/7] dict not object --- src/github/logic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/github/logic.py b/src/github/logic.py index da3acd38..18efc353 100644 --- a/src/github/logic.py +++ b/src/github/logic.py @@ -301,7 +301,7 @@ def maybe_delete_branch_if_merged(pull_request: PullRequest, repository): owner = pull_request.repository_owner_handle() repo_name = pull_request.repository_name() branch_name = pull_request.head_ref_name() - if branch_name != repository.default_branch and repository.deleteBranchOnMerge: + if branch_name != repository["default_branch"] and repository["deleteBranchOnMerge"]: github_client.delete_branch_if_exists(owner, repo_name, branch_name)