feat: implement update experiment REST no command (#1482)

api-description/web-api.swagger.yaml

@@ -3494,6 +3494,54 @@ paths:
             $ref: '#/definitions/experimentCreateExperimentRequest'
       tags:
         - experiment
+    patch:
+      summary: Update
+      description: Update an experiment.
+      operationId: web.v1.experiment.update
+      responses:
+        "200":
+          description: A successful response.
+          schema:
+            $ref: '#/definitions/experimentUpdateExperimentResponse'
+        "400":
+          description: Returned for bad requests that may have failed validation.
+          schema:
+            $ref: '#/definitions/googlerpcStatus'
+          examples:
+            application/json:
+              code: 3
+              details: []
+              message: invalid arguments error
+        "401":
+          description: Request could not be authenticated (authentication required).
+          schema:
+            $ref: '#/definitions/googlerpcStatus'
+          examples:
+            application/json:
+              code: 16
+              details: []
+              message: not authenticated
+        "503":
+          description: Returned for internal errors.
+          schema:
+            $ref: '#/definitions/googlerpcStatus'
+          examples:
+            application/json:
+              code: 13
+              details: []
+              message: internal
+        default:
+          description: An unexpected error response.
+          schema:
+            $ref: '#/definitions/googlerpcStatus'
+      parameters:
+        - name: body
+          in: body
+          required: true
+          schema:
+            $ref: '#/definitions/experimentUpdateExperimentRequest'
+      tags:
+        - experiment
   /v1/experiments:
     get:
       summary: List
@@ -5193,6 +5241,11 @@ definitions:
     properties:
       role:
         $ref: '#/definitions/AccountV2RoleOrganization'
+  UpdateExperimentRequestUpdatedStatus:
+    type: object
+    properties:
+      status:
+        $ref: '#/definitions/experimentExperimentStatus'
   UpdateFeatureTargetingRequestFrom:
     type: string
     enum:
@@ -7091,8 +7144,45 @@ definitions:
     type: object
   experimentStopExperimentResponse:
     type: object
+  experimentUpdateExperimentRequest:
+    type: object
+    properties:
+      id:
+        type: string
+      changeExperimentPeriodCommand:
+        $ref: '#/definitions/experimentChangeExperimentPeriodCommand'
+        description: deprecated
+      changeNameCommand:
+        $ref: '#/definitions/experimentChangeExperimentNameCommand'
+        description: deprecated
+      changeDescriptionCommand:
+        $ref: '#/definitions/experimentChangeExperimentDescriptionCommand'
+        description: deprecated
+      environmentId:
+        type: string
+      name:
+        type: string
+      description:
+        type: string
+      startAt:
+        type: string
+        format: int64
+      stopAt:
+        type: string
+        format: int64
+      archived:
+        type: boolean
+        description: if true, the experiment will be archived
+      status:
+        $ref: '#/definitions/UpdateExperimentRequestUpdatedStatus'
+    required:
+      - id
+      - environmentId
   experimentUpdateExperimentResponse:
     type: object
+    properties:
+      experiment:
+        $ref: '#/definitions/experimentExperiment'
   experimentUpdateGoalRequest:
     type: object
     properties:

pkg/domainevent/domain/message.go

@@ -431,6 +431,14 @@ func LocalizedMessage(eventType proto.Event_Type, localizer locale.Localizer) *p
 				localizer.MustLocalizeWithTemplate(locale.Experiment),
 			),
 		}
+	case proto.Event_EXPERIMENT_UPDATED:
+		return &proto.LocalizedMessage{
+			Locale: localizer.GetLocale(),
+			Message: localizer.MustLocalizeWithTemplate(
+				locale.UpdatedTemplate,
+				localizer.MustLocalizeWithTemplate(locale.Goal),
+			),
+		}
 	case proto.Event_EXPERIMENT_DELETED:
 		return &proto.LocalizedMessage{
 			Locale: localizer.GetLocale(),

pkg/experiment/api/error.go

@@ -30,6 +30,7 @@ var (
 	statusInvalidGoalID          = gstatus.New(codes.InvalidArgument, "experiment: invalid goal id")
 	statusGoalNameRequired       = gstatus.New(codes.InvalidArgument, "experiment: goal name must be specified")
 	statusPeriodTooLong          = gstatus.New(codes.InvalidArgument, "experiment: period too long")
+	statusPeriodInvalid          = gstatus.New(codes.InvalidArgument, "experiment: period is invalid")
 	statusInvalidOrderBy         = gstatus.New(codes.InvalidArgument, "expriment: order_by is invalid")
 	statusNotFound               = gstatus.New(codes.NotFound, "experiment: not found")
 	statusGoalNotFound           = gstatus.New(codes.NotFound, "experiment: goal not found")

pkg/experiment/api/experiment.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"strconv"
 
+	pb "github.com/golang/protobuf/proto"
 	"github.com/jinzhu/copier"
 	"go.uber.org/zap"
 	"google.golang.org/genproto/googleapis/rpc/errdetails"
@@ -655,6 +656,11 @@ func (s *experimentService) UpdateExperiment(
 	if err != nil {
 		return nil, err
 	}
+	if req.ChangeExperimentPeriodCommand == nil &&
+		req.ChangeNameCommand == nil &&
+		req.ChangeDescriptionCommand == nil {
+		return s.updateExperimentNoCommand(ctx, req, editor, localizer)
+	}
 	if err := validateUpdateExperimentRequest(req, localizer); err != nil {
 		return nil, err
 	}
@@ -730,7 +736,106 @@ func (s *experimentService) UpdateExperiment(
 		return experimentStorage.UpdateExperiment(ctx, experiment, req.EnvironmentId)
 	})
 	if err != nil {
-		if err == v2es.ErrExperimentNotFound || err == v2es.ErrExperimentUnexpectedAffectedRows {
+		if errors.Is(err, v2es.ErrExperimentNotFound) || errors.Is(err, v2es.ErrExperimentUnexpectedAffectedRows) {
+			dt, err := statusNotFound.WithDetails(&errdetails.LocalizedMessage{
+				Locale:  localizer.GetLocale(),
+				Message: localizer.MustLocalize(locale.NotFoundError),
+			})
+			if err != nil {
+				return nil, statusInternal.Err()
+			}
+			return nil, dt.Err()
+		}
+		s.logger.Error(
+			"Failed to update experiment",
+			log.FieldsFromImcomingContext(ctx).AddFields(
+				zap.Error(err),
+				zap.String("environmentId", req.EnvironmentId),
+			)...,
+		)
+		dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
+			Locale:  localizer.GetLocale(),
+			Message: localizer.MustLocalize(locale.InternalServerError),
+		})
+		if err != nil {
+			return nil, statusInternal.Err()
+		}
+		return nil, dt.Err()
+	}
+	return &proto.UpdateExperimentResponse{}, nil
+}
+
+func (s *experimentService) updateExperimentNoCommand(
+	ctx context.Context,
+	req *proto.UpdateExperimentRequest,
+	editor *eventproto.Editor,
+	localizer locale.Localizer,
+) (*proto.UpdateExperimentResponse, error) {
+	err := validateUpdateExperimentNoCommandRequest(req, localizer)
+	if err != nil {
+		s.logger.Error(
+			"Failed validate update experiment no command req",
+			log.FieldsFromImcomingContext(ctx).AddFields(
+				zap.Error(err),
+				zap.String("environmentId", req.EnvironmentId),
+			)...,
+		)
+		return nil, err
+	}
+
+	err = s.mysqlClient.RunInTransactionV2(ctx, func(ctxWithTx context.Context, _ mysql.Transaction) error {
+		experimentStorage := v2es.NewExperimentStorage(s.mysqlClient)
+		experiment, err := experimentStorage.GetExperiment(ctxWithTx, req.Id, req.EnvironmentId)
+		if err != nil {
+			return err
+		}
+		updated, err := experiment.Update(
+			req.Name,
+			req.Description,
+			req.StartAt,
+			req.StopAt,
+			req.Status,
+			req.Archived,
+		)
+		if err != nil {
+			return err
+		}
+
+		var eventMsg pb.Message
+		if req.Archived != nil {
+			eventMsg = &eventproto.ExperimentArchivedEvent{
+				Id: req.Id,
+			}
+		} else {
+			eventMsg = &eventproto.ExperimentUpdatedEvent{
+				Id:          experiment.Id,
+				Name:        updated.Name,
+				Description: updated.Description,
+				StartAt:     updated.StartAt,
+				StopAt:      updated.StopAt,
+				Status:      updated.Status,
+			}
+		}
+		event, err := domainevent.NewEvent(
+			editor,
+			eventproto.Event_EXPERIMENT,
+			experiment.Id,
+			eventproto.Event_EXPERIMENT_UPDATED,
+			eventMsg,
+			req.EnvironmentId,
+			updated,
+			experiment,
+		)
+		if err != nil {
+			return err
+		}
+		if err := s.publisher.Publish(ctxWithTx, event); err != nil {
+			return err
+		}
+		return experimentStorage.UpdateExperiment(ctxWithTx, updated, req.EnvironmentId)
+	})
+	if err != nil {
+		if errors.Is(err, v2es.ErrExperimentNotFound) || errors.Is(err, v2es.ErrExperimentUnexpectedAffectedRows) {
 			dt, err := statusNotFound.WithDetails(&errdetails.LocalizedMessage{
 				Locale:  localizer.GetLocale(),
 				Message: localizer.MustLocalize(locale.NotFoundError),
@@ -782,6 +887,53 @@ func validateUpdateExperimentRequest(req *proto.UpdateExperimentRequest, localiz
 	return nil
 }
 
+func validateUpdateExperimentNoCommandRequest(
+	req *proto.UpdateExperimentRequest,
+	localizer locale.Localizer,
+) error {
+	if req.Id == "" {
+		dt, err := statusExperimentIDRequired.WithDetails(&errdetails.LocalizedMessage{
+			Locale:  localizer.GetLocale(),
+			Message: localizer.MustLocalizeWithTemplate(locale.RequiredFieldTemplate, "id"),
+		})
+		if err != nil {
+			return statusInternal.Err()
+		}
+		return dt.Err()
+	}
+	if req.Name != nil && req.Name.Value == "" {
+		dt, err := statusExperimentNameRequired.WithDetails(&errdetails.LocalizedMessage{
+			Locale:  localizer.GetLocale(),
+			Message: localizer.MustLocalizeWithTemplate(locale.RequiredFieldTemplate, "name"),
+		})
+		if err != nil {
+			return statusInternal.Err()
+		}
+		return dt.Err()
+	}
+	if (req.StartAt != nil && req.StopAt == nil) ||
+		(req.StartAt == nil && req.StopAt != nil) {
+		dt, err := statusPeriodInvalid.WithDetails(&errdetails.LocalizedMessage{
+			Locale:  localizer.GetLocale(),
+			Message: localizer.MustLocalizeWithTemplate(locale.InvalidArgumentError, "period"),
+		})
+		if err != nil {
+			return statusInternal.Err()
+		}
+		return dt.Err()
+	}
+	if req.StartAt != nil && req.StopAt != nil {
+		if err := validateExperimentPeriod(
+			req.StartAt.Value,
+			req.StopAt.Value,
+			localizer,
+		); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (s *experimentService) StartExperiment(
 	ctx context.Context,
 	req *proto.StartExperimentRequest,