feat(notifications): improve notification template handling making it…#273
Open
julius-malcovsky wants to merge 9 commits intomainfrom
Open
feat(notifications): improve notification template handling making it…#273julius-malcovsky wants to merge 9 commits intomainfrom
julius-malcovsky wants to merge 9 commits intomainfrom
Conversation
… generic to support api and events
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes approval notifications more generic so templates can work across both API subscriptions and event subscriptions by introducing normalized resource placeholders and collapsing target kinds into a shared “subscription” kind for purpose selection.
Changes:
- Add
resource_name/resource_typeproperties derived from requesterbasePathoreventType. - Normalize notification purpose target kind so
ApiSubscriptionandEventSubscriptionboth map tosubscription. - Extend requester extraction tests to cover the new resource placeholders (including an event subscription case).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
approval/internal/handler/util/notification.go |
Adds generic resource placeholders, introduces target-kind normalization for purposes, updates default properties initialization. |
approval/internal/handler/util/notification_test.go |
Expands extractRequester tests to assert resource_name / resource_type for API and event subscription inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
lukas016
reviewed
Mar 17, 2026
julius-malcovsky
requested review from
BjoernKarma,
iagarba,
ron96g and
stefan-ctrl
ron96g
reviewed
Mar 18, 2026
…stead of targetKind Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>
Member
|
approval/README.md (lines 196-231) needs updating to reflect the changes in this PR:
|
Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>
ron96g
approved these changes
Mar 20, 2026
Member
ron96g
left a comment
ron96g
left a comment
There was a problem hiding this comment.
LGTM. Thanks for updating the readme
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.
… generic to support api and events