[BE] feat: 리뷰 폼 응답 재구현 #295

Kimprodp · 2024-08-10T18:50:28Z

resolves [BE] 리뷰 쓰기 위해 받아오는 정보 응답 기능을 재구현한다. #286

🚀 어떤 기능을 구현했나요 ?

리뷰 폼 응답 기능을 재구현했습니다.

🔥 어떻게 해결했나요 ?

mapper를 통해 응답 객체를 매핑하여 응답합니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

현재 테스트 중 TemplateMapperTest의 리뷰_그룹과_템플릿으로_템플릿_응답을_매핑한다 테스트에서 문제가 발생하고 있습니다.
단일로 돌리면 성공하지만, 다른 테스트와 같이 돌리면 실패하게 됩니다.
테스트 실패의 원인은 TemplateMapper.mapToSectionResponse() 메서드 내부에서 sectionRepository.getSectionById(sectionId) 부분이 실행될 때, 갑자기 section에 저장된 questionId 가 동일한 id로 한번 더 저장된 채 반환되고 있습니다. (디버깅 했지만 이유를 찾지 못했습니다.)

📚 참고 자료, 할 말

response에서 dto 텍스트 검증이 필요할까요?
옵션그룹이 옵션을 모르기 때문에 옵션 그룹에 옵션들이 알맞게 들어갔는지 검증할 수 없다는 문제가 있을 것 같아요

github-actions · 2024-08-10T18:51:51Z

Test Results

57 tests +8 57 ✅ +8 2s ⏱️ -1s
22 suites +2 0 💤 ±0
22 files +2 0 ❌ ±0

Results for commit d5c3b3d. ± Comparison against base commit 48e2876.

♻️ This comment has been updated with latest results.

donghoony

늦은밤까지 고생했어요~!

donghoony · 2024-08-11T03:22:16Z

backend/src/main/java/reviewme/template/controller/TemplateController.java

+
+    private final TemplateService templateService;
+
+    @GetMapping("/reviews/write")


기존 컨트롤러 지우면 프론트엔드가 간극이 있을 수 있으니 버전 업 하는 게 좋을 듯해요. v2 prefix를 들고가보는 건 어떨까요 /

(이미 반영되었네요)

donghoony · 2024-08-11T03:24:11Z

backend/src/main/java/reviewme/template/dto/response/QuestionResponse.java

+        OptionGroupResponse optionGroup,
+        boolean hasGuideline,
+        String guideline


null할 수 있는건 response단에서 @Nullable 명시해주는 건 어떤가요 ?

donghoony · 2024-08-11T03:24:50Z

backend/src/main/java/reviewme/review/domain/exception/InvalidAnswerLengthException.java

@@ -7,7 +7,7 @@
 public class InvalidAnswerLengthException extends BadRequestException {

    public InvalidAnswerLengthException(int answerLength, int minLength, int maxLength) {
-        super("답변의 길이는 %d자 이상 %d자 이하여야 합니다.".formatted(minLength, maxLength));
+        super("답변의 길이는 %d자 이상 %d자 이하여야 해요.".formatted(minLength, maxLength));
        log.info("AnswerLength is out of bound - answerLength:{}, minLength: {}, maxLength: {}",


Suggested change

log.info("AnswerLength is out of bound - answerLength:{}, minLength: {}, maxLength: {}",

log.info("AnswerLength is out of bound - answerLength: {}, minLength: {}, maxLength: {}",

donghoony · 2024-08-11T03:25:28Z

backend/src/main/java/reviewme/question/domain/Question2.java

+    public boolean hasGuideline() {
+        return !guideline.isEmpty();
+    }


guideline이 nullable해서, 체크할 필요도 있습니다! 이거 추가로 테스트도 작성해야겠네요

donghoony · 2024-08-11T03:26:14Z

backend/src/main/java/reviewme/template/domain/exception/NoRegisteredTemplatesException.java

+
+    public NoRegisteredTemplatesException() {
+        super("서버 내부에서 문제가 발생했어요. 서버에 문의해주세요.");
+        log.warn("NoRegisteredTemplatesException is occurred");


템플릿 아이디와 같은 문맥이 필요해요. 로그를 적을 때에는 달랑 이 메시지만 보고 어디에서 발생하는지에 대한 정보가 풍부하면 좋겠습니다

donghoony · 2024-08-11T04:35:18Z

backend/src/main/java/reviewme/template/domain/exception/SectionNotFoundException.java

+
+    public SectionNotFoundException(long id) {
+        super("서버 내부에서 문제가 발생했어요. 서버에 문의해주세요.");
+        log.info("SectionNotFoundException is occurred - id: {}", id);


문의 바람은 warn이 낫겠네요!

donghoony · 2024-08-11T04:35:55Z

backend/src/main/java/reviewme/template/repository/TemplateRepository.java

+@Repository
+public interface TemplateRepository extends JpaRepository<Template, Long> {
+
+    Optional<Template> findTopByOrderByIdDesc();


템플릿을 찾는 건데, id로 찾는 게 더 낫지 않나요? 나중에 여러 개가 들어오면 확장하기 어려울 것 같아요

donghoony · 2024-08-11T04:36:39Z

backend/src/main/java/reviewme/template/service/TemplateMapper.java

+        List<SectionResponse> sectionResponses = template.getSectionIds()
+                .stream()
+                .map(this::mapToSectionResponse)
+                .toList();


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

donghoony · 2024-08-11T04:38:03Z

backend/src/test/java/reviewme/support/DatabaseCleaner.java

+            if (!tableName.equals("section_ids") && !tableName.equals("question_ids")) {
+                entityManager.createNativeQuery(ALTER_FORMAT.formatted(tableName)).executeUpdate();
+            }


이..건 일대다 단방향을 맺는 게 나아보입니다 ㅋㅋㅋ

donghoony · 2024-08-11T04:38:10Z

backend/src/main/java/reviewme/template/service/TemplateService.java

@@ -19,7 +19,7 @@ public class TemplateService {
    private final TemplateRepository templateRepository;
    private final TemplateMapper templateMapper;

-    @Transactional
+    @Transactional(readOnly = true)


skylar1220

dto의 content까지 검증하지 않아도 된다고 생각해요~
코멘트 남긴 것 정도(객관식, 주관식일 때의 각각 null)만 더 검증이 있으면 된다고 생각합니다!
수고수고쓰~~

skylar1220 · 2024-08-11T04:18:39Z

backend/src/main/java/reviewme/template/service/TemplateMapper.java

+        OptionGroupResponse optionGroupResponse = optionGroupRepository.findByQuestionId(question.getId())
+                .map(this::mapToOptionGroupResponse)
+                .orElse(null);
+
+        return new QuestionResponse(
+                question.getId(),
+                question.isRequired(),
+                question.getContent(),
+                question.getContent(),
+                optionGroupResponse,


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),

);

skylar1220 · 2024-08-11T04:40:04Z

backend/src/test/java/reviewme/template/service/TemplateMapperTest.java

+    ReviewGroupRepository reviewGroupRepository;
+
+    @Test
+    void 리뷰_그룹과_템플릿으로_템플릿_응답을_매핑한다() {


QuestionResponse 매핑시 optionGroup이 없으면 null로 들어가는 로직 등이 있기때문에 이에 대한 테스트가 필요하다고 생각해요!
현제 테스트에 주관식 섹션을 추가해서 optionGroup은 null로 들어간 것을 검증하는 형식으로요!

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

skylar1220 · 2024-08-11T04:51:37Z

backend/src/main/java/reviewme/template/service/TemplateMapper.java

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

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

skylar1220

코멘트로 남겨서 RC 추가!

nayonsoso

제가 작성하는 코드 스타일과 비슷한 것 같네요 ㅋㅎㅎ
별로 코멘트 남길게 없습니당!

그리고 ElementCollection 에 Fetch EGAGER 을 해주신 것은 저도 필요성을 느끼긴 했었는데,
ElementCollection 을 사용하는 것 자체에 대해서 괜찮은 방법이었을지 의구심이 들긴 하네요
여러 PR을 읽으면서 느낀 점입니다. (특히 아루의 조회 PR)

일단 리뷰 반영하면서 더 생각해보기로 해요!

nayonsoso · 2024-08-11T04:54:26Z

backend/src/main/java/reviewme/template/service/TemplateService.java

+        Template defaultTemplate = templateRepository.findTopByOrderByIdDesc()
+                .orElseThrow(NoRegisteredTemplatesException::new);


ReviewGroup에 templateId 필드를 두고, 거기로부터 templateId 를 가져와서 조회하는게 좋을 것 같아요! (지금은 templateId 가 review에 있는데 개인적으로 ReviewGroup에 옮겨야 한다고 생각합니다)

nayonsoso · 2024-08-11T04:57:32Z

backend/src/main/java/reviewme/template/service/TemplateMapper.java

+    public TemplateResponse mapToTemplateResponse(ReviewGroup reviewGroup, Template template) {
+        List<SectionResponse> sectionResponses = template.getSectionIds()
+                .stream()
+                .map(this::mapToSectionResponse)
+                .toList();


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

donghoony

👍🏻

donghoony · 2024-08-11T07:49:42Z

backend/src/main/java/reviewme/template/service/TemplateService.java

 import reviewme.template.dto.response.TemplateResponse;
 import reviewme.template.repository.TemplateRepository;

 @Service
 @RequiredArgsConstructor
 public class TemplateService {

+    private static final Long USE_TEMPLATE_ID = 1L;


널러블하지 않으니 long으로 가시죠!

…at/286-review-creation-setup # Conflicts: # backend/src/main/java/reviewme/question/domain/OptionItem.java # backend/src/main/java/reviewme/question/domain/Question2.java # backend/src/main/java/reviewme/question/repository/OptionGroupRepository.java # backend/src/main/java/reviewme/question/repository/OptionItemRepository.java # backend/src/main/java/reviewme/question/repository/Question2Repository.java # backend/src/main/java/reviewme/template/domain/Section.java # backend/src/main/java/reviewme/template/domain/Template.java # backend/src/main/java/reviewme/template/domain/exception/SectionNotFoundException.java # backend/src/main/java/reviewme/template/repository/SectionRepository.java # backend/src/main/java/reviewme/template/repository/TemplateRepository.java

skylar1220

리프룹

Kimprodp added 13 commits August 10, 2024 16:15

feat: TemplateController 추가 및 리뷰 템플릿 응답 기능 구현

2ea69e6

feat: 리뷰 템플릿 응답을 위한 dto 생성

c424c06

refactor: 에러 응답 메세지 변경

5abbf92

refactor: onSelectedOptionId 타입 변경

8d10101

refactor: 도메인 id 제외 생성자 추가

d532c2b

feat: TemplateService 추가 및 기본 템플릿 응답 기능 추가

8cdaabe

feat: TemplateMapper 구현

9039ec8

refactor: mapping url 중복으로 인한 context 로딩 에러 해결을 위해 임시적으로 url 경로 변경

d6f708f

refactor: 로깅 레벨 warn으로 변경

9c50413

refactor: 질문에 옵션 그룹이 없을 경우 null을 사용하도록 변경

fa0ae42

refactor: id 리스트를 즉시 로딩할 수 있도록 변경

1811deb

refactor: 메서드명 변경

f2edd17

test: 테스트 추가

2c93220

Kimprodp added ✨ BE 🚀 기능 labels Aug 10, 2024

Kimprodp added this to the 4차 스프린트: 최종 milestone Aug 10, 2024

Kimprodp requested review from donghoony, nayonsoso and skylar1220 August 10, 2024 18:50

Kimprodp self-assigned this Aug 10, 2024

Kimprodp linked an issue Aug 10, 2024 that may be closed by this pull request

[BE] 리뷰 쓰기 위해 받아오는 정보 응답 기능을 재구현한다. #286

Closed

Kimprodp added 2 commits August 11, 2024 11:32

test: DatabaseCleaner에서 CollectionTable로 생성된 테이블 초기화 하도록 변경

bb2245c

refactor: Transactional readOnly 설정

040cbea

donghoony requested changes Aug 11, 2024

View reviewed changes

skylar1220 reviewed Aug 11, 2024

View reviewed changes

skylar1220 requested changes Aug 11, 2024

View reviewed changes

nayonsoso reviewed Aug 11, 2024

View reviewed changes

Kimprodp added 2 commits August 11, 2024 14:22

refactor: 컨트롤러 요청 경로 변경

274d8d3

refactor: response에서 null 가능한 필드 명시

31999b1

Kimprodp added 9 commits August 11, 2024 14:28

style: 오타 수정

de0c64d

refactor: Question2 hasGuideline 메서드에서 guideline이 null 인 경우도 추가

f6c309c

refactor: 기본 사용 템플릿을 찾을 수 없는 경우에 대한 예외명과 로그 메세지 수정

f7b1ec3

refactor: 옵션 그룹에 옵션 아이템이 없는 경우에 대한 예외명 수정

207eccc

refactor: SectionNotFoundException의 로그 레벨 수정 (info -> warn)

fef29f6

refactor: 사용하지 않는 메서드 삭제

bc28681

test: 가이드라인이 제공되지 않는 경우 테스트 추가

d26b268

test: 옵션 그룹에 없는 질문이 없는 경우 테스트 추가

3092be1

test: 섹션의 선택된 옵션이 필요없는 경우 테스트 추가

892c73b

donghoony approved these changes Aug 11, 2024

View reviewed changes

refactor: long 타입 변경

be6b8d5

skylar1220 approved these changes Aug 11, 2024

View reviewed changes

nayonsoso approved these changes Aug 11, 2024

View reviewed changes

skylar1220 approved these changes Aug 12, 2024

View reviewed changes

nayonsoso approved these changes Aug 12, 2024

View reviewed changes

Kimprodp merged commit c2b75de into develop Aug 12, 2024
4 checks passed

donghoony deleted the be/feat/286-review-creation-setup branch August 20, 2024 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] feat: 리뷰 폼 응답 재구현 #295

[BE] feat: 리뷰 폼 응답 재구현 #295

Kimprodp commented Aug 10, 2024

github-actions bot commented Aug 10, 2024 •

edited

Loading

donghoony left a comment

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

donghoony Aug 11, 2024

skylar1220 left a comment

skylar1220 Aug 11, 2024

skylar1220 Aug 11, 2024

skylar1220 Aug 11, 2024 •

edited

Loading

skylar1220 left a comment

nayonsoso left a comment

nayonsoso Aug 11, 2024

nayonsoso Aug 11, 2024

donghoony left a comment

donghoony Aug 11, 2024

skylar1220 left a comment


		private final TemplateService templateService;

		@GetMapping("/reviews/write")

	log.info("AnswerLength is out of bound - answerLength:{}, minLength: {}, maxLength: {}",
	log.info("AnswerLength is out of bound - answerLength: {}, minLength: {}, maxLength: {}",

		Template defaultTemplate = templateRepository.findTopByOrderByIdDesc()
		.orElseThrow(NoRegisteredTemplatesException::new);

[BE] feat: 리뷰 폼 응답 재구현 #295

[BE] feat: 리뷰 폼 응답 재구현 #295

Conversation

Kimprodp commented Aug 10, 2024

🚀 어떤 기능을 구현했나요 ?

🔥 어떻게 해결했나요 ?

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

github-actions bot commented Aug 10, 2024 • edited Loading

Test Results

donghoony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skylar1220 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skylar1220 Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

skylar1220 left a comment

Choose a reason for hiding this comment

nayonsoso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donghoony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skylar1220 left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 10, 2024 •

edited

Loading

skylar1220 Aug 11, 2024 •

edited

Loading