Merge pull request #113 from hackaburg/copilot/fix-notfound-error-replacement

Split RatingController into RatingController and CriteriaController; fix ProjectController bugs

Author: sezanzeb

backend/src/controllers/criteria-controller.ts

@@ -0,0 +1,70 @@
+import {
+  Authorized,
+  Delete,
+  JsonController,
+  CurrentUser,
+  Param,
+  Put,
+  Post,
+  Body
+} from "routing-controllers";
+import { Inject } from "typedi";
+import { UserRole } from "../entities/user-role";
+import { RatingServiceToken, IRatingService } from "../services/rating-service";
+import {
+  CriteriaDTO,
+  SuccessResponseDTO,
+  convertBetweenEntityAndDTO
+} from "./dto";
+import { User } from "../entities/user";
+import { Criteria } from "../entities/criteria";
+
+@JsonController("/criteria")
+export class CriteriaController {
+  public constructor(
+    @Inject(RatingServiceToken) private readonly _ratings: IRatingService,
+  ) {}
+
+  /**
+   * Create criteria.
+   */
+  @Post("/")
+  @Authorized(UserRole.Root)
+  public async createCriteria(
+    @Body() { data: criteriaDTO }: { data: CriteriaDTO },
+  ): Promise<CriteriaDTO> {
+    const criteria = convertBetweenEntityAndDTO(criteriaDTO, Criteria);
+    const createdCriteria = await this._ratings.createCriteria(criteria);
+    return convertBetweenEntityAndDTO(createdCriteria, CriteriaDTO);
+  }
+
+  /**
+   * Update criteria.
+   */
+  @Put("/:id")
+  @Authorized(UserRole.Root)
+  public async updateCriteria(
+    @Param("id") criteriaId: number,
+    @Body() { data: criteriaDTO }: { data: CriteriaDTO },
+  ): Promise<CriteriaDTO> {
+    // TODO There is a TeamUpdateDTO. CriteriaUpdateDTO?
+    const criteria = convertBetweenEntityAndDTO(criteriaDTO, Criteria);
+    const updateCriteria = await this._ratings.updateCriteria(criteria);
+    return convertBetweenEntityAndDTO(updateCriteria, CriteriaDTO);
+  }
+
+  /**
+   * Delete criteria.
+   */
+  @Delete("/:id")
+  @Authorized(UserRole.Root)
+  public async deleteCriteria(
+    @Param("id") criteriaId: number,
+    @CurrentUser() user: User,
+  ): Promise<SuccessResponseDTO> {
+    await this._ratings.deleteCriteriaByID(criteriaId, user);
+    const response = new SuccessResponseDTO();
+    response.success = true;
+    return response;
+  }
+}

backend/src/controllers/project-controller.ts

@@ -1,7 +1,10 @@
-import { Authorized, Delete, JsonController, NotFoundError } from "routing-controllers";
+import { Authorized, Delete, JsonController, NotFoundError, Put, Param, Body, CurrentUser } from "routing-controllers";
 import { Inject } from "typedi";
 import { UserRole } from "../entities/user-role";
 import { IProjectService, ProjectServiceToken } from "../services/project-service";
+import { ProjectDTO, convertBetweenEntityAndDTO } from "./dto";
+import { Project } from "../entities/project";
+import { User } from "../entities/user";
 
 // TODO for every team, add a new project automatically with the correct teamId
 
@@ -19,13 +22,17 @@ export class ProjectController {
   public async updateProject(
     @Param("id") projectId: number,
     @Body() { data: projectDTO }: { data: ProjectDTO },
-  ): Promise<TeamDTO> {
+    @CurrentUser() user: User,
+  ): Promise<ProjectDTO> {
     // TODO ProjectUpdateDTO?
-    const project = convertBetweenEntityAndDTO(projectDTO, Project);
+    const existing = await this._projects.getProjectByID(projectId);
 
-    // TODO how to make actual not found errors for incorrect ids?
+    if (existing == null) {
+      throw new NotFoundError();
+    }
 
-    const updateProject = await this._ratings.updateProject(project, user);
-    return convertBetweenEntityAndDTO(updateProject, ProjectDTO);
+    const project = convertBetweenEntityAndDTO(projectDTO, Project);
+    const updatedProject = await this._projects.updateProject(project, user);
+    return convertBetweenEntityAndDTO(updatedProject, ProjectDTO);
   }
 }

backend/src/controllers/rating-controller.ts

@@ -1,11 +1,7 @@
 import {
   Authorized,
-  Delete,
   JsonController,
-  ForbiddenError,
   CurrentUser,
-  Param,
-  Put,
   Post,
   Body
 } from "routing-controllers";
@@ -15,16 +11,11 @@ import { SettingsServiceToken, ISettingsService } from "../services/settings-ser
 import { RatingServiceToken, IRatingService } from "../services/rating-service";
 import {
   RatingDTO,
-  CriteriaDTO,
-  SuccessResponseDTO,
   convertBetweenEntityAndDTO
 } from "./dto";
 import { User } from "../entities/user";
-import { Criteria } from "../entities/criteria";
 import { Rating } from "../entities/rating";
 
-// TODO separate rating and criteria controller?
-
 @JsonController("/ratings")
 export class RatingController {
   public constructor(
@@ -69,48 +60,5 @@ export class RatingController {
     return convertBetweenEntityAndDTO(createdRating, RatingDTO);
   }
 
-  /**
-   * Create criteria.
-   */
-  @Post("/criteria")
-  @Authorized(UserRole.Root)
-  public async createCriteria(
-    @Body() { data: criteriaDTO }: { data: CriteriaDTO },
-  ): Promise<CriteriaDTO> {
-    const criteria = convertBetweenEntityAndDTO(criteriaDTO, Criteria);
-    const createdCriteria = await this._ratings.createCriteria(criteria);
-    return convertBetweenEntityAndDTO(createdCriteria, CriteriaDTO);
-  }
-
-  /**
-   * Update criteria.
-   */
-  @Put("/criteria/:id")
-  @Authorized(UserRole.Root)
-  public async updateCriteria(
-    @Param("id") criteriaId: number,
-    @Body() { data: criteriaDTO }: { data: CriteriaDTO },
-  ): Promise<CriteriaDTO> {
-    // TODO There is a TeamUpdateDTO. CriteriaUpdateDTO?
-    const criteria = convertBetweenEntityAndDTO(criteriaDTO, Criteria);
-    const updateCriteria = await this._ratings.updateCriteria(criteria);
-    return convertBetweenEntityAndDTO(updateCriteria, CriteriaDTO);
-  }
-
-  /**
-   * Delete criteria.
-   */
-  @Delete("/criteria/:id")
-  @Authorized(UserRole.Root)
-  public async deleteCriteria(
-    @Param("id") criteriaId: number,
-    @CurrentUser() user: User,
-  ): Promise<SuccessResponseDTO> {
-    await this._ratings.deleteCriteriaByID(criteriaId, user);
-    const response = new SuccessResponseDTO();
-    response.success = true;
-    return response;
-  }
-
   // TODO write test that all the root endpoints are not accessible by users
 }

backend/src/services/rating-service.ts

@@ -1,4 +1,4 @@
-import { ForbiddenError } from "routing-controllers";
+import { ForbiddenError, NotFoundError } from "routing-controllers";
 import { Inject, Service, Token } from "typedi";
 import { Repository } from "typeorm";
 import { IService } from ".";
@@ -109,7 +109,7 @@ export class RatingService implements IRatingService {
     // TODO only if user matches
     await this.checkPermission(rating, user);
     if (!originRating) {
-      throw new ForbiddenError("Rating not found");
+      throw new NotFoundError("Rating not found");
     }
     const originRatingUser = originRating.user;
     if (user.id != originRatingUser.id) {
@@ -146,7 +146,7 @@ export class RatingService implements IRatingService {
     const rating = await this._ratings.findOneBy({ id });
 
     if (!rating) {
-      throw new ForbiddenError("Rating not found");
+      throw new NotFoundError("Rating not found");
     }
 
     await this.checkPermission(rating, currentUserId);
@@ -167,7 +167,7 @@ export class RatingService implements IRatingService {
 
     const project = await this._projects.findOneBy({ id: rating.project.id });
     if (!project) {
-      throw new ForbiddenError("Project not found");
+      throw new NotFoundError("Project not found");
     }
     if (!project.allowRating) {
       // TODO test
@@ -176,7 +176,7 @@ export class RatingService implements IRatingService {
 
     const team = await this._teams.findOneBy({ id: project.team.id })
     if (!team) {
-      throw new ForbiddenError("Team not found");
+      throw new NotFoundError("Team not found");
     }
     if (team.users.includes(user.id)) {
       // TODO test