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 all commits
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 @@ -9,6 +9,6 @@ public record KeywordResponse(
long id,

@Schema(description = "키워드명")
String detail
String content
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import reviewme.keyword.dto.response.KeywordResponse;
import reviewme.keyword.dto.response.KeywordsResponse;
import reviewme.keyword.repository.KeywordRepository;

@Service
Expand All @@ -13,11 +13,11 @@ public class KeywordService {

private final KeywordRepository keywordRepository;

public KeywordsResponse findAllKeywords() {
List<KeywordResponse> responses = keywordRepository.findAll()
@Transactional(readOnly = true)
public List<KeywordResponse> findAllKeywords() {
return keywordRepository.findAll()
.stream()
.map(keyword -> new KeywordResponse(keyword.getId(), keyword.getContent()))
.toList();
return new KeywordsResponse(responses);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.ArrayList;
import java.util.List;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import reviewme.member.domain.exception.InvalidDescriptionLengthException;
Expand All @@ -29,6 +30,7 @@
@Entity
@Table(name = "reviewer_group")
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@EqualsAndHashCode(of = "id")
@Getter
public class ReviewerGroup {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ public ReviewerGroupGithubIds(ReviewerGroup reviewerGroup, List<GithubId> github
Set<GithubIdReviewerGroup> reviewers = githubIds.stream()
.map(githubId -> new GithubIdReviewerGroup(githubId, reviewerGroup))
.collect(Collectors.toSet());
if (reviewers.size() != githubIds.size()) {
throw new DuplicateReviewerException();
}
// if (reviewers.size() != githubIds.size()) {
// throw new DuplicateReviewerException();
// }
this.reviewerGithubIds = reviewers;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package reviewme.member.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;
import java.time.LocalDateTime;

@Schema(description = "리뷰 생성 시 필요한 리뷰어 그룹 응답")
public record ReviewCreationReviewerGroupResponse(

@Schema(description = "리뷰어 그룹 아이디")
long id,

@Schema(description = "리뷰 그룹 이름 (레포지토리명)")
String name,

@Schema(description = "그룹 소개")
String description,

@Schema(description = "리뷰 작성 기한")
LocalDateTime deadline,

@Schema(description = "썸네일 URL")
String thumbnailUrl,

@Schema(description = "리뷰이")
MemberResponse reviewee
Copy link
Contributor

Choose a reason for hiding this comment

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

우리 요청마다 DTO 를 만들어주기로 했었던 것 같은데,
이 MemberResponse 는 다른 DTO에서도 사용되고 있는 것 같아요.
ReviewCreationRevieweerGroupRevieweeResponse 이런식으로 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.

그거슨... 약간 또 저만의(?) 생각에서 나온 것인데요!
Member의 필드 id, name, githubId 중에 모든 경우에서 id, name이 쓰이는 것이 많은 멤버 응답의 기본형태일 것이라고 생각했어요!

  1. githubId는 id와 동일하게 식별자 역할을 하지만 github 종속적이기때문에 식별자가 필요하면 id를 쓸 것임. 즉, githubId는 응답으로 보내줄 일이 없음.
  2. name만 보내줄 일이 있지 않을까?: 그럼 그 때 name만 반환하는 DTO를 만들게되지 않을까?

따라서 id, name만을 가지는 MemberResponse를 기본형태라고 생각하고 컨텍스트를 추가하지 않은 기본형태로 다용도로 쓸 수 있는 약간의 예외 상황이라고 생각했습니다!

만 논의를 하지는 않았네요😂

Copy link
Contributor

Choose a reason for hiding this comment

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

으앙 저도 dto 는 그냥 막 만들고 재사용하지 말자 주의였긴 했는데,
막 철저하게 나누려다보니 너무 중복 코드가 많아지는 것 같아 흔들리네요🫨
이것도 이슈에 남겨둘게요! 일단 지금은 넘어가도 될 것 같아요

) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import reviewme.member.domain.Member;
import reviewme.member.domain.ReviewerGroup;
import reviewme.member.dto.response.MemberResponse;
import reviewme.member.dto.response.ReviewerGroupResponse;
import reviewme.member.repository.ReviewerGroupRepository;
import reviewme.member.dto.response.ReviewCreationReviewerGroupResponse;

@Service
@RequiredArgsConstructor
Expand All @@ -24,4 +26,18 @@ public ReviewerGroupResponse findReviewerGroup(long reviewerGroupId) {
new MemberResponse(reviewee.getId(), reviewee.getName())
);
}

@Transactional(readOnly = true)
public ReviewCreationReviewerGroupResponse findReviewCreationReviewerGroup(long reviewerGroupId) {
ReviewerGroup reviewerGroup = reviewerGroupRepository.getReviewerGroupById(reviewerGroupId);
Member reviewee = reviewerGroup.getReviewee();
return new ReviewCreationReviewerGroupResponse(
reviewerGroup.getId(),
reviewerGroup.getGroupName(),
reviewerGroup.getDescription(),
reviewerGroup.getDeadline(),
reviewerGroup.getThumbnailUrl(),
new MemberResponse(reviewee.getId(), reviewee.getName())
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestParam;
import reviewme.review.dto.response.ReviewCreationResponse;
import reviewme.review.dto.request.CreateReviewRequest;
import reviewme.review.dto.response.ReviewDetailResponse;

Expand All @@ -23,5 +25,11 @@ public interface ReviewApi {
summary = "리뷰 조회",
description = "단일 리뷰를 조회한다."
)
ResponseEntity<ReviewDetailResponse> findReview(@PathVariable long id);
ResponseEntity<ReviewDetailResponse> findReview(@PathVariable long id, @RequestParam long memberId);

@Operation(
summary = "리뷰 생성 시 필요한 정보 조회",
description = "리뷰 생성 시 필요한 정보를 조회한다."
)
ResponseEntity<ReviewCreationResponse> findReviewCreationSetup(@RequestParam long reviewerGroupId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@
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.response.ReviewCreationResponse;
import reviewme.review.dto.request.CreateReviewRequest;
import reviewme.review.dto.response.ReviewDetailResponse;
import reviewme.review.service.ReviewService;

@RestController
@RequiredArgsConstructor
public class ReviewController {
public class ReviewController implements ReviewApi{

private final ReviewService 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,14 @@
package reviewme.review.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;

@Schema(description = "리뷰 문항 응답")
public record QuestionResponse(

@Schema(description = "리뷰 문항 ID")
long id,

@Schema(description = "리뷰 문항")
String content
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package reviewme.review.dto.response;

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

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

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

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

@Schema(description = "키워드 목록")
List<KeywordResponse> keywords
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package reviewme.review.service;

import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import reviewme.review.dto.response.QuestionResponse;
import reviewme.review.repository.QuestionRepository;

@Service
@RequiredArgsConstructor
public class QuestionService {

private final QuestionRepository questionRepository;

@Transactional(readOnly = true)
public List<QuestionResponse> findAllQuestions() {
return questionRepository.findAll()
.stream()
.map(question -> new QuestionResponse(question.getId(), question.getContent()))
.toList();
}
}
Comment on lines +17 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

지금 약간쓰 혼란스러운게 있는데, 'Controller에서 호출되지 않는 서비스의 함수'가 dto를 리턴해도 괜찮을까요?

저희는 지금 dto와 요청 or 응답을 1 : 1 관계로 두고 있어요. 그러니 지금 이 함수처럼 서비스의 함수가 dto 를 리턴하게 된다면, 서비스의 모든 함수가 요청과 1 : 1 관계가 될 것인데요! 그럼 너무 재사용성이 떨어질 것 같다는 생각이 듭니다😓

예를 들어서 "모든 질문을 가져오되, 그 응답 내용이나 형식이 다르게 되는 api" 가 추가되는 상황을 생각해봐요. 그럼 dto 를 추가로 만들어야될 것이고, 그 dto를 반환하는 서비스의 함수가 또 생기게 될 것 같아요🤔 이건 다시 비효율적이라는 생각이 들어요!

그러니

  • 컨트롤러에서 호출되지 않는 서비스 함수는 dto가 아니라 도메인을 반환하도록 하거나
  • 아니면 컨트롤러에서 호출되는 서비스 함수 내부에 두거나

하면 어떨까 생각합니다,

그런데 여기까지는 제 생각이라 한번 이야기해보는 것도 좋을 것 같네요!
@donghoony @Kimprodp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아하.. 컨트롤러에서 호출되지 않는 서비스 함수는 dto가 아니라 도메인을 반환하도록 하거나 부분 동의합니다!
이렇게 되면, 리뷰 그룹, 질문, 키워드를 각 서비스에서 호출해오지 않고 ReviewService 내에서 한번에 응답을 생성하는 게 더 적절하겠네요!(응답으로 변환하는 걸 제외하면 각 서비스로 분리한 메서드가 도메인만 반환하는 로직밖에 남지 않음)

ReviewService로 다시 몰아넣고, 테스트도 ReviewServiceTest로 통합하는 작업이라 좀 커서 추후 리팩토링 때 반영할게요!😂

Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,20 @@
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.Question;
import reviewme.review.domain.Review;
import reviewme.review.domain.ReviewContent;
import reviewme.review.dto.response.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 @@ -28,6 +34,9 @@
@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;
Expand Down Expand Up @@ -100,4 +109,12 @@ public ReviewDetailResponse findReview(long reviewId, long memberId) {
keywordContents
);
}

@Transactional(readOnly = true)
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);
}
}
18 changes: 18 additions & 0 deletions backend/src/test/java/reviewme/fixture/QuestionFixure.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package reviewme.fixture;

import reviewme.review.domain.Question;

public enum QuestionFixure {

소프트스킬이_어떤가요,
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

Choose a reason for hiding this comment

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

이모티콘이 많이 섭섭해보이네요 바로 반영하겠습니다

기술역량이_어떤가요,
;

public Question create() {
return new Question(replaceUnderscores());
}

private String replaceUnderscores() {
return name().replace("_", " ");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
@Getter
public enum ReviewerGroupFixture {

리뷰_그룹("리뷰 그룹", "그룹 설명", LocalDateTime.of(2024, 1, 1, 12, 0)),
데드라인_남은_그룹("데드라인 이전 그룹", "설명", LocalDateTime.now().plusDays(1)),
데드라인_지난_그룹("데드라인 지난 그룹", "설명", LocalDateTime.now().minusDays(1)),
;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package reviewme.keyword.service;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import reviewme.fixture.KeywordFixture;
import reviewme.keyword.dto.response.KeywordResponse;
import reviewme.keyword.repository.KeywordRepository;
import reviewme.support.ServiceTest;

@ServiceTest
class KeywordServiceTest {

@Autowired
KeywordService keywordService;

@Autowired
KeywordRepository keywordRepository;

@Test
void 모든_키워드를_조회한다() {
// given
keywordRepository.save(KeywordFixture.회의를_이끌어요.create());

// when
List<KeywordResponse> keywords = keywordService.findAllKeywords();

// then
assertThat(keywords).hasSize(1);
}
}
Loading