[BE] refactor: 기존 api가 리뷰 그룹 정보를 포함하도록 수정 #1050
Conversation
nayonsoso
requested review from
donghoony,
Kimprodp and
skylar1220
as code owners
Test Results157 tests 154 ✅ 4s ⏱️ Results for commit 7a52ab2. |
| @@ -17,8 +18,9 @@ public class HighlightController { | |||
|
|
|||
| private final HighlightService highlightService; | |||
|
|
|||
| @PostMapping("/v2/highlight") | |||
| @PostMapping("/v2/groups/{reviewGroupId}/highlight") | |||
There was a problem hiding this comment.
request에 포함해서 보내는 것이 자연스러워 보여요. 하이라이트가 리뷰 그룹의 하위라는 게 직관적으로 다가오지 않아서요!
There was a problem hiding this comment.
해당 api들이 리뷰 그룹을 필요로 하는 상황이 다 다르고, 그에 따라 방식이 다른 건 당연하다고 생각해서 코멘트 남겨요~
음 생각해보니 그렇네요..! 🙂
굳이 방식을 통일 할 필요가 없었던 것 같아요.
저는 커비에게 설득되었는데, 다른 분들 의견도 더 들어볼게요!
There was a problem hiding this comment.
저도 동의합니다. 우선은 request에 포함되는 것이 좋겠어요.
| @@ -15,8 +16,9 @@ public class SectionController { | |||
|
|
|||
| private final SectionService sectionService; | |||
|
|
|||
| @GetMapping("/v2/sections") | |||
| @GetMapping("/v2/groups/{reviewGroupId}/sections") | |||
There was a problem hiding this comment.
섹션이 리뷰 그룹 하위는 아니라고 느껴져요. 리뷰 그룹 request를 받아서 내려주는 게 어떨까요?
There was a problem hiding this comment.
섹션이 리뷰 그룹 하위는 아니라고 느껴져요.
동의합니다.!
리뷰 그룹 request를 받아서 내려주는 게 어떨까요?
이 부분은 조금 고민되는게, Get이 RequestBody 를 받는게 이상해서요😓
사실 제일 논리적인건 /v2/templates/{templateId}/sections 같아서 이걸로 바꾸고 싶은데,
그런데 이렇게 하려면 클라이언트에서 templateId를 알고 있어야 해서
ReceivedReviewsSummaryResponse 에서 templateId 를 포함해서 내려줘야해요.
AS-IS
{"projectName":"우테코","revieweeName":"산초","totalReviewCount":6}
TO-BE
{"projectName":"우테코","revieweeName":"산초","totalReviewCount":6,"templateId":1}
이 방식은 어떤가요?
There was a problem hiding this comment.
저도 Boby보단, /v2/templates/{templateId}/sections 의견에 동의하나, 우선은 템플릿이 하나이고, 템플릿 추가 예정이 아직은 없으니 기존 uri로 냅두고 템플릿 고정값으로 응답 하는게 좋아보입니다.
| @@ -17,8 +18,9 @@ public class HighlightController { | |||
|
|
|||
| private final HighlightService highlightService; | |||
|
|
|||
| @PostMapping("/v2/highlight") | |||
| @PostMapping("/v2/groups/{reviewGroupId}/highlight") | |||
There was a problem hiding this comment.
저도 동의합니다. 우선은 request에 포함되는 것이 좋겠어요.
| @@ -15,8 +16,9 @@ public class SectionController { | |||
|
|
|||
| private final SectionService sectionService; | |||
|
|
|||
| @GetMapping("/v2/sections") | |||
| @GetMapping("/v2/groups/{reviewGroupId}/sections") | |||
There was a problem hiding this comment.
저도 Boby보단, /v2/templates/{templateId}/sections 의견에 동의하나, 우선은 템플릿이 하나이고, 템플릿 추가 예정이 아직은 없으니 기존 uri로 냅두고 템플릿 고정값으로 응답 하는게 좋아보입니다.
🚀 어떤 기능을 구현했나요 ?
그래서 몇가지 api 에 ❗️리뷰 그룹에 대한 정보를 추가해야 합니다❗️
예를 들어 하이라이트를 추가하는 POST /highlight의 경우,
회원이 이를 호출한다고 하면 "어떤 리뷰 그룹(프로젝트)에 대한 하이라이트 추가인지"를 전달해줄 정보가 필요합니다.
변경이 필요한 uri 들은 아래와 같습니다.
/groups/{id}/highlight/groups/{id}/reviews/groups/{id}/reviews/summary/groups/{id}/reviews/gather/groups/{id}/groups/{id}/sections🔥 어떻게 해결했나요 ?
/groups/1/sections처럼 다소 uri 가 길어지는 단점이 있었으나, 이정도는 넘어갈만 하다 생각합니다. 그리고 위 6개의 api 들에 모두 적용될 수 있다는 점도 장점입니다. 통일감은 중요한 요소이니깐요.📝 어떤 부분에 집중해서 리뷰해야 할까요?
📚 참고 자료, 할 말
간단한 PR이니 이번 코드리뷰는 속도감 있게 갔으면 좋겠습니다.
리뷰 그룹 요약 조회 : GET /groups부분이 커,테가 작업한 부분이랑 겹칩니다. 아마 테스트 코드에서 컨틀릭 날거예요.뭘 먼저 머지하던지 리베이스한 다음 해소하고 머지해야 합니다.