Fallback to main branch protection rules rather than skipping non-mai…

…n PRs

catalog/dags/maintenance/pr_review_reminders/pr_review_reminders.py

@@ -3,6 +3,8 @@
 from collections import defaultdict
 from dataclasses import dataclass
 
+from requests import HTTPError
+
 from common.github import GitHubAPI
 
 
@@ -135,9 +137,7 @@ def base_repo_name(pr: dict):
 _BRANCH_PROTECTION_CACHE = defaultdict(dict)
 
 
-def get_branch_protection(gh: GitHubAPI, pr: dict) -> dict:
-    repo = base_repo_name(pr)
-    branch_name = pr["base"]["ref"]
+def get_branch_protection(gh: GitHubAPI, repo: str, branch_name: str) -> dict:
     if branch_name not in _BRANCH_PROTECTION_CACHE[repo]:
         _BRANCH_PROTECTION_CACHE[repo][branch_name] = gh.get_branch_protection(
             repo, branch_name
@@ -147,7 +147,20 @@ def get_branch_protection(gh: GitHubAPI, pr: dict) -> dict:
 
 
 def get_min_required_approvals(gh: GitHubAPI, pr: dict) -> int:
-    branch_protection_rules = get_branch_protection(gh, pr)
+    repo = base_repo_name(pr)
+    branch_name = pr["base"]["ref"]
+
+    try:
+        branch_protection_rules = get_branch_protection(gh, repo, branch_name)
+    except HTTPError as e:
+        # If the base branch does not have protection rules, the request
+        # above will 404. In that case, fall back to the rules for `main`
+        # as a safe default.
+        if e.response is not None and e.response.status_code == 404:
+            branch_protection_rules = get_branch_protection(gh, repo, "main")
+        else:
+            raise e
+
     return branch_protection_rules["required_pull_request_reviews"][
         "required_approving_review_count"
     ]
@@ -158,11 +171,7 @@ def post_reminders(github_pat: str, dry_run: bool):
 
     open_prs = []
     for repo in REPOSITORIES:
-        open_prs += [
-            pr
-            for pr in gh.get_open_prs(repo)
-            if (not pr["draft"] and pr["base"]["label"] == "WordPress:main")
-        ]
+        open_prs += [pr for pr in gh.get_open_prs(repo) if not pr["draft"]]
 
     urgent_prs = []
     for pr in open_prs:

catalog/tests/dags/maintenance/test_pr_review_reminders.py

@@ -2,6 +2,7 @@
 from datetime import datetime, timedelta
 
 import pytest
+from requests import HTTPError, Request, Response
 
 from catalog.dags.maintenance.pr_review_reminders.pr_review_reminders import (
     Urgency,
@@ -100,6 +101,13 @@ def get_reviews(*args, **kwargs):
     def get_branch_protection(*args, **kwargs):
         repo = args[1]
         branch = args[2]
+        if repo not in branch_protection or branch not in branch_protection[repo]:
+            response = Response()
+            response.status_code = 404
+            request = Request(method="GET", url="https://api.github.com/")
+            response.request = request
+            raise HTTPError(response=response)
+
         return branch_protection[repo][branch]
 
     def patch_gh_fn(fn, impl):
@@ -131,11 +139,16 @@ def freeze_friday(freeze_time):
 
 
 def _setup_branch_protection(github: dict, pr: dict, min_required_approvals: int = 2):
-    branch_protection = make_branch_protection(min_required_approvals)
-
     repo = pr["base"]["repo"]["name"]
     branch = pr["base"]["ref"]
 
+    _setup_branch_protection_for_branch(github, repo, branch, min_required_approvals)
+
+
+def _setup_branch_protection_for_branch(
+    github: dict, repo: str, branch: str, min_required_approvals: int = 2
+):
+    branch_protection = make_branch_protection(min_required_approvals)
     github["branch_protection"][repo][branch] = branch_protection
 
 
@@ -247,22 +260,25 @@ def test_does_reping_past_due_if_reminder_is_outdated(github, urgency):
     # tested elsewhere.
     ("APPROVED",) + UNAPPROVED_REVIEW_STATES,
 )
+@pytest.mark.parametrize("base_branch", ("main", "not_main"))
 def test_does_ping_if_pr_has_less_than_min_required_approvals(
-    github, urgency, first_review_state, second_review_state
+    github, urgency, first_review_state, second_review_state, base_branch
 ):
     reviews = [
         make_review(state)
         for state in (first_review_state, second_review_state)
         if state is not None
     ]
-    past_due_pull = make_pull(urgency, past_due=True)
+    past_due_pull = make_pull(urgency, past_due=True, base_branch=base_branch)
     past_due_pull["requested_reviewers"] = [
         make_requested_reviewer(f"reviewer-due-{i}") for i in range(2)
     ]
 
-    _setup_branch_protection(
+    # Always use `main` to exercise fallback for non-main branches
+    _setup_branch_protection_for_branch(
         github,
-        pr=past_due_pull,
+        repo="openverse",
+        branch="main",
         min_required_approvals=2,
     )
 
@@ -275,17 +291,26 @@ def test_does_ping_if_pr_has_less_than_min_required_approvals(
 
 
 @parametrize_urgency
-def test_does_not_ping_if_pr_has_min_required_approvals(github, urgency):
-    past_due_pull = make_pull(urgency, past_due=True)
+@pytest.mark.parametrize(
+    ("base_branch"),
+    (
+        "main",
+        "not_main",  # should fallback to use `main`
+    ),
+)
+def test_does_not_ping_if_pr_has_min_required_approvals(github, urgency, base_branch):
+    past_due_pull = make_pull(urgency, past_due=True, base_branch=base_branch)
     past_due_pull["requested_reviewers"] = [
         make_requested_reviewer(f"reviewer-due-{i}") for i in range(2)
     ]
 
     min_required_approvals = 4
 
-    _setup_branch_protection(
+    # Always use `main` to exercise fallback for non-main branches
+    _setup_branch_protection_for_branch(
         github,
-        pr=past_due_pull,
+        repo="openverse",
+        branch="main",
         min_required_approvals=min_required_approvals,
     )
 
@@ -317,12 +342,3 @@ def test_does_not_ping_if_no_reviewers(github, urgency):
 
     assert past_due_pull["number"] not in github["posted_comments"]
     assert not_due_pull["number"] not in github["posted_comments"]
-
-
-@parametrize_urgency
-def test_skips_non_main_target(github, urgency):
-    non_main_target = make_pull(urgency, past_due=False)
-    non_main_target["base"]["label"] = "WordPress:main_old"
-
-    post_reminders("not_set", dry_run=False)
-    assert non_main_target["number"] not in github["posted_comments"]