-
Notifications
You must be signed in to change notification settings - Fork 0
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
야간잔류 신청 기능 구현 #26
야간잔류 신청 기능 구현 #26
Conversation
* Application 엔티티에 컬럼 추가 * ApplicationController에서 ApiResponse를 이용한 응답 및 에러 처리 * ApplicationRequest에서 요청 DTO 구현 * ApplicationResponse에서 응답 DTO 구현 * ApplicationRepository와 MemberRepository 생성 * ApplicationService 구현 Ref: #21
Test Coverage Report
|
@PostMapping | ||
public ResponseEntity<ApiResponse<ApplicationResponse>> createApplication(@RequestBody ApplicationRequest applicationRequest) { |
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.
ResponseEntity와 ApiResponse는 거의 같은 기능이어서 한 번 더 감싸지 않아도 될 것 같습니다!
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.
넵! 좋습니다. ApiResponse에 상태 코드별 메서드를 만들면 더 간단하게 구현할 수 있을 것 같습니다.
.body(new ApiResponse<>( | ||
new ApiResponse.Status(HttpStatus.BAD_REQUEST, "필수 값이 누락되었습니다"), | ||
null | ||
)); |
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.
넵 간단하게 추가해주세요! 더 필요한 부분이 있다면 제가 나중에 추가하겠습니다.
try { | ||
ApplicationResponse applicationResponse = applicationService.createApplication(applicationRequest); | ||
return ResponseEntity.ok(new ApiResponse<>( | ||
new ApiResponse.Status(HttpStatus.CREATED, "신청이 완료되었습니다"), | ||
applicationResponse | ||
)); | ||
} catch (IllegalArgumentException e) { | ||
// 401 에러 처리 | ||
return ResponseEntity | ||
.status(HttpStatus.UNAUTHORIZED) | ||
.body(new ApiResponse<>( | ||
new ApiResponse.Status(HttpStatus.UNAUTHORIZED, "권한이 없습니다"), | ||
null | ||
)); | ||
} |
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.
try-catch 문을 반복적으로 사용해서 코드가 조금 길어지는 것 같습니다!
이 부분은 예외 상황에서 Http 응답이 아닌 예외만 반환하게 하고, 추후에 전역 예외 핸들러를 통해 통일된 Http 응답 형식으로 반환하는 방식으로 수정할 수 있을 것 같습니다.
@ExceptionHandler(RuntimeException.class)
public ApiResponse<Void> handle(RuntimeException exception, HttpServletRequest request) {
logInfo(exception, request);
return ApiResponse.exception(exception);
}
요런 느낌인데 나중에 시간나면 추가해볼게유
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.
오 좋습니다! 일단 해당 코드는 주석 처리하고 나중에 예외 throw 하는 방식으로 바꾸겠습니다.
private String name; | ||
private MemberType type; | ||
private Long memberId; | ||
private String loginId; | ||
private String phoneNumber; | ||
private String email; | ||
private Integer participantCount; | ||
private List<String> coParticipantNames; | ||
private Boolean privacyAgreement; | ||
private LocalDateTime startTime; | ||
private LocalDateTime endTime; |
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.
회원이 로그인한 상태로 야간잔류를 신청한다는 걸 감안하면 request dto에서 신청자의 정보를 받을 필요 없이 현재 로그인된 회원의 정보를 그대로 가져올 수 있어서, 관련 필드 변수들을 빼도 될 것 같습니다!
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.
넵 수정하겠습니다.
private String loginId; | ||
private String name; | ||
private MemberType type; | ||
private Long memberId; | ||
private String phoneNumber; | ||
private String email; | ||
private Integer participantCount; | ||
private List<String> coParticipantNames; | ||
private Boolean privacyAgreement; | ||
private LocalDateTime startTime; | ||
private LocalDateTime endTime; |
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.
여기서 회원 관련 정보는 히원 정보 DTO로 묶으면 좋을 것 같고, coParticipantNames도 단순 이름이 아닌 회원 정보 DTO 내용을 담으면 좋을 것 같습니다!
(관련 DTO 구현이 아직 해당 PR에는 반영되지 않아서 추후에 변경해도 괜찮을 것 같습니다.)
또한 participantCount의 경우, coParticipantNames.size() + 1로 자체 계산할 수 있을 것 같습니다.
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.
넵 수정하겠습니다! 감사합니다
충돌 해결해주시면 감사하겠습니다! |
* ResponseEntity를 ApiResponse로 통일 * ApiResponse.exception을 활용해 응답
* Spring Security 도입하여 코드 리팩토링 * ApplicationController에서 Authentication을 이용해 loginId 가져옴 * ApplicationService에서 findByLoginId 적용
Test Coverage Report
|
Test Coverage Report
|
Test Coverage Report
|
Test Coverage Report
|
#️⃣ 연관된 이슈
#21
📝 작업 내용
📷 스크린샷 (선택)
회원가입 후, 로그인을 한 뒤에 야간잔류 신청 기능을 이용할 수 있습니다.
![image](https://private-user-images.githubusercontent.com/97730320/389280838-26a9dbd7-b443-437f-bc8c-5c8d37db65ac.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1ODgxNjcsIm5iZiI6MTczOTU4Nzg2NywicGF0aCI6Ii85NzczMDMyMC8zODkyODA4MzgtMjZhOWRiZDctYjQ0My00MzdmLWJjOGMtNWM4ZDM3ZGI2NWFjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDAyNTEwN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTJlNzM3ZTVlNWNmODg3NTFkN2U4Mzc3YzA3OTA3MzI1Mzc5N2Q0Mjc1YWU2ZTY3Y2U2ZGM0MjRmMzVkYmQ0ZDImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.Ejr3iXb_LNH4dgJPNCn_IEv6d-JQXUjjg3o5ryzENCg)
coParticipants 엔티티 별도로 생성했습니다.
![image](https://private-user-images.githubusercontent.com/97730320/389280865-94594ba0-951f-4570-a13d-8a2b6dd50732.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1ODgxNjcsIm5iZiI6MTczOTU4Nzg2NywicGF0aCI6Ii85NzczMDMyMC8zODkyODA4NjUtOTQ1OTRiYTAtOTUxZi00NTcwLWExM2QtOGEyYjZkZDUwNzMyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDAyNTEwN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTA0NWZmMjJkZWZkNjAyY2JiMjc3NzZiOTVhZTk1MGJjZDQ0ZjYyZjYyMmNkMGQzODc3MmJmNTQyNjg2ZjkzOGMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.XBvi_tJfbgBuW2Jv89g_xuahOkM-xnaHjg1Fjp1xbSM)
💬 리뷰 요구사항 (선택)
피드백 많이 남겨주세요!!
구현 완료했습니다. 🫠