feat(issue-workflow): Add notification platform support for deploy notifs#120398
Conversation
1c85775 to
623fdc4
Compare
ecbadc2 to
b489875
Compare
Sentry Snapshot Testing
|
| DeployReleaseCommit( | ||
| author_name=author_name, | ||
| date=commit.date_added.isoformat() if commit.date_added else "", | ||
| sha=commit.key[:7], |
There was a problem hiding this comment.
Incorrect commit SHA shortening
Low Severity
Commit SHAs are truncated with commit.key[:7] instead of commit.short_id. short_id only shortens 40-character hashes and otherwise returns the full key, which matches legacy deploy emails. Non-SHA keys longer than 7 characters will be truncated incorrectly in the new notifications.
Reviewed by Cursor Bugbot for commit 623fdc4. Configure here.
b489875 to
d91d1dd
Compare
623fdc4 to
4963aef
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4963aef. Configure here.
| author_name=author_name, | ||
| date=commit.date_added.isoformat() if commit.date_added else "", | ||
| sha=commit.key[:7], | ||
| message=commit.message or "", |
There was a problem hiding this comment.
Full commit message in notifications
Medium Severity
Commit bodies are stored as the full commit.message. Legacy deploy emails use commit.title (first line only). Multi-line commit messages will inflate Slack/email deploy notifications with long bodies that the previous experience intentionally omitted.
Reviewed by Cursor Bugbot for commit 4963aef. Configure here.
| if not deploy or not release: | ||
| return |
There was a problem hiding this comment.
Bug: If get_deploy() or get_release() returns None, deploy notifications are silently dropped without falling back to the legacy system.
Severity: MEDIUM
Suggested Fix
Modify _send_deploy_activity_notification to handle cases where get_deploy() or get_release() return None. The function should explicitly call the legacy notification function, _send_legacy_activity_notification, as a fallback to ensure notifications are not silently dropped.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/tasks/activity.py#L43-L44
Potential issue: For organizations with access to the new notification platform, if
`get_deploy()` or `get_release()` returns `None` (e.g., due to a `DoesNotExist`
exception), the `_send_deploy_activity_notification` function will return prematurely.
This new logic prevents a fallback to the legacy notification system, which was the
behavior prior to this change for `DEPLOY` activity. Consequently, deploy notifications
in these scenarios will be silently dropped instead of being sent through the old
system.
Did we get this right? 👍 / 👎 to inform future reviews.
| ParagraphSection, | ||
| PlainTextBlock, | ||
| ) | ||
| from sentry.notifications.utils import get_environment_for_deploy, get_group_counts_by_project |
There was a problem hiding this comment.
Deploy notification crashes if environment is deleted
get_environment_for_deploy does Environment.objects.get(id=deploy.environment_id) without handling DoesNotExist. If the environment is deleted after the deploy is created, the send_activity_notifications task crashes. Fix: wrap the .get() in try/except Environment.DoesNotExist inside get_environment_for_deploy so it falls back to 'Default Environment'.
Evidence
build_deploy_release_datacallsget_environment_for_deploy(deploy)without guarding forEnvironment.DoesNotExist.get_environment_for_deployinsentry/notifications/utils/__init__.pydoesEnvironment.objects.get(id=deploy.environment_id)directly with no try/except.- This is called from
_send_deploy_activity_notificationinsidesend_activity_notifications, which is a Celery task; an unhandledDoesNotExistwill crash and retry the task. Deploy.environment_idis a raw integer field with no FK constraint, so the environment record can be deleted independently.
Also found at 1 additional location
src/sentry/notifications/platform/templates/deploy.py:272
Identified by Warden sentry-backend-bugs · R3E-LPN
ceorourke
left a comment
There was a problem hiding this comment.
broken record but does this need tests too?
|
|
||
| def build_deploy_release_data(deploy: Deploy, release: Release) -> DeployReleaseDataResult: | ||
| organization = release.organization | ||
| projects = set(release.projects.all()) |
There was a problem hiding this comment.
do we need to limit this to some number?
| author_name=author_name, | ||
| date=commit.date_added.isoformat() if commit.date_added else "", | ||
| sha=commit.key[:7], | ||
| message=commit.message or "", |


Adds a new route specifically for deploy notifications. They are not like any other activity notification because the activity does not matter for them, they need a deploy/release (not group).
The template already exists, this just adds a helper to build the data, and a strategy to notify committers + people who want to know about releases.
Ideally this wouldn't be tied to activity at all, but that was the trigger system for these notificiations before and we aren't changing that here
Stack created with GitHub Stacks CLI • Give Feedback 💬