-
Notifications
You must be signed in to change notification settings - Fork 296
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
Fix demo alert for inbound email #2081
Changes from 8 commits
cae0923
c533632
d2fa9a6
9679ead
54ab40d
f07cea4
a35d199
70c4275
3f33983
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
|
||
from apps.alerts.models import AlertReceiveChannel | ||
from common.api_helpers.utils import create_engine_url | ||
from common.exceptions import UnableToSendDemoAlert | ||
|
||
|
||
@pytest.mark.django_db | ||
|
@@ -145,6 +146,21 @@ def test_send_demo_alert_alertmanager_payload_shape( | |
assert mocked_create_alert.call_args.args[1]["force_route_id"] is None | ||
|
||
|
||
@mock.patch("apps.integrations.tasks.create_alert.apply_async", return_value=None) | ||
@pytest.mark.parametrize( | ||
"integration", [config.slug for config in AlertReceiveChannel._config if not config.is_demo_alert_enabled] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
) | ||
@pytest.mark.django_db | ||
def test_send_demo_alert_not_enabled(mocked_create_alert, make_organization, make_alert_receive_channel, integration): | ||
organization = make_organization() | ||
alert_receive_channel = make_alert_receive_channel(organization, integration=integration) | ||
|
||
with pytest.raises(UnableToSendDemoAlert): | ||
alert_receive_channel.send_demo_alert() | ||
|
||
assert not mocked_create_alert.called | ||
|
||
|
||
@pytest.mark.django_db | ||
def test_notify_maintenance_no_general_channel(make_organization, make_alert_receive_channel): | ||
organization = make_organization(general_log_channel_id=None) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,3 +102,26 @@ def test_default_templates_are_valid(): | |
jinja_template_env.from_string(template) | ||
except TemplateSyntaxError as e: | ||
pytest.fail(e.message) | ||
|
||
|
||
@pytest.mark.parametrize("config", AlertReceiveChannel._config) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice ✌️ |
||
def test_is_demo_alert_enabled(config): | ||
# is_demo_alert_enabled must be defined | ||
try: | ||
assert isinstance(config.is_demo_alert_enabled, bool), "is_demo_alert_enabled must be bool" | ||
except AttributeError: | ||
pytest.fail("is_demo_alert_enabled must be defined") | ||
|
||
# example_payload must be defined | ||
try: | ||
assert config.example_payload is None or isinstance( | ||
config.example_payload, dict | ||
), "example_payload must be dict or None" | ||
except AttributeError: | ||
pytest.fail("example_payload must be defined") | ||
|
||
# example_payload must be provided when is_demo_alert_enabled is True | ||
if config.is_demo_alert_enabled: | ||
assert config.example_payload, "example_payload must be defined and non-empty" | ||
else: | ||
assert config.example_payload is None, "example_payload must be None if is_demo_alert_enabled is False" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -172,20 +172,16 @@ def get_queryset(self, eager=True, ignore_filtering_by_available_teams=False): | |
@action(detail=True, methods=["post"], throttle_classes=[DemoAlertThrottler]) | ||
def send_demo_alert(self, request, pk): | ||
alert_receive_channel = AlertReceiveChannel.objects.get(public_primary_key=pk) | ||
demo_alert_payload = request.data.get("demo_alert_payload", None) | ||
payload = request.data.get("demo_alert_payload", None) | ||
|
||
if not demo_alert_payload: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
# If no payload provided, use the demo payload for backword compatibility | ||
payload = alert_receive_channel.config.example_payload | ||
else: | ||
if type(demo_alert_payload) != dict: | ||
raise BadRequest(detail="Payload for demo alert must be a valid json object") | ||
payload = demo_alert_payload | ||
if payload is not None and not isinstance(payload, dict): | ||
raise BadRequest(detail="Payload for demo alert must be a valid json object") | ||
|
||
try: | ||
alert_receive_channel.send_demo_alert(payload=payload) | ||
except UnableToSendDemoAlert as e: | ||
raise BadRequest(detail=str(e)) | ||
|
||
return Response(status=status.HTTP_200_OK) | ||
|
||
@action(detail=False, methods=["get"]) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,3 +54,5 @@ | |
resolve_condition = None | ||
|
||
acknowledge_condition = None | ||
|
||
example_payload = None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,4 +26,4 @@ | |
|
||
acknowledge_condition = None | ||
|
||
example_payload = {"foo": "bar"} | ||
example_payload = None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,3 +54,5 @@ | |
resolve_condition = None | ||
|
||
acknowledge_condition = None | ||
|
||
example_payload = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to simplify things a bit and reduce the number of indentation levels.
The only logical change is checking for
if not self.is_demo_alert_enabled
before anything else.