feat(billing): write feature title and metadata as given on UpdateFeature

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{

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 {

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))
 	}
 

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())
+		})
+	}
+}