-
Notifications
You must be signed in to change notification settings - Fork 5
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
[Feature] - 로그인 한 사용자만 여행기를 작성할 수 있도록 리팩토링 #175
Conversation
Test Results53 tests 52 ✅ 4s ⏱️ Results for commit 5754dc6. ♻️ 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.
문제 없어보입니닷!
approve 드립니다!
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.
질문이랑 리뷰 한 개씩 남겼습니다!
미룬이가 되지 않은 리건이 참 멋지네요 👍
String memberId = Objects.requireNonNull(request).getAttribute("memberId").toString(); | ||
|
||
if (memberId == null) { | ||
throw new BadRequestException("로그인이 필요합니다."); |
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.
로그인 필요할 땐 UnauthorizedException
이 좋을 것 같슴다!
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.
예외에 대해서 고민을 많이했는데 UnauthorizedException
이 좋을 것 같습니다!
public ResponseEntity<TravelogueResponse> createTravelogue(@Valid @RequestBody TravelogueRequest request) { | ||
TravelogueResponse response = travelogueFacadeService.createTravelogue(request); | ||
public ResponseEntity<TravelogueResponse> createTravelogue( | ||
@NotNull MemberAuth member, |
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.
질문) MethodArgumentResolver에서 null 검증하고 있는데 여기서 또 validation 붙인 이유가 있나요?.?
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.
고생하셨습니다리건 🔫
public ResponseEntity<TravelogueResponse> createTravelogue( | ||
@NotNull MemberAuth member, |
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.
@NotNull
validation은 @Valid
가 있을 때 의미 있다고 알고 있어요!
@NotNull
만 사용되고 있어 리뷰 남깁니다.
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.
👍🏻
반영 확인했습니다
* refactor: 패키지 구조 변경 * refactor: 인증된 사용자만 여행기를 작성할 수 있도록 리팩토링 * feat: 사용자 서비스 구현 * refactor: 여행기 관련 테스트에 사용자(작성자) 정보를 추가 * refactor: 로그인 되어 있지 않을 경우 발생하는 예외 타입 변경 * test: 로그인 되어 있지 않을 경우 발생하는 예외 테스트 작성 * chore: 불필요한 어노테이션 제거 * chore: MemberAuth validation 추가 * fix: 변경된 패키지에 맞게 import 구문 수정 * refactor: Project group 수정
✅ 작업 내용