[DDING-66] ClubMember 목록 조회 API 응답값 변경 #201
Conversation
워크스루이 풀 리퀘스트는 클럽 멤버 정보를 반환하는 방식을 변경하는 리팩토링 작업입니다. 기존의 변경 사항
가능한 관련 PR
제안된 레이블
제안된 리뷰어
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/ClubMemberListResponse.java (1)
6-9: Schema 설명과 클래스 이름의 불일치 수정 필요클래스 이름이
CentralClubMemberListResponse에서ClubMemberListResponse로 변경되어 더 일반적인 의미를 가지게 되었습니다. 하지만 Schema 설명은 여전히 "중앙동아리"로 특정되어 있습니다. 일관성을 위해 Schema 설명도 수정이 필요합니다.@Schema( name = "ClubMemberResponse", - description = "중앙동아리 - 동아리원 조회 응답" + description = "동아리원 조회 응답" )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/ddingdong/ddingdongBE/domain/club/api/CentralClubApi.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/CentralClubController.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/AllClubMemberInfoResponse.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/ClubMemberListResponse.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/dto/query/AllClubMemberInfoQuery.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/dto/query/ClubMemberListQuery.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (10)
src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/dto/query/AllClubMemberInfoQuery.java (2)
5-8: 클래스 이름과 필드 명명이 명확합니다.Record 클래스를 사용하여 불변성을 보장하고, 필드명이 의도를 잘 표현하고 있습니다.
10-12: 정적 팩토리 메서드가 잘 구현되어 있습니다.
of메서드를 통해 객체 생성의 의도가 명확히 드러나며, 생성자 사용을 캡슐화하고 있습니다.src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberService.java (1)
11-11: 반환 타입 변경이 요구사항과 잘 부합합니다.
getAllMyClubMember메서드의 반환 타입을AllClubMemberInfoQuery로 변경한 것이 클라이언트의 요구사항(동아리명 포함)을 잘 반영하고 있습니다.src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/dto/query/ClubMemberListQuery.java (2)
Line range hint
5-12: 클래스 이름 단순화가 적절합니다.
Central접두어를 제거하여 더 간결하고 명확해졌습니다.개인정보 처리에 대한 검토가 필요합니다.
전화번호와 같은 민감한 개인정보가 포함되어 있습니다. 보안을 위해 다음 사항들을 검토해주세요:
- 로그에 개인정보가 노출되지 않도록 주의
- 필요한 경우 전화번호 마스킹 처리 고려
Line range hint
14-22: 정적 팩토리 메서드가 잘 구현되어 있습니다.엔티티에서 DTO로의 변환 로직이 깔끔하게 구현되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/AllClubMemberInfoResponse.java (2)
8-13: API 문서화가 잘 되어있습니다.Swagger 어노테이션을 통해 API 응답 구조가 명확하게 문서화되어 있습니다.
클래스 이름이 적절합니다.
PR 목표에서 언급된 'wrapping' 대신
AllClubMemberInfoResponse라는 명확한 이름을 사용한 것이 좋습니다.
15-20: 매핑 로직이 깔끔하게 구현되어 있습니다.Stream API를 활용하여 Query에서 Response로의 변환이 간결하게 구현되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java (1)
35-41: 구현이 PR 목적에 잘 부합합니다!클럽 이름을 포함하도록 응답 구조를 변경한 것이 PR의 목적에 잘 부합합니다. 스트림 API를 사용하여 깔끔하게 구현되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/club/controller/CentralClubController.java (1)
57-61: 컨트롤러 구현이 깔끔합니다!다른 메서드들과 동일한 패턴을 따르며, 응답 구조 변경이 깔끔하게 구현되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/club/api/CentralClubApi.java (1)
54-58: API 문서화가 잘 되어있습니다!응답 타입 변경에 따른 Swagger 어노테이션 업데이트가 잘 되어있습니다.
| public record AllClubMemberInfoResponse( | ||
| @Schema(name = "동아리명", example = "COW") | ||
| String clubName, | ||
| @ArraySchema(schema = @Schema(implementation = ClubMemberListResponse.class)) | ||
| List<ClubMemberListResponse> clubMembers | ||
| ) { | ||
|
|
||
| public static AllClubMemberInfoResponse from(AllClubMemberInfoQuery query) { | ||
| List<ClubMemberListResponse> responses = query.clubMemberListQueries().stream() | ||
| .map(ClubMemberListResponse::from) | ||
| .toList(); | ||
| return new AllClubMemberInfoResponse(query.clubName(), responses); | ||
| } | ||
| } |
There was a problem hiding this comment.
p3)
별도의 클래스로 분리하지 말고 하나의 이너 클래스로 두어 관리하는건 어떨까요?
| public record AllClubMemberInfoResponse( | |
| @Schema(name = "동아리명", example = "COW") | |
| String clubName, | |
| @ArraySchema(schema = @Schema(implementation = ClubMemberListResponse.class)) | |
| List<ClubMemberListResponse> clubMembers | |
| ) { | |
| public static AllClubMemberInfoResponse from(AllClubMemberInfoQuery query) { | |
| List<ClubMemberListResponse> responses = query.clubMemberListQueries().stream() | |
| .map(ClubMemberListResponse::from) | |
| .toList(); | |
| return new AllClubMemberInfoResponse(query.clubName(), responses); | |
| } | |
| } | |
| public record ClubMemberListResponse( | |
| @Schema(name = "동아리명", example = "COW") | |
| String clubName, | |
| @ArraySchema(schema = @Schema(implementation = ClubMemberListResponse.class)) | |
| List<ClubMemberResponse> clubMembers | |
| ) { | |
| public static AllClubMemberInfoResponse from(AllClubMemberInfoQuery query) { | |
| List<ClubMemberListResponse> responses = query.clubMemberListQueries().stream() | |
| .map(ClubMemberResponse::from) | |
| .toList(); | |
| return new ClubMemberListResponse(query.clubName(), responses); | |
| } | |
| @Schema( | |
| name = "ClubMemberResponse", | |
| description = "중앙동아리 - 동아리원 정보 조회 응답" | |
| ) | |
| public record ClubMemberListResponse( | |
| @Schema(description = "식별자", example = "1") | |
| Long id, | |
| @Schema(description = "이름", example = "홍길동") | |
| String name, | |
| @Schema(description = "학번", example = "60001111") | |
| String studentNumber, | |
| @Schema(description = "전화번호", example = "010-1234-5678") | |
| String phoneNumber, | |
| @Schema(description = "동아리원 역할", | |
| example = "LEADER", | |
| allowableValues = {"LEADER", "EXECUTION", "MEMBER"} | |
| ) | |
| String position, | |
| @Schema(description = "학과", example = "학과") | |
| String department | |
| ) { | |
| public static ClubMemberResponse from(ClubMemberListQuery query) { | |
| return new ClubMemberListResponse( | |
| query.id(), | |
| query.name(), | |
| query.studentNumber(), | |
| query.phoneNumber(), | |
| query.position(), | |
| query.department() | |
| ); | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
수정 완료하였습니다!
56d8d84
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/dto/query/AllClubMemberInfoQuery.java (2)
6-9: 클래스 이름을 더 명확하게 변경하는 것을 고려해 보세요.
AllClubMemberInfoQuery라는 이름보다ClubMemberListWithNameQuery와 같이 클래스의 목적을 더 명확하게 표현하는 이름이 좋을 것 같습니다.
11-13: null 체크 로직 추가를 고려해 보세요.팩토리 메서드에서
clubName과clubMembers에 대한 null 체크가 없습니다. 유효성 검사를 추가하는 것이 안전할 것 같습니다.public static AllClubMemberInfoQuery of(String clubName, List<ClubMember> clubMembers) { + Objects.requireNonNull(clubName, "클럽 이름은 null일 수 없습니다"); + Objects.requireNonNull(clubMembers, "클럽 멤버 목록은 null일 수 없습니다"); return new AllClubMemberInfoQuery(clubName, clubMembers); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/AllClubMemberInfoResponse.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/dto/query/AllClubMemberInfoQuery.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (1)
src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java (1)
34-37: 예외 처리 로직 추가가 필요합니다.
clubService.getByUserId()에서 클럽을 찾지 못했을 경우의 예외 처리가 명시적으로 보이지 않습니다. 적절한 예외 처리를 추가하는 것이 좋겠습니다.다음 스크립트로 예외 처리 여부를 확인해보세요:
| @Schema(description = "동아리원 역할", | ||
| example = "LEADER", | ||
| allowableValues = {"LEADER", "EXECUTION", "MEMBER"} | ||
| ) | ||
| String position, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Position 필드를 enum으로 변경하는 것이 좋겠습니다.
현재 position 필드가 String으로 되어있는데, 허용 가능한 값이 제한적이므로 enum을 사용하는 것이 타입 안전성을 보장할 수 있습니다.
public enum ClubMemberPosition {
LEADER, EXECUTION, MEMBER
}그리고 다음과 같이 변경하세요:
- String position,
+ ClubMemberPosition position,| public static ClubMemberListResponse from(ClubMember clubMember) { | ||
| return new ClubMemberListResponse( | ||
| clubMember.getId(), | ||
| clubMember.getName(), | ||
| clubMember.getStudentNumber(), | ||
| clubMember.getPhoneNumber(), | ||
| clubMember.getPosition().getName(), | ||
| clubMember.getDepartment() | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
from 메서드에서 position 매핑 로직 개선이 필요합니다.
position 값을 가져올 때 getName()을 호출하고 있는데, enum으로 변경 후에는 직접 enum 값으로 매핑하는 것이 좋겠습니다.
- clubMember.getPosition().getName(),
+ clubMember.getPosition(),Committable suggestion skipped: line range outside the PR's diff.
|
🚀 작업 내용
💬 리뷰 중점사항
래핑한 클래스 이름이 적절한지 봐주세요!
Summary by CodeRabbit
새로운 기능
리팩토링
문서화