feat: 발송 옵션 조회 기능 일부 수정 & 옵션 선택에 따른 질문 조회 기능 #10
Conversation
623nana
added
enhancement
| return new BaseResponse<>(200, 0, "", options); | ||
| } | ||
|
|
||
| @GetMapping("/letters/options/{optionId}") | ||
| public BaseResponse<List<QuestionResponse>> findQuestionsByOptionId(@PathVariable("optionId") Long optionId) { |
There was a problem hiding this comment.
응답에 해당 option 의 questions 뿐 만 아니라 OptionResponse 에 있는 id, text, covidStat 도 함께 전달하는건 어떨까요?
questions 만 반환하려는 경우라면 /letters/options/{optionsId}/questions url 이 이렇게 되어야 할 것 같습니다.
There was a problem hiding this comment.
그 때 발송 옵션 따로 질문 따로 나누자고 하셔서 각각으로 개발하고 있어요.
질문만 리턴되는 형태라 말씀 주신 형태로 반영할게요!
| import com.nexters.covid.letter.domain.SendOption; | ||
| import java.util.List; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
|
|
||
| @Getter | ||
| @Setter | ||
| public class OptionResponse { |
There was a problem hiding this comment.
발송 옵션이라 직역했어요,,
첨에 그냥 option이라고 했다가 무슨 옵션인지 애매해서?
저거도 일관성 있게 SendOptionResponse로 바꾸는게 좋을 것 같네요!
| public BaseResponse<List<OptionResponse>> options() { | ||
| List<OptionResponse> options = letterService.options(); | ||
| @GetMapping("/letters") | ||
| public BaseResponse<List<LetterResponse>> findLettersByEmail(Authentication authentication) { |
There was a problem hiding this comment.
authentication 객체를 곧바로 사용하지않고 필터쪽에서 저희쪽에서 사용하는 user 와 같은 객체로 변경 후 controller쪽에서 사용할 수 있도록 하면 어떨까요?
There was a problem hiding this comment.
헉 좋은 아이디어인 것 같은데
스프링 시큐리티 로직을 좀더 제가 보고 개선 해볼게요!
|
|
||
| @Transactional(readOnly = true) | ||
| public List<QuestionResponse> findQuestionsByOptionId(Long questionId) { | ||
| return questionRepository.findQuestionsBySendOptionIdEqualsOrSendOptionIdEquals(questionId, 3L) |
There was a problem hiding this comment.
3L을 상수화하여 어떤 의미인지 명시적으로 확인할 수 있어도 좋겠습니다.
| @GetMapping("/letters") | ||
| public BaseResponse<List<LetterResponse>> findLettersByEmail(Authentication authentication) { | ||
| List<LetterResponse> letters = letterService.findLettersByEmail(authentication.getName()); | ||
| return new BaseResponse<>(200, 0, "", letters); |
There was a problem hiding this comment.
첫번째 인자가 HTTP Status code 를 의미하는것이라면
org.springframework.http.HttpStatus 값을 활용하는 것도 좋겠습니다.
| @Test | ||
| @DisplayName("[QuestionRepository] 선택한 옵션에 따른 질문과 공통 질문 조회 테스트") | ||
| void findQuestionByOptionTest() { | ||
| List<Question> questions = questionRepository.findQuestionsBySendOptionIdEqualsOrSendOptionIdEquals(1L, 3L); |
There was a problem hiding this comment.
Long normalSendOptionId = 1L;
Long commonSendOptionId = 3L;
명시적으로 표시하고 테스트내용에서 사용해도 좋겠습니다.
편지 API 개발 #3