-
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
feat: 일반 관리자 기능 구현 #14
Conversation
- Group 엔티티 명을 Role 로 변경 - ManyToOne Fetch Type 을 Lazy로 설정 - Member 엔티티 오타 수정
main 브랜치 변경이 있어 |
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.
기본 기능을 잘 구현해주셨네요 👍
몇가지 코멘트 드렸으니 다음 pr에 반영해주시면 될 것 같습니다!
@DeleteMapping("/me") | ||
public ResponseEntity<ApiResponse<Void>> deleteMyAccount() { | ||
adminService.deleteAdmin(TEMP_CURRENT_ID); | ||
return ResponseEntity.ok(ApiResponse.ok(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.
Response.noContent 로 리턴해도 될 것 같습니다!
private final MemberRepository memberRepository; | ||
|
||
@Transactional(readOnly = true) | ||
public Object getMembers(List<Long> memberIds) { |
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.
리턴 타입을 명확히 해주면 좋을 것 같아요!
List<MemberSummaryGetResponse>
private final AdminService adminService; | ||
|
||
@GetMapping("/members") | ||
public ResponseEntity<ApiResponse<?>> getMembers(@RequestParam(required = false) List<Long> memberIds) { |
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.
기존 구현이 파라미터의 존재유무에 따라 다른 값을 반환하도록 구현되어 있어
/members와 /members/details 로 엔드포인트 분리하였습니다!
String phoneNumber, | ||
String email | ||
) { | ||
public static MemberSummaryGetResponse toDto(Member 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.
정적 팩토리 메서드 네이밍 컨벤션 참고해주세요! 참고
from 으로 통일해주는건 어떨까요~?
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나 repository를 같이 사용해야 할 것 같아서 최소한의 구현만 하고 PR 올립니다.
Custom Exception 및 Controller Advice는 아직 구현하지 않았고, GlobalControllerAdvice에 대한 논의가 필요할 것 같습니다.
resolve: #10