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] refactor: 리뷰 등록시 검증 보완 #414

Merged
merged 14 commits into from
Aug 19, 2024

Conversation

skylar1220
Copy link
Contributor

@skylar1220 skylar1220 commented Aug 18, 2024


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

  • 리뷰 등록시 검증을 추가하고 오류를 수정했습니다.

🔥 어떻게 해결했나요 ?

  • 서술형 답변 길이 검증 추가
  • 필수가 아닌 서술형 응답 생성되지 않도록 로직 수정

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

  • 필수가 아닌 서술형 응답은 TextAnswer 객체로 생성이 안되게 로직을 짜려다보니 validator 밖에 조건을 추가하는 형태로 구현했어요. 그런데 이렇게 하니 검증이 validator 안, 밖 두곳에서 이뤄지는 느낌이 들어서 찝찝합니다. 만약 정책이 바뀌어서 필수가 아닌 서술형 응답에 대한 처리 로직이 바뀌어야한다고 할 때 찾기 쉬운가?하는 고민이 듭니다!
  • 검증해야하는 내용이 기존에 더해서 많이 떠오르지 않아서 일단 떠오르는 것만 추가했어요~ 혹시 더 있다면 알려주세요!

📚 참고 자료, 할 말

Copy link

github-actions bot commented Aug 18, 2024

Test Results

56 tests  +5   56 ✅ +5   2s ⏱️ ±0s
20 suites +1    0 💤 ±0 
20 files   +1    0 ❌ ±0 

Results for commit 8a131dd. ± Comparison against base commit 272bff4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@donghoony donghoony left a comment

Choose a reason for hiding this comment

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

생각할 점을 남겨둡니다!

Comment on lines 22 to 23
public static final int MIN_LENGTH = 20;
public static final int MAX_LENGTH = 1_000;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • private으로 할까요?
  • 하나의 정책처럼 보입니다. 도메인에 있어야 하는 값이 맞을까요? 다른 답변이 500자 제한이라면, 도메인을 재사용할 수 없을 듯해요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

각 텍스트 형 답변마다 서로 다른 최소 최대글자수를 갖는 상황이 있을 수 있겠네요.
그렇다면 도메인이 아니라 검증 객체로 빼야 하겠습니다!

더불어 컬럼에 글자수 제한이 원래 있었는데 이것도 같은 맥락으로
텍스트 답변마다 다른 글자수 제한을 가질 수 있기 때문에 제거하였습니다~~

Copy link
Contributor

Choose a reason for hiding this comment

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

텍스트 답변마다 다른 글자수 제한을 가질 수 있기 때문에

그렇다면 서술형 질문에 대해서 Question 객체를 바로 사용하지 않고,
최대 최소 길이를 컬럼으로 갖게 하는 새로운 엔티티가 필요해질 수도 있겠네요!
다만 아직 정말로 필요한 것은 아니라 하나의 가능성만 제시해봅니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

최대 최소 길이를 컬럼으로 갖게 하는 새로운 엔티티가 필요해질 수도 있겠네요!

오호 생각해봐야겠네요!

@@ -60,7 +60,7 @@ private Long saveReview(CreateReviewRequest request, ReviewGroup reviewGroup) {
for (CreateReviewAnswerRequest answerRequests : request.answers()) {
Question question = questionRepository.getQuestionById(answerRequests.questionId());
QuestionType questionType = question.getQuestionType();
if (questionType == QuestionType.TEXT) {
if (questionType == QuestionType.TEXT && !answerRequests.text().isBlank()) {
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.

반영 완료!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영 완료!

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.

앗ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

@@ -60,7 +60,7 @@ private Long saveReview(CreateReviewRequest request, ReviewGroup reviewGroup) {
for (CreateReviewAnswerRequest answerRequests : request.answers()) {
Question question = questionRepository.getQuestionById(answerRequests.questionId());
QuestionType questionType = question.getQuestionType();
if (questionType == QuestionType.TEXT) {
if (questionType == QuestionType.TEXT && answerRequests.isNotBlank()) {
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.

약간 달라요..!
text 타입이면서 내용이 빈문자열이 아닌 경우만 저장 로직을 실행하게 하기위해서입니다~

Comment on lines 22 to 23
public static final int MIN_LENGTH = 20;
public static final int MAX_LENGTH = 1_000;
Copy link
Contributor

Choose a reason for hiding this comment

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

텍스트 답변마다 다른 글자수 제한을 가질 수 있기 때문에

그렇다면 서술형 질문에 대해서 Question 객체를 바로 사용하지 않고,
최대 최소 길이를 컬럼으로 갖게 하는 새로운 엔티티가 필요해질 수도 있겠네요!
다만 아직 정말로 필요한 것은 아니라 하나의 가능성만 제시해봅니다

Comment on lines 9 to 11
public CheckBoxAnswerIncludedTextException() {
super("텍스트형 응답은 옵션 항목을 포함할 수 없어요.");
super("체크박스형 응답은 텍스트를 포함할 수 없어요.");
log.info("Text type answer cannot have option items");
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.

하하 제가 하나만 수정하고 둘은 수정하지 않았네요😇

Copy link
Contributor

@donghoony donghoony left a comment

Choose a reason for hiding this comment

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

👍🏻

Copy link
Contributor

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

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

테드 생일 축하해!!

@skylar1220 skylar1220 merged commit 39fc08b into develop Aug 19, 2024
5 checks passed
@donghoony donghoony deleted the be/refactor/383-review-creation-validation branch August 19, 2024 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BE] 리뷰 등록 시 검증을 강화한다.
4 participants