incorporate code review feedback

ls1intum · Oct 1, 2024 · 280514e · 280514e
1 parent 9300fe9
commit 280514e
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 14 deletions.
diff --git a/src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathService.java b/src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathService.java
@@ -11,7 +11,6 @@
 import java.util.stream.Collectors;
 
 import jakarta.validation.constraints.NotNull;
-import jakarta.ws.rs.BadRequestException;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,6 +40,7 @@
 import de.tum.cit.aet.artemis.core.dto.SearchResultPageDTO;
 import de.tum.cit.aet.artemis.core.dto.pageablesearch.SearchTermPageableSearchDTO;
 import de.tum.cit.aet.artemis.core.exception.AccessForbiddenException;
+import de.tum.cit.aet.artemis.core.exception.ConflictException;
 import de.tum.cit.aet.artemis.core.repository.CourseRepository;
 import de.tum.cit.aet.artemis.core.repository.UserRepository;
 import de.tum.cit.aet.artemis.core.util.PageUtil;
@@ -267,14 +267,14 @@ public LearningPathDTO generateLearningPathForCurrentUser(long courseId) {
         final var currentUser = userRepository.getUser();
         final var course = courseRepository.findWithEagerCompetenciesAndPrerequisitesByIdElseThrow(courseId);
         if (learningPathRepository.findByCourseIdAndUserId(courseId, currentUser.getId()).isPresent()) {
-            throw new BadRequestException("Learning path already exists.");
+            throw new ConflictException("Learning path already exists.", "LearningPath", "learningPathAlreadyExists");
         }
         final var learningPath = generateLearningPathForUser(course, currentUser);
         return LearningPathDTO.of(learningPath);
     }
 
     /**
-     * Start the learning path for the current user in the given course.
+     * Start the learning path for the current user
      *
      * @param learningPathId the id of the learning path
      */
@@ -285,7 +285,7 @@ public void startLearningPathForCurrentUser(long learningPathId) {
             throw new AccessForbiddenException("You are not allowed to start this learning path.");
         }
         else if (learningPath.isStartedByStudent()) {
-            throw new BadRequestException("Learning path already started.");
+            throw new ConflictException("Learning path already started.", "LearningPath", "learningPathAlreadyStarted");
         }
         learningPath.setStartedByStudent(true);
         learningPathRepository.save(learningPath);

diff --git a/...e/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts b/...e/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts
@@ -44,10 +44,10 @@ export class LearningPathStudentPageComponent {
     readonly isLearningPathNavigationLoading = this.learningPathNavigationService.isLoading;
 
     constructor() {
-        effect(() => this.loadLearningPathId(this.courseId()), { allowSignalWrites: true });
+        effect(() => this.loadLearningPath(this.courseId()), { allowSignalWrites: true });
     }
 
-    private async loadLearningPathId(courseId: number): Promise<void> {
+    private async loadLearningPath(courseId: number): Promise<void> {
         try {
             this.isLearningPathLoading.set(true);
             const learningPath = await this.learningApiService.getLearningPathForCurrentUser(courseId);

diff --git a/src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts b/src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts
@@ -31,7 +31,7 @@ export abstract class BaseApiHttpService {
      * @param url     The endpoint URL excluding the base server url (/api).
      * @param options The HTTP options to send with the request.
      *
-     * @return  An `Promise` of the response body of type `T`.
+     * @return  A `Promise` of the response body of type `T`.
      */
     private async request<T>(
         method: HttpMethod,
@@ -73,7 +73,7 @@ export abstract class BaseApiHttpService {
      * @param options The HTTP options to send with the request.
      * @protected
      *
-     * @return An `Promise` of type `Object` (T),
+     * @return A `Promise` of type `Object` (T),
      */
     protected async get<T>(
         url: string,
@@ -102,7 +102,7 @@ export abstract class BaseApiHttpService {
      * @param options The HTTP options to send with the request.
      * @protected
      *
-     * @return An `Promise` of type `Object` (T),
+     * @return A `Promise` of type `Object` (T),
      */
     protected async post<T>(
         url: string,
@@ -124,15 +124,15 @@ export abstract class BaseApiHttpService {
     }
 
     /**
-     * Constructs a `PUT` request that interprets the body as JSON and
+     * Constructs a `PATCH` request that interprets the body as JSON and
      * returns a Promise of an object of type `T`.
      *
      * @param url The endpoint URL excluding the base server url (/api).
      * @param body The content to include in the body of the request.
      * @param options The HTTP options to send with the request.
      * @protected
      *
-     * @return An `Promise` of type `Object` (T),
+     * @return A `Promise` of type `Object` (T),
      */
     protected async patch<T>(
         url: string,

diff --git a/src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java
@@ -222,7 +222,7 @@ private void deleteCompetencyRESTCall(Competency competency) throws Exception {
     @WithMockUser(username = STUDENT1_OF_COURSE, roles = "USER")
     void testAll_asStudent() throws Exception {
         this.testAllPreAuthorize();
-        request.get("/api/courses/" + course.getId() + "/learning-path-id", HttpStatus.BAD_REQUEST, Long.class);
+        request.get("/api/courses/" + course.getId() + "/learning-path/me", HttpStatus.BAD_REQUEST, LearningPathDTO.class);
     }
 
     @Test
@@ -613,7 +613,7 @@ void shouldReturnForbiddenIfNotEnabled() throws Exception {
         @WithMockUser(username = STUDENT1_OF_COURSE, roles = "USER")
         void shouldReturnBadRequestIfAlreadyExists() throws Exception {
             course = learningPathUtilService.enableAndGenerateLearningPathsForCourse(course);
-            request.postWithResponseBody("/api/courses/" + course.getId() + "/learning-path", null, LearningPathDTO.class, HttpStatus.BAD_REQUEST);
+            request.postWithResponseBody("/api/courses/" + course.getId() + "/learning-path", null, LearningPathDTO.class, HttpStatus.CONFLICT);
         }
 
         @Test
@@ -652,7 +652,7 @@ void shouldReturnBadRequestIfAlreadyStarted() throws Exception {
             final var learningPath = learningPathRepository.findByCourseIdAndUserIdElseThrow(course.getId(), student.getId());
             learningPath.setStartedByStudent(true);
             learningPathRepository.save(learningPath);
-            request.patch("/api/learning-path/" + learningPath.getId() + "/start", null, HttpStatus.BAD_REQUEST);
+            request.patch("/api/learning-path/" + learningPath.getId() + "/start", null, HttpStatus.CONFLICT);
         }
 
         @Test