Skip to content
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] feat: 리뷰에 필요한 정보 조회 기능 추가 #103

Merged
merged 18 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import reviewme.review.dto.ReviewCreationResponse;
import reviewme.review.dto.request.CreateReviewRequest;
import reviewme.review.dto.response.ReviewDetailResponse;
import reviewme.review.service.ReviewService;
Expand All @@ -32,4 +33,10 @@ public ResponseEntity<ReviewDetailResponse> findReview(@PathVariable long id,
ReviewDetailResponse response = reviewService.findReview(id, memberId);
return ResponseEntity.ok(response);
}

@GetMapping("/reviews/write")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SwaggerDocs 사용 위해서 ReviewApi 등록하는 게 좋아보여요 ! (Dto에서 적어 둔 어노테이션이 무용지물이 되었습니다 😨 )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

호호호... 다 만들고 작동을 안시켜준 격이군요😅 반영했습니다!

public ResponseEntity<ReviewCreationResponse> findReviewCreationSetup(@RequestParam long reviewerGroupId) {
ReviewCreationResponse response = reviewService.findReviewCreationSetup(reviewerGroupId);
return ResponseEntity.ok(response);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package reviewme.review.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;
import reviewme.keyword.dto.response.KeywordResponse;
import reviewme.member.dto.response.ReviewCreationReviewerGroupResponse;
import reviewme.review.dto.response.QuestionResponse;

@Schema(description = "리뷰 생성 시 필요한 정보 응답")
public record ReviewCreationResponse(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReviewCreationReviewerGroupResponse 랑 다른 패키지에 있네요 😲
(ReviewCreationReviewerGroupResponse : member / ReviewCreationResponse : review)

dto 가 요청과 일대일 관계라면, 그 요청을 처리하는 컨트롤러가 있는 패키지에 있는게 자연스러울 것 같아요. 그런데 다른 패키지에 둔 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReviewCreationReviewerGroupResponse는 ReviewerGroupService를 통해 반환되고,
ReviewCreationResponse는 ReviewerService를 통해 반환되는 DTO라서
반환되는 각 서비스에 해당하는 패키지에 있어야한다고 생각했어요!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오~ 서비스에서 반환하는 것을 기준으로 나눈다는 것을 생각하면 일리가 있...는 것 같기도 하는데
아무래도 헷갈리네요.. ㅠㅠ
뭔가 하나의 응답 dto 와 관련된 것들은 한 곳에 모여있어야 할 것 같은 느낌이 듭니당

다음에 '컨트롤러에서 호출되지 않는 서비스가 dto를 반환하게 하지 않게 리팩터링' 할 때 옮겨주면 좋을 것 같아요!


@Schema(description = "리뷰어 그룹")
ReviewCreationReviewerGroupResponse reviewerGroup,

@Schema(description = "리뷰 문항 목록")
List<QuestionResponse> questions,

@Schema(description = "키워드 목록")
List<KeywordResponse> keywords
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KeywordResponse 내부에서 detailcontent로 변경해야 할 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와우 놓쳤던 부분! 감사감사🙏

) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
import org.springframework.transaction.annotation.Transactional;
import reviewme.keyword.domain.Keyword;
import reviewme.keyword.domain.Keywords;
import reviewme.keyword.dto.response.KeywordResponse;
import reviewme.keyword.repository.KeywordRepository;
import reviewme.keyword.service.KeywordService;
import reviewme.member.domain.Member;
import reviewme.member.domain.ReviewerGroup;
import reviewme.member.repository.MemberRepository;
import reviewme.member.repository.ReviewerGroupRepository;
import reviewme.member.service.ReviewerGroupService;
import reviewme.review.domain.Review;
import reviewme.review.domain.ReviewContent;
import reviewme.review.dto.ReviewCreationResponse;
import reviewme.review.dto.response.QuestionResponse;
import reviewme.member.dto.response.ReviewCreationReviewerGroupResponse;
import reviewme.review.dto.request.CreateReviewRequest;
import reviewme.review.dto.response.ReviewDetailResponse;
import reviewme.review.dto.response.ReviewDetailReviewContentResponse;
Expand All @@ -25,9 +30,11 @@
@RequiredArgsConstructor
public class ReviewService {

private final ReviewerGroupService reviewerGroupService;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤 도메인은 Service를 사용하고 어떤 도메인은 Repository를 사용하고, 어떤 도메인은 둘 다 사용하고 있어요.
Servicerepository 사용에 대한 기준이 명확했으면 합니다. 또 Service에서 다른 Service를 참조하는 것이 좋을까는 다음번에 같이 고민해보면 좋을 것 같네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제 기준) 서비스에 1:1로 해당하는 엔티티가 아닌 다른 엔티티 관련 repo를 사용하는 로직은 다 해당 엔티티의 서비스로 분리해보았어요! 로직상 불가피한 경우가 아니라면 최대한 분리해서 책임을 나누고, 각 서비스가 지나치게 커지는 걸 방지하고 싶었어요. 하지만 아직 이 방식에 대해 논의해본적이 없으니 함께 고민해보면 좋겠네요!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다음 리팩터링 때 고려해볼 사항으로 추가하죠~

private final KeywordService keywordService;
private final QuestionService questionService;
private final ReviewRepository reviewRepository;
private final MemberRepository memberRepository;
private final ReviewerGroupRepository reviewerGroupRepository;
private final ReviewContentRepository reviewContentRepository;
private final KeywordRepository keywordRepository;

Expand Down Expand Up @@ -75,4 +82,11 @@ public ReviewDetailResponse findReview(long reviewId, long memberId) {
keywordContents
);
}

public ReviewCreationResponse findReviewCreationSetup(long reviewerGroupId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setup말고 좋은 네이밍이 있을 것 같은데 같이 고민해봐요 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요리조리 고민해봤으나 역시 네이밍 애매하군요..! 여러분의 도움이 필요합니다😂

ReviewCreationReviewerGroupResponse reviewerGroup = reviewerGroupService.findReviewCreationReviewerGroup(reviewerGroupId);
Comment on lines +114 to +115
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우왕 컨트롤러의 함수와 서비스의 함수가 이름이 다른데 혹시 어떤 의미가 있으셨을까요!!

Copy link
Contributor Author

@skylar1220 skylar1220 Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅎㅎ산초가 아니라고 반영안해도 된다고함(진짜임)

List<QuestionResponse> questions = questionService.findAllQuestions();
List<KeywordResponse> keywords = keywordService.findAllKeywords();
return new ReviewCreationResponse(reviewerGroup, questions, keywords);
}
}