-
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(#24): 답변 작성 API 구현 #55
The head ref may contain hidden characters: "feature/#24-feat-\uB2F5\uBCC0-\uC791\uC131-api"
Conversation
Test Results57 tests 56 ✅ 3s ⏱️ Results for commit 8fb91a7. ♻️ This comment has been updated with latest results. |
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.
수고하셨습니다! 👍
@@ -23,4 +23,14 @@ public ApiResponse<SurveyResponse> getNextSurvey( | |||
@AuthenticationPrincipal CustomUserDetails member, @PathVariable Long bundleId) { | |||
return ApiResponse.success(surveyService.getNextSurvey(bundleId, member.getMemberId())); | |||
} | |||
|
|||
@ResponseStatus(HttpStatus.NO_CONTENT) | |||
@PostMapping("/submit/{surveyId}") |
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.
p2.
동사는 지양하면 좋을 것 같아요!
이미 http 메서드 자체가 어떤 액션을 취하는지 잘 드러나는 것 같습니다.
/api/v1/surveys/{surveyId}
가 설문 질문 자체를 수정하는 것 같아 앞에 동사를 붙여주신 거면, 뒤에 명사를 적어도 좋을 것 같습니다.- 아니면
SurveySubmissionService
를 만들어도 좋을 것 같습니다. 추후에 답변 목록을 내려줄 때 (월별)surveyId
로 설문을 조회하고 내려주진 않을 것 같아요
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.
넵 확실히 동사를 사용하기보다 submission
을 뒤에 붙여서 /api/v1/surveys/{surveyId}/submission
처럼 활용해보겠습니다 !
// when | ||
// then |
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.
제가 템플릿을 쓰고있어서 test 를 치면 자동으로
@DisplayName("")
@Test
void test() {
//given
//when
//then
}
이렇게 생성되게 사용하고있어요... 손에 너무 습관이 들어서 안지운것 같습니다
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.
고생하셨습니다!
final Member member, | ||
final Survey survey, | ||
final SurveyOption selectedOption, | ||
final String comment) { | ||
this.member = member; | ||
this.survey = survey; | ||
this.selectedOption = selectedOption; | ||
this.comment = comment; |
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.
pn2. 필드명 수정
comment
라는 필드는 비교적 추상적인 표현인 것 같아요!- 프론트엔드 팀의 경우,
회고
라는 것을 명확하게 표현하기 위해retrospective
라는 네이밍을 사용하더라고요. 해당 네이밍을 사용하는 건 어떠신가요? (스펙도 맞출겸해서 좋을 것 같습니다!)
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.
감사합니다 ! 프론트엔드의 네이밍과 맞추는것도 좋을 것 같습니다 !
@@ -0,0 +1,3 @@ | |||
package org.nexters.jaknaesocore.domain.survey.dto; | |||
|
|||
public record SurveySubmissionServiceRequest(Long optionId, String comment) {} |
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.
저희 그동안 service에 전달하는 DTO 객체 네이밍을 Command
로 하고 있었는데, Command
라고 통일하는 건 어떨까요?
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.
Command
라고 사용하고 있는줄은 몰랐네요..! 명령이라는 의미로 Command
로 통일한걸까요 ??
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.
- 넵 그렇게 이해하셔도 될 것 같아요.
- 특정한 행위를 트리거하는 역할이라는 것을 강조하기 위해 해당 네이밍을 사용하면 좋을 것 같아요.
@PostMapping("/{surveyId}/submission") | ||
public ApiResponse<?> submitSurvey( |
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.
명사형으로 바꾼 것 좋습니다👍
- comment 보다 회고 retrospective라는 정확한 필드명 사용
- 제출이라는 의미를 주고 싶기 때문에 submit 동사 대신 submission 명사 사용
4ea917a
to
a787195
Compare
- 특정한 행위를 트리거하는 역할이라는 것을 강조하기 위한 네이밍
작업 개요
작업 사항