feat(slack): optional blokkit payload for Slack notifications - #5414
feat(slack): optional blokkit payload for Slack notifications#5414TheMeier wants to merge 1 commit into
Conversation
|
This is the approach to acheive option 2 from this comment #2217 (comment) using other means. It relies on |
Signed-off-by: Christoph Maser <christoph.maser+github@gmail.com>
📝 WalkthroughWalkthroughSlack receivers can now select templated Block Kit payloads or retain plain attachment payloads. The change adds configuration fields, payload composition, message update handling, documentation, and tests. ChangesSlack Block Kit support
Mattermost template formatting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Alertmanager
participant SlackNotifier
participant BlockKitComposer
participant SlackAPI
Alertmanager->>SlackNotifier: send notification
SlackNotifier->>BlockKitComposer: render configured blocks
BlockKitComposer-->>SlackNotifier: return channel, text, and blocks
SlackNotifier->>SlackAPI: encode and submit selected payload
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/notifiers.go`:
- Around line 348-349: Rename the exported fields BlocKitEnabeld and
BlocKitPayload to BlockKitEnabled and BlockKitPayload in the notifier
configuration, then update every notifier and test reference to use the
corrected names while preserving their existing YAML and JSON tags.
In `@notify/slack/message_blockkit.go`:
- Around line 27-35: Update the rendering flow around DeepCopyWithTemplate so
template substitutions are rendered without reparsing rendered string values as
YAML, preserving Block Kit text fields as strings. Keep the existing block
structure and template behavior intact, and add coverage for rendered text
containing colon-space and scalar-looking values.
In `@notify/slack/slack.go`:
- Around line 86-99: Update the payload construction flow around
composePlainRequest and composeBlockKitPayload to render only the selected
format: call composePlainRequest only in the plain branch, and check tmplTextErr
after rendering either payload. In Block Kit mode, include errors from the
top-level channel and message_text template rendering before returning the
request; preserve the existing payload and channelForError assignments for valid
templates.
In `@template/default.tmpl`:
- Around line 236-238: Remove the added blank lines after the closing template
block in template/default.tmpl, leaving the file unchanged and preserving the
existing mattermost.default.text template content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2fce32da-a53c-4609-aa5b-b92f8735a31b
📒 Files selected for processing (8)
config/notifiers.godocs/configuration.mdnotify/slack/message_blockkit.gonotify/slack/message_blockkit_test.gonotify/slack/message_plain.gonotify/slack/slack.gonotify/slack/slack_test.gotemplate/default.tmpl
| BlocKitEnabeld *bool `yaml:"use_block_kit,omitempty" json:"use_block_kit,omitempty"` | ||
| BlocKitPayload any `yaml:"block_kit_payload,omitempty" json:"block_kit_payload,omitempty"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the exported Block Kit field names.
BlocKitEnabeld and BlocKitPayload expose misspelled Go API names. Rename them to BlockKitEnabled and BlockKitPayload before release. Update all notifier and test references.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/notifiers.go` around lines 348 - 349, Rename the exported fields
BlocKitEnabeld and BlocKitPayload to BlockKitEnabled and BlockKitPayload in the
notifier configuration, then update every notifier and test reference to use the
corrected names while preserving their existing YAML and JSON tags.
| renderedBlocks, err := template.DeepCopyWithTemplate(blocksTmpl, tmplTextFunc) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return map[string]any{ | ||
| "channel": channel, | ||
| "text": text, | ||
| "blocks": renderedBlocks, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Keep rendered Block Kit values as strings.
DeepCopyWithTemplate reparses rendered string values. A valid Slack text value such as summary: disk full becomes a map, which violates the Block Kit schema. The documented quote workaround does not make normal templated text safe.
Render template substitutions without YAML-decoding the rendered result. Add coverage for colon-space and scalar-looking rendered text.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notify/slack/message_blockkit.go` around lines 27 - 35, Update the rendering
flow around DeepCopyWithTemplate so template substitutions are rendered without
reparsing rendered string values as YAML, preserving Block Kit text fields as
strings. Keep the existing block structure and template behavior intact, and add
coverage for rendered text containing colon-space and scalar-looking values.
| useBlockKit := n.conf.BlocKitEnabeld != nil && *n.conf.BlocKitEnabeld | ||
| req := composePlainRequest(n.conf, tmplText, logger) | ||
| payload := any(req) | ||
| channelForError := req.Channel | ||
| var bkPayload map[string]any | ||
|
|
||
| if useBlockKit { | ||
| bkPayload, err = composeBlockKitPayload(n.conf.BlocKitPayload, tmplText(n.conf.Channel), tmplText(n.conf.MessageText), tmplText, &tmplTextErr) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to render block kit payload: %w", err) | ||
| } | ||
| payload = bkPayload | ||
| channelForError = tmplText(n.conf.Channel) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Render only the selected payload format and check template errors.
When useBlockKit is true, Line 87 still renders every legacy attachment field. An invalid unused legacy template can fail a valid Block Kit notification. In plain mode, tmplTextErr is never checked before the request is encoded.
Compose composePlainRequest only in the plain branch. Check tmplTextErr after rendering the selected payload, including top-level Block Kit channel and message_text values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notify/slack/slack.go` around lines 86 - 99, Update the payload construction
flow around composePlainRequest and composeBlockKitPayload to render only the
selected format: call composePlainRequest only in the plain branch, and check
tmplTextErr after rendering either payload. In Block Kit mode, include errors
from the top-level channel and message_text template rendering before returning
the request; preserve the existing payload and channelForError assignments for
valid templates.
| {{ end }} | ||
|
|
||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove this unrelated template change.
This PR adds configurable Slack Block Kit payloads and does not introduce a new default notification template. Lines 237-238 only add blank source lines after mattermost.default.text; remove them to keep template/default.tmpl unchanged.
As per coding guidelines, update template/default.tmpl only when introducing new default notification templates.
Proposed cleanup
{{ end }}
-
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{ end }} | |
| {{ end }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@template/default.tmpl` around lines 236 - 238, Remove the added blank lines
after the closing template block in template/default.tmpl, leaving the file
unchanged and preserving the existing mattermost.default.text template content.
Source: Coding guidelines
1 participant
Pull Request Checklist
Please check all the applicable boxes.
benchstatto compare benchmarksWhich user-facing changes does this PR introduce?