[BE] feat: 변경된 도메인 구조가 적용된 리뷰 작성 API 구현 #296

nayonsoso · 2024-08-10T20:56:01Z

resolves [BE] 리뷰 쓰기 기능을 구현한다. #292

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

리뷰 작성 기능을 구현했습니다.

🔥 어떻게 해결했나요 ?

충돌이 나는 것을 피하기 위해 & 서비스 분리가 필요하다 생각해서 CreateReviewService 를 만들어서 거기서 작업했습니다.
dto 도 패키지 별 분리를 하면 좋겠다 생각해서 create 라는 패키지를 파서 거기에 dto 추가 생성했습니다.
서술형 질문, 객관식 질문에 대해서 저장을 할 때 검증이 많아서 객체를 분리했습니다.
특히 객관식 질문 검증 짱마늠

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

이번에 서비스도 분리하고 dto 패키지도 분리하는 등 제가 주관대로 한 부분들이 조금 있는데, 혹시 마음에 안들면 코멘트 남겨주세용~

📚 참고 자료, 할 말

다음에 도메인 엎고 역할 분배할 순간이 다시 온다면, "생성자와 레포지토리 & default 함수 정도"는 만들고 분배해야 할 듯🤯

공통 코드에서 리팩터링 필요하다 느낀 부분

선택지를 뜻하는 checkbox 와 option 이라는 단어를 통일해서 하나만 사용해야 한다.
Review 가 templateId 를 갖는게 아니라 ReviewGroup 이 갖게 해야 한다.
테스트 코드를 작성할 때, Template 이 SessionId 를 List sessionIds 으로 참조하고 있는 상태에서, template.getSesstionIds() 를 호출하니 lazily initialize exception 이 발생했다. ElementCollection으로 참조하는 것도 순전히 자바 객체처럼 참조하는 것은 아니고, 내부적으로 트랜잭션을 신경써야 하는 것 같다. 일단 테스트코드에서 그런 상황이 발생하지 않게끔 갯수만 검증하는 식으로 구현하긴 했다.

- 명시적이게 Question2 로 변경

github-actions · 2024-08-10T20:57:10Z

Test Results

49 tests 49 ✅ 1s ⏱️
20 suites 0 💤
20 files 0 ❌

Results for commit 04f5b66.

♻️ This comment has been updated with latest results.

donghoony

밤까지 고생했습니다 👍🏻

.../reviewme/review/service/exception/CheckBoxAnswerIncludedNotProvidedOptionItemException.java

backend/src/main/java/reviewme/review/service/CreateCheckBoxAnswerRequestValidator.java

backend/src/main/java/reviewme/review/service/CreateTextAnswerRequestValidator.java

backend/src/main/java/reviewme/review/service/exception/TextAnswerIncudedOptionException.java

...src/main/java/reviewme/review/service/exception/RequiredQuestionMustBeAnsweredException.java

...d/src/main/java/reviewme/review/service/exception/SelectedCheckBoxAnswerCountOutOfRange.java

backend/src/test/java/reviewme/review/service/CreateCheckBoxAnswerRequestValidatorTest.java

backend/src/main/java/reviewme/review/controller/ReviewController.java

backend/src/main/java/reviewme/review/domain/Review2.java

backend/src/main/java/reviewme/template/domain/Section.java

Kimprodp

전체적으로 좋네요 급하게 수정되어야 할 부분은 없을 것 같고, 논의가 필요한 부분만 다 같이 논의해보면 좋을듯!

참고로 로깅 너무 잘해놔서 내 기능 구현할때 편했어요 (●'◡'●)👍

Kimprodp · 2024-08-11T04:49:54Z

backend/src/main/java/reviewme/review/domain/Review2.java

@@ -25,7 +26,7 @@ public class Review2 {
    private Long id;

    @Column(name = "template_id", nullable = false)
-    private long templateId;
+    private long templateId; // todo: templateId 를 여기가 아니라 reviewGroup 안에 둬야 함


이 부분 다시 논의 해보면 좋겠지만, 중간에 템플릿이 변경되면 리뷰 그룹을 다시 생성해야 하나요?
리뷰 그룹이 templateId를 가지고 있으면 리뷰 그룹 하나는 하나의 템플릿만 사용하지 못할 것 같은데..

리뷰 그룹이 templateId를 가지고 있으면 리뷰 그룹 하나는 하나의 템플릿만 사용하지 못할 것 같은데..

하나의 리뷰 그룹은 하나의 템플릿을 사용하는게 자연스럽다고 생각해요!
'리뷰 그룹'이라고 함은, 리뷰를 작성하는 대상들을 정의하는 것인데,
작성자들마다 다른 템플릿을 보여주진 않을 것 같아요.
혹시 테드가 다르게 생각하고 있는 부분이 있다면 알려주세용~

중간에 템플릿이 변경되면 리뷰 그룹을 다시 생성해야 하나요?

이건 다 같이 논의되지 않았긴 했는데, 제 생각에는 중간에 템플릿이 바뀌지 않을 것 같습니다

backend/src/main/java/reviewme/review/controller/ReviewController.java

backend/src/main/java/reviewme/review/service/CreateReviewService.java

skylar1220

검증 수고했어여🥲👏

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

skylar1220 · 2024-08-11T05:50:17Z

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

+
+    public OptionItemNotFoundException(long id) {
+        super("옵션 아이템이 존재하지 않아요.");
+        log.info("OptionItemNotFoundException is occurred - id: {}", id);


예외의 info / warn / error를 책정하는 기준이 있어야겠네요!
이 예외도 일어날 가능성이 적다는 측면이라면 warn 정도도 괜찮을 것 같아요!

예외의 info / warn / error를 책정하는 기준이 있어야겠네요!

동의합니다~
서버가 내려준 대로 응답하면 발생하지 않을 예외들은 warn 으로 두는게 좋을 것 같아요.
누군가 사이트를 통하지 않고 이상한 형식으로 리뷰를 제출하려 한다는 거니까요.
이건 지금 고칠건 아닌 것 같고, 나중에 상의할 필요가 있다는 생각이 들어 이슈에 추가해뒀습니다!

skylar1220 · 2024-08-11T06:01:16Z

backend/src/main/java/reviewme/review/service/CreateReviewService.java

+                .orElseThrow(() -> new ReviewGroupNotFoundByRequestReviewCodeException(reviewRequestCode));
+    }
+
+    private void validateSubmittedQuestionAndProvidedQuestionMatch(CreateReviewRequest request, Template template) {


validateFormAndTemplateQuestionMatch
응답한 폼은 Form, 양식은 Template 으로 명칭을 통일해서 이름을 간략화하는 건 어떨까요?
다른 pr에도 코멘트로 남겼는데, 이 명칭이 혼용되고 있는 것에 대해 문제를 느꼈었어서요!
이건 전체 논의가 필요할 것 같네요!

@Kimprodp @donghoony

응답한 폼은 Form, 양식은 Template 으로 명칭을 통일해서 이름을 간략화하는 건 어떨까요?

이 문장에서

응답한 폼은 Form, 양식은 Template 으로 명칭을 통일

하는 것은 좋은 의견같습니다!

이름을 간략화하는 건 어떨까요?

그런데 요 부분은 다른 생각인게,
이 검증 함수는 '질문의 id'를 검증하는 함수이기 때문에 길더라도 지금의 이름이 의미를 잘 드러낸다고 생각해요. 심지어 이 클래스 안에 validate 가 많기 때문에 의미를 명확히 하는게 더 중요하다고 생각고요! 그래서 일단 이렇게 두겠습니다 ㅎㅎ 🫰🏻

backend/src/main/java/reviewme/review/service/CreateCheckBoxAnswerRequestValidator.java

skylar1220 · 2024-08-11T06:25:30Z

backend/src/main/java/reviewme/review/service/CreateCheckBoxAnswerRequestValidator.java

+    private void validateQuestionRequired(CreateReviewAnswerRequest request, Question2 question) {
+        if (question.isRequired() && request.selectedOptionIds() == null) {
+            throw new RequiredQuestionMustBeAnsweredException(question.getId());
+        }


(수정사항 아님! 논의할점!)
그리고 request.selectedOptionIds() == null 이 부분 리스트인데 null 검증을 하고있는 상황을 보면서 생각이 들었는데요!
현재 api 문서에는 주관식인경우 selectedOptionIds가 null로 오고있는데 null 위험성을 경계하자면 프론트에게 빈 list로 보내달라고 요청하는 건 어떨까? 하는 생각이 들어서 논의해보고 싶네요!

backend/src/main/java/reviewme/review/service/CreateCheckBoxAnswerRequestValidator.java

skylar1220 · 2024-08-11T06:38:28Z

backend/src/main/java/reviewme/review/service/CreateReviewService.java

+        return reviewGroupRepository.findByReviewRequestCode(reviewRequestCode)
+                .orElseThrow(() -> new ReviewGroupNotFoundByRequestReviewCodeException(reviewRequestCode));


createService를 분리하여 관심사도 분리하고, 응집도 up! 서비스 가벼워지는 것 좋아요~
다만 이런 부분이 reviewService(분리 후에는 조회에 집중되어있음)에도 중복 로직이 있어서 이런 부분을 아예 조회 서비스에서 불러와서 쓰게해야할지, 중복을 감수할지 고민이네요!

응집도 up! 서비스 가벼워지는 것 좋아요~

🥰

다만 이런 부분이 reviewService(분리 후에는 조회에 집중되어있음)에도 중복 로직이 있어서 이런 부분을 아예 조회 서비스에서 불러와서 쓰게해야할지, 중복을 감수할지 고민이네요!

서비스를 각각 분리한다면 충분히 생길만한 고민이네요! 그런데 그런 순간이 왔을 때 해결책을 찾아보는 것도 좋을 것 같아요~ 어떤게 중복되는지에 따라 달라질 것 같아서요!

skylar1220 · 2024-08-11T06:42:02Z

backend/src/main/java/reviewme/review/service/CreateTextAnswerRequestValidator.java

+    private Question2 validateQuestionExists(CreateReviewAnswerRequest request) {
+        long questionId = request.questionId();
+        return question2Repository.getQuestionById(questionId);
+    }


이 부분은 선택형 validator, 서술형 validator에 중복으로 들어있는 로직이네요.
만약 수정사항이 생기면 두 부분의 검증 메서드의 정합성을 신경써야한다는 점이 걸려요!

완전 공통 validate 부분은 서비스에서 검증하기

공통 검증 validator를 하나 만들기
두개정도 떠오르는데 2번은 과하니 1번 방법은 어떤가요?

예리하게 보셨네요...
그런데 더욱 놀라운 사실은 이미 1번을 서비스에서 하고 있습니다.!

그래서 왜 서비스에서 검증하는 것을 XXXAnswerRequestValidator에도 검증하느냐 한다면,
저는 XXXAnswerRequestValidator 그 자체로 "검증해주는 객체"로 생각했어요. 그래서 앞에서 어떤 함수를 선언했는지에 상관 없이 그 안에서 QuestionId에 대한 검증을 하게 해줬어요. 제출된 응답에 대해 검증을 하는 객체이니까, 그 응답이 존재하는 questionId에 대한것인지 검증하는 책임을 갖는게 당연하다 생각했지요!

수정사항이 생기면 두 부분의 검증 메서드의 정합성을 신경써야한다는 점이 걸려요!

그와 별개로 이 부분에는 동의를 하는데요, 상속을 사용하면 참 좋겠다는 생각을 했습니다. 이건 나중에 더 이야기해봐요!

- Service -> Component

- private 함수를 호출하지 사용하지 않고, repository 함수를 바로 호출하도록 변경

- question 과 template 패키지에 각각 존재하던 것을 question 패키지로 통합

…ature/292-review-create-api # Conflicts: # 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/review/controller/ReviewController.java # backend/src/main/java/reviewme/review/domain/Review2.java # backend/src/main/java/reviewme/template/repository/SectionRepository.java

- merge 후 발생하는 컨플릭을 해결한 뒤 발생한 컴파일 에러를 해결한다.

skylar1220

궈궈

nayonsoso added 26 commits August 10, 2024 22:59

feat: 리뷰 작성 요청 dto 생성

99d04af

feat: 리뷰 레포지토리 생성

214347e

feat: 템플릿 레포지토리 생성 및 기본 함수 등록

5b13842

feat: 섹션 레포지토리 생성 및 기본 함수 등록

1f2ba2f

feat: 질문 레포지토리 생성 및 기본 함수 등록

9326b3f

feat: 옵션 아이템 레포지토리 생성 및 기본 함수 등록

59479f2

feat: 옵션 그룹 레포지토리 생성 및 기본 함수 등록

98507f6

feat: 엔티티에 생성자 추가

e33c1b3

feat: templateId 를 필요한 곳에 추가

bcdc74f

refactor: 레포지토리 이름 변경

6f1867e

- 명시적이게 Question2 로 변경

feat: 리뷰 저장 검증 실패시 발생시킬 예외 생성

cf03f9a

feat: 리뷰 저장 검증 시 필요한 레포지토리 함수 생성

65b2e8d

feat: 선택형 대답 검증기 생성

194239e

feat: 서술형 대답 검증기 생성

63f0a2f

feat: 리뷰 저장 기능 구현

03b4404

test: 서술형 응답 검증 테스트

7c4c79d

feat: 필수 질문 검증 기능 구현

334bcf7

test: 텍스트 필수 질문 검증 테스트 추가

8db2800

feat: OptionGroup에 생성자 추가

a456a97

feat: 선택지 갯수 검증 기능 추가

f003aaa

test: 선택 문항 검증 테스트 추가

5d30d5c

refactor: 컨트롤러에서 호출하는 서비스 변경

4f174fb

refactor: 리뷰 저장 함수에 Transactional 추가

3e05f1a

feat: 테스트 코드 작성을 위해 필요한 생성자, 레포지토리 추가

9c0ad21

test: 리뷰 작성 서비스 테스트

52920e9

style: 개행 및 코드 재정렬

ce2dfc6

nayonsoso added ✨ BE 🚀 기능 🫠 힘들다... labels Aug 10, 2024

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

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

[BE] 리뷰 쓰기 기능을 구현한다. #292

Closed

10 tasks

donghoony changed the title ~~[BE] 변경된 도메인 구조가 적용된 리뷰 작성 API 를 구현한다.~~ [BE] feat: 변경된 도메인 구조가 적용된 리뷰 작성 API 구현 Aug 11, 2024

donghoony requested changes Aug 11, 2024

View reviewed changes

Kimprodp approved these changes Aug 11, 2024

View reviewed changes

skylar1220 requested changes Aug 11, 2024

View reviewed changes

skylar1220 mentioned this pull request Aug 11, 2024

[BE] feat: 리뷰 단건 조회 API 업데이트 #294

Merged

nayonsoso added 8 commits August 11, 2024 22:37

refactor: 없는 선택지 제출 시 발생하는 예외 로그 보강

fe7f203

refactor: validator 어노테이션 수정

f27375e

- Service -> Component

refactor: 예외 클래스 이름 수정

53533a6

refactor: 예외 클래스 이름 수정

f9c1a91

refactor: 예외 로그 보강

aaac7e4

style: 개행 통일

4800f70

refactor: api 버전 구분

b8e9d28

style: 논리적 개행

8cb7c95

nayonsoso mentioned this pull request Aug 11, 2024

[BE] 예외 발생 상황에 따라 로그 레벨을 다르게 둔다. #300

Closed

nayonsoso added 6 commits August 11, 2024 23:20

refactor: 예외 클래스 이름 변경

b6137fc

refactor: 예외 메세지 수정

c150f29

refactor: DB 존재 검증 로직 위치 변경

ff64fe4

- private 함수를 호출하지 사용하지 않고, repository 함수를 바로 호출하도록 변경

Merge branch 'develop' into be/feature/292-review-create-api

4ce7d15

refactor: OptionItem, OptionGroup 레포지토리 통합

a2d3bfb

- question 과 template 패키지에 각각 존재하던 것을 question 패키지로 통합

style: 주석 삭제

d418a24

nayonsoso added 💪 아자아자!! and removed 🫠 힘들다... labels Aug 12, 2024

nayonsoso added 2 commits August 12, 2024 13:34

fix: 컴파일 에러 해결

04f5b66

- merge 후 발생하는 컨플릭을 해결한 뒤 발생한 컴파일 에러를 해결한다.

donghoony approved these changes Aug 12, 2024

View reviewed changes

skylar1220 approved these changes Aug 12, 2024

View reviewed changes

nayonsoso merged commit 48e2876 into develop Aug 12, 2024
4 checks passed

donghoony deleted the be/feature/292-review-create-api branch August 20, 2024 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] feat: 변경된 도메인 구조가 적용된 리뷰 작성 API 구현 #296

[BE] feat: 변경된 도메인 구조가 적용된 리뷰 작성 API 구현 #296

nayonsoso commented Aug 10, 2024

github-actions bot commented Aug 10, 2024 •

edited

Loading

donghoony left a comment

Kimprodp left a comment

Kimprodp Aug 11, 2024

nayonsoso Aug 11, 2024

skylar1220 left a comment

skylar1220 Aug 11, 2024

nayonsoso Aug 11, 2024

skylar1220 Aug 11, 2024

nayonsoso Aug 11, 2024 •

edited

Loading

skylar1220 Aug 11, 2024

skylar1220 Aug 11, 2024

nayonsoso Aug 11, 2024 •

edited

Loading

skylar1220 Aug 11, 2024

nayonsoso Aug 11, 2024

skylar1220 left a comment

		return reviewGroupRepository.findByReviewRequestCode(reviewRequestCode)
		.orElseThrow(() -> new ReviewGroupNotFoundByRequestReviewCodeException(reviewRequestCode));

[BE] feat: 변경된 도메인 구조가 적용된 리뷰 작성 API 구현 #296

[BE] feat: 변경된 도메인 구조가 적용된 리뷰 작성 API 구현 #296

Conversation

nayonsoso 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

Kimprodp 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 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

nayonsoso Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nayonsoso Aug 11, 2024 • edited Loading

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

github-actions bot commented Aug 10, 2024 •

edited

Loading

nayonsoso Aug 11, 2024 •

edited

Loading

nayonsoso Aug 11, 2024 •

edited

Loading