From 28ecb158a913422c1b96be328812cb6e5b6c662d Mon Sep 17 00:00:00 2001 From: olav Date: Fri, 17 Jun 2022 08:15:56 +0200 Subject: [PATCH] refactor: add schemas to feedback controller (#1698) * refactor: remove previous getProjects route * refactor: add schemas to feedback controller --- src/lib/openapi/index.ts | 2 + src/lib/openapi/spec/feedback-schema.ts | 27 ++++ src/lib/routes/admin-api/project/index.ts | 2 - src/lib/routes/admin-api/user-feedback.ts | 116 +++++++++++------- src/test/e2e/api/admin/feedback.e2e.test.ts | 2 +- src/test/e2e/api/admin/state.e2e.test.ts | 3 - .../__snapshots__/openapi.e2e.test.ts.snap | 93 ++++++++++++++ 7 files changed, 197 insertions(+), 48 deletions(-) create mode 100644 src/lib/openapi/spec/feedback-schema.ts diff --git a/src/lib/openapi/index.ts b/src/lib/openapi/index.ts index 364009cd69..0a97c6aae6 100644 --- a/src/lib/openapi/index.ts +++ b/src/lib/openapi/index.ts @@ -12,6 +12,7 @@ import { featureTypeSchema } from './spec/feature-type-schema'; import { featureTypesSchema } from './spec/feature-types-schema'; import { featureVariantsSchema } from './spec/feature-variants-schema'; import { featuresSchema } from './spec/features-schema'; +import { feedbackSchema } from './spec/feedback-schema'; import { healthOverviewSchema } from './spec/health-overview-schema'; import { healthReportSchema } from './spec/health-report-schema'; import { mapValues } from '../util/map-values'; @@ -54,6 +55,7 @@ export const schemas = { featureTypesSchema, featureVariantsSchema, featuresSchema, + feedbackSchema, healthOverviewSchema, healthReportSchema, overrideSchema, diff --git a/src/lib/openapi/spec/feedback-schema.ts b/src/lib/openapi/spec/feedback-schema.ts new file mode 100644 index 0000000000..390c2870a9 --- /dev/null +++ b/src/lib/openapi/spec/feedback-schema.ts @@ -0,0 +1,27 @@ +import { FromSchema } from 'json-schema-to-ts'; + +export const feedbackSchema = { + $id: '#/components/schemas/feedbackSchema', + type: 'object', + additionalProperties: false, + required: [], + properties: { + userId: { + type: 'number', + }, + feedbackId: { + type: 'string', + }, + neverShow: { + type: 'boolean', + }, + given: { + type: 'string', + format: 'date-time', + nullable: true, + }, + }, + components: {}, +} as const; + +export type FeedbackSchema = FromSchema; diff --git a/src/lib/routes/admin-api/project/index.ts b/src/lib/routes/admin-api/project/index.ts index abb6845bf7..4e97d5f3c3 100644 --- a/src/lib/routes/admin-api/project/index.ts +++ b/src/lib/routes/admin-api/project/index.ts @@ -26,8 +26,6 @@ export default class ProjectApi extends Controller { this.projectService = services.projectService; this.openApiService = services.openApiService; - this.get('/', this.getProjects); - this.route({ path: '', method: 'get', diff --git a/src/lib/routes/admin-api/user-feedback.ts b/src/lib/routes/admin-api/user-feedback.ts index 46dcbe2c5f..00ff7bf9f3 100644 --- a/src/lib/routes/admin-api/user-feedback.ts +++ b/src/lib/routes/admin-api/user-feedback.ts @@ -1,5 +1,4 @@ import { Response } from 'express'; - import Controller from '../controller'; import { Logger } from '../../logger'; import { IUnleashConfig } from '../../types/option'; @@ -7,73 +6,106 @@ import { IUnleashServices } from '../../types/services'; import UserFeedbackService from '../../services/user-feedback-service'; import { IAuthRequest } from '../unleash-types'; import { NONE } from '../../types/permissions'; - -interface IFeedbackBody { - neverShow?: boolean; - feedbackId: string; - given?: Date; -} +import { OpenApiService } from '../../services/openapi-service'; +import { + feedbackSchema, + FeedbackSchema, +} from '../../openapi/spec/feedback-schema'; +import { serializeDates } from '../../types/serialize-dates'; +import { parseISO } from 'date-fns'; +import { createRequestSchema, createResponseSchema } from '../../openapi'; +import BadDataError from '../../error/bad-data-error'; class UserFeedbackController extends Controller { private logger: Logger; private userFeedbackService: UserFeedbackService; + private openApiService: OpenApiService; + constructor( config: IUnleashConfig, - { userFeedbackService }: Pick, + { + userFeedbackService, + openApiService, + }: Pick, ) { super(config); this.logger = config.getLogger('feedback-controller.ts'); this.userFeedbackService = userFeedbackService; + this.openApiService = openApiService; - this.post('/', this.recordFeedback, NONE); - this.put('/:id', this.updateFeedbackSettings, NONE); + this.route({ + method: 'post', + path: '', + handler: this.createFeedback, + permission: NONE, + middleware: [ + openApiService.validPath({ + tags: ['admin'], + operationId: 'createFeedback', + requestBody: createRequestSchema('feedbackSchema'), + responses: { 200: createResponseSchema('feedbackSchema') }, + }), + ], + }); + + this.route({ + method: 'put', + path: '/:id', + handler: this.updateFeedback, + permission: NONE, + middleware: [ + openApiService.validPath({ + tags: ['admin'], + operationId: 'updateFeedback', + requestBody: createRequestSchema('feedbackSchema'), + responses: { 200: createResponseSchema('feedbackSchema') }, + }), + ], + }); } - private async recordFeedback( - req: IAuthRequest, - res: Response, + private async createFeedback( + req: IAuthRequest, + res: Response, ): Promise { - const BAD_REQUEST = 400; - const { user } = req; - - const { feedbackId } = req.body; - - if (!feedbackId) { - res.status(BAD_REQUEST).json({ - error: 'feedbackId must be present.', - }); - return; + if (!req.body.feedbackId) { + throw new BadDataError('Missing feedbackId'); } - const feedback = { - ...req.body, - userId: user.id, + const updated = await this.userFeedbackService.updateFeedback({ + feedbackId: req.body.feedbackId, + userId: req.user.id, given: new Date(), neverShow: req.body.neverShow || false, - }; + }); - const updated = await this.userFeedbackService.updateFeedback(feedback); - res.json(updated); + this.openApiService.respondWithValidation( + 200, + res, + feedbackSchema.$id, + serializeDates(updated), + ); } - private async updateFeedbackSettings( - req: IAuthRequest, - res: Response, + private async updateFeedback( + req: IAuthRequest<{ id: string }, unknown, FeedbackSchema>, + res: Response, ): Promise { - const { user } = req; - const { id } = req.params; - - const feedback = { - ...req.body, - feedbackId: id, - userId: user.id, + const updated = await this.userFeedbackService.updateFeedback({ + feedbackId: req.params.id, + userId: req.user.id, neverShow: req.body.neverShow || false, - }; + given: req.body.given && parseISO(req.body.given), + }); - const updated = await this.userFeedbackService.updateFeedback(feedback); - res.json(updated); + this.openApiService.respondWithValidation( + 200, + res, + feedbackSchema.$id, + serializeDates(updated), + ); } } diff --git a/src/test/e2e/api/admin/feedback.e2e.test.ts b/src/test/e2e/api/admin/feedback.e2e.test.ts index dc9205ed89..3596712437 100644 --- a/src/test/e2e/api/admin/feedback.e2e.test.ts +++ b/src/test/e2e/api/admin/feedback.e2e.test.ts @@ -65,7 +65,7 @@ test('it gives 400 when feedback is not present', async () => { .expect('Content-Type', /json/) .expect(400) .expect((res) => { - expect(res.body.error).toBeTruthy(); + expect(res.body.name).toEqual('BadDataError'); }); }); diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index c6623c55b7..7ccf1856c7 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -435,8 +435,5 @@ test(`should clean apitokens for not existing environment after import with drop .expect(202); const apiTokens = await app.services.apiTokenService.getAllTokens(); - - console.log(apiTokens); - expect(apiTokens.length).toEqual(0); }); diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 5c1d2bfccc..23db22dfc6 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -422,6 +422,27 @@ Object { ], "type": "object", }, + "feedbackSchema": Object { + "additionalProperties": false, + "properties": Object { + "feedbackId": Object { + "type": "string", + }, + "given": Object { + "format": "date-time", + "nullable": true, + "type": "string", + }, + "neverShow": Object { + "type": "boolean", + }, + "userId": Object { + "type": "number", + }, + }, + "required": Array [], + "type": "object", + }, "healthOverviewSchema": Object { "additionalProperties": false, "properties": Object { @@ -1429,6 +1450,78 @@ Object { ], }, }, + "/api/admin/feedback": Object { + "post": Object { + "operationId": "createFeedback", + "requestBody": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/feedbackSchema", + }, + }, + }, + "description": "feedbackSchema", + "required": true, + }, + "responses": Object { + "200": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/feedbackSchema", + }, + }, + }, + "description": "feedbackSchema", + }, + }, + "tags": Array [ + "admin", + ], + }, + }, + "/api/admin/feedback/{id}": Object { + "put": Object { + "operationId": "updateFeedback", + "parameters": Array [ + Object { + "in": "path", + "name": "id", + "required": true, + "schema": Object { + "type": "string", + }, + }, + ], + "requestBody": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/feedbackSchema", + }, + }, + }, + "description": "feedbackSchema", + "required": true, + }, + "responses": Object { + "200": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/feedbackSchema", + }, + }, + }, + "description": "feedbackSchema", + }, + }, + "tags": Array [ + "admin", + ], + }, + }, "/api/admin/projects": Object { "get": Object { "operationId": "getProjects",