From 53ecfc002547b1feafd0da1499c36bda3e328ffa Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Wed, 13 Sep 2023 16:43:48 -0400 Subject: [PATCH] Enable toggling for codecov slack app notifications (#88) This commit enables toggling for the slack app notifications via the codecov config slack_app option. Signed-off-by: joseph-sentry --- requirements.in | 2 +- requirements.txt | 4 +-- services/notification/__init__.py | 19 +++++++----- .../notifiers/codecov_slack_app.py | 11 ++++++- .../tests/unit/test_codecov_slack_app.py | 31 +++++++++++++------ services/yaml/tests/test_yaml.py | 1 + 6 files changed, 46 insertions(+), 22 deletions(-) diff --git a/requirements.in b/requirements.in index ccbee6ceb..25cddc4d7 100644 --- a/requirements.in +++ b/requirements.in @@ -1,4 +1,4 @@ -git+ssh://git@github.com/codecov/shared.git@f0174635ccaebc3463a5641b9ad640a46b5fd472#egg=shared +git+ssh://git@github.com/codecov/shared.git@bef9260a4b6218b5ce4b5b9152a71d7e6d63a954#egg=shared git+ssh://git@github.com/codecov/opentelem-python.git@v0.0.4a1#egg=codecovopentelem boto3 celery diff --git a/requirements.txt b/requirements.txt index cac8f38ed..3e4d57f1a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 @@ -251,7 +251,7 @@ s3transfer==0.3.4 # via boto3 sentry-sdk==1.19.1 # via -r requirements.in -shared @ git+ssh://git@github.com/codecov/shared.git@f0174635ccaebc3463a5641b9ad640a46b5fd472 +shared @ git+ssh://git@github.com/codecov/shared.git@bef9260a4b6218b5ce4b5b9152a71d7e6d63a954 # via -r requirements.in six==1.15.0 # via diff --git a/services/notification/__init__.py b/services/notification/__init__.py index 190dfbd89..35c5266c6 100644 --- a/services/notification/__init__.py +++ b/services/notification/__init__.py @@ -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) + 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: diff --git a/services/notification/notifiers/codecov_slack_app.py b/services/notification/notifiers/codecov_slack_app.py index c711d3124..bf592c295 100644 --- a/services/notification/notifiers/codecov_slack_app.py +++ b/services/notification/notifiers/codecov_slack_app.py @@ -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 diff --git a/services/notification/notifiers/tests/unit/test_codecov_slack_app.py b/services/notification/notifiers/tests/unit/test_codecov_slack_app.py index 27d5e8e3e..d63ba2a81 100644 --- a/services/notification/notifiers/tests/unit/test_codecov_slack_app.py +++ b/services/notification/notifiers/tests/unit/test_codecov_slack_app.py @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/services/yaml/tests/test_yaml.py b/services/yaml/tests/test_yaml.py index 1a39f56ac..f55814320 100644 --- a/services/yaml/tests/test_yaml.py +++ b/services/yaml/tests/test_yaml.py @@ -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()