-
Notifications
You must be signed in to change notification settings - Fork 1
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
[FEAT] 7일 이후 문답 구현 #122
[FEAT] 7일 이후 문답 구현 #122
Conversation
parentchild.initQnA(); | ||
parentchild.addQnA(newQnA); | ||
parentchild.initQna(); | ||
parentchild.setQna(newQnA); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
초기 세팅으로 Qna를 세팅할 때는 setQna를,
7일 이후에 Qna가 추가될 때는 addQna를 쓰는 방식으로 구현했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
복잡한 로직 짜느라 .. 수고 많으셨습니다~ 짱짱
@PatchMapping("/qna/restart") | ||
@ResponseStatus(HttpStatus.OK) | ||
public ApiResponse restartQna(Principal principal) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p5;
이거 좀 신경쓰이네요..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 굉장히 킹받지만,, 이때 이렇게 다 짰었어서 추후 리팩토링때 변경할 예정입니다!
// 5. 이 경우 아예 추가될 질문이 없으므로 예외 발생시킴 | ||
List<Question> targetQuestions = questionRepository.findByTypeInAndIdNotIn(types, doneQuestionIds); | ||
if (targetQuestions.isEmpty()) { | ||
throw new CustomException(QUESTION_NOT_FOUND_ERROR); | ||
} | ||
|
||
QuestionSection section = qnaList.get(parentchild.getCount() - 1).getQuestion().getSection(); | ||
List<Question> differentSectionQuestions = targetQuestions.stream() | ||
.filter(question -> !question.getSection().equals(section)) | ||
.collect(Collectors.toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 부분 나중에 쿼리로 한번에 처리해도 좋을 것 같아요!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
targetQuestions 중에서 section과 다른 differentSectionQuestions, section과 일치하는 equalSectionQuestions이 둘다 필요할 수도 있기 때문에 (section과 다른 질문이 없을 경우)
쿼리로 묶을 수 있는 범위가 targetQuestions까지라고 생각했습니다!
질문의 수에 따라서 한번에 쿼리로 처리하는게 나을지에 대한 해답이 달라질 것 같습니다
일단 현재는 질문이 30개 수준이기 때문에 쿼리 수를 늘리기보다는 현재처럼 처리하다가, 추후에 질문이 많아지게 되면 고려해보겠습니당!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굳굳 수고하셨습니다!! 👍
if (targetQuestions.isEmpty()) { | ||
throw new CustomException(QUESTION_NOT_FOUND_ERROR); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
질문이 더 이상 없는 경우에 대해 기획 측에서 넘겨주는 내용에 따라 예외로 던지지 않는 방향도 고려할 수 있을 것 같아요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분 501 에러로 처리하도록 적절히 변경했습니다! (서버 알림과도 연동)
📌 관련 이슈
closed #121
✨ 어떤 이유로 변경된 내용인지
"엄빠도 어렸다"는 더이상 7일에 멈추는 서비스가 아닙니다.
지금 현재 DB에 있는 질문들의 타입
7일 이후에 새롭게 추가될 때 해야하는 작업
🙏 검토 혹은 리뷰어에게 남기고 싶은 말