Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: be able to group slack notifications by feature flag tag #1472

Merged
merged 17 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions api-description/web-api.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7893,6 +7893,10 @@ definitions:
$ref: '#/definitions/notificationRecipient'
name:
type: string
featureFlagTags:
type: array
items:
type: string
notificationCreateSubscriptionRequest:
type: object
properties:
Expand All @@ -7908,6 +7912,10 @@ definitions:
$ref: '#/definitions/SubscriptionSourceType'
recipient:
$ref: '#/definitions/notificationRecipient'
featureFlagTags:
type: array
items:
type: string
required:
- environmentId
- name
Expand Down Expand Up @@ -8096,8 +8104,19 @@ definitions:
type: string
environmentName:
type: string
featureFlagTags:
type: array
items:
type: string
notificationUpdateAdminSubscriptionResponse:
type: object
notificationUpdateSubscriptionFeatureFlagTagsCommand:
type: object
properties:
featureFlagTags:
type: array
items:
type: string
notificationUpdateSubscriptionRequest:
type: object
properties:
Expand All @@ -8124,6 +8143,13 @@ definitions:
type: boolean
description: if true, the subscription is disabled, otherwise enabled.
title: disabled
featureFlagTags:
type: array
items:
type: string
updateSubscriptionFeatureTagsCommand:
$ref: '#/definitions/notificationUpdateSubscriptionFeatureFlagTagsCommand'
description: Deprecated. Use feature_flag_tags instead.
required:
- id
- environmentId
Expand Down
6 changes: 3 additions & 3 deletions manifests/bucketeer/charts/web/values.yaml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE `subscription` ADD COLUMN `feature_flag_tags` JSON NOT NULL AFTER `name`;

-- Populate feature_flag_tags with an empty array
UPDATE `subscription` SET `feature_flag_tags` = '[]';
3 changes: 2 additions & 1 deletion migration/mysql/atlas.sum
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
h1:VXTmzqYh41HnDCRZyYb9OvaSQx2m1+GLavQcpqgGCjQ=
h1:w7fx0uljXF2/NlqThOynR/9BbVayje1TcjST6rEkRoI=
20240626022133_initialization.sql h1:u9qmPkwWX7PN92qEcDDfR92lrMpwadQSMxUJgv6LjQ0=
20240708065726_update_audit_log_table.sql h1:k7gK8Njv1yHMsYXAQtSgMaSbXy4lxyZ9MPzbJyMyyoM=
20240815043128_update_auto_ops_rule_table.sql h1:6ib+XfS1uu9AUO3qESvkpUfOu3qUsLwHm9KHcrGEz0E=
Expand All @@ -24,3 +24,4 @@ h1:VXTmzqYh41HnDCRZyYb9OvaSQx2m1+GLavQcpqgGCjQ=
20250116054703_remove_environment_namespace_column.sql h1:UNiN/syYAs8nUQOkMw3RRvDQBQU/kKYv7B3XBhL6qPM=
20250116060049_update_goal_table.sql h1:zEhXMf2hntdk2Im9klR76vx6PEAh+cJwk3fKgm2gGPs=
20250122261735_add_code_ref_table.sql h1:ZqE6Ekn3PnhBFdpuczJYo5KjwTvcgeqSnkuLCNeHoCA=
20250122261736_update_subscription_table.sql h1:+TyXXsVb1NxYHBNQsnBENojIxV5AFORw48dcG9D0gxQ=
8 changes: 8 additions & 0 deletions pkg/domainevent/domain/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,14 @@ func LocalizedMessage(eventType proto.Event_Type, localizer locale.Localizer) *p
localizer.MustLocalizeWithTemplate(locale.Notification),
),
}
case proto.Event_SUBSCRIPTION_FEATURE_FLAG_TAGS_UPDATED:
return &proto.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(
locale.FeatureFlagTagsUpdatedTemplate,
localizer.MustLocalizeWithTemplate(locale.Notification),
),
}
case proto.Event_SUBSCRIPTION_UPDATED:
return &proto.LocalizedMessage{
Locale: localizer.GetLocale(),
Expand Down
1 change: 1 addition & 0 deletions pkg/locale/localizedata/en.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ MultipleUpdated: "{{ .Field_1 }} have been updated"
DescriptionUpdated: "{{ .Field_1 }} description has been updated"
Incremented: "{{ .Field_1 }} has been incremented"
NameUpdated: "{{ .Field_1 }} name has been updated"
FeatureFlagTagsUpdated: "{{ .Field_1 }} feature flag tags have been updated"
Started: "{{ .Field_1 }} has been started"
Stopped: "{{ .Field_1 }} has been stopped"
Finished: "{{ .Field_1 }} has been finished"
Expand Down
1 change: 1 addition & 0 deletions pkg/locale/localizedata/ja.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ Changed: "{{ .Field_1 }}が変更されました"
DescriptionUpdated: "{{ .Field_1 }}の説明文が更新されました"
Incremented: "{{ .Field_1 }}が増加されました"
NameUpdated: "{{ .Field_1 }}の名前が更新されました"
FeatureFlagTagsUpdated: "{{ .Field_1 }} フィチャーフラグのタグが更新されました"
Updated: "{{ .Field_1 }}が更新されました"
MultipleUpdated: "{{ .Field_1 }}が更新されました"
Started: "{{ .Field_1 }}が開始されました"
Expand Down
55 changes: 28 additions & 27 deletions pkg/locale/localizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,33 +135,34 @@ const (

// domain events
const (
UnknownOperation = "UnknownOperation"
CreatedTemplate = "Created"
EnabledTemplate = "Enabled"
DisabledTemplate = "Disabled"
ArchivedTemplate = "Archived"
UnarchivedTemplate = "Unarchived"
DeletedTemplate = "Deleted"
AddedTemplate = "Added"
ConditionAddedTemplate = "ConditionAdded"
ConditionDeletedTemplate = "ConditionDeleted"
ChangedTemplate = "Changed"
DescriptionUpdatedTemplate = "DescriptionUpdated"
IncrementedTemplate = "Incremented"
NameUpdatedTemplate = "NameUpdated"
UpdatedTemplate = "Updated"
MultipleUpdatedTemplate = "MultipleUpdated"
StartedTemplate = "Started"
StoppedTemplate = "Stopped"
FinishedTemplate = "Finished"
ExecutedTemplate = "Executed"
UploadedTemplate = "Uploaded"
ResetTemplate = "Reset"
ClonedTemplate = "Cloned"
TrialConverted = "TrialConverted"
ValueAddedTemplate = "ValueAdded"
ValueUpdatedTemplate = "ValueUpdated"
ValueDeletedTemplate = "ValueDeleted"
UnknownOperation = "UnknownOperation"
CreatedTemplate = "Created"
EnabledTemplate = "Enabled"
DisabledTemplate = "Disabled"
ArchivedTemplate = "Archived"
UnarchivedTemplate = "Unarchived"
DeletedTemplate = "Deleted"
AddedTemplate = "Added"
ConditionAddedTemplate = "ConditionAdded"
ConditionDeletedTemplate = "ConditionDeleted"
ChangedTemplate = "Changed"
DescriptionUpdatedTemplate = "DescriptionUpdated"
IncrementedTemplate = "Incremented"
NameUpdatedTemplate = "NameUpdated"
FeatureFlagTagsUpdatedTemplate = "FeatureFlagTagsUpdated"
UpdatedTemplate = "Updated"
MultipleUpdatedTemplate = "MultipleUpdated"
StartedTemplate = "Started"
StoppedTemplate = "Stopped"
FinishedTemplate = "Finished"
ExecutedTemplate = "Executed"
UploadedTemplate = "Uploaded"
ResetTemplate = "Reset"
ClonedTemplate = "Cloned"
TrialConverted = "TrialConverted"
ValueAddedTemplate = "ValueAdded"
ValueUpdatedTemplate = "ValueUpdated"
ValueDeletedTemplate = "ValueDeleted"
)

func init() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/notification/api/admin_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (s *NotificationService) CreateAdminSubscription(
if err := s.validateCreateAdminSubscriptionRequest(req, localizer); err != nil {
return nil, err
}
subscription, err := domain.NewSubscription(req.Command.Name, req.Command.SourceTypes, req.Command.Recipient)
subscription, err := domain.NewSubscription(req.Command.Name, req.Command.SourceTypes, req.Command.Recipient, nil)
if err != nil {
s.logger.Error(
"Failed to create a new admin subscription",
Expand Down
2 changes: 1 addition & 1 deletion pkg/notification/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func putSubscription(t *testing.T, s storage.Client, kind, namespace string, dis
Type: proto.Recipient_SlackChannel,
SlackChannelRecipient: &proto.SlackChannelRecipient{WebhookUrl: "url"},
}
subscription, err := domain.NewSubscription("sname", sourceTypes, recipient)
subscription, err := domain.NewSubscription("sname", sourceTypes, recipient, []string{"tag"})
subscription.Disabled = disabled
require.NoError(t, err)
err = s.Put(context.Background(), key, subscription.Subscription)
Expand Down
40 changes: 29 additions & 11 deletions pkg/notification/api/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ func (s *NotificationService) CreateSubscription(
if err := s.validateCreateSubscriptionRequest(req, localizer); err != nil {
return nil, err
}
subscription, err := domain.NewSubscription(req.Command.Name, req.Command.SourceTypes, req.Command.Recipient)
subscription, err := domain.NewSubscription(
req.Command.Name,
req.Command.SourceTypes,
req.Command.Recipient,
req.Command.FeatureFlagTags,
)
if err != nil {
s.logger.Error(
"Failed to create a new subscription",
Expand Down Expand Up @@ -165,7 +170,12 @@ func (s *NotificationService) createSubscriptionNoCommand(
if err := s.validateCreateSubscriptionNoCommandRequest(req, localizer); err != nil {
return nil, err
}
subscription, err := domain.NewSubscription(req.Name, req.SourceTypes, req.Recipient)
subscription, err := domain.NewSubscription(
req.Name,
req.SourceTypes,
req.Recipient,
req.FeatureFlagTags,
)
if err != nil {
s.logger.Error(
"Failed to create a new subscription",
Expand Down Expand Up @@ -241,9 +251,10 @@ func (s *NotificationService) createSubscriptionNoCommand(
subscription.Id,
eventproto.Event_SUBSCRIPTION_CREATED,
&eventproto.SubscriptionCreatedEvent{
SourceTypes: subscription.SourceTypes,
Recipient: subscription.Recipient,
Name: subscription.Name,
SourceTypes: subscription.SourceTypes,
Recipient: subscription.Recipient,
Name: subscription.Name,
FeatureFlagTags: subscription.FeatureFlagTags,
},
req.EnvironmentId,
subscription.Subscription,
Expand Down Expand Up @@ -330,6 +341,7 @@ func (s *NotificationService) updateSubscriptionNoCommand(
req.Name,
req.SourceTypes,
req.Disabled,
req.FeatureFlagTags,
editor,
localizer,
)
Expand Down Expand Up @@ -515,6 +527,7 @@ func (s *NotificationService) updateSubscriptionMySQLNoCommand(
name *wrapperspb.StringValue,
sourceTypes []notificationproto.Subscription_SourceType,
disabled *wrapperspb.BoolValue,
featureFlagTags []string,
editor *eventproto.Editor,
localizer locale.Localizer,
) (*notificationproto.Subscription, error) {
Expand Down Expand Up @@ -545,7 +558,7 @@ func (s *NotificationService) updateSubscriptionMySQLNoCommand(
if err != nil {
return err
}
updated, err := subscription.UpdateSubscription(name, sourceTypes, disabled)
updated, err := subscription.UpdateSubscription(name, sourceTypes, disabled, featureFlagTags)
if err != nil {
return err
}
Expand All @@ -556,10 +569,11 @@ func (s *NotificationService) updateSubscriptionMySQLNoCommand(
subscription.Id,
eventproto.Event_SUBSCRIPTION_UPDATED,
&eventproto.SubscriptionUpdatedEvent{
Id: ID,
SourceTypes: sourceTypes,
Name: name,
Disabled: disabled,
Id: ID,
SourceTypes: sourceTypes,
Name: name,
Disabled: disabled,
FeatureFlagTags: featureFlagTags,
},
ID,
updatedSubscription,
Expand Down Expand Up @@ -617,7 +631,8 @@ func (s *NotificationService) updateSubscriptionMySQLNoCommand(
func (s *NotificationService) isNoUpdateSubscriptionCommand(req *notificationproto.UpdateSubscriptionRequest) bool {
return req.AddSourceTypesCommand == nil &&
req.DeleteSourceTypesCommand == nil &&
req.RenameSubscriptionCommand == nil
req.RenameSubscriptionCommand == nil &&
req.UpdateSubscriptionFeatureTagsCommand == nil
}

func (s *NotificationService) DeleteSubscription(
Expand Down Expand Up @@ -736,6 +751,9 @@ func (s *NotificationService) createUpdateSubscriptionCommands(
if req.RenameSubscriptionCommand != nil {
commands = append(commands, req.RenameSubscriptionCommand)
}
if req.UpdateSubscriptionFeatureTagsCommand != nil {
commands = append(commands, req.UpdateSubscriptionFeatureTagsCommand)
}
return commands
}

Expand Down
56 changes: 56 additions & 0 deletions pkg/notification/api/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ func (s *NotificationService) validateCreateSubscriptionRequest(
}
return dt.Err()
}
// We should only save the tags if the Feature source type is in the list
if req.Command.FeatureFlagTags != nil {
var found bool
for _, sourceType := range req.Command.SourceTypes {
if sourceType == notificationproto.Subscription_DOMAIN_EVENT_FEATURE {
found = true
break
}
}
if !found {
dt, err := statusSourceTypesRequired.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(locale.InvalidArgumentError, "SourceTypes"),
})
if err != nil {
return statusInternal.Err()
}
return dt.Err()
}
}
if err := s.validateRecipient(req.Command.Recipient, localizer); err != nil {
return err
}
Expand Down Expand Up @@ -75,6 +95,26 @@ func (s *NotificationService) validateCreateSubscriptionNoCommandRequest(
}
return dt.Err()
}
// We should only save the tags if the Feature source type is in the list
if req.FeatureFlagTags != nil {
Comment on lines +98 to +99
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the feature_flags_tags belongs to the Feature notification policy, we must validate it.

var found bool
for _, sourceType := range req.SourceTypes {
if sourceType == notificationproto.Subscription_DOMAIN_EVENT_FEATURE {
found = true
break
}
}
if !found {
dt, err := statusSourceTypesRequired.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(locale.InvalidArgumentError, "SourceTypes"),
})
if err != nil {
return statusInternal.Err()
}
return dt.Err()
}
}
if err := s.validateRecipient(req.Recipient, localizer); err != nil {
return err
}
Expand Down Expand Up @@ -234,6 +274,22 @@ func (s *NotificationService) validateUpdateSubscriptionRequest(
}
return dt.Err()
}
if req.UpdateSubscriptionFeatureTagsCommand != nil {
if req.DeleteSourceTypesCommand != nil {
for _, st := range req.DeleteSourceTypesCommand.SourceTypes {
if st == notificationproto.Subscription_DOMAIN_EVENT_FEATURE {
dt, err := statusNameRequired.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(locale.InvalidArgumentError, "feature_flag_tags"),
})
if err != nil {
return statusInternal.Err()
}
return dt.Err()
}
}
}
}
return nil
}

Expand Down
17 changes: 17 additions & 0 deletions pkg/notification/command/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ func (h *subscriptionCommandHandler) Handle(ctx context.Context, cmd Command) er
return h.disable(ctx, c)
case *proto.RenameSubscriptionCommand:
return h.rename(ctx, c)
case *proto.UpdateSubscriptionFeatureFlagTagsCommand:
return h.updateFeatureFlagTags(ctx, c)
}
return errUnknownCommand
}
Expand Down Expand Up @@ -140,6 +142,21 @@ func (h *subscriptionCommandHandler) rename(ctx context.Context, cmd *proto.Rena
return h.createEvent(ctx, eventproto.Event_SUBSCRIPTION_RENAMED, &eventproto.SubscriptionRenamedEvent{Name: cmd.Name})
}

func (h *subscriptionCommandHandler) updateFeatureFlagTags(
ctx context.Context,
cmd *proto.UpdateSubscriptionFeatureFlagTagsCommand,
) error {
err := h.subscription.UpdateFeatureFlagTags(cmd.FeatureFlagTags)
if err != nil {
return err
}
return h.createEvent(
ctx,
eventproto.Event_SUBSCRIPTION_FEATURE_FLAG_TAGS_UPDATED,
&eventproto.SubscriptionFeatureFlagTagsUpdatedEvent{FeatureFlagTags: cmd.FeatureFlagTags},
)
}

func (h *subscriptionCommandHandler) createEvent(
ctx context.Context,
eventType eventproto.Event_Type,
Expand Down
Loading
Loading