feat: be able to group slack notifications by feature flag tag #1472
Conversation
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
cre8ivejp
force-pushed
the
feat-slack-tags
branch
5 times, most recently
from
8d2dff7 to
890be22
Compare
cre8ivejp
changed the title
cre8ivejp
force-pushed
the
feat-slack-tags
branch
5 times, most recently
from
64e27f8 to
b243d57
Compare
cre8ivejp
commented
Jan 28, 2025
Comment on lines
+225
to
+226
| // When a flag changes it must be checked before sending notifications | ||
| func (s *sender) checkForFeatureDomainEvent( |
There was a problem hiding this comment.
This function handles the domain events to ensure that they are from the FEATURE entity.
Also, it checks if there are tags in the subscription and if the tags configured in the flag match the tags configured in the subscription.
| @@ -53,4 +53,5 @@ message Subscription { | |||
| string name = 7; | |||
| string environment_id = 8; | |||
| string environment_name = 9; | |||
| repeated string feature_flag_tags = 10; | |||
There was a problem hiding this comment.
This field will hold the tags that will be used to send the notifications.
cre8ivejp
requested review from
Ubisoft-potato and
kentakozuka
and removed request for
kentakozuka
cre8ivejp
commented
Jan 28, 2025
Comment on lines
+129
to
+133
| // Check if the subcription tag is configured in the feature flag | ||
| // If not, we skip the notification | ||
| if !send { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Here is where we decide whether to send the notification.
cre8ivejp
commented
Jan 28, 2025
Comment on lines
+98
to
+99
| // We should only save the tags if the Feature source type is in the list | ||
| if req.FeatureFlagTags != nil { |
There was a problem hiding this comment.
Because the feature_flags_tags belongs to the Feature notification policy, we must validate it.
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
…mmand Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
cre8ivejp
force-pushed
the
feat-slack-tags
branch
from
b243d57 to
58e7a6f
Compare
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.
Fix of #1230
Thins done
feature_flag_tagscolumn to subscription tablefeature_flag_tagsfield in the subscription APIsThis pull request introduces the concept of
featureFlagTagsto the subscription feature in the notification service. The changes include updates to the API definitions, database schema, validation logic, and various parts of the notification service to support the newfeatureFlagTagsfield.API and Schema Updates:
featureFlagTagsto multiple definitions inapi-description/web-api.swagger.yaml. [1] [2] [3] [4]feature_flag_tagscolumn and populated it with an empty array inmigration/mysql/20250122261736_update_subscription_table.sql.Service and Validation Logic:
NewSubscriptionfunction and related methods inpkg/notification/domain/subscription.goto handlefeatureFlagTags. [1] [2]featureFlagTagsare only saved if theFeaturesource type is present inpkg/notification/api/validation.go. [1] [2] [3]Command Handling and Localization:
UpdateSubscriptionFeatureFlagTagsCommandand its handler inpkg/notification/command/subscription.go.pkg/locale/localizedata/en.yaml,pkg/locale/localizedata/ja.yaml, andpkg/locale/localizer.go. [1] [2] [3]Test and Miscellaneous Updates:
featureFlagTagsfield. [1] [2] [3]These changes ensure that the notification service can now handle feature flag tags within subscriptions, providing more granular control over notification settings.