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

Enable toggling for codecov slack app notifications #88

Merged
merged 1 commit into from
Sep 13, 2023
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
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
git+ssh://[email protected]/codecov/shared.git@f0174635ccaebc3463a5641b9ad640a46b5fd472#egg=shared
git+ssh://[email protected]/codecov/shared.git@bef9260a4b6218b5ce4b5b9152a71d7e6d63a954#egg=shared
git+ssh://[email protected]/codecov/[email protected]#egg=codecovopentelem
boto3
celery
Expand Down
4 changes: 2 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# This file is autogenerated by pip-compile with Python 3.10
# This file is autogenerated by pip-compile with Python 3.8
# by the following command:
#
# pip-compile requirements.in
Expand Down Expand Up @@ -251,7 +251,7 @@ s3transfer==0.3.4
# via boto3
sentry-sdk==1.19.1
# via -r requirements.in
shared @ git+ssh://[email protected]/codecov/shared.git@f0174635ccaebc3463a5641b9ad640a46b5fd472
shared @ git+ssh://[email protected]/codecov/shared.git@bef9260a4b6218b5ce4b5b9152a71d7e6d63a954
# via -r requirements.in
six==1.15.0
# via
Expand Down
19 changes: 11 additions & 8 deletions services/notification/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,17 @@ def get_notifiers_instances(self) -> Iterator[AbstractBaseNotifier]:
decoration_type=self.decoration_type,
)

yield CodecovSlackAppNotifier(
repository=self.repository,
title="codecov-slack-app",
notifier_yaml_settings={},
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
)
# yield notifier if slack_app field is True, nonexistent, or a non-empty dict
slack_app_yaml_field = read_yaml_field(self.current_yaml, ("slack_app",), True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this makes the slack app enabled by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

if slack_app_yaml_field:
yield CodecovSlackAppNotifier(
repository=self.repository,
title="codecov-slack-app",
notifier_yaml_settings=slack_app_yaml_field,
notifier_site_settings={},
current_yaml=self.current_yaml,
decoration_type=self.decoration_type,
)

comment_yaml_field = read_yaml_field(self.current_yaml, ("comment",))
if comment_yaml_field:
Expand Down
11 changes: 10 additions & 1 deletion services/notification/notifiers/codecov_slack_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,16 @@ def notification_type(self) -> Notification:
return Notification.codecov_slack_app

def is_enabled(self) -> bool:
return True
# if yaml settings are a dict, then check the enabled key and return that
# the enabled field should always exist if the yaml settings are a dict because otherwise it would fail the validation

# else if the yaml settings is a boolean then just return that

# in any case, self.notifier_yaml_settings should either be a bool or a dict always and should never be None
if isinstance(self.notifier_yaml_settings, dict):
return self.notifier_yaml_settings.get("enabled", False)
elif isinstance(self.notifier_yaml_settings, bool):
return self.notifier_yaml_settings

def store_results(self, comparison: Comparison, result: NotificationResult):
pass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,30 @@ def test_is_enabled(self, dbsession, mock_configuration, sample_comparison):
notifier = CodecovSlackAppNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_yaml_settings={"enabled": True},
notifier_site_settings=True,
current_yaml={},
current_yaml={"slack_app": {"enabled": True}},
)
assert notifier.is_enabled() == True

def test_is_enable_false(self, dbsession, mock_configuration, sample_comparison):
notifier = CodecovSlackAppNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
notifier_yaml_settings={"enabled": False},
notifier_site_settings=True,
current_yaml={"slack_app": {"enabled": False}},
)

assert notifier.is_enabled() is False

def test_notification_type(self, dbsession, mock_configuration, sample_comparison):
notifier = CodecovSlackAppNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_yaml_settings={"enabled": True},
notifier_site_settings=True,
current_yaml={},
current_yaml={"slack_app": {"enabled": True}},
)
assert notifier.notification_type == Notification.codecov_slack_app

Expand All @@ -36,9 +47,9 @@ async def test_notify(
notifier = CodecovSlackAppNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_yaml_settings={"enabled": True},
notifier_site_settings=True,
current_yaml={},
current_yaml={"slack_app": {"enabled": True}},
)
result = await notifier.notify(sample_comparison)
assert result.notification_successful == True
Expand All @@ -54,9 +65,9 @@ async def test_notify_failure(
notifier = CodecovSlackAppNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_yaml_settings={"enabled": True},
notifier_site_settings=True,
current_yaml={},
current_yaml={"slack_app": {"enabled": True}},
)
result = await notifier.notify(sample_comparison)
assert result.notification_successful == False
Expand All @@ -74,9 +85,9 @@ async def test_notify_request_being_called(
notifier = CodecovSlackAppNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_yaml_settings={"enabled": True},
notifier_site_settings=True,
current_yaml={},
current_yaml={"slack_app": {"enabled": True}},
)
result = await notifier.notify(sample_comparison)
assert result.notification_successful == True
Expand Down
1 change: 1 addition & 0 deletions services/yaml/tests/test_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def test_get_final_yaml_no_thing_set_at_all(self, mocker, mock_configuration):
"show_carryforward_flags": False,
},
"github_checks": {"annotations": True},
"slack_app": True,
}
result = UserYaml.get_final_yaml(owner_yaml={}, repo_yaml={}, commit_yaml={})
assert expected_result == result.to_dict()
Expand Down
Loading