Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logic to remove checks from team plan customer #463

Merged
merged 6 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 63 additions & 34 deletions services/notification/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
from celery.exceptions import CeleryError, SoftTimeLimitExceeded
from shared.config import get_config
from shared.helpers.yaml import default_if_true
from shared.plan.constants import TEAM_PLAN_REPRESENTATIONS
from shared.yaml import UserYaml

from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME, Owner
from helpers.metrics import metrics
from services.comparison import ComparisonProxy
from services.decoration import Decoration
Expand All @@ -24,6 +25,7 @@
create_or_update_commit_notification_from_notification_result,
)
from services.notification.notifiers import (
StatusType,
get_all_notifier_classes_mapping,
get_pull_request_notifiers,
get_status_notifier_class,
Expand All @@ -36,6 +38,7 @@
ChecksWithFallback,
)
from services.notification.notifiers.codecov_slack_app import CodecovSlackAppNotifier
from services.notification.notifiers.status.base import StatusNotifier
from services.yaml import read_yaml_field
from services.yaml.reader import get_components_from_yaml

Expand All @@ -55,15 +58,28 @@ def __init__(
self.decoration_type = decoration_type
self.gh_installation_name_to_use = gh_installation_name_to_use

def _should_use_checks_notifier(self) -> bool:
def _should_use_status_notifier(self, status_type: StatusType) -> bool:
owner: Owner = self.repository.owner

if owner.plan in TEAM_PLAN_REPRESENTATIONS:
if status_type != StatusType.PATCH.value:
return False

return True

def _should_use_checks_notifier(self, status_type: StatusType) -> bool:
checks_yaml_field = read_yaml_field(self.current_yaml, ("github_checks",))
if checks_yaml_field is False:
return False

owner = self.repository.owner
owner: Owner = self.repository.owner
if owner.service not in ["github", "github_enterprise"]:
return False

if owner.plan in TEAM_PLAN_REPRESENTATIONS:
if status_type != StatusType.PATCH.value:
return False

app_installation_filter = filter(
lambda obj: (
obj.name == self.gh_installation_name_to_use and obj.is_configured()
Expand All @@ -81,6 +97,46 @@ def _should_use_checks_notifier(self) -> bool:
and (self.repository.owner.service in ["github", "github_enterprise"])
)

def _use_status_and_possibly_checks_notifiers(
self,
key: StatusType,
title: str,
status_config: dict,
) -> AbstractBaseNotifier | StatusNotifier:
status_notifier_class = get_status_notifier_class(key, "status")
if self._should_use_checks_notifier(status_type=key):
checks_notifier = get_status_notifier_class(key, "checks")
return ChecksWithFallback(
checks_notifier=checks_notifier(
repository=self.repository,
title=title,
notifier_yaml_settings=status_config,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
gh_installation_name_to_use=self.gh_installation_name_to_use,
),
status_notifier=status_notifier_class(
repository=self.repository,
title=title,
notifier_yaml_settings=status_config,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
gh_installation_name_to_use=self.gh_installation_name_to_use,
),
)
else:
return status_notifier_class(
repository=self.repository,
title=title,
notifier_yaml_settings=status_config,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
gh_installation_name_to_use=self.gh_installation_name_to_use,
)

def get_notifiers_instances(self) -> Iterator[AbstractBaseNotifier]:
mapping = get_all_notifier_classes_mapping()
yaml_field = read_yaml_field(self.current_yaml, ("coverage", "notify"))
Expand All @@ -101,38 +157,11 @@ def get_notifiers_instances(self) -> Iterator[AbstractBaseNotifier]:

current_flags = [rf.flag_name for rf in self.repository.flags if not rf.deleted]
for key, title, status_config in self.get_statuses(current_flags):
status_notifier_class = get_status_notifier_class(key, "status")
if self._should_use_checks_notifier():
checks_notifier = get_status_notifier_class(key, "checks")
yield ChecksWithFallback(
checks_notifier=checks_notifier(
repository=self.repository,
title=title,
notifier_yaml_settings=status_config,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
gh_installation_name_to_use=self.gh_installation_name_to_use,
),
status_notifier=status_notifier_class(
repository=self.repository,
title=title,
notifier_yaml_settings=status_config,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
gh_installation_name_to_use=self.gh_installation_name_to_use,
),
)
else:
yield status_notifier_class(
repository=self.repository,
if self._should_use_status_notifier(status_type=key):
yield self._use_status_and_possibly_checks_notifiers(
key=key,
title=title,
notifier_yaml_settings=status_config,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
gh_installation_name_to_use=self.gh_installation_name_to_use,
status_config=status_config,
)

# yield notifier if slack_app field is True, nonexistent, or a non-empty dict
Expand Down
19 changes: 13 additions & 6 deletions services/notification/notifiers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from enum import Enum
from typing import Dict, List, Type

from services.notification.notifiers.base import AbstractBaseNotifier
Expand Down Expand Up @@ -29,20 +30,26 @@ def get_all_notifier_classes_mapping() -> Dict[str, Type[AbstractBaseNotifier]]:
}


class StatusType(Enum):
PATCH = "patch"
PROJECT = "project"
CHANGES = "changes"


def get_status_notifier_class(
status_type: str, class_type: str = "status"
) -> Type[AbstractBaseNotifier]:
if status_type == "patch" and class_type == "checks":
if status_type == StatusType.PATCH.value and class_type == "checks":
return PatchChecksNotifier
if status_type == "project" and class_type == "checks":
if status_type == StatusType.PROJECT.value and class_type == "checks":
return ProjectChecksNotifier
if status_type == "changes" and class_type == "checks":
if status_type == StatusType.CHANGES.value and class_type == "checks":
return ChangesChecksNotifier
if status_type == "patch" and class_type == "status":
if status_type == StatusType.PATCH.value and class_type == "status":
return PatchStatusNotifier
if status_type == "project" and class_type == "status":
if status_type == StatusType.PROJECT.value and class_type == "status":
return ProjectStatusNotifier
if status_type == "changes" and class_type == "status":
if status_type == StatusType.CHANGES.value and class_type == "status":
return ChangesStatusNotifier


Expand Down
111 changes: 106 additions & 5 deletions services/notification/tests/unit/test_notification_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import mock
import pytest
from celery.exceptions import SoftTimeLimitExceeded
from shared.plan.constants import PlanName
from shared.reports.resources import Report, ReportFile, ReportLine
from shared.yaml import UserYaml

Expand All @@ -17,6 +18,7 @@
from services.comparison import ComparisonProxy
from services.comparison.types import Comparison, EnrichedPull, FullCommit
from services.notification import NotificationService
from services.notification.notifiers import StatusType
from services.notification.notifiers.base import NotificationResult
from services.notification.notifiers.checks import ProjectChecksNotifier
from services.notification.notifiers.checks.checks_with_fallback import (
Expand Down Expand Up @@ -58,7 +60,10 @@ def test_should_use_checks_notifier_yaml_field_false(self, dbsession):
repository = RepositoryFactory.create()
current_yaml = {"github_checks": False}
service = NotificationService(repository, current_yaml)
assert service._should_use_checks_notifier() == False
assert (
service._should_use_checks_notifier(status_type=StatusType.PROJECT.value)
== False
)

@pytest.mark.parametrize(
"repo_data,outcome",
Expand Down Expand Up @@ -104,7 +109,10 @@ def test_should_use_checks_notifier_deprecated_flow(
current_yaml = {"github_checks": True}
assert repository.owner.github_app_installations == []
service = NotificationService(repository, current_yaml)
assert service._should_use_checks_notifier() == outcome
assert (
service._should_use_checks_notifier(status_type=StatusType.PROJECT.value)
== outcome
)

def test_should_use_checks_notifier_ghapp_all_repos_covered(self, dbsession):
repository = RepositoryFactory.create(owner__service="github")
Expand All @@ -119,7 +127,94 @@ def test_should_use_checks_notifier_ghapp_all_repos_covered(self, dbsession):
current_yaml = {"github_checks": True}
assert repository.owner.github_app_installations == [ghapp_installation]
service = NotificationService(repository, current_yaml)
assert service._should_use_checks_notifier() == True
assert (
service._should_use_checks_notifier(status_type=StatusType.PROJECT.value)
== True
)

def test_use_checks_notifier_for_team_plan(self, dbsession):
repository = RepositoryFactory.create(
owner__service="github", owner__plan=PlanName.TEAM_MONTHLY.value
)
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
installation_id=456789,
owner=repository.owner,
repository_service_ids=None,
)
dbsession.add(ghapp_installation)
dbsession.flush()
current_yaml = {"github_checks": True}
assert repository.owner.github_app_installations == [ghapp_installation]
service = NotificationService(repository, current_yaml)
assert (
service._should_use_checks_notifier(status_type=StatusType.PROJECT.value)
== False
)
assert (
service._should_use_checks_notifier(status_type=StatusType.CHANGES.value)
== False
)
assert (
service._should_use_checks_notifier(status_type=StatusType.PATCH.value)
== True
)

def test_use_status_notifier_for_team_plan(self, dbsession):
repository = RepositoryFactory.create(
owner__service="github", owner__plan=PlanName.TEAM_MONTHLY.value
)
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
installation_id=456789,
owner=repository.owner,
repository_service_ids=None,
)
dbsession.add(ghapp_installation)
dbsession.flush()
current_yaml = {"github_checks": True}
assert repository.owner.github_app_installations == [ghapp_installation]
service = NotificationService(repository, current_yaml)
assert (
service._should_use_status_notifier(status_type=StatusType.PROJECT.value)
== False
)
assert (
service._should_use_checks_notifier(status_type=StatusType.CHANGES.value)
== False
)
assert (
service._should_use_checks_notifier(status_type=StatusType.PATCH.value)
== True
)

def test_use_status_notifier_for_non_team_plan(self, dbsession):
repository = RepositoryFactory.create(
owner__service="github", owner__plan=PlanName.CODECOV_PRO_MONTHLY.value
)
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
installation_id=456789,
owner=repository.owner,
repository_service_ids=None,
)
dbsession.add(ghapp_installation)
dbsession.flush()
current_yaml = {"github_checks": True}
assert repository.owner.github_app_installations == [ghapp_installation]
service = NotificationService(repository, current_yaml)
assert (
service._should_use_status_notifier(status_type=StatusType.PROJECT.value)
== True
)
assert (
service._should_use_checks_notifier(status_type=StatusType.CHANGES.value)
== True
)
assert (
service._should_use_checks_notifier(status_type=StatusType.PATCH.value)
== True
)

@pytest.mark.parametrize(
"gh_installation_name",
Expand All @@ -145,9 +240,15 @@ def test_should_use_checks_notifier_ghapp_some_repos_covered(
service = NotificationService(
repository, current_yaml, gh_installation_name_to_use=gh_installation_name
)
assert service._should_use_checks_notifier() == True
assert (
service._should_use_checks_notifier(status_type=StatusType.PROJECT.value)
== True
)
service = NotificationService(other_repo_same_owner, current_yaml)
assert service._should_use_checks_notifier() == False
assert (
service._should_use_checks_notifier(status_type=StatusType.PROJECT.value)
== False
)

def test_get_notifiers_instances_only_third_party(
self, dbsession, mock_configuration
Expand Down
Loading