-
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
[LIME-141] 채팅방 목록 조회, 채팅방 참여, 채팅방 인원수 조회, 채팅방 나가기 기능 구현 #70
Conversation
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 final ChatService chatService; | ||
|
||
@Operation(summary = "채티방 전체 조회", description = "채팅방을 조회합니다.") |
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.
아! 이 부분은 다음 채팅 PR이 완료되면 넣기 위해 주석처리하여습니다.
따로 말씀 못드려서 혼동이 왔네요 죄송합니다.
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.
수정하였습니다! ㅎㅎ 2bcd9c6
@Operation(summary = "채팅방 인원수 조회", description = "채팅방의 인원수를 조회합니다.") | ||
@GetMapping("/{chatRoomId}/members/count") | ||
public ResponseEntity<Integer> countChatRoomMembers(@PathVariable final Long chatRoomId) { | ||
return ResponseEntity.ok( | ||
chatRoomService.countChatRoomMembersByChatRoomId(chatRoomId) | ||
); | ||
} |
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.
인원수 조회하는 api가 따로 필요한 이유가 호기심으로 궁금해요
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.
수고하셨습니다~! 몇 가지 리뷰 남겼어요!!
|
||
private final ChatRoomService chatRoomService; | ||
|
||
//private final ChatService chatService; |
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.
P5;
ChatService는 사용하지 않는 건가요?? 그럼 삭제하는 게 어떨까요!
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이 완료되면 다시 넣을 예정 입니다.
public record ChatRoomGetListResponse( | ||
List<ChatRoomInfo> chatRoomInfos | ||
) { | ||
public static ChatRoomGetListResponse from(ChatRoomGetServiceListResponse chatRoomGetServiceListResponse) { |
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.
P1;
final 키워드 추가해주세여~
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.
추가하였습니다! 98cfbd5
if(Objects.isNull(memberId)) { | ||
chatRoomInfos = chatRoomReader.readOpenChatRooms(); | ||
} else { | ||
chatRoomInfos = chatRoomReader.readOpenChatRoomsByMemberId(memberId); | ||
} |
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.
P5; (궁금)
회원 아이디 유무에 따라서 조회하는 채팅방이 다른것 같은데 어떻게 다른건가요??
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.
memberId에 따라 구현 계층에서 호출하는 메서드가 달랐습니다.
null 확인은 구현계층에서 하도록 변경하였습니다. d675452
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.
회원 아이디 유무에 따라서 조회하는 채팅방이 다른것 같은데 어떻게 다른건가요??
- 아닙니다!! 같은 채팅방을 조회 하는데 사용 됩니다.
회원과 비회원이 들어갈 수 있는 채팅방이 따로 있는건가요?!
- 회원과 비회원이 구분되어 있는 채팅방은 없습니다!
전체적으로 lime.domains.chatroom.repository.ChatRoomQueryDslImpl에 구현되어 있는 로직을 보셔야 할 거 같아요
); | ||
} | ||
|
||
public void joinChatRoom(Long chatRoomId) { |
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.
p1;
final 추가 해주세요~ 다른 곳에도 빠져있네요!
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.
추가하였습니다! d3d1cfa
📌 PR 종류
어떤 종류의 PR인지 아래 항목 중에 체크 해주세요.
📌 어떤 기능이 추가 되었나요?
총 네 가지의 기능이 추가 되었습니다.
Issue Number
LIME-141
기능 설명
채팅방 목록 조회(전체)
채팅방 참여
채팅방 인원수 조회
채팅방 나가기
📌 기존에 있던 기능에 영향을 주나요?