Skip to content

Commit

Permalink
fix: cannot delete feature source type when sending feature flag tags
Browse files Browse the repository at this point in the history
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
  • Loading branch information
cre8ivejp committed Jan 22, 2025
1 parent 42fa925 commit 890be22
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 1 deletion.
16 changes: 16 additions & 0 deletions pkg/notification/api/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,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
12 changes: 12 additions & 0 deletions pkg/notification/domain/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ func (s *Subscription) UpdateSubscription(
updated.Name = name.Value
}
if len(sourceTypes) > 0 {
// We must check the feature source type is being deleted.
// If so, we must reset the tags
for _, sourceType := range updated.SourceTypes {
if sourceType == proto.Subscription_DOMAIN_EVENT_FEATURE {
updated.FeatureFlagTags = []string{}
break
}
}
updated.SourceTypes = sourceTypes
}
if disabled != nil {
Expand Down Expand Up @@ -158,6 +166,10 @@ func (s *Subscription) DeleteSourceTypes(sourceTypes []proto.Subscription_Source
return ErrSourceTypesMustHaveAtLeastOne
}
for _, nt := range sourceTypes {
// Reset the tags if the feature source type is being deleted
if nt == proto.Subscription_DOMAIN_EVENT_FEATURE {
s.FeatureFlagTags = []string{}
}
idx, err := indexSourceType(nt, s.SourceTypes)
if err != nil {
return err
Expand Down
51 changes: 50 additions & 1 deletion pkg/notification/domain/subscription_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ func TestUpdateNotification(t *testing.T) {
Name: "origin",
SourceTypes: []proto.Subscription_SourceType{
proto.Subscription_DOMAIN_EVENT_ACCOUNT,
proto.Subscription_DOMAIN_EVENT_FEATURE,
},
Recipient: &proto.Recipient{
Type: proto.Recipient_SlackChannel,
Expand All @@ -169,13 +170,61 @@ func TestUpdateNotification(t *testing.T) {
},
},
inputData: &input{
sourceTypes: []proto.Subscription_SourceType{
proto.Subscription_DOMAIN_EVENT_ACCOUNT,
},
featureFlagTags: []string{"update-tag"},
},
expected: nil,
expectedErr: ErrCannotUpdateFeatureFlagTags,
},
{
desc: "update subscription's feature flag tags",
desc: "success: reset feature flag tags when the feature source type is also being deleted",
origin: &Subscription{
&proto.Subscription{
Id: "id",
Name: "origin",
SourceTypes: []proto.Subscription_SourceType{
proto.Subscription_DOMAIN_EVENT_ACCOUNT,
proto.Subscription_DOMAIN_EVENT_FEATURE,
},
Recipient: &proto.Recipient{
Type: proto.Recipient_SlackChannel,
SlackChannelRecipient: &proto.SlackChannelRecipient{
WebhookUrl: "https://slack-hooks.exp",
},
},
Disabled: false,
FeatureFlagTags: []string{"tag"},
},
},
inputData: &input{
sourceTypes: []proto.Subscription_SourceType{
proto.Subscription_DOMAIN_EVENT_ACCOUNT,
},
},
expected: &Subscription{
&proto.Subscription{
Id: "id",
Name: "origin",
SourceTypes: []proto.Subscription_SourceType{
proto.Subscription_DOMAIN_EVENT_ACCOUNT,
},
Recipient: &proto.Recipient{
Type: proto.Recipient_SlackChannel,
SlackChannelRecipient: &proto.SlackChannelRecipient{
WebhookUrl: "https://slack-hooks.exp",
},
},
Disabled: false,
UpdatedAt: time.Now().Unix(),
FeatureFlagTags: []string{},
},
},
expectedErr: nil,
},
{
desc: "success: update subscription's feature flag tags",
origin: &Subscription{
&proto.Subscription{
Id: "id",
Expand Down

0 comments on commit 890be22

Please sign in to comment.