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: 리뷰 폼 응답 재구현 #295

Merged
merged 28 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2ea69e6
feat: TemplateController 추가 및 리뷰 템플릿 응답 기능 구현
Kimprodp Aug 10, 2024
c424c06
feat: 리뷰 템플릿 응답을 위한 dto 생성
Kimprodp Aug 10, 2024
5abbf92
refactor: 에러 응답 메세지 변경
Kimprodp Aug 10, 2024
8d10101
refactor: onSelectedOptionId 타입 변경
Kimprodp Aug 10, 2024
d532c2b
refactor: 도메인 id 제외 생성자 추가
Kimprodp Aug 10, 2024
8cdaabe
feat: TemplateService 추가 및 기본 템플릿 응답 기능 추가
Kimprodp Aug 10, 2024
9039ec8
feat: TemplateMapper 구현
Kimprodp Aug 10, 2024
d6f708f
refactor: mapping url 중복으로 인한 context 로딩 에러 해결을 위해 임시적으로 url 경로 변경
Kimprodp Aug 10, 2024
9c50413
refactor: 로깅 레벨 warn으로 변경
Kimprodp Aug 10, 2024
fa0ae42
refactor: 질문에 옵션 그룹이 없을 경우 null을 사용하도록 변경
Kimprodp Aug 10, 2024
1811deb
refactor: id 리스트를 즉시 로딩할 수 있도록 변경
Kimprodp Aug 10, 2024
f2edd17
refactor: 메서드명 변경
Kimprodp Aug 10, 2024
2c93220
test: 테스트 추가
Kimprodp Aug 10, 2024
bb2245c
test: DatabaseCleaner에서 CollectionTable로 생성된 테이블 초기화 하도록 변경
Kimprodp Aug 11, 2024
040cbea
refactor: Transactional readOnly 설정
Kimprodp Aug 11, 2024
274d8d3
refactor: 컨트롤러 요청 경로 변경
Kimprodp Aug 11, 2024
31999b1
refactor: response에서 null 가능한 필드 명시
Kimprodp Aug 11, 2024
de0c64d
style: 오타 수정
Kimprodp Aug 11, 2024
f6c309c
refactor: Question2 hasGuideline 메서드에서 guideline이 null 인 경우도 추가
Kimprodp Aug 11, 2024
f7b1ec3
refactor: 기본 사용 템플릿을 찾을 수 없는 경우에 대한 예외명과 로그 메세지 수정
Kimprodp Aug 11, 2024
207eccc
refactor: 옵션 그룹에 옵션 아이템이 없는 경우에 대한 예외명 수정
Kimprodp Aug 11, 2024
fef29f6
refactor: SectionNotFoundException의 로그 레벨 수정 (info -> warn)
Kimprodp Aug 11, 2024
bc28681
refactor: 사용하지 않는 메서드 삭제
Kimprodp Aug 11, 2024
d26b268
test: 가이드라인이 제공되지 않는 경우 테스트 추가
Kimprodp Aug 11, 2024
3092be1
test: 옵션 그룹에 없는 질문이 없는 경우 테스트 추가
Kimprodp Aug 11, 2024
892c73b
test: 섹션의 선택된 옵션이 필요없는 경우 테스트 추가
Kimprodp Aug 11, 2024
be6b8d5
refactor: long 타입 변경
Kimprodp Aug 11, 2024
d5c3b3d
Merge remote-tracking branch 'refs/remotes/origin/develop' into be/fe…
Kimprodp Aug 12, 2024
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 @@ -53,6 +53,6 @@ public boolean isSelectable() {
}

public boolean hasGuideline() {
return guideline != null;
return guideline != null && !guideline.isEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.question.domain.exception;

import lombok.extern.slf4j.Slf4j;
import reviewme.global.exception.NotFoundException;

@Slf4j
public class MissingOptionItemsInOptionGroupException extends NotFoundException {

public MissingOptionItemsInOptionGroupException(long optionGroupId) {
super("서버 내부에서 문제가 발생했어요. 서버에 문의해주세요.");
log.warn("OptionGroup has no OptionItems - optionGroupId: {}", optionGroupId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.question.domain.exception;

import lombok.extern.slf4j.Slf4j;
import reviewme.global.exception.NotFoundException;

@Slf4j
public class OptionGroupNotFoundByQuestionException extends NotFoundException {

public OptionGroupNotFoundByQuestionException(long questionId) {
super("서버 내부에서 문제가 발생했어요. 서버에 문의해주세요.");
log.warn("OptionGroup not found by question - questionId: {}", questionId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public interface OptionItemRepository extends JpaRepository<OptionItem, Long> {

List<OptionItem> findAllByOptionGroupId(long optionGroupId);

boolean existsByOptionTypeAndId(OptionType optionType, long id);

@Query(value = """
SELECT o.id FROM option_item o
LEFT JOIN checkbox_answer ca
Expand All @@ -38,8 +40,6 @@ public interface OptionItemRepository extends JpaRepository<OptionItem, Long> {
""", nativeQuery = true)
List<OptionItem> findSelectedOptionItemsByReviewIdAndQuestionId(long reviewId, long questionId);

boolean existsByOptionTypeAndId(OptionType optionType, long id);

default OptionItem getOptionItemById(long id) {
return findById(id).orElseThrow(() -> new OptionItemNotFoundException(id));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package reviewme.question.repository;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;
import reviewme.question.domain.Question2;
import reviewme.question.domain.exception.QuestionNotFoundException;

@Repository
public interface Question2Repository extends JpaRepository<Question2, Long> {

default Question2 getQuestionById(long id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
public class InvalidAnswerLengthException extends BadRequestException {

public InvalidAnswerLengthException(int answerLength, int minLength, int maxLength) {
super("답변의 길이는 %d자 이상 %d자 이하여야 합니다.".formatted(minLength, maxLength));
log.info("AnswerLength is out of bound - answerLength:{}, minLength: {}, maxLength: {}",
super("답변의 길이는 %d자 이상 %d자 이하여야 해요.".formatted(minLength, maxLength));
log.info("AnswerLength is out of bound - answerLength: {}, minLength: {}, maxLength: {}",
answerLength, minLength, maxLength);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package reviewme.template.controller;

import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import reviewme.template.dto.response.TemplateResponse;
import reviewme.template.service.TemplateService;

@RestController
@RequiredArgsConstructor
public class TemplateController {

private final TemplateService templateService;

@GetMapping("/v2/reviews/write")
public ResponseEntity<TemplateResponse> getReviewForm(@RequestParam String reviewRequestCode) {
TemplateResponse response = templateService.generateReviewForm(reviewRequestCode);
return ResponseEntity.ok(response);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package reviewme.template.domain;

import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.template.domain.exception;

import lombok.extern.slf4j.Slf4j;
import reviewme.global.exception.NotFoundException;

@Slf4j
public class DefaultTemplateNotFoundException extends NotFoundException {

public DefaultTemplateNotFoundException(long id) {
super("서버 내부에서 문제가 발생했어요. 서버에 문의해주세요.");
log.warn("DefaultTemplateNotFoundException is occurred - id: {}", id);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
public class SectionNotFoundException extends NotFoundException {

public SectionNotFoundException(long id) {
super("섹션이 존재하지 않아요.");
log.info("SectionNotFoundException is occurred - id: {}", id);
super("서버 내부에서 문제가 발생했어요. 서버에 문의해주세요.");
log.warn("SectionNotFoundException is occurred - id: {}", id);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package reviewme.template.dto.response;

import java.util.List;

public record OptionGroupResponse(

long optionGroupId,
int minCount,
int maxCount,
List<OptionItemResponse> options
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package reviewme.template.dto.response;

public record OptionItemResponse(

long optionId,
String content
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package reviewme.template.dto.response;

import jakarta.annotation.Nullable;

public record QuestionResponse(

long questionId,
boolean required,
String content,
String questionType,
@Nullable OptionGroupResponse optionGroup,
boolean hasGuideline,
@Nullable String guideline
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package reviewme.template.dto.response;

import jakarta.annotation.Nullable;
import java.util.List;

public record SectionResponse(

long sectionId,
String visible,
@Nullable Long onSelectedOptionId,
String header,
List<QuestionResponse> questions
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package reviewme.template.dto.response;

import java.util.List;

public record TemplateResponse(

long formId,
String revieweeName,
String projectName,
List<SectionResponse> sections
) {
}
102 changes: 102 additions & 0 deletions backend/src/main/java/reviewme/template/service/TemplateMapper.java
Copy link
Contributor

@skylar1220 skylar1220 Aug 11, 2024

Choose a reason for hiding this comment

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

(급한 건 아닙니다! 얘기해볼 점!)
Mapper를 쓰니 서비스가 깔끔해서 보기 좋은데 현재 수준에서 Mapper로 분리가 필요한 정도는 아니지 않을까?하는 의견입니다!

지금 생각으로는 TemplateService에 더 들어올 로직이 떠오르는 게 없는데, 그렇다면 현재 TemplateSevice는 곧 템플릿 생성이라는 역할만을 전적으로 담당하는 서비스인데, Mapper가 그 생성을 다시 한 번 나눠가진 느낌이 들어요.
(별개로 더 복잡해지는 경우 응집도를 위해서 Mapper 사용은 좋은 것 같습니다. 확실히 깔끔하네요!)

Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package reviewme.template.service;

import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import reviewme.question.domain.OptionGroup;
import reviewme.question.domain.OptionItem;
import reviewme.question.domain.Question2;
import reviewme.question.domain.exception.MissingOptionItemsInOptionGroupException;
import reviewme.question.repository.OptionGroupRepository;
import reviewme.question.repository.OptionItemRepository;
import reviewme.question.repository.Question2Repository;
import reviewme.reviewgroup.domain.ReviewGroup;
import reviewme.template.domain.Section;
import reviewme.template.domain.SectionQuestion;
import reviewme.template.domain.Template;
import reviewme.template.domain.TemplateSection;
import reviewme.template.dto.response.OptionGroupResponse;
import reviewme.template.dto.response.OptionItemResponse;
import reviewme.template.dto.response.QuestionResponse;
import reviewme.template.dto.response.SectionResponse;
import reviewme.template.dto.response.TemplateResponse;
import reviewme.template.repository.SectionRepository;

@Component
@RequiredArgsConstructor
public class TemplateMapper {

private final SectionRepository sectionRepository;
private final Question2Repository questionRepository;
private final OptionGroupRepository optionGroupRepository;
private final OptionItemRepository optionItemRepository;

public TemplateResponse mapToTemplateResponse(ReviewGroup reviewGroup, Template template) {
List<SectionResponse> sectionResponses = template.getSectionIds()
.stream()
.map(this::mapToSectionResponse)
.toList();
Comment on lines +35 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

섹션 하나당 쿼리가 한 개 나갈텐데, 레포에서 쿼리 묶어서 하는 게 더 효율은 좋겠습니다. 일단 구현이 급선무니 이건 넘어가도 좋아요

Comment on lines +34 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋ 연쇄매핑마.. 그래도 보기 깔끔해서 좋네요!


return new TemplateResponse(
template.getId(),
reviewGroup.getReviewee(),
reviewGroup.getProjectName(),
sectionResponses
);
}

private SectionResponse mapToSectionResponse(TemplateSection templateSection) {
Section section = sectionRepository.getSectionById(templateSection.getSectionId());
List<QuestionResponse> questionResponses = section.getQuestionIds()
.stream()
.map(this::mapToQuestionResponse)
.toList();

return new SectionResponse(
section.getId(),
section.getVisibleType().name(),
section.getOnSelectedOptionId(),
section.getHeader(),
questionResponses
);
}

private QuestionResponse mapToQuestionResponse(SectionQuestion sectionQuestion) {
Question2 question = questionRepository.getQuestionById(sectionQuestion.getQuestionId());
OptionGroupResponse optionGroupResponse = optionGroupRepository.findByQuestionId(question.getId())
.map(this::mapToOptionGroupResponse)
.orElse(null);

return new QuestionResponse(
question.getId(),
question.isRequired(),
question.getContent(),
question.getContent(),
optionGroupResponse,
Comment on lines +66 to +75
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional로 null 가능성을 명시하면서 안전성을 챙기는 건 어떨까요?

Suggested change
OptionGroupResponse optionGroupResponse = optionGroupRepository.findByQuestionId(question.getId())
.map(this::mapToOptionGroupResponse)
.orElse(null);
return new QuestionResponse(
question.getId(),
question.isRequired(),
question.getContent(),
question.getContent(),
optionGroupResponse,
Optional<OptionGroupResponse> optionGroupResponse = optionGroupRepository.findByQuestionId(question.getId())
.map(this::mapToOptionGroupResponse);
return new QuestionResponse(
question.getId(),
question.isRequired(),
question.getContent(),
question.getContent(),
optionGroupResponse.orElse(null),
);

question.hasGuideline(),
question.getGuideline()
);
}

private OptionGroupResponse mapToOptionGroupResponse(OptionGroup optionGroup) {
List<OptionItem> optionItems = optionItemRepository.findAllByOptionGroupId(optionGroup.getId());
if (optionItems.isEmpty()) {
throw new MissingOptionItemsInOptionGroupException(optionGroup.getId());
}

List<OptionItemResponse> optionItemResponses = optionItems.stream()
.map(this::mapToOptionItemResponse)
.toList();

return new OptionGroupResponse(
optionGroup.getId(),
optionGroup.getMinSelectionCount(),
optionGroup.getMaxSelectionCount(),
optionItemResponses
);
}

private OptionItemResponse mapToOptionItemResponse(OptionItem optionItem) {
return new OptionItemResponse(optionItem.getId(), optionItem.getContent());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package reviewme.template.service;

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import reviewme.review.domain.exception.ReviewGroupNotFoundByRequestReviewCodeException;
import reviewme.reviewgroup.domain.ReviewGroup;
import reviewme.reviewgroup.repository.ReviewGroupRepository;
import reviewme.template.domain.Template;
import reviewme.template.domain.exception.DefaultTemplateNotFoundException;
import reviewme.template.dto.response.TemplateResponse;
import reviewme.template.repository.TemplateRepository;

@Service
@RequiredArgsConstructor
public class TemplateService {

private static final long USE_TEMPLATE_ID = 1L;

private final ReviewGroupRepository reviewGroupRepository;
private final TemplateRepository templateRepository;
private final TemplateMapper templateMapper;

@Transactional(readOnly = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

public TemplateResponse generateReviewForm(String reviewRequestCode) {
ReviewGroup reviewGroup = reviewGroupRepository.findByReviewRequestCode(reviewRequestCode)
.orElseThrow(() -> new ReviewGroupNotFoundByRequestReviewCodeException(reviewRequestCode));

Template defaultTemplate = templateRepository.findById(USE_TEMPLATE_ID)
.orElseThrow(() -> new DefaultTemplateNotFoundException(USE_TEMPLATE_ID));

return templateMapper.mapToTemplateResponse(reviewGroup, defaultTemplate);
}
}
5 changes: 3 additions & 2 deletions backend/src/test/java/reviewme/support/DatabaseCleaner.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import jakarta.persistence.Table;
import jakarta.persistence.metamodel.EntityType;
import jakarta.transaction.Transactional;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;

Expand All @@ -22,10 +23,10 @@ public class DatabaseCleaner {
@PostConstruct
public void afterPropertiesSet() {
Set<EntityType<?>> entities = entityManager.getMetamodel().getEntities();
tableNames = entities.stream()
tableNames = new ArrayList<>(entities.stream()
.filter(entity -> entity.getJavaType().isAnnotationPresent(Table.class))
.map(entity -> entity.getJavaType().getAnnotation(Table.class).name())
.toList();
.toList());
}

@Transactional
Expand Down
Loading