diff --git a/libs/shared/shared/bots/github_apps.py b/libs/shared/shared/bots/github_apps.py index 6965872c8c..5e94aa5b0f 100644 --- a/libs/shared/shared/bots/github_apps.py +++ b/libs/shared/shared/bots/github_apps.py @@ -112,6 +112,19 @@ def _get_apps_from_weighted_selection( ) if default_apps: apps_to_consider.extend(default_apps) + if ( + owner_uses_sentry(owner) + and installation_name != settings.GITHUB_SENTRY_APP_NAME + ): + # Sentry-linked owners prefer the Codecov app, but the Sentry app is kept + # as a fallback in case the preferred app(s) are unavailable. + sentry_apps = filter( + lambda obj: _can_use_this_app( + obj, settings.GITHUB_SENTRY_APP_NAME, repository + ), + DjangoSQLAlchemyOwnerWrapper.get_github_app_installations(owner), + ) + apps_to_consider.extend(sentry_apps) return apps_to_consider @@ -266,9 +279,9 @@ def get_github_app_info_for_owner( Any GitHub App info returned needs to cover this repo installation_name (str | None): The installation name to search for in the available apps. GitHubAppInstallation.name must be equal to installation_name for it to be returned. - If the owner has a sentry_org_id linked through their account, this parameter will be - overridden and the sentry app will be used instead. If None and no sentry_org_id exists, - the installation name will be determined automatically. + If the owner has a sentry_org_id linked through their account, the Codecov app is still + preferred, but the sentry app is added as a fallback in case the preferred app(s) are + unavailable. Returns: (ordered) List[GithubInstallationInfo]: where index 0 is the main app and the others are fallback options @@ -276,9 +289,6 @@ def get_github_app_info_for_owner( Raises: NoConfiguredAppsAvailable: Owner has app installations available, but they are all currently rate limited. """ - if owner_uses_sentry(owner): - installation_name = settings.GITHUB_SENTRY_APP_NAME - extra_info_to_log = { "ownerid": owner.ownerid, "repoid": getattr(repository, "repoid", None), diff --git a/libs/shared/tests/unit/bots/test_github_apps.py b/libs/shared/tests/unit/bots/test_github_apps.py index 7fb0242837..88fcb407af 100644 --- a/libs/shared/tests/unit/bots/test_github_apps.py +++ b/libs/shared/tests/unit/bots/test_github_apps.py @@ -138,7 +138,43 @@ def test_raise_NoAppsConfiguredAvailable_if_suspended_or_rate_limited( @pytest.mark.django_db @override_settings(GITHUB_SENTRY_APP_NAME="test_sentry_app") - def test_sentry_org_id_overrides_installation_name(self, mocker): + def test_sentry_org_prefers_codecov_app_with_sentry_fallback(self, mocker): + owner = OwnerFactory(service="github") + account = Account.objects.create(name="test_account", sentry_org_id=12345) + owner.account = account + owner.save() + + codecov_app = GithubAppInstallation( + owner=owner, + installation_id=1000, + app_id=10, + name=GITHUB_APP_INSTALLATION_DEFAULT_NAME, + pem_path="codecov_pem", + ) + codecov_app.save() + sentry_app = GithubAppInstallation( + owner=owner, + installation_id=2000, + app_id=20, + name="test_sentry_app", + pem_path="sentry_pem", + ) + sentry_app.save() + + result = get_github_app_info_for_owner( + owner, installation_name=GITHUB_APP_INSTALLATION_DEFAULT_NAME + ) + + # The Codecov app is preferred, with the Sentry app kept as a fallback. + assert len(result) == 2 + assert result[0]["installation_id"] == 1000 + assert result[0]["app_id"] == 10 + assert result[1]["installation_id"] == 2000 + assert result[1]["app_id"] == 20 + + @pytest.mark.django_db + @override_settings(GITHUB_SENTRY_APP_NAME="test_sentry_app") + def test_sentry_org_falls_back_to_sentry_app_when_no_codecov_app(self, mocker): owner = OwnerFactory(service="github") account = Account.objects.create(name="test_account", sentry_org_id=12345) owner.account = account