-
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
Conversation
if payload is None: | ||
payload = self.config.example_payload | ||
if self.is_demo_alert_enabled: |
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.
|
||
if not demo_alert_payload: |
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.
AlertReceiveChannel.send_demo_alert
handles payload=None
cases
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.
LGTM
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.
LGTM! You might want to double check this integration file here in the private repo
@@ -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 comment
The reason will be displayed to describe this comment to others. Learn more.
nice ✌️
@@ -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 comment
The reason will be displayed to describe this comment to others. Learn more.
👍
# Conflicts: # CHANGELOG.md
What this PR does
Fix HTTP 500 when sending demo alert for inbound integration email.
is_demo_alert_enabled
andexample_payload
valuesChecklist
pr:no public docs
PR label added if not required)CHANGELOG.md
updated (orpr:no changelog
PR label added if not required)