-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ref(notifications): org request notification #29362
Merged
Merged
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
bcaaa5a
starting work with org request notification
251f9eb
refactor and check in missing files
18ba783
temp checkin
51ece34
more progress
bd0fde8
fixing imports
1f70612
fix broken test
16bdf0d
fix tests and work on hooking up with downstream notification work
8c1e4e5
get downstream changdes to work
927d936
fix type errors
f667be7
fix two PR suggestions
d9034e4
Merge branch 'master' of github.com:getsentry/sentry into ref/org-req…
4d98721
cleanup
efa3e86
minor changes
a65332c
implement PR suggestions
b29a07c
fix type errors
ae0bc06
remove string cast
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 30 additions & 0 deletions
30
src/sentry/integrations/slack/message_builder/organization_requests.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
from typing import TYPE_CHECKING, Any, Mapping, Union | ||
|
||
from sentry.integrations.slack.message_builder import SlackBody | ||
from sentry.integrations.slack.message_builder.notifications import SlackNotificationsMessageBuilder | ||
from sentry.notifications.notifications.organization_request import OrganizationRequestNotification | ||
|
||
if TYPE_CHECKING: | ||
from sentry.models import Team, User | ||
|
||
|
||
class SlackOrganizationRequestMessageBuilder(SlackNotificationsMessageBuilder): | ||
def __init__( | ||
self, | ||
notification: OrganizationRequestNotification, | ||
context: Mapping[str, Any], | ||
recipient: Union["Team", "User"], | ||
) -> None: | ||
super().__init__(notification, context, recipient) | ||
# TODO: use generics here to do this | ||
self.notification: OrganizationRequestNotification = notification | ||
|
||
def build(self) -> SlackBody: | ||
# may need to pass more args to _build and pass recipient to certain helper functions | ||
return self._build( | ||
title=self.notification.build_attachment_title(), | ||
text=self.notification.get_message_description(), | ||
footer=self.notification.build_notification_footer(self.recipient), | ||
actions=self.notification.get_actions(), | ||
color="info", | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,84 +5,37 @@ | |
|
||
from sentry import options | ||
from sentry.models import Project, ProjectOption, Team, User | ||
from sentry.notifications.notifications.activity.base import ActivityNotification | ||
from sentry.notifications.notifications.base import BaseNotification | ||
from sentry.notifications.notifications.rules import AlertRuleNotification | ||
from sentry.notifications.notifications.base import BaseNotification, ProjectNotification | ||
from sentry.notifications.notify import register_notification_provider | ||
from sentry.types.integrations import ExternalProviders | ||
from sentry.utils import json | ||
from sentry.utils.email import MessageBuilder, group_id_to_email | ||
from sentry.utils.email import MessageBuilder | ||
from sentry.utils.linksign import generate_signed_link | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def get_headers(notification: BaseNotification) -> Mapping[str, Any]: | ||
headers = { | ||
"X-Sentry-Project": notification.project.slug, | ||
"X-SMTPAPI": json.dumps({"category": notification.get_category()}), | ||
} | ||
|
||
group = getattr(notification, "group", None) | ||
if group: | ||
headers.update( | ||
{ | ||
"X-Sentry-Logger": group.logger, | ||
"X-Sentry-Logger-Level": group.get_level_display(), | ||
"X-Sentry-Reply-To": group_id_to_email(group.id), | ||
} | ||
) | ||
|
||
return headers | ||
|
||
|
||
def build_subject_prefix(project: "Project", mail_option_key: Optional[str] = None) -> str: | ||
key = mail_option_key or "mail:subject_prefix" | ||
def build_subject_prefix(project: "Project") -> str: | ||
key = "mail:subject_prefix" | ||
return force_text( | ||
ProjectOption.objects.get_value(project, key) or options.get("mail.subject-prefix") | ||
) | ||
|
||
|
||
def get_subject_with_prefix( | ||
notification: BaseNotification, | ||
context: Optional[Mapping[str, Any]] = None, | ||
mail_option_key: Optional[str] = None, | ||
) -> bytes: | ||
|
||
prefix = build_subject_prefix(notification.project, mail_option_key) | ||
return f"{prefix}{notification.get_subject(context)}".encode() | ||
|
||
|
||
def get_unsubscribe_link( | ||
user_id: int, resource_id: int, key: str = "issue", referrer: Optional[str] = None | ||
) -> str: | ||
return generate_signed_link( | ||
user_id, | ||
f"sentry-account-email-unsubscribe-{key}", | ||
referrer, | ||
kwargs={f"{key}_id": resource_id}, | ||
return str( | ||
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. This got lost in a previous comment but you should avoid casting to satisfy mypy. signed_link: str = generate_signed_link(
user_id,
f"sentry-account-email-unsubscribe-{key}",
referrer,
kwargs={f"{key}_id": resource_id},
)
return signed_link 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. @mgaeta good catch! Let me add that in |
||
generate_signed_link( | ||
user_id, | ||
f"sentry-account-email-unsubscribe-{key}", | ||
referrer, | ||
kwargs={f"{key}_id": resource_id}, | ||
) | ||
) | ||
|
||
|
||
def log_message(notification: BaseNotification, recipient: Union["Team", "User"]) -> None: | ||
extra = { | ||
"project_id": notification.project.id, | ||
"actor_id": recipient.actor_id, | ||
} | ||
group = getattr(notification, "group", None) | ||
if group: | ||
extra.update({"group": group.id}) | ||
|
||
if isinstance(notification, AlertRuleNotification): | ||
extra.update( | ||
{ | ||
"target_type": notification.target_type, | ||
"target_identifier": notification.target_identifier, | ||
} | ||
) | ||
elif isinstance(notification, ActivityNotification): | ||
extra.update({"activity": notification.activity}) | ||
|
||
extra = notification.get_log_params(recipient) | ||
logger.info("mail.adapter.notify.mail_user", extra=extra) | ||
|
||
|
||
|
@@ -103,8 +56,9 @@ def get_context( | |
} | ||
# TODO(mgaeta): The unsubscribe system relies on `user_id` so it doesn't | ||
# work with Teams. We should add the `actor_id` to the signed link. | ||
if isinstance(recipient, User) and notification.get_unsubscribe_key(): | ||
key, resource_id, referrer = notification.get_unsubscribe_key() | ||
unsubscribe_key = notification.get_unsubscribe_key() | ||
if isinstance(recipient, User) and unsubscribe_key: | ||
key, resource_id, referrer = unsubscribe_key | ||
context.update( | ||
{"unsubscribe_link": get_unsubscribe_link(recipient.id, resource_id, key, referrer)} | ||
) | ||
|
@@ -119,25 +73,36 @@ def send_notification_as_email( | |
shared_context: Mapping[str, Any], | ||
extra_context_by_user_id: Optional[Mapping[int, Mapping[str, Any]]], | ||
) -> None: | ||
headers = get_headers(notification) | ||
|
||
for recipient in recipients: | ||
if isinstance(recipient, Team): | ||
# TODO(mgaeta): MessageBuilder only works with Users so filter out Teams for now. | ||
continue | ||
extra_context = (extra_context_by_user_id or {}).get(recipient.id, {}) | ||
log_message(notification, recipient) | ||
context = get_context(notification, recipient, shared_context, extra_context) | ||
subject = get_subject_with_prefix(notification, context=context) | ||
msg = MessageBuilder( | ||
subject=subject, | ||
context=context, | ||
template=notification.get_template(), | ||
html_template=notification.get_html_template(), | ||
headers=headers, | ||
reference=notification.get_reference(), | ||
reply_reference=notification.get_reply_reference(), | ||
type=notification.get_type(), | ||
**get_builder_args(notification, recipient, shared_context, extra_context_by_user_id) | ||
) | ||
msg.add_users([recipient.id], project=notification.project) | ||
# TODO: find better way of handling this | ||
if isinstance(notification, ProjectNotification): | ||
msg.add_users([recipient.id], project=notification.project) | ||
msg.send_async() | ||
|
||
|
||
def get_builder_args( | ||
notification: BaseNotification, | ||
recipient: "User", | ||
shared_context: Optional[Mapping[str, Any]] = None, | ||
extra_context_by_user_id: Optional[Mapping[int, Mapping[str, Any]]] = None, | ||
) -> Mapping[str, Any]: | ||
# TODO: move context logic to single notification class method | ||
extra_context = (extra_context_by_user_id or {}).get(recipient.id, {}) | ||
context = get_context(notification, recipient, shared_context or {}, extra_context) | ||
return { | ||
"subject": notification.get_subject_with_prefix(context=context), | ||
"context": context, | ||
"template": notification.get_template(), | ||
"html_template": notification.get_html_template(), | ||
"headers": notification.get_headers(), | ||
"reference": notification.get_reference(), | ||
"reply_reference": notification.get_reply_reference(), | ||
"type": notification.get_type(), | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Maybe there is a better way to do this but I'm not sure