From ff4c1e01fc6cb41c2f2dad5c6a75f2431b3466bc Mon Sep 17 00:00:00 2001 From: Ivan Alglave <ivanalglave@outlook.fr> Date: Sun, 11 Dec 2022 11:49:49 +0100 Subject: [PATCH] fix: PUT internships/studentId doesn't require a tracking body member anymore, and will correctly update. Changed save to make sure studentId is unique on insert. Custom error handling. --- src/internships/dao/internships.dao.ts | 67 ++++++++++++------- .../dto/nested-create/tracking.dto.ts | 4 +- .../nested-entities/tracking.entity.ts | 2 +- src/internships/internships.controller.ts | 15 ++++- src/internships/internships.service.ts | 3 +- src/shared/HttpError.ts | 13 ++++ src/shared/InternshipState.ts | 20 ++++++ src/shared/InternshipStatus.ts | 4 ++ 8 files changed, 95 insertions(+), 33 deletions(-) create mode 100644 src/shared/HttpError.ts create mode 100644 src/shared/InternshipState.ts create mode 100644 src/shared/InternshipStatus.ts diff --git a/src/internships/dao/internships.dao.ts b/src/internships/dao/internships.dao.ts index c42a2b4..8a573c1 100644 --- a/src/internships/dao/internships.dao.ts +++ b/src/internships/dao/internships.dao.ts @@ -1,25 +1,23 @@ -import { - Injectable, - NotFoundException, - InternalServerErrorException, -} from '@nestjs/common'; +import { Injectable, NotFoundException } from '@nestjs/common'; import { InjectModel } from '@nestjs/mongoose'; import { Model } from 'mongoose'; +import { CONFLICT } from 'src/shared/HttpError'; +import { STATE_1 } from 'src/shared/InternshipState'; +import { STATUS_NOK } from 'src/shared/InternshipStatus'; import { CreateInternshipDto } from '../dto/create-internship.dto'; import { InternshipDto } from '../dto/internship.dto'; -import { TrackingDto } from '../dto/nested-create/tracking.dto'; import { Internship } from '../schemas/internship.schema'; @Injectable() export class InternshipDao { constructor( @InjectModel(Internship.name) - private readonly _groupModel: Model<Internship>, + private readonly _internshipModel: Model<Internship>, ) {} find = (): Promise<Internship[]> => new Promise((resolve, reject) => { - this._groupModel.find({}, {}, {}, (err, value) => { + this._internshipModel.find({}, {}, {}, (err, value) => { if (err) reject(err.message); if (!value) reject('No values'); resolve(value); @@ -28,7 +26,7 @@ export class InternshipDao { findByStudentId = (studentId: string): Promise<Internship | void> => new Promise((resolve, reject) => { - this._groupModel.findOne({ studentId }, {}, {}, (err, value) => { + this._internshipModel.findOne({ studentId }, {}, {}, (err, value) => { if (err) reject(err.message); if (!value) reject(new NotFoundException()); resolve(value); @@ -37,29 +35,34 @@ export class InternshipDao { save = (internship: CreateInternshipDto): Promise<Internship> => new Promise((resolve, reject) => { - // do smth - const _internship: InternshipDto = { - ...internship, - tracking: { - state: 'state-1', - status: 'pending', + // Use updateOne with `upsert: true` to only insert when no other document has the same studentId to prevent duplicata + const decoratedInternship = this.toInternshipDtoWithTracking(internship); + this._internshipModel.updateOne( + { studentId: internship.studentId }, + { $setOnInsert: decoratedInternship }, + { + upsert: true, + runValidators: true, }, - }; - new this._groupModel(_internship).save((err, value) => { - if (err) reject(err.message); - if (!value) reject(new InternalServerErrorException()); - resolve(value); - }); + (err, value) => { + const { upsertedCount } = value; + if (err) reject(err.message); + if (upsertedCount === 0) reject(CONFLICT); + resolve(decoratedInternship as Internship); + }, + ); }); findByStudentIdAndUpdate = ( studentId: string, - internship: InternshipDto, + internship: CreateInternshipDto, ): Promise<Internship | void> => new Promise((resolve, reject) => { - this._groupModel.findOneAndReplace( - { studentId }, - internship, + // Check if information modification is allowed -> current state is information input by student and updating is allowed + const decoratedInternship = this.toInternshipDtoWithTracking(internship); + this._internshipModel.findOneAndReplace( + { studentId, 'tracking.state': STATE_1, 'tracking.status': STATUS_NOK }, + decoratedInternship, { new: true, runValidators: true, @@ -73,9 +76,21 @@ export class InternshipDao { findByStudentIdAndRemove = (studentId: string): Promise<Internship | void> => new Promise((resolve, reject) => { - this._groupModel.findOneAndDelete({ studentId }, {}, (err) => { + this._internshipModel.findOneAndDelete({ studentId }, {}, (err) => { if (err) reject(err.message); resolve(); }); }); + + toInternshipDtoWithTracking = ( + createInternshipDto: CreateInternshipDto, + ): InternshipDto => { + return { + ...createInternshipDto, + tracking: { + state: 'state-1', + status: 'nok', + }, + }; + }; } diff --git a/src/internships/dto/nested-create/tracking.dto.ts b/src/internships/dto/nested-create/tracking.dto.ts index 05f9a83..b14354b 100644 --- a/src/internships/dto/nested-create/tracking.dto.ts +++ b/src/internships/dto/nested-create/tracking.dto.ts @@ -3,9 +3,9 @@ import { IsString, IsNotEmpty } from 'class-validator'; export class TrackingDto { @IsString() @IsNotEmpty() - status: string; + state: string; @IsString() @IsNotEmpty() - state: string; + status: string; } diff --git a/src/internships/entities/nested-entities/tracking.entity.ts b/src/internships/entities/nested-entities/tracking.entity.ts index 6838aeb..a41f5e7 100644 --- a/src/internships/entities/nested-entities/tracking.entity.ts +++ b/src/internships/entities/nested-entities/tracking.entity.ts @@ -1,6 +1,6 @@ export class TrackingEntity { - status: string; state: string; + status: string; constructor(partial: Partial<TrackingEntity>) { Object.assign(this, partial); diff --git a/src/internships/internships.controller.ts b/src/internships/internships.controller.ts index 066e877..0c43ac8 100644 --- a/src/internships/internships.controller.ts +++ b/src/internships/internships.controller.ts @@ -8,9 +8,10 @@ import { Body, UseInterceptors, } from '@nestjs/common'; +import { BAD_TRACKING_STATE, CUSTOM } from 'src/shared/HttpError'; +import * as InternshipStates from 'src/shared/InternshipState'; import { HttpInterceptor } from '../interceptors/http.interceptor'; import { CreateInternshipDto } from './dto/create-internship.dto'; -import { InternshipDto } from './dto/internship.dto'; import { InternshipEntity } from './entities/internship.entity'; import { InternshipService } from './internships.service'; @@ -41,11 +42,21 @@ export class InternshipsController { @Put(':studentId') update( @Param() params: { studentId: string }, - @Body() internshipDto: InternshipDto, + @Body() internshipDto: CreateInternshipDto, ): Promise<InternshipEntity | void> { return this._groupsService.update(params.studentId, internshipDto); } + @Put(':studentId/tracking') + updateState( + @Param() params: { studentId: string }, + @Body() body: { state: string; content: string }, + ) { + if (!InternshipStates.isAllowedState(body.state)) + throw BAD_TRACKING_STATE(body.state); + // Treat request and update tracking -> implement service + dao + } + @Delete(':studentId') delete( @Param() params: { studentId: string }, diff --git a/src/internships/internships.service.ts b/src/internships/internships.service.ts index 89d8f08..5bdb50d 100644 --- a/src/internships/internships.service.ts +++ b/src/internships/internships.service.ts @@ -1,7 +1,6 @@ import { Injectable } from '@nestjs/common'; import { InternshipDao } from './dao/internships.dao'; import { CreateInternshipDto } from './dto/create-internship.dto'; -import { InternshipDto } from './dto/internship.dto'; import { InternshipEntity } from './entities/internship.entity'; @Injectable() @@ -19,7 +18,7 @@ export class InternshipService { update = ( studentId: string, - internship: InternshipDto, + internship: CreateInternshipDto, ): Promise<InternshipEntity | void> => this._internshipsDao.findByStudentIdAndUpdate(studentId, internship); diff --git a/src/shared/HttpError.ts b/src/shared/HttpError.ts new file mode 100644 index 0000000..70f9310 --- /dev/null +++ b/src/shared/HttpError.ts @@ -0,0 +1,13 @@ +import { + HttpException, + NotFoundException, + ConflictException, + UnprocessableEntityException, +} from '@nestjs/common'; + +export const NOT_FOUND = new NotFoundException(); +export const CONFLICT = new ConflictException(); +export const BAD_TRACKING_STATE = (badState: string) => + new UnprocessableEntityException(`Unknown state [${badState}]`); +export const CUSTOM = (reason: string, errorStatus: number) => + new HttpException(reason, errorStatus); diff --git a/src/shared/InternshipState.ts b/src/shared/InternshipState.ts new file mode 100644 index 0000000..26b492d --- /dev/null +++ b/src/shared/InternshipState.ts @@ -0,0 +1,20 @@ +export const STATE_1 = 'state-1'; +export const STATE_2 = 'state-2'; +export const STATE_3 = 'state-3'; +export const STATE_4 = 'state-4'; +export const STATE_5 = 'state-5'; +export const STATE_6 = 'state-6'; +export const STATE_7 = 'state-7'; + +export const STATES = [ + STATE_1, + STATE_2, + STATE_3, + STATE_4, + STATE_5, + STATE_6, + STATE_7, +]; + +export const isAllowedState = (potentialState: string): boolean => + STATES.includes(potentialState); diff --git a/src/shared/InternshipStatus.ts b/src/shared/InternshipStatus.ts new file mode 100644 index 0000000..d375607 --- /dev/null +++ b/src/shared/InternshipStatus.ts @@ -0,0 +1,4 @@ +export const STATUS_OK = 'ok'; +export const STATUS_NOK = 'nok'; + +export type InternshipState = 'ok' | 'nok'; -- GitLab