From 99600f1bb86cbe14d8b011d108c0bd13b22967c3 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 6 Nov 2026 13:58:19 +0530 Subject: [PATCH] feat(billing): write feature title and metadata as given on UpdateFeature --- billing/product/service.go | 19 ++ billing/product/service_test.go | 191 ++++++++++++++++++ .../api/v1beta1connect/billing_product.go | 4 +- .../v1beta1connect/billing_product_test.go | 123 +++++++++++ internal/api/v1beta1connect/interfaces.go | 1 + .../v1beta1connect/mocks/product_service.go | 57 ++++++ 6 files changed, 393 insertions(+), 2 deletions(-) diff --git a/billing/product/service.go b/billing/product/service.go index e37fc91863..31bc52cc83 100644 --- a/billing/product/service.go +++ b/billing/product/service.go @@ -13,6 +13,7 @@ import ( "github.com/google/uuid" billingerrors "github.com/raystack/frontier/billing/errors" + "github.com/raystack/frontier/pkg/metadata" "github.com/raystack/frontier/pkg/utils" "github.com/stripe/stripe-go/v79/client" ) @@ -596,6 +597,24 @@ func (s *Service) UpsertFeature(ctx context.Context, feature Feature) (Feature, return s.featureRepository.UpdateByName(ctx, existingFeature) } +func (s *Service) UpdateFeature(ctx context.Context, feature Feature) (Feature, error) { + if len(feature.Name) == 0 { + return Feature{}, fmt.Errorf("feature name is required: %w", ErrInvalidFeatureDetail) + } + existingFeature, err := s.featureRepository.GetByName(ctx, feature.Name) + if err != nil { + return Feature{}, err + } + + existingFeature.Title = feature.Title + existingFeature.ProductIDs = utils.Deduplicate(feature.ProductIDs) + existingFeature.Metadata = feature.Metadata + if existingFeature.Metadata == nil { + existingFeature.Metadata = metadata.Metadata{} + } + return s.featureRepository.UpdateByName(ctx, existingFeature) +} + func (s *Service) updateProductFeatures(ctx context.Context, existingProduct Product, product Product) error { var featureErr error existingFeatures, err := s.ListFeatures(ctx, Filter{ diff --git a/billing/product/service_test.go b/billing/product/service_test.go index f6c9597e13..3f70f59f92 100644 --- a/billing/product/service_test.go +++ b/billing/product/service_test.go @@ -2,6 +2,7 @@ package product_test import ( "context" + "errors" "reflect" "testing" @@ -12,6 +13,7 @@ import ( "github.com/raystack/frontier/billing/product" "github.com/raystack/frontier/billing/product/mocks" stripemock "github.com/raystack/frontier/billing/stripetest/mocks" + "github.com/raystack/frontier/pkg/metadata" "github.com/stretchr/testify/mock" "github.com/stripe/stripe-go/v79/client" ) @@ -1227,6 +1229,195 @@ func TestService_UpsertFeature(t *testing.T) { } } +func TestService_UpdateFeature(t *testing.T) { + ctx := context.Background() + tests := []struct { + name string + feature product.Feature + want product.Feature + wantErr error + setup func() *product.Service + }{ + { + name: "should write title, metadata and product ids as given", + feature: product.Feature{ + ID: "1", + Name: "feature1", + ProductIDs: []string{"2", "2"}, + }, + want: product.Feature{ + ID: "1", + Name: "feature1", + ProductIDs: []string{"2"}, + Metadata: metadata.Metadata{}, + }, + setup: func() *product.Service { + stripeClient, _, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t) + mockFeatureRepo.EXPECT().GetByName(ctx, "feature1").Return(product.Feature{ + ID: "1", + Name: "feature1", + Title: "Feature 1", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "tooltip"}, + }, nil) + mockFeatureRepo.EXPECT().UpdateByName(ctx, product.Feature{ + ID: "1", + Name: "feature1", + ProductIDs: []string{"2"}, + Metadata: metadata.Metadata{}, + }).Return(product.Feature{ + ID: "1", + Name: "feature1", + ProductIDs: []string{"2"}, + Metadata: metadata.Metadata{}, + }, nil) + return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo) + }, + }, + { + name: "should write empty metadata when metadata is nil", + feature: product.Feature{ + Name: "feature1", + Title: "Feature 1", + Metadata: nil, + }, + want: product.Feature{ + ID: "1", + Name: "feature1", + Title: "Feature 1", + Metadata: metadata.Metadata{}, + }, + setup: func() *product.Service { + stripeClient, _, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t) + mockFeatureRepo.EXPECT().GetByName(ctx, "feature1").Return(product.Feature{ + ID: "1", + Name: "feature1", + Title: "Feature 1", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "tooltip"}, + }, nil) + mockFeatureRepo.EXPECT().UpdateByName(ctx, product.Feature{ + ID: "1", + Name: "feature1", + Title: "Feature 1", + Metadata: metadata.Metadata{}, + }).Return(product.Feature{ + ID: "1", + Name: "feature1", + Title: "Feature 1", + Metadata: metadata.Metadata{}, + }, nil) + return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo) + }, + }, + { + name: "should write a non-empty title and metadata unchanged", + feature: product.Feature{ + Name: "feature1", + Title: "Audit logs", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "x"}, + }, + want: product.Feature{ + ID: "1", + Name: "feature1", + Title: "Audit logs", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "x"}, + }, + setup: func() *product.Service { + stripeClient, _, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t) + mockFeatureRepo.EXPECT().GetByName(ctx, "feature1").Return(product.Feature{ + ID: "1", + Name: "feature1", + Title: "Feature 1", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "tooltip"}, + }, nil) + mockFeatureRepo.EXPECT().UpdateByName(ctx, product.Feature{ + ID: "1", + Name: "feature1", + Title: "Audit logs", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "x"}, + }).Return(product.Feature{ + ID: "1", + Name: "feature1", + Title: "Audit logs", + ProductIDs: []string{"1"}, + Metadata: metadata.Metadata{"info": "x"}, + }, nil) + return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo) + }, + }, + { + name: "should look up a feature by name even when the name looks like a uuid", + feature: product.Feature{ + Name: "7f1c2d3e-4a5b-4c6d-8e7f-9a0b1c2d3e4f", + Title: "Feature 1", + }, + want: product.Feature{ + ID: "1", + Name: "7f1c2d3e-4a5b-4c6d-8e7f-9a0b1c2d3e4f", + Title: "Feature 1", + Metadata: metadata.Metadata{}, + }, + setup: func() *product.Service { + stripeClient, _, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t) + mockFeatureRepo.EXPECT().GetByName(ctx, "7f1c2d3e-4a5b-4c6d-8e7f-9a0b1c2d3e4f").Return(product.Feature{ + ID: "1", + Name: "7f1c2d3e-4a5b-4c6d-8e7f-9a0b1c2d3e4f", + }, nil) + mockFeatureRepo.EXPECT().UpdateByName(ctx, product.Feature{ + ID: "1", + Name: "7f1c2d3e-4a5b-4c6d-8e7f-9a0b1c2d3e4f", + Title: "Feature 1", + Metadata: metadata.Metadata{}, + }).Return(product.Feature{ + ID: "1", + Name: "7f1c2d3e-4a5b-4c6d-8e7f-9a0b1c2d3e4f", + Title: "Feature 1", + Metadata: metadata.Metadata{}, + }, nil) + return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo) + }, + }, + { + name: "should return not found if feature does not exist", + feature: product.Feature{ + Name: "feature1", + }, + wantErr: product.ErrFeatureNotFound, + setup: func() *product.Service { + stripeClient, _, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t) + mockFeatureRepo.EXPECT().GetByName(ctx, "feature1").Return(product.Feature{}, product.ErrFeatureNotFound) + return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo) + }, + }, + { + name: "should return invalid detail if name is empty", + feature: product.Feature{}, + wantErr: product.ErrInvalidFeatureDetail, + setup: func() *product.Service { + stripeClient, _, mockProductRepo, mockPriceRepo, mockFeatureRepo := mockService(t) + return product.NewService(stripeClient, mockProductRepo, mockPriceRepo, mockFeatureRepo) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := tt.setup() + got, err := s.UpdateFeature(ctx, tt.feature) + if !errors.Is(err, tt.wantErr) { + t.Fatalf("UpdateFeature() error = %v, wantErr %v", err, tt.wantErr) + } + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("UpdateFeature() mismatch (-want +got):\n%s", diff) + } + }) + } +} + func TestService_AddFeatureToProduct(t *testing.T) { ctx := context.Background() type args struct { diff --git a/internal/api/v1beta1connect/billing_product.go b/internal/api/v1beta1connect/billing_product.go index 8626aaa8e4..d12ec1f3e0 100644 --- a/internal/api/v1beta1connect/billing_product.go +++ b/internal/api/v1beta1connect/billing_product.go @@ -229,7 +229,7 @@ func (h *ConnectHandler) CreateFeature(ctx context.Context, request *connect.Req func (h *ConnectHandler) UpdateFeature(ctx context.Context, request *connect.Request[frontierv1beta1.UpdateFeatureRequest]) (*connect.Response[frontierv1beta1.UpdateFeatureResponse], error) { metaDataMap := metadata.Build(request.Msg.GetBody().GetMetadata().AsMap()) - updatedFeature, err := h.productService.UpsertFeature(ctx, product.Feature{ + updatedFeature, err := h.productService.UpdateFeature(ctx, product.Feature{ ID: request.Msg.GetId(), Name: request.Msg.GetBody().GetName(), Title: request.Msg.GetBody().GetTitle(), @@ -240,7 +240,7 @@ func (h *ConnectHandler) UpdateFeature(ctx context.Context, request *connect.Req if errors.Is(err, product.ErrInvalidFeatureDetail) { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) } - return nil, mapBillingError(ctx, fmt.Errorf("UpdateFeature.UpsertFeature: feature_id=%s feature_name=%s feature_title=%s product_ids=%v: %w", + return nil, mapBillingError(ctx, fmt.Errorf("UpdateFeature.UpdateFeature: feature_id=%s feature_name=%s feature_title=%s product_ids=%v: %w", request.Msg.GetId(), request.Msg.GetBody().GetName(), request.Msg.GetBody().GetTitle(), request.Msg.GetBody().GetProductIds(), err)) } diff --git a/internal/api/v1beta1connect/billing_product_test.go b/internal/api/v1beta1connect/billing_product_test.go index a4c9316088..93c7c2fc36 100644 --- a/internal/api/v1beta1connect/billing_product_test.go +++ b/internal/api/v1beta1connect/billing_product_test.go @@ -13,6 +13,7 @@ import ( frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "google.golang.org/protobuf/types/known/structpb" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -1170,3 +1171,125 @@ func TestConnectHandler_GetFeature(t *testing.T) { }) } } + +func TestConnectHandler_UpdateFeature(t *testing.T) { + populatedMetadata, err := structpb.NewStruct(map[string]any{"info": "See [docs](https://example.com)."}) + assert.NoError(t, err) + tests := []struct { + name string + setup func(ps *mocks.ProductService) + req *connect.Request[frontierv1beta1.UpdateFeatureRequest] + want *frontierv1beta1.Feature + wantMetadata map[string]any + wantErrCode connect.Code + }{ + { + name: "should write empty title and metadata as given", + setup: func(ps *mocks.ProductService) { + ps.EXPECT().UpdateFeature(mock.Anything, product.Feature{ + ID: "feature-1", + Name: "basic-analytics", + ProductIDs: []string{"product-1"}, + Metadata: metadata.Metadata{}, + }).Return(product.Feature{ + ID: "feature-1", + Name: "basic-analytics", + ProductIDs: []string{"product-1"}, + Metadata: metadata.Metadata{}, + }, nil) + }, + req: connect.NewRequest(&frontierv1beta1.UpdateFeatureRequest{ + Id: "feature-1", + Body: &frontierv1beta1.FeatureRequestBody{ + Name: "basic-analytics", + ProductIds: []string{"product-1"}, + }, + }), + want: &frontierv1beta1.Feature{ + Id: "feature-1", + Name: "basic-analytics", + ProductIds: []string{"product-1"}, + }, + wantMetadata: map[string]any{}, + }, + { + name: "should pass a populated title and metadata through", + setup: func(ps *mocks.ProductService) { + ps.EXPECT().UpdateFeature(mock.Anything, product.Feature{ + ID: "feature-1", + Name: "basic-analytics", + Title: "Basic Analytics", + ProductIDs: []string{"product-1"}, + Metadata: metadata.Metadata{"info": "See [docs](https://example.com)."}, + }).Return(product.Feature{ + ID: "feature-1", + Name: "basic-analytics", + Title: "Basic Analytics", + ProductIDs: []string{"product-1"}, + Metadata: metadata.Metadata{"info": "See [docs](https://example.com)."}, + }, nil) + }, + req: connect.NewRequest(&frontierv1beta1.UpdateFeatureRequest{ + Id: "feature-1", + Body: &frontierv1beta1.FeatureRequestBody{ + Name: "basic-analytics", + Title: "Basic Analytics", + ProductIds: []string{"product-1"}, + Metadata: populatedMetadata, + }, + }), + want: &frontierv1beta1.Feature{ + Id: "feature-1", + Name: "basic-analytics", + Title: "Basic Analytics", + ProductIds: []string{"product-1"}, + }, + wantMetadata: map[string]any{"info": "See [docs](https://example.com)."}, + }, + { + name: "should return not found if feature does not exist", + setup: func(ps *mocks.ProductService) { + ps.EXPECT().UpdateFeature(mock.Anything, mock.Anything).Return(product.Feature{}, product.ErrFeatureNotFound) + }, + req: connect.NewRequest(&frontierv1beta1.UpdateFeatureRequest{ + Id: "feature-1", + Body: &frontierv1beta1.FeatureRequestBody{Name: "basic-analytics"}, + }), + wantErrCode: connect.CodeNotFound, + }, + { + name: "should return invalid argument if feature detail is invalid", + setup: func(ps *mocks.ProductService) { + ps.EXPECT().UpdateFeature(mock.Anything, mock.Anything).Return(product.Feature{}, product.ErrInvalidFeatureDetail) + }, + req: connect.NewRequest(&frontierv1beta1.UpdateFeatureRequest{ + Id: "feature-1", + Body: &frontierv1beta1.FeatureRequestBody{}, + }), + wantErrCode: connect.CodeInvalidArgument, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + productService := mocks.NewProductService(t) + tt.setup(productService) + h := &ConnectHandler{ + productService: productService, + } + got, err := h.UpdateFeature(context.Background(), tt.req) + if tt.wantErrCode != 0 { + assert.Equal(t, tt.wantErrCode, connect.CodeOf(err)) + assert.Nil(t, got) + return + } + assert.NoError(t, err) + gotFeature := got.Msg.GetFeature() + assert.Equal(t, tt.want.GetId(), gotFeature.GetId()) + assert.Equal(t, tt.want.GetName(), gotFeature.GetName()) + assert.Equal(t, tt.want.GetTitle(), gotFeature.GetTitle()) + assert.Equal(t, tt.want.GetProductIds(), gotFeature.GetProductIds()) + assert.Equal(t, tt.wantMetadata, gotFeature.GetMetadata().AsMap()) + }) + } +} diff --git a/internal/api/v1beta1connect/interfaces.go b/internal/api/v1beta1connect/interfaces.go index 50288490df..20ab700362 100644 --- a/internal/api/v1beta1connect/interfaces.go +++ b/internal/api/v1beta1connect/interfaces.go @@ -117,6 +117,7 @@ type ProductService interface { Update(ctx context.Context, product product.Product) (product.Product, error) List(ctx context.Context, filter product.Filter) ([]product.Product, error) UpsertFeature(ctx context.Context, feature product.Feature) (product.Feature, error) + UpdateFeature(ctx context.Context, feature product.Feature) (product.Feature, error) GetFeatureByID(ctx context.Context, id string) (product.Feature, error) ListFeatures(ctx context.Context, filter product.Filter) ([]product.Feature, error) } diff --git a/internal/api/v1beta1connect/mocks/product_service.go b/internal/api/v1beta1connect/mocks/product_service.go index 3682d04ae7..a805ee5a6e 100644 --- a/internal/api/v1beta1connect/mocks/product_service.go +++ b/internal/api/v1beta1connect/mocks/product_service.go @@ -368,6 +368,63 @@ func (_c *ProductService_Update_Call) RunAndReturn(run func(context.Context, pro return _c } +// UpdateFeature provides a mock function with given fields: ctx, feature +func (_m *ProductService) UpdateFeature(ctx context.Context, feature product.Feature) (product.Feature, error) { + ret := _m.Called(ctx, feature) + + if len(ret) == 0 { + panic("no return value specified for UpdateFeature") + } + + var r0 product.Feature + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, product.Feature) (product.Feature, error)); ok { + return rf(ctx, feature) + } + if rf, ok := ret.Get(0).(func(context.Context, product.Feature) product.Feature); ok { + r0 = rf(ctx, feature) + } else { + r0 = ret.Get(0).(product.Feature) + } + + if rf, ok := ret.Get(1).(func(context.Context, product.Feature) error); ok { + r1 = rf(ctx, feature) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ProductService_UpdateFeature_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UpdateFeature' +type ProductService_UpdateFeature_Call struct { + *mock.Call +} + +// UpdateFeature is a helper method to define mock.On call +// - ctx context.Context +// - feature product.Feature +func (_e *ProductService_Expecter) UpdateFeature(ctx interface{}, feature interface{}) *ProductService_UpdateFeature_Call { + return &ProductService_UpdateFeature_Call{Call: _e.mock.On("UpdateFeature", ctx, feature)} +} + +func (_c *ProductService_UpdateFeature_Call) Run(run func(ctx context.Context, feature product.Feature)) *ProductService_UpdateFeature_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(product.Feature)) + }) + return _c +} + +func (_c *ProductService_UpdateFeature_Call) Return(_a0 product.Feature, _a1 error) *ProductService_UpdateFeature_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *ProductService_UpdateFeature_Call) RunAndReturn(run func(context.Context, product.Feature) (product.Feature, error)) *ProductService_UpdateFeature_Call { + _c.Call.Return(run) + return _c +} + // UpsertFeature provides a mock function with given fields: ctx, feature func (_m *ProductService) UpsertFeature(ctx context.Context, feature product.Feature) (product.Feature, error) { ret := _m.Called(ctx, feature)