Conversation
There was a problem hiding this comment.
Pull request overview
이 PR은 어드민용 채팅 기능을 추가하여, 어드민이 관련 채팅방 목록을 조회하고 메시지를 조회/전송할 수 있도록 합니다.
Changes:
- 어드민 전용 채팅방/메시지 조회 및 전송을 위한 Service, DTO, Controller, API 인터페이스 추가.
- 어드민 채팅 방/메시지 조회에 필요한 JPA Repository 쿼리 추가 (어드민이 포함된 채팅방, 어드민 대상 미읽음 메시지 집계 등).
- 관련 서브모듈 리비전 업데이트.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/config | 서브모듈 커밋 해시 업데이트. |
| ChatRoomRepository.java | 어드민이 포함된 채팅방 조회 및 유저-어드민 채팅방 조회용 메서드 추가. |
| ChatMessageRepository.java | 어드민 대상 미읽음 메시지 조회 및 채팅방별 미읽음 메시지 카운트 조회용 메서드 추가. |
| AdminChatService.java | 어드민 채팅방 생성/조회, 메시지 조회 및 전송 비즈니스 로직 구현. |
| AdminChatRoomsResponse.java | 어드민 채팅방 목록 응답 DTO 및 일반 사용자 정보/미읽음 수 매핑 로직 구현. |
| AdminChatMessagesResponse.java | 어드민 채팅방 메시지 페이지 응답 DTO 및 메시지 단건 응답 매핑 로직 구현. |
| AdminChatController.java | 어드민 채팅 API 구현체로 서비스와 HTTP 응답 매핑. |
| AdminChatApi.java | 어드민 채팅 관련 REST API 스펙 및 Swagger 문서 정의. |
| ); | ||
| unreadMessages.forEach(ChatMessage::markAsRead); | ||
|
|
||
| PageRequest pageable = PageRequest.of(page - 1, limit); |
There was a problem hiding this comment.
page와 limit에 대한 값 검증이 없어, page가 0 이하이거나 limit가 1 미만일 경우 PageRequest.of(page - 1, limit)에서 IllegalArgumentException이 발생할 수 있습니다. 컨트롤러 단에서 @RequestParam에 @Min(1) 등의 제약을 추가하거나, 서비스에서 최소값을 보장하는 검증/보정 로직을 추가해 잘못된 입력으로 인한 500 오류를 방지하는 것이 좋습니다.
| PageRequest pageable = PageRequest.of(page - 1, limit); | |
| int safePage = (page == null || page < 1) ? 1 : page; | |
| int safeLimit = (limit == null || limit < 1) ? 1 : limit; | |
| PageRequest pageable = PageRequest.of(safePage - 1, safeLimit); |
| private void validateAdminChatRoom(ChatRoom chatRoom) { | ||
| boolean hasAdmin = chatRoom.getSender().getRole() == UserRole.ADMIN | ||
| || chatRoom.getReceiver().getRole() == UserRole.ADMIN; | ||
| if (!hasAdmin) { | ||
| throw CustomException.of(FORBIDDEN_CHAT_ROOM_ACCESS); | ||
| } | ||
| } | ||
|
|
||
| private User getNormalUser(ChatRoom chatRoom) { | ||
| if (chatRoom.getSender().getRole() == UserRole.ADMIN) { | ||
| return chatRoom.getReceiver(); | ||
| } | ||
| return chatRoom.getSender(); | ||
| } |
There was a problem hiding this comment.
getNormalUser의 어드민/일반 사용자 구분 로직이 AdminChatRoomsResponse.InnerAdminChatRoomResponse 내부의 getNormalUser와 중복되어 있습니다. 동일한 역할 분기 로직이 여러 곳에 퍼져 있으면 향후 UserRole 또는 ChatRoom 구조 변경 시 유지보수가 어려워지므로, ChatRoom 도메인 메서드나 공용 유틸로 추출하여 한 곳에서만 관리하는 것을 권장합니다.
| @Override | ||
| public ResponseEntity<AdminChatRoomsResponse> getChatRooms(Integer adminId) { | ||
| AdminChatRoomsResponse response = adminChatService.getChatRooms(); | ||
|
|
||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @Override | ||
| public ResponseEntity<AdminChatMessagesResponse> getChatRoomMessages( | ||
| Integer chatRoomId, | ||
| Integer page, | ||
| Integer limit, | ||
| Integer adminId | ||
| ) { | ||
| AdminChatMessagesResponse response = adminChatService.getChatRoomMessages( | ||
| chatRoomId, page, limit | ||
| ); | ||
|
|
||
| return ResponseEntity.ok(response); | ||
| } |
There was a problem hiding this comment.
getChatRooms와 getChatRoomMessages의 adminId 파라미터는 메서드 내부에서 사용되지 않고 있습니다. 실제로 필요하지 않은 값이라면 컨트롤러/서비스 시그니처에서 제거하거나, 인증된 어드민 컨텍스트를 활용해 로깅/권한 체크 등으로 활용하여 불필요한 파라미터로 인한 혼동을 줄이는 것이 좋습니다.
|
| @Override | ||
| public ResponseEntity<AdminChatRoomsResponse> getChatRooms(@UserId Integer adminId) { | ||
| AdminChatRoomsResponse response = adminChatService.getChatRooms(); | ||
|
|
||
| return ResponseEntity.ok(response); | ||
| } |
There was a problem hiding this comment.
메서드 파라미터 adminId가 메서드 내부에서 사용되지 않고 있습니다. 사용 의도가 없다면 파라미터를 제거하거나, 특정 어드민의 채팅방만 필터링하는 등의 로직에 활용해 불필요한 파라미터를 없애는 것이 좋습니다.
| @Validated | ||
| @RestController | ||
| @RequiredArgsConstructor | ||
| @RequestMapping("/admin/chats") | ||
| public class AdminChatController implements AdminChatApi { |
There was a problem hiding this comment.
이미 AdminChatApi 인터페이스에서 @RequestMapping(\"/admin/chats\")를 선언하고 있는데 구현 클래스에서도 동일한 매핑을 다시 선언하고 있어 중복입니다. 경로 매핑을 한 곳(일반적으로 구현 클래스)으로 통일하거나, 인터페이스에는 Swagger/@Auth만 두고 타입 레벨 @RequestMapping은 제거해 매핑 구성을 명확하게 하는 것이 좋습니다.
| import jakarta.validation.Valid; | ||
|
|
||
| @Tag(name = "(Admin) Chat: 채팅", description = "어드민 채팅 API") | ||
| @RequestMapping("/admin/chats") |
There was a problem hiding this comment.
이 인터페이스의 @RequestMapping(\"/admin/chats\")는 구현 클래스(AdminChatController)에서도 동일하게 선언되어 있어 어떤 쪽이 실제 URL prefix를 담당하는지 혼동을 줄 수 있습니다. URL 경로 설정과 보안 설정(예: @Auth)을 명확히 분리하기 위해, 경로 매핑은 컨트롤러 구현 쪽으로만 두고 인터페이스에는 문서화/권한 메타데이터만 유지하는 방향을 고려해볼 수 있습니다.
| @RequestMapping("/admin/chats") |
|
1 participant
🔍 개요
🚀 주요 변경 내용
💬 참고 사항
✅ Checklist (완료 조건)