Merge pull request #2076 from CarnegieLearningWeb/enhancement/params-…

…decorator-issue-2066

enhancement for api path params validators and removing rejected api responses

backend/packages/Upgrade/src/api/DTO/ExperimentDTO.ts

@@ -10,6 +10,7 @@ import {
   IsObject,
   IsOptional,
   IsString,
+  IsUUID,
   MaxLength,
   MinLength,
   ValidateIf,
@@ -458,6 +459,12 @@ export class ExperimentDTO {
   public type: EXPERIMENT_TYPE;
 }
 
+export class ExperimentIdValidator {
+  @IsNotEmpty()
+  @IsUUID()
+  public id: string;
+}
+
 export interface ExperimentFile {
   fileName: string;
   fileContent: string;

backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v5.ts

@@ -106,7 +106,7 @@ export class ExperimentClientController {
     public metricService: MetricService
   ) {}
 
-  private async checkIfUserExist(
+  public async checkIfUserExist(
     userId: string,
     logger: UpgradeLogger,
     api?: string

backend/packages/Upgrade/src/api/controllers/ExperimentController.ts

@@ -3,7 +3,6 @@ import {
   Get,
   JsonController,
   OnUndefined,
-  Param,
   Post,
   Put,
   Delete,
@@ -17,8 +16,6 @@ import { Experiment } from '../models/Experiment';
 import { ExperimentNotFoundError } from '../errors/ExperimentNotFoundError';
 import { ExperimentService } from '../services/ExperimentService';
 import { ExperimentAssignmentService } from '../services/ExperimentAssignmentService';
-import { SERVER_ERROR } from 'upgrade_types';
-import { isUUID } from 'class-validator';
 import { ExperimentCondition } from '../models/ExperimentCondition';
 import { ExperimentPaginatedParamsValidator } from './validators/ExperimentPaginatedParamsValidator';
 import { UserDTO } from '../DTO/UserDTO';
@@ -28,7 +25,7 @@ import { AppRequest, PaginationResponse } from '../../types';
 import { ExperimentDTO, ExperimentFile, ValidatedExperimentError } from '../DTO/ExperimentDTO';
 import { ExperimentIds } from './validators/ExperimentIdsValidator';
 import { NotFoundException } from '@nestjs/common/exceptions';
-import { IdValidator } from './validators/FeatureFlagValidator';
+import { ExperimentIdValidator } from '../DTO/ExperimentDTO';
 
 interface ExperimentPaginationInfo extends PaginationResponse {
   nodes: Experiment[];
@@ -634,7 +631,7 @@ export class ExperimentController {
    *            description: AuthorizationRequiredError
    */
   @Get()
-  public find(@Req() request: AppRequest): Promise<Experiment[]> {
+  public find(@Req() request: AppRequest): Promise<ExperimentDTO[]> {
     return this.experimentService.find(request.logger);
   }
 
@@ -743,14 +740,6 @@ export class ExperimentController {
     @Req()
     request: AppRequest
   ): Promise<ExperimentPaginationInfo> {
-    if (!paginatedParams) {
-      return Promise.reject(
-        new Error(
-          JSON.stringify({ type: SERVER_ERROR.MISSING_PARAMS, message: ' : paginatedParams should not be null.' })
-        )
-      );
-    }
-
     const [experiments, count] = await Promise.all([
       this.experimentService.findPaginated(
         paginatedParams.skip,
@@ -841,7 +830,10 @@ export class ExperimentController {
    */
   @Get('/single/:id')
   @OnUndefined(ExperimentNotFoundError)
-  public one(@Params({ validate: true }) { id }: IdValidator, @Req() request: AppRequest): Promise<Experiment> {
+  public one(
+    @Params({ validate: true }) { id }: ExperimentIdValidator,
+    @Req() request: AppRequest
+  ): Promise<ExperimentDTO> {
     return this.experimentService.getSingleExperiment(id, request.logger);
   }
 
@@ -920,7 +912,7 @@ export class ExperimentController {
   @Get('/conditions/:id')
   @OnUndefined(ExperimentNotFoundError)
   public async getCondition(
-    @Params({ validate: true }) { id }: IdValidator,
+    @Params({ validate: true }) { id }: ExperimentIdValidator,
     @Req() request: AppRequest
   ): Promise<ExperimentCondition[]> {
     return this.experimentService.getExperimentalConditions(id, request.logger);
@@ -1042,7 +1034,7 @@ export class ExperimentController {
 
   @Delete('/:id')
   public async delete(
-    @Params({ validate: true }) { id }: IdValidator,
+    @Params({ validate: true }) { id }: ExperimentIdValidator,
     @CurrentUser() currentUser: UserDTO,
     @Req() request: AppRequest
   ): Promise<Experiment | undefined> {
@@ -1140,21 +1132,14 @@ export class ExperimentController {
    */
   @Put('/:id')
   public update(
-    @Param('id') id: string,
+    @Params({ validate: true }) { id }: ExperimentIdValidator,
     @Body({ validate: true })
     experiment: ExperimentDTO,
     @CurrentUser() currentUser: UserDTO,
     @Req() request: AppRequest
-  ): Promise<Experiment> {
-    if (!isUUID(id)) {
-      return Promise.reject(
-        new Error(
-          JSON.stringify({ type: SERVER_ERROR.INCORRECT_PARAM_FORMAT, message: ' : id should be of type UUID.' })
-        )
-      );
-    }
+  ): Promise<ExperimentDTO> {
     request.logger.child({ user: currentUser });
-    return this.experimentService.update(experiment, currentUser, request.logger);
+    return this.experimentService.update({ ...experiment, id }, currentUser, request.logger);
   }
 
   /**
@@ -1345,7 +1330,7 @@ export class ExperimentController {
   @Get('/getGroupAssignmentStatus/:id')
   @OnUndefined(ExperimentNotFoundError)
   public async getGroupAssignmentStatus(
-    @Params({ validate: true }) { id }: IdValidator,
+    @Params({ validate: true }) { id }: ExperimentIdValidator,
     @Req() request: AppRequest
   ): Promise<number> | undefined {
     return this.experimentAssignmentService.getGroupAssignmentStatus(id, request.logger);

backend/packages/Upgrade/src/api/controllers/ExperimentUserController.ts

@@ -1,12 +1,14 @@
-import { JsonController, Req, Get, OnUndefined, Param, Post, Put, Body, Authorized } from 'routing-controllers';
+import { JsonController, Req, Get, Post, Put, Body, Authorized, Params } from 'routing-controllers';
 import { ExperimentUserService } from '../services/ExperimentUserService';
 import { ExperimentUser } from '../models/ExperimentUser';
-import { UserNotFoundError } from '../errors/UserNotFoundError';
-import { SERVER_ERROR } from 'upgrade_types';
-import { isUUID } from 'class-validator';
 import { AppRequest } from '../../types';
-import { ExperimentUserArrayValidator, ExperimentUserValidator } from './validators/ExperimentUserValidator';
+import {
+  ExperimentUserArrayValidator,
+  ExperimentUserValidator,
+  IdValidator,
+} from './validators/ExperimentUserValidator';
 import { InsertResult } from 'typeorm';
+import { ExperimentClientController } from './ExperimentClientController.v5';
 
 // TODO delete this from experiment system
 /**
@@ -34,7 +36,10 @@ import { InsertResult } from 'typeorm';
 @Authorized()
 @JsonController('/experimentusers')
 export class UserController {
-  constructor(public userService: ExperimentUserService) {}
+  constructor(
+    public userService: ExperimentUserService,
+    public experimentClientController: ExperimentClientController
+  ) {}
 
   /**
    * @swagger
@@ -46,6 +51,8 @@ export class UserController {
    *       responses:
    *          '200':
    *            description: Successful
+   *          '401':
+   *            description: AuthorizationRequiredError
    */
   @Get()
   public find(@Req() request: AppRequest): Promise<ExperimentUser[]> {
@@ -71,15 +78,19 @@ export class UserController {
    *       responses:
    *          '200':
    *            description: Get user By Id
+   *          '400':
+   *            description: BadRequestError - InvalidParameterValue
+   *          '401':
+   *            description: AuthorizationRequiredError
    *          '404':
-   *            description: user not found
+   *            description: Experiment User not defined
    */
   @Get('/:id')
-  @OnUndefined(UserNotFoundError)
-  public one(@Param('id') id: string, @Req() request: AppRequest): Promise<ExperimentUser> {
-    if (!isUUID(id)) {
-      return Promise.reject(new Error(SERVER_ERROR.INCORRECT_PARAM_FORMAT + ' : id should be of type UUID.'));
-    }
+  public async one(
+    @Params({ validate: true }) { id }: IdValidator,
+    @Req() request: AppRequest
+  ): Promise<ExperimentUser> {
+    await this.experimentClientController.checkIfUserExist(id, request.logger);
     return this.userService.findOne(id, request.logger);
   }
 
@@ -105,6 +116,10 @@ export class UserController {
    *       responses:
    *          '200':
    *            description: New ExperimentUser is created
+   *          '400':
+   *            description: BadRequestError - InvalidParameterValue
+   *          '401':
+   *            description: AuthorizationRequiredError
    */
   @Post()
   public create(
@@ -143,13 +158,20 @@ export class UserController {
    *       responses:
    *          '200':
    *            description: ExperimentUser is updated
+   *          '400':
+   *            description: BadRequestError - InvalidParameterValue
+   *          '401':
+   *            description: AuthorizationRequiredError
+   *          '404':
+   *            description: Experiment User not defined
    */
   @Put('/:id')
-  public update(
-    @Param('id') id: string,
+  public async update(
+    @Params({ validate: true }) { id }: IdValidator,
     @Body({ validate: true }) user: ExperimentUserValidator,
     @Req() request: AppRequest
   ): Promise<ExperimentUser> {
+    await this.experimentClientController.checkIfUserExist(id, request.logger);
     return this.userService.update(id, user, request.logger);
   }
 }

backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts

@@ -251,14 +251,6 @@ export class FeatureFlagsController {
     paginatedParams: FeatureFlagPaginatedParamsValidator,
     @Req() request: AppRequest
   ): Promise<FeatureFlagsPaginationInfo> {
-    if (!paginatedParams) {
-      return Promise.reject(
-        new Error(
-          JSON.stringify({ type: SERVER_ERROR.MISSING_PARAMS, message: ' : paginatedParams should not be null.' })
-        )
-      );
-    }
-
     const [featureFlags, count] = await Promise.all([
       this.featureFlagService.findPaginated(
         paginatedParams.skip,
@@ -453,7 +445,7 @@ export class FeatureFlagsController {
     @CurrentUser() currentUser: UserDTO,
     @Req() request: AppRequest
   ): Promise<FeatureFlag> {
-    return this.featureFlagService.update(flag, currentUser, request.logger);
+    return this.featureFlagService.update({ ...flag, id }, currentUser, request.logger);
   }
 
   /**

backend/packages/Upgrade/src/api/controllers/MetricController.ts

@@ -1,8 +1,8 @@
-import { Authorized, JsonController, Get, Delete, Param, Post, Req, Body, Params } from 'routing-controllers';
+import { Authorized, JsonController, Get, Delete, Post, Req, Body, Params } from 'routing-controllers';
 import { MetricService } from '../services/MetricService';
 import { IMetricUnit } from 'upgrade_types';
 import { AppRequest } from '../../types';
-import { MetricKeyValidator, MetricValidator } from './validators/MetricValidator';
+import { ContextValidator, MetricKeyValidator, MetricValidator } from './validators/MetricValidator';
 
 /**
  * @swagger
@@ -56,7 +56,10 @@ export class MetricController {
    *            description: Metrics Context not found
    */
   @Get('/:context')
-  public getMetricsByContext(@Param('context') context: string, @Req() request: AppRequest): Promise<IMetricUnit[]> {
+  public getMetricsByContext(
+    @Params({ validate: true }) { context }: ContextValidator,
+    @Req() request: AppRequest
+  ): Promise<IMetricUnit[]> {
     return this.metricService.getMetricsByContext(context, request.logger);
   }
 

backend/packages/Upgrade/src/api/controllers/PreviewUserController.ts

@@ -1,12 +1,22 @@
-import { JsonController, Get, OnUndefined, Param, Post, Put, Body, Authorized, Delete, Req } from 'routing-controllers';
+import {
+  JsonController,
+  Get,
+  OnUndefined,
+  Post,
+  Put,
+  Body,
+  Authorized,
+  Delete,
+  Req,
+  Params,
+} from 'routing-controllers';
 import { UserNotFoundError } from '../errors/UserNotFoundError';
-import { SERVER_ERROR } from 'upgrade_types';
 import { PreviewUserService } from '../services/PreviewUserService';
 import { PreviewUser } from '../models/PreviewUser';
-import { isString } from 'class-validator';
 import { PaginatedParamsValidator } from './validators/PaginatedParamsValidator';
 import { AppRequest, PaginationResponse } from '../../types';
 import { PreviewUserValidator } from './validators/PreviewUserValidator';
+import { IdValidator } from './validators/ExperimentUserValidator';
 
 interface PreviewUserPaginationInfo extends PaginationResponse {
   nodes: PreviewUser[];
@@ -83,14 +93,6 @@ export class PreviewUserController {
     @Body({ validate: true }) paginatedParams: PaginatedParamsValidator,
     @Req() request: AppRequest
   ): Promise<PreviewUserPaginationInfo> {
-    if (!paginatedParams) {
-      return Promise.reject(
-        new Error(
-          JSON.stringify({ type: SERVER_ERROR.MISSING_PARAMS, message: ' : paginatedParams should not be null.' })
-        )
-      );
-    }
-
     const [previewUsers, count] = await Promise.all([
       this.previewUserService.findPaginated(paginatedParams.skip, paginatedParams.take, request.logger),
       this.previewUserService.getTotalCount(request.logger),
@@ -126,14 +128,10 @@ export class PreviewUserController {
    */
   @Get('/:id')
   @OnUndefined(UserNotFoundError)
-  public one(@Param('id') id: string, @Req() request: AppRequest): Promise<PreviewUser | undefined> {
-    if (!isString(id)) {
-      return Promise.reject(
-        new Error(
-          JSON.stringify({ type: SERVER_ERROR.INCORRECT_PARAM_FORMAT, message: ' : id should be of type string.' })
-        )
-      );
-    }
+  public one(
+    @Params({ validate: true }) { id }: IdValidator,
+    @Req() request: AppRequest
+  ): Promise<PreviewUser | undefined> {
     return this.previewUserService.findOne(id, request.logger);
   }
 
@@ -161,7 +159,10 @@ export class PreviewUserController {
    *            description: New ExperimentUser is created
    */
   @Post()
-  public create(@Body({ validate: true }) users: PreviewUserValidator, @Req() request: AppRequest): Promise<PreviewUser> {
+  public create(
+    @Body({ validate: true }) users: PreviewUserValidator,
+    @Req() request: AppRequest
+  ): Promise<PreviewUser> {
     return this.previewUserService.create(users, request.logger);
   }
 
@@ -196,7 +197,7 @@ export class PreviewUserController {
    */
   @Put('/:id')
   public update(
-    @Param('id') id: string,
+    @Params({ validate: true }) { id }: IdValidator,
     @Body({ validate: true }) user: PreviewUserValidator,
     @Req() request: AppRequest
   ): Promise<PreviewUser> {
@@ -224,7 +225,10 @@ export class PreviewUserController {
    *            description: Delete User By Id
    */
   @Delete('/:id')
-  public delete(@Param('id') id: string, @Req() request: AppRequest): Promise<PreviewUser | undefined> {
+  public delete(
+    @Params({ validate: true }) { id }: IdValidator,
+    @Req() request: AppRequest
+  ): Promise<PreviewUser | undefined> {
     return this.previewUserService.delete(id, request.logger);
   }
 
@@ -252,7 +256,10 @@ export class PreviewUserController {
    *            description: Assignment is created
    */
   @Post('/assign')
-  public assign(@Body({ validate: true }) user: PreviewUserValidator, @Req() request: AppRequest): Promise<PreviewUser> {
+  public assign(
+    @Body({ validate: true }) user: PreviewUserValidator,
+    @Req() request: AppRequest
+  ): Promise<PreviewUser> {
     return this.previewUserService.upsertExperimentConditionAssignment(user, request.logger);
   }
 }