[Feature] - 여행 계획 상세 조회 API 구현 #56

eunjungL · 2024-07-18T10:24:41Z

✅ 작업 내용

Swagger label 표시 수정 (여행기 -> 여행계획)
여행 계획 상세 조회 API 구현

📸 스크린샷

🙈 참고 사항

github-actions · 2024-07-18T10:42:49Z

Unit Test Results

0 tests 0 ✔️ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit 7d57b0c.

♻️ This comment has been updated with latest results.

hangillee

한 가지 고민해볼 내용 제외하면 큰 문제 없는 것 같습니다!

코멘트 확인해주세요!

hangillee · 2024-07-19T01:09:57Z

backend/src/main/java/woowacourse/touroot/travelplan/dto/response/TravelPlanResponse.java

+        @Schema(description = "여행 계획 날짜별 정보") List<TravelPlanDayResponse> days
+) {
+
+    public static TravelPlanResponse of(TravelPlan travelPlan, List<TravelPlanDayResponse> days) {


정적 팩토리 메소드 네이밍 👍

hangillee · 2024-07-19T01:13:15Z

backend/src/test/java/woowacourse/touroot/travelplan/controller/TravelPlanControllerTest.java

+        PlanPlaceCreateRequest planPlaceCreateRequest = new PlanPlaceCreateRequest(
+                "잠실한강공원",
+                "신나는 여행 장소",
+                0,
+                locationRequest
+        );


어차피 여러 줄에 걸쳐서 생성자 인자를 넘길 것이라면 @Builder를 활용하는 건 어떻게 생각하시나요?

동감합니다리 빌더나 fixture운용하는 것이 가장 좋아보여요
검증하는 부분이 강조될 수 있도록요!

가독성을 높이기 위해 builder를 도입했습니다!
Fixture도 TravelPlan 패키지에 request Fixture를 따로 만들어서 사용해볼까 생각 중입니다.
이 부분은 같이 논의해보고 도입할게요!

Libienz

안녕하세요 클로버!
작업 고생하셨습니다 👍

코드에 대한 제 견해 코멘트로 남겨보았어요.
다만 dev브랜치에 빠르게 기능 추가하는 것을 우선하고 있는 저희로써 나중에 생각해볼만한 거리들이라고 생각해요

이에 우선 approve 드립니다!

Libienz · 2024-07-19T03:37:56Z

backend/src/main/java/woowacourse/touroot/global/exception/GlobalExceptionHandler.java

+    @ExceptionHandler(BadRequestException.class)
+    public ResponseEntity<ExceptionResponse> handleBadRequestException(BadRequestException exception) {
+        log.info("BAD_REQUEST_EXCEPTION :: message = {}", exception.getMessage());
+        ExceptionResponse data = new ExceptionResponse(exception.getMessage());
+        return ResponseEntity.badRequest()
+                .body(data);
+    }


예외 핸들러 확인했습니다.
다음에 작성할 일이 있으면 비슷한 포맷으로 저도 작성하도록 할게요!

Libienz · 2024-07-19T03:39:36Z

backend/src/main/java/woowacourse/touroot/global/exception/dto/ExceptionResponse.java

+package woowacourse.touroot.global.exception.dto;
+
+public record ExceptionResponse(String message) {
+}


현재는 예외의 종류가 많지 않아서 이정도가 담백한 구현인 듯 해요!
다만 BadRequest의 종류가 다양해진다면 별도로 운용하는 에러 코드가 필요해질수도 있을 것 같다는 생각이에요 🤔
반영은 저희 나중에 생각하시죠..!
다만 생각은 하고 있으면 좋을 것 같아 코멘트 남겨놉니다 👍

에러 코드 도입하는건 저도 긍정적으로 생각합니다~!
이후 케이스가 많아졌을 떄 한 번 얘기해보죵

좀 더 표준화된 방식인 ProblemDetail을 사용하는 것에 대해서는 어떻게 생각하실까요?!

problem detail이 제공되는건 처음 알았네요~! 예외 리팩토링 진행 시 반영하겠습니다
굳굳 낙낙쓰

Libienz · 2024-07-19T03:42:53Z

backend/src/test/java/woowacourse/touroot/travelplan/controller/TravelPlanControllerTest.java

+        PlanPlaceCreateRequest planPlaceCreateRequest = new PlanPlaceCreateRequest(
+                "잠실한강공원",
+                "신나는 여행 장소",
+                0,
+                locationRequest
+        );


동감합니다리 빌더나 fixture운용하는 것이 가장 좋아보여요
검증하는 부분이 강조될 수 있도록요!

Libienz · 2024-07-19T04:14:14Z

backend/src/test/java/woowacourse/touroot/travelplan/controller/TravelPlanControllerTest.java

+        RestAssured.given().log().all()
+                .contentType(ContentType.JSON)
+                .when().log().all()
+                .get("/api/v1/travel-plans/" + id)
+                .then().log().all()
+                .statusCode(400)
+                .body("message", is("존재하지 않는 여행 계획입니다."));
+    }


바디 검증을 특정 필드만이 아닌 객체전체를 진행하는 것은 어떻게 생각하시나요?

저는 모든 필드의 검증이 필요하다고 생각해서 ObjectMapper의 writeValueAsString을 선호하는 편입니다.
만약 정하게 된다면 저희 RestAssured 테스트의 컨벤션으로 승화될 수 있겠네요.

생각해볼 필요가 있겠습니다 🤔

저는 DB에서 값을 꺼내주는 만큼 특정 값이 검증되면 충분하다고 생각하는데요, 객체 전체 테스트의 필요성에 대해서도 동감합니다!
RestAssured 컨벤션으로 팀원 전체가 맞춰볼 수 있게 리비가 스크럼 때 말씀 한 번 해주시죠!
요 부분에 대해서는 컨벤션이 정해지면 한 번 전체적으로 리팩토링하겠습니다 👍

eunjungL

말씀해주신 부분 반영했습니다!
다들 어프로브 주셔서 일단 머지할게요~
다른 부분은 스크럼 때 한 번 컨벤션을 정해봐요

eunjungL · 2024-07-19T04:38:50Z

backend/src/main/java/woowacourse/touroot/global/exception/dto/ExceptionResponse.java

+package woowacourse.touroot.global.exception.dto;
+
+public record ExceptionResponse(String message) {
+}


에러 코드 도입하는건 저도 긍정적으로 생각합니다~!
이후 케이스가 많아졌을 떄 한 번 얘기해보죵

eunjungL · 2024-07-19T04:46:19Z

backend/src/test/java/woowacourse/touroot/travelplan/controller/TravelPlanControllerTest.java

+        PlanPlaceCreateRequest planPlaceCreateRequest = new PlanPlaceCreateRequest(
+                "잠실한강공원",
+                "신나는 여행 장소",
+                0,
+                locationRequest
+        );


가독성을 높이기 위해 builder를 도입했습니다!
Fixture도 TravelPlan 패키지에 request Fixture를 따로 만들어서 사용해볼까 생각 중입니다.
이 부분은 같이 논의해보고 도입할게요!

eunjungL · 2024-07-19T04:49:29Z

backend/src/test/java/woowacourse/touroot/travelplan/controller/TravelPlanControllerTest.java

+        RestAssured.given().log().all()
+                .contentType(ContentType.JSON)
+                .when().log().all()
+                .get("/api/v1/travel-plans/" + id)
+                .then().log().all()
+                .statusCode(400)
+                .body("message", is("존재하지 않는 여행 계획입니다."));
+    }


저는 DB에서 값을 꺼내주는 만큼 특정 값이 검증되면 충분하다고 생각하는데요, 객체 전체 테스트의 필요성에 대해서도 동감합니다!
RestAssured 컨벤션으로 팀원 전체가 맞춰볼 수 있게 리비가 스크럼 때 말씀 한 번 해주시죠!
요 부분에 대해서는 컨벤션이 정해지면 한 번 전체적으로 리팩토링하겠습니다 👍

nak-honest

예외 관련해서 간단한 의견만 남겨보았습니닷!
고생 많으셨어욥 응로버~~~

nak-honest · 2024-07-20T05:48:54Z

backend/src/main/java/woowacourse/touroot/global/exception/dto/ExceptionResponse.java

+package woowacourse.touroot.global.exception.dto;
+
+public record ExceptionResponse(String message) {
+}


좀 더 표준화된 방식인 ProblemDetail을 사용하는 것에 대해서는 어떻게 생각하실까요?!

nak-honest · 2024-07-20T05:52:07Z

backend/src/main/java/woowacourse/touroot/global/exception/GlobalExceptionHandler.java

+    @ExceptionHandler(BadRequestException.class)
+    public ResponseEntity<ExceptionResponse> handleBadRequestException(BadRequestException exception) {
+        log.info("BAD_REQUEST_EXCEPTION :: message = {}", exception.getMessage());
+        ExceptionResponse data = new ExceptionResponse(exception.getMessage());
+        return ResponseEntity.badRequest()
+                .body(data);
+    }


지금은 400 에러밖에 없지만, 이후 다양한 번호의 에러를 응답해 줘야 할때 조금 복잡해질수도 있을 것 같아요..!
예외 자체에 StatusCode를 두고, 이를 ResponseEntity가 받게하는 구조는 어떻게 생각하시나욥!!

예외 자체에 status code를 두는 방법 좋은 것 같습니다! 예외 메시지도 아예 둬버리면 더 통일되게 관리할 수도 있을 것 같네용
예외 관련해서 BE 회의 한 번 진행하시죠~! 👍

* fix: TravelPlanController Swagger label 여행기 -> 여행 계획으로 변경 * fix: PlanPlaceCreateRequest 장소명과 설명 예시 바뀐 곳 수정 * feat: 여행 계획 상세 조회 API 구현 * feat: 여행 계획 상세 조회 swagger 설명 추가 * feat: TravelPlanController Swagger에 Error 응답 추가 * feat: GlobalExceptionHandler 추가 * feat: TravelPlan 응답에서 날짜, 장소 순서별로 정렬 추가 * fix: TravelPlanResponse에 TravelPlan id 추가 * test: TravelPlanController 테스트 추가 * test: TravelPlanService 테스트 추가

eunjungL added 10 commits July 18, 2024 17:05

fix: TravelPlanController Swagger label 여행기 -> 여행 계획으로 변경

837b7c8

fix: PlanPlaceCreateRequest 장소명과 설명 예시 바뀐 곳 수정

a262831

feat: 여행 계획 상세 조회 API 구현

b6bd753

feat: 여행 계획 상세 조회 swagger 설명 추가

1d36bdd

feat: TravelPlanController Swagger에 Error 응답 추가

f25a089

feat: GlobalExceptionHandler 추가

63d0088

feat: TravelPlan 응답에서 날짜, 장소 순서별로 정렬 추가

6465dd6

fix: TravelPlanResponse에 TravelPlan id 추가

bb36c54

test: TravelPlanController 테스트 추가

0afa90f

test: TravelPlanService 테스트 추가

6a1eac2

eunjungL added the BE label Jul 18, 2024

eunjungL added this to the sprint 2 milestone Jul 18, 2024

eunjungL self-assigned this Jul 18, 2024

eunjungL requested review from nak-honest and hangillee July 18, 2024 10:24

hangillee approved these changes Jul 19, 2024

View reviewed changes

Libienz approved these changes Jul 19, 2024

View reviewed changes

refactor: TravelPlan test 코드 build 도입 및 생성자 주입으로 변경

7d57b0c

eunjungL commented Jul 19, 2024

View reviewed changes

eunjungL merged commit ebe87b2 into develop/be Jul 19, 2024
3 checks passed

eunjungL deleted the feature/be/#52 branch July 19, 2024 04:51

nak-honest reviewed Jul 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] - 여행 계획 상세 조회 API 구현 #56

[Feature] - 여행 계획 상세 조회 API 구현 #56

eunjungL commented Jul 18, 2024

github-actions bot commented Jul 18, 2024 •

edited

Loading

hangillee left a comment

hangillee Jul 19, 2024

hangillee Jul 19, 2024

Libienz Jul 19, 2024

eunjungL Jul 19, 2024

Libienz left a comment

Libienz Jul 19, 2024

Libienz Jul 19, 2024

eunjungL Jul 19, 2024

nak-honest Jul 20, 2024

eunjungL Jul 20, 2024

Libienz Jul 19, 2024

Libienz Jul 19, 2024

eunjungL Jul 19, 2024

eunjungL left a comment

eunjungL Jul 19, 2024

eunjungL Jul 19, 2024

eunjungL Jul 19, 2024

nak-honest left a comment

nak-honest Jul 20, 2024

nak-honest Jul 20, 2024

eunjungL Jul 20, 2024

[Feature] - 여행 계획 상세 조회 API 구현 #56

[Feature] - 여행 계획 상세 조회 API 구현 #56

Conversation

eunjungL commented Jul 18, 2024

✅ 작업 내용

📸 스크린샷

🙈 참고 사항

github-actions bot commented Jul 18, 2024 • edited Loading

Unit Test Results

hangillee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Libienz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjungL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nak-honest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 18, 2024 •

edited

Loading