-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BE] docs: 작성한 리뷰 목록 조회, 회원용 리뷰 그룹 생성, 리뷰 그룹 정보, 리뷰 등록 API 문서를 작성한다. #1017
base: develop
Are you sure you want to change the base?
Changes from all commits
490780f
1585e94
3eff679
5dd5322
b1c5cb4
4e3f3df
bd2d97a
1d05f2a
8751114
13c10ed
de17b00
201363b
20010fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
==== 자신이 받은 리뷰 목록 조회 | ||
|
||
operation::received-review-list-with-pagination[snippets="curl-request,request-cookies,query-parameters,http-response,response-fields"] | ||
|
||
==== 자신이 작성한 리뷰 목록 조회 | ||
|
||
operation::written-review-list-with-pagination[snippets="curl-request,query-parameters,http-response,response-fields"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
import reviewme.review.service.dto.response.gathered.ReviewsGatheredBySectionResponse; | ||
import reviewme.review.service.dto.response.list.ReceivedReviewsResponse; | ||
import reviewme.review.service.dto.response.list.ReceivedReviewsSummaryResponse; | ||
import reviewme.review.service.dto.response.list.WrittenReviewsResponse; | ||
import reviewme.reviewgroup.controller.ReviewGroupSession; | ||
import reviewme.reviewgroup.domain.ReviewGroup; | ||
|
||
|
@@ -35,6 +36,7 @@ public class ReviewController { | |
|
||
@PostMapping("/v2/reviews") | ||
public ResponseEntity<Void> createReview(@Valid @RequestBody ReviewRegisterRequest request) { | ||
// 회원 세션 추후 추가해야 함 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 다시 보니, 이 api는 /v2/reviews api 하나로 처리해도 상관없어 보여요.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
회원이 리뷰를 작성할 땐, '내가 작성한 리뷰'에 추가해줘야 하니깐요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #1017 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코멘트 남겼어요 확인 부탁합니다~ |
||
long savedReviewId = reviewRegisterService.registerReview(request); | ||
return ResponseEntity.created(URI.create("/reviews/" + savedReviewId)).build(); | ||
} | ||
|
@@ -75,4 +77,15 @@ public ResponseEntity<ReviewsGatheredBySectionResponse> getReceivedReviewsBySect | |
reviewGatheredLookupService.getReceivedReviewsBySectionId(reviewGroup, sectionId); | ||
return ResponseEntity.ok(response); | ||
} | ||
|
||
@GetMapping("/v2/written") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 api의 url에서 "written"만 있는 것은 어떤 자원을 찾는지 명확하지 않아보여서 변경이 필요해 보이네요.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1번 방법이 좋아보여요. 내가 받은 리뷰가 /v2/reviews인데 이것도 함께 변경하면 좋을 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
어떤걸 선택한거죠?? 😮😮 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 메서드, 내가 작성한 리뷰는 왜냐면 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
동의합니다만! 내가 받은 리뷰 : v2/reviews/received 로 형용사로 통일하는건 어떤가요? |
||
public ResponseEntity<WrittenReviewsResponse> findWrittenReviews( | ||
@RequestParam(required = false) Long lastReviewId, | ||
nayonsoso marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@RequestParam(required = false) Integer size | ||
// @MemberSession Member member | ||
// TODO: 세션을 활용한 권한 체계에 따른 추가 조치 필요 | ||
nayonsoso marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) { | ||
WrittenReviewsResponse response = reviewListLookupService.getWrittenReviews(lastReviewId, size); | ||
return ResponseEntity.ok(response); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package reviewme.review.service.dto.response.list; | ||
|
||
import java.time.LocalDate; | ||
import java.util.List; | ||
|
||
public record WrittenReviewElementResponse( | ||
long reviewId, | ||
String revieweeName, | ||
String projectName, | ||
LocalDate createdAt, | ||
String contentPreview, | ||
List<ReviewCategoryResponse> categories | ||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package reviewme.review.service.dto.response.list; | ||
|
||
import java.util.List; | ||
|
||
public record WrittenReviewsResponse( | ||
List<WrittenReviewElementResponse> reviews, | ||
long lastReviewId, | ||
boolean isLastPage | ||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package reviewme.reviewgroup.service.dto; | ||
|
||
import jakarta.validation.constraints.NotEmpty; | ||
|
||
public record MemberReviewGroupCreationRequest( | ||
|
||
@NotEmpty(message = "리뷰이 이름을 입력해주세요.") | ||
String revieweeName, | ||
|
||
@NotEmpty(message = "프로젝트 이름을 입력해주세요.") | ||
String projectName | ||
) { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하나의 api 를 사용하되, 세션에 따라서 회원과 비회원을 구분한다는 선택지도 있었을 것 같아요.
그것을 선택하지 않고 이렇게 api 를 분리한 이유가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
회원/비회원 리뷰 생성 API를 하나로 했을때를 생각해보았는데요.
이때, 비회원일 경우 세션이 없지만, resolver에서 예외를 터뜨리거나 하지 않고 컨트롤러까지 다시 member 객체를 null로 반환하는 형식으로 처리를 해줘야합니다. 그리고 서비스에서 이를 확인해서 비회원용 로직으로 처리하게 되겠죠. 이렇게, 하나의 API에서 세션이 null인 것을 예외가 아닌 비회원임으로 인식하고 처리하기 위해서 예외를 터뜨려야하는데 다르게 처리하는 등으로 로직이 복잡해진다고 생각했어요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지난 회의에서 이야기 나온 것처럼, 하나의 api를 여러곳에서 쓰고, 권한에 맞게 별도의 로직을 제공하려는 우리의 취지를 생각해봤어요. 그럼 하나의 api에서 처리하는게 맞는 것 같아요.
그리고 저도 확실하지 않아서 지피티 & 클로드에게 물어보니, 회원/비회원 기능을 하나의 api에서 제공하고 내부적으로 분기하는 것은 일반적인 패턴이라고 합니다. 아래의 예시 코드처럼요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
회원/비회원의 동작 차이가 크지 않으면 동일한 비즈니스 로직을 하나로 사용하는 것이 좋다고 생각해요.
서버는 단순히 로그인 세션이 존재하면 회원이 요청한 것으로 알고 처리, 없으면 비회원이 요청한 것으로 알고 처리. 이렇게 구현하면 리졸버에서 null을 반환하는 것이 자연스러워 보입니다.
오히려 같은 동작인데 api를 분리하면 권한 중심으로 설계된다고 생각해요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 테드와 같은 생각입니다.
덧붙이자면 우리는 "한가지 api에 대해서 권한에 따라 다르게 기능하기"로 합의했고,
지금은 api 설계 단계이니, 이 목표에 구현을 맞춰야 하지
구현이 되지 않았으므로 설계를 두개로 나눌 필요는 없다 생각해요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
중간 계층을 두거나 하는 방식으로 복잡도를 줄이고 '한가지 api에 대해서 권한에 따라 다르게 기능하기'를 최대한 활용해보는 것에 동의합니다! 반영 완료!