feat(signals): add team-level default Slack channel for inbox reports#60722
Merged
Conversation
Contributor
|
Size Change: +4.32 kB (+0.01%) Total Size: 80.8 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
joshsny
force-pushed
the
posthog-code/signals-team-slack-channel
branch
from
7399da1 to
101ee31
Compare
Contributor
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 0 Needs Review | 0 Blocked ✅ SafeBrief or no lock, backwards compatible 📚 How to Deploy These Changes SafelyAddField: This operation acquires a brief lock but doesn't rewrite the table. Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up. Last updated: 2026-06-01 11:13 UTC (5f2d6b4) |
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
products/signals/backend/serializers.py:151-159
An empty string `""` sent for `slack_notification_channel` will pass validation and be stored as `""` rather than `None`, but the help text documents `null` as the canonical "not set" state. Downstream consumers (e.g. `_channel_id_from_target`) would receive an empty string and likely produce an invalid Slack API call. Adding `"allow_blank": False` ensures clients must use an explicit `null` to clear the field, matching the documented semantics.
```suggestion
extra_kwargs = {
"slack_notification_channel": {
"allow_blank": False,
"help_text": (
"Default Slack channel for this team's signal inbox notifications, in the same "
"`channel_id|#channel-name` shape PostHog uses elsewhere (only the channel id is required). "
"Null means no team-level default; per-user channels still apply."
),
},
}
```
Reviews (1): Last reviewed commit: "feat(signals): add team-level default Sl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a team-level default Slack channel field to SignalTeamConfig and exposes it through the existing team-config API, so teams can configure a single inbox channel for signal notifications. The notification dispatcher itself is not yet wired up to use the field (still TBD per the PR description).
Changes:
- Add nullable
slack_notification_channelCharField toSignalTeamConfigmodel and migration 0021. - Surface the field in
SignalTeamConfigSerializerwith help_text documenting thechannel_id|#channel-nameshape. - Add API tests covering GET, 404, set/clear via POST, and partial-update preservation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| products/signals/backend/models.py | Adds the new CharField on SignalTeamConfig. |
| products/signals/backend/migrations/0021_signalteamconfig_slack_notification_channel.py | New AddField migration. |
| products/signals/backend/migrations/max_migration.txt | Bumps max migration pointer to 0021. |
| products/signals/backend/serializers.py | Exposes field with help_text in the team config serializer. |
| products/signals/backend/test/test_signal_team_config_api.py | New API test module for the team config endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
andrewm4894
approved these changes
May 29, 2026
2 tasks
joshsny
force-pushed
the
posthog-code/signals-team-slack-channel
branch
from
101ee31 to
3e8e7c1
Compare
sortafreel
approved these changes
May 29, 2026
joshsny
force-pushed
the
posthog-code/signals-team-slack-channel
branch
from
3e8e7c1 to
4870f77
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
joshsny
force-pushed
the
posthog-code/signals-team-slack-channel
branch
from
4870f77 to
bb169ea
Compare
Add SignalTeamConfig.slack_notification_channel (nullable) with its migration, and expose it on the existing team config serializer/endpoint, so a team-level default Slack channel can be stored for signal inbox reports. The frontend for configuring this lives in PostHog Code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
joshsny
force-pushed
the
posthog-code/signals-team-slack-channel
branch
from
bb169ea to
5f2d6b4
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adds a team level
slack_notification_channel, we will send all signals notifications here and tag the relevant slack user if there is an issue.We will keep the user level setting and forward notifications for that user to the channel (TBD on if we forward or post there).
Goal is to have a single place where a team can see all their signal reports, similar to the experience of mendral.
Note: this doesn't FK to the slack integration so that it's robust to reconnections
This is part 2 of 2 in a stack made with GitButler: