peer review comments for params updates in api

CarnegieLearningWeb · Oct 29, 2024 · 727438c · 727438c
1 parent fc4cf35
commit 727438c
Show file tree

Hide file tree

Showing 8 changed files with 11 additions and 19 deletions.
diff --git a/backend/packages/Upgrade/src/api/controllers/ExperimentController.ts b/backend/packages/Upgrade/src/api/controllers/ExperimentController.ts
@@ -1137,9 +1137,9 @@ export class ExperimentController {
     experiment: ExperimentDTO,
     @CurrentUser() currentUser: UserDTO,
     @Req() request: AppRequest
-  ): Promise<Experiment> {
+  ): Promise<ExperimentDTO> {
     request.logger.child({ user: currentUser });
-    return this.experimentService.update(experiment, currentUser, request.logger);
+    return this.experimentService.update({ ...experiment, id }, currentUser, request.logger);
   }
 
   /**

diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts
@@ -445,7 +445,7 @@ export class FeatureFlagsController {
     @CurrentUser() currentUser: UserDTO,
     @Req() request: AppRequest
   ): Promise<FeatureFlag> {
-    return this.featureFlagService.update(flag, id, currentUser, request.logger);
+    return this.featureFlagService.update({ ...flag, id }, currentUser, request.logger);
   }
 
   /**

diff --git a/backend/packages/Upgrade/src/api/controllers/PreviewUserController.ts b/backend/packages/Upgrade/src/api/controllers/PreviewUserController.ts
@@ -15,7 +15,8 @@ import { PreviewUserService } from '../services/PreviewUserService';
 import { PreviewUser } from '../models/PreviewUser';
 import { PaginatedParamsValidator } from './validators/PaginatedParamsValidator';
 import { AppRequest, PaginationResponse } from '../../types';
-import { IdValidator, PreviewUserValidator } from './validators/PreviewUserValidator';
+import { PreviewUserValidator } from './validators/PreviewUserValidator';
+import { IdValidator } from './validators/ExperimentUserValidator';
 
 interface PreviewUserPaginationInfo extends PaginationResponse {
   nodes: PreviewUser[];

diff --git a/backend/packages/Upgrade/src/api/controllers/validators/PreviewUserValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/PreviewUserValidator.ts
@@ -1,4 +1,4 @@
-import { ValidateNested, IsOptional, IsString, IsArray, IsNotEmpty } from 'class-validator';
+import { ValidateNested, IsOptional, IsString, IsArray } from 'class-validator';
 import { Type } from 'class-transformer';
 import { Experiment } from '../../../../src/api/models/Experiment';
 import { ExperimentCondition } from '../../../../src/api/models/ExperimentCondition';
@@ -26,9 +26,3 @@ export class PreviewUserValidator {
   @Type(() => ExplicitIndividualAssignment)
   public assignments?: ExplicitIndividualAssignment[];
 }
-
-export class IdValidator {
-  @IsNotEmpty()
-  @IsString()
-  public id: string;
-}
diff --git a/backend/packages/Upgrade/src/api/services/ExperimentService.ts b/backend/packages/Upgrade/src/api/services/ExperimentService.ts
@@ -363,7 +363,7 @@ export class ExperimentService {
     });
   }
 
-  public async update(experiment: ExperimentDTO, currentUser: UserDTO, logger: UpgradeLogger): Promise<Experiment> {
+  public async update(experiment: ExperimentDTO, currentUser: UserDTO, logger: UpgradeLogger): Promise<ExperimentDTO> {
     if (logger) {
       logger.info({ message: `Update the experiment`, details: experiment });
     }

diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts
@@ -322,12 +322,9 @@ export class FeatureFlagService {
 
   public async update(
     flagDTO: FeatureFlagValidation,
-    flagId: string,
     currentUser: UserDTO,
     logger: UpgradeLogger
   ): Promise<FeatureFlag> {
-    // update the flag with the flagId from the path params:
-    flagDTO.id = flagId;
     logger.info({ message: `Update a Feature Flag => ${flagDTO.toString()}` });
     const result = await this.featureFlagRepository.validateUniqueKey(flagDTO);
 

diff --git a/...d/packages/Upgrade/test/integration/Experiment/conditionAndPartition/ConditionPayloads.ts b/...d/packages/Upgrade/test/integration/Experiment/conditionAndPartition/ConditionPayloads.ts
@@ -103,7 +103,7 @@ export default async function ConditionPayload(): Promise<void> {
   updatedExperimentDoc = await experimentService.getSingleExperiment(
     updatedExperimentDoc.id as any,
     new UpgradeLogger()
-  ) as any;
+  );
 
   expect(updatedExperimentDoc.conditionPayloads.length).toEqual(1);
   expect(updatedExperimentDoc.conditionPayloads).toEqual(
@@ -128,6 +128,6 @@ export default async function ConditionPayload(): Promise<void> {
   updatedExperimentDoc = await experimentService.getSingleExperiment(
     updatedExperimentDoc.id as any,
     new UpgradeLogger()
-  ) as any;
+  );
   expect(updatedExperimentDoc.conditionPayloads.length).toEqual(0);
 }
diff --git a/backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts b/backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts
@@ -370,15 +370,15 @@ describe('Feature Flag Service Testing', () => {
   });
 
   it('should update the flag', async () => {
-    const results = await service.update(mockFlag2, uuid(), mockUser1, logger);
+    const results = await service.update(mockFlag2, mockUser1, logger);
     expect(isUUID(results.id)).toBeTruthy();
   });
 
   it('should throw an error when unable to update flag', async () => {
     const err = new Error('insert error');
     flagRepo.updateFeatureFlag = jest.fn().mockRejectedValue(err);
     expect(async () => {
-      await service.update(mockFlag2, uuid(), mockUser1, logger);
+      await service.update(mockFlag2, mockUser1, logger);
     }).rejects.toThrow(
       new Error('Error in updating feature flag document "updateFeatureFlagInDB" Error: insert error')
     );