Feature/#105 #110
Merged
Feature/#105 #110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dongchandev
reviewed
Sep 4, 2024
| @@ -40,6 +41,11 @@ public Response delete(@PathVariable String id) { | |||
| return commandUseCase.delete(id); | |||
| } | |||
|
|
|||
| @PatchMapping("/status/{id}") | |||
| public Response updateStatus(@PathVariable("id") String id, @RequestParam ActiveStatus status){ | |||
dongchandev
reviewed
Sep 4, 2024
| @@ -51,6 +51,13 @@ public ResponseData<List<MemberInfoRes>> getDeactivateMembers() { | |||
| .toList()); | |||
| } | |||
|
|
|||
| public ResponseData<List<MemberInfoRes>> getPendingMembers(){ | |||
| return ResponseData.ok("보류중인 멤버 조회 성공", memberRepository.findByStatusOrderByStudent(ActiveStatus.PENDING) | |||
There was a problem hiding this comment.
해당 메서드도 pending만 불러오는것이 아닌 한개의 메서드로 통일시켜 파라미터로 구분하도록 수정하겠습니다
dongchandev
reviewed
Sep 4, 2024
| @@ -15,7 +15,7 @@ public enum MemberExceptionCode implements ExceptionCode { | |||
| TEACHER_NOT_FOUND(404, "없는 선생님"), | |||
| MEMBER_DUPLICATION(409, "이미 존재하는 멤버"), | |||
| BROADCAST_CLUB_MEMBER_DUPLICATION(409, "이미 존재하는 방송부원"), | |||
| UNMODIFIABLE_ROLE(403, "해당 유저는 수정이 불가합니다"); | |||
| UNAUTHORIZED(403, "권한 없음"); | |||
There was a problem hiding this comment.
UNMODIFIABLE_ROLE는 목적인 대상을 수정 할 수 없다는 뜻이고
UNAUTHORIZED는 사용자의 권한이 없다는 뜻입니다.
에러의 네이밍이 사용하는데 헷갈릴것같아 수정하였습니다.
There was a problem hiding this comment.
http에서 unauthorized는 401을 의미하기 때문에 다른 네이밍을 고려하는게 좋을 것 같아요.
There was a problem hiding this comment.
코드 한 번 보니까, 해당 exception code를 사용하는 곳이 없어서 해당 라인을 없애는게 좋아보이네요
suw0n
reviewed
Sep 4, 2024
| @@ -83,6 +83,13 @@ public Response delete(String id) { | |||
| return Response.noContent("멤버 삭제 성공"); | |||
| } | |||
|
|
|||
| @CacheEvict(value = "members-cache", key = "'activeMembers'") | |||
| public Response status(String id, ActiveStatus status) { | |||
There was a problem hiding this comment.
현재 updateStatus메서드를 사용하는 기존 메서드가 있어서 임시로 네이밍을 해뒀습니다
웹에서 변경사항에 맞춰 수정하면 기존메서드를 삭제하고 메서드 명도 updateStatus로 변경할 예정입니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
현재 : 회원가입시 DEACTIVATE로 가입
변경 : 회원가입시 PENDING으로 가입
추가 : 유저 불러올때 ACITVE, DEACTIVATE 메서드가 분리되어있음 -> 메서드를 하나로 통일 후 파라미터를 통해 상태를 받아 반환