[FEAT] 채팅 모달 서버 연동 - #88
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 16 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough견적 상세 화면의 채팅 모달을 컨테이너 기반으로 변경했습니다. 컨테이너와 공개 훅이 채팅방 생성·조회, 메시지 페이징·전송, 소켓 연결, 오류 및 토스트 상태를 관리합니다. 메시지 목록은 발신자별 스타일과 한국 시간 표시를 제공합니다. Changes견적 채팅 통합
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant 견적상세화면
participant ChatRoomModalContainer
participant useChatRoomModalController
participant useConnectedChatRoomModalController
participant Socket
견적상세화면->>ChatRoomModalContainer: estimateId 전달
ChatRoomModalContainer->>useChatRoomModalController: 채팅방 조회 또는 생성
useChatRoomModalController-->>ChatRoomModalContainer: 활성 채팅방 반환
ChatRoomModalContainer->>useConnectedChatRoomModalController: 메시지 연결
useConnectedChatRoomModalController->>Socket: 소켓 연결
Socket-->>useConnectedChatRoomModalController: 실시간 메시지 전달
useConnectedChatRoomModalController-->>ChatRoomModalContainer: 병합·정렬된 메시지 반환
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/components/estimate/pending/PendingEstimateDetailView.tsx (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value배럴 export를 추가했지만 두 화면 모두 파일 경로를 직접 import합니다. 이 PR은
src/components/chat/index.ts에ChatRoomModalContainernamed export를 추가했습니다. 진입점을 통일해 주세요.
src/components/estimate/pending/PendingEstimateDetailView.tsx#L5-L5:import { ChatRoomModalContainer } from "@/components/chat";로 변경해 주세요.src/components/estimate/sent/SentEstimateDetailPage.tsx#L5-L5: 동일하게 배럴 import로 변경해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/estimate/pending/PendingEstimateDetailView.tsx` at line 5, Update the ChatRoomModalContainer imports in src/components/estimate/pending/PendingEstimateDetailView.tsx#L5-L5 and src/components/estimate/sent/SentEstimateDetailPage.tsx#L5-L5 to use the named export from "`@/components/chat`" instead of direct file-path imports.src/components/chat/ChatRoomModalContainer.tsx (3)
102-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value메시지 목록에 목록 시맨틱을 적용해 주세요.
메시지 항목을
div로만 구성했습니다. 스크린 리더는 항목 수와 경계를 알리지 못합니다. 목록은ul/li로 표현해 주세요. 시각적 스타일은list-none으로 유지할 수 있습니다.시간 값은
<time dateTime={message.createdAt}>으로 감싸면 의미가 명확해집니다.프로젝트 코딩 가이드라인의 "목록은 목록 요소를 사용한다" 항목을 근거로 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/chat/ChatRoomModalContainer.tsx` around lines 102 - 136, Update the message list rendering around the mapped message items to use a ul container and an li element for each message instead of div-only structure, preserving the current visual layout with list-none styling. Wrap each formatted message timestamp in a time element with dateTime set to message.createdAt.Source: Coding guidelines
37-50: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Intl.DateTimeFormat인스턴스를 모듈 스코프로 올려 주세요.
formatMessageTime은 호출마다 포맷터를 새로 만듭니다. 메시지 수만큼 렌더마다 생성됩니다.Intl.DateTimeFormat생성은 포맷 실행보다 비용이 큽니다.♻️ 포맷터 재사용안
+const MESSAGE_TIME_FORMATTER = new Intl.DateTimeFormat("ko-KR", { + hour: "2-digit", + minute: "2-digit", + hour12: false, + timeZone: "Asia/Seoul", +}); + function formatMessageTime(createdAt: string): string { const date = new Date(createdAt); if (Number.isNaN(date.getTime())) { return ""; } - return new Intl.DateTimeFormat("ko-KR", { - hour: "2-digit", - minute: "2-digit", - hour12: false, - timeZone: "Asia/Seoul", - }).format(date); + return MESSAGE_TIME_FORMATTER.format(date); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/chat/ChatRoomModalContainer.tsx` around lines 37 - 50, Move the Intl.DateTimeFormat instance used by formatMessageTime to module scope and reuse it for every call, preserving the existing ko-KR, 24-hour, minute, and Asia/Seoul formatting options.
99-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win현재 작성자인 편별 판정 근거를 명확히 분리해 주세요.
chat.ts의ChatParticipant에id와role모두 존재하므로 현재 판정은 타입 문제는 없습니다. 다만participantRoleprop이src/components/estimate/...Detail*.tsx에서 상대방 역할(CUSTOMER/MOVER)을 전달하고 있어isMine = sender.role === participantRole이 “내 메시지”인지 오해하기 쉽습니다.ChatMessage의senderId와 현재 사용자 ID로 판정하거나, prop 이름을currentUserRole처럼 현재 작성자 기준으로 명확히 해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/chat/ChatRoomModalContainer.tsx` around lines 99 - 101, ChatRoomModalContainer의 messages.map 내부 isMine 판정을 현재 사용자 기준으로 명확히 수정하세요. sender.role과 상대방 역할을 전달하는 participantRole 비교는 제거하고, ChatMessage.senderId와 현재 사용자 ID를 비교하거나 participantRole을 실제 현재 사용자 역할로 일관되게 변경해 작성자 판정이 오해되지 않도록 하세요.src/hooks/useChatRoomModalController.ts (1)
29-39: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value메시지 병합 비용을 확인해 주세요.
mergeMessages는 매 병합마다 전체 배열을Map으로 재구성하고 다시 정렬합니다.appendMessages도 기존liveMessages전체를 대상으로 같은 작업을 반복합니다. 메시지가 수백 건 이상 쌓이면 소켓 수신마다 O(n log n) 정렬이 반복됩니다.현재 페이지 크기 기준으로는 문제가 없을 수 있습니다. 무한 스크롤로 누적되는 양이 크다면 삽입 위치만 찾는 방식으로 바꾸는 것을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useChatRoomModalController.ts` around lines 29 - 39, Optimize mergeMessages and the appendMessages flow to avoid rebuilding the full message Map and sorting the entire liveMessages collection on every socket update. Preserve deduplication by message id and chronological/id ordering, but merge incoming messages by locating their insertion or replacement positions and only adjusting the affected range; retain existing behavior for duplicate and already-ordered messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/chat/ChatRoomModalContainer.tsx`:
- Around line 80-98: ChatRoomModalContainer의 메시지 목록에 하단 sentinel ref를 추가하고,
messages 길이가 변경될 때 해당 요소의 scrollIntoView를 호출해 새 메시지 수신·전송 시 스크롤 컨테이너가 하단으로 이동하도록
구현하세요. 기존 overflow-y-auto 영역과 이전 메시지 로딩 동작은 유지하세요.
- Around line 218-244: Add a “다시 시도” action to the !activeRoom fallback when
chatRoom.isChatRoomPending is false, matching the existing failure-state UI
around the retry control. Wire it to the retry method exposed by
useChatRoomModalController so users can re-request the chat room without closing
the modal; keep the pending message and current modal behavior unchanged.
In `@src/hooks/useChatRoomModalController.ts`:
- Around line 55-80: 채팅방 준비 실패 시 자동 재요청이 반복되지 않도록
src/hooks/useChatRoomModalController.ts 55-80행의 onError에서 requestedEstimateIdRef
초기화를 제거하고, 명시적으로 요청을 다시 실행하는 retryChatRoom 함수를 반환하세요.
src/components/chat/ChatRoomModalContainer.tsx 218-244행에서는 isChatRoomPending이
거짓인 실패 상태에 메시지 조회 실패 UI(166-178행)와 동일한 형태의 “다시 시도” 버튼을 추가하고 retryChatRoom을
연결하세요.
- Around line 144-168: Update handleSendMessage so the sendMessage call is
wrapped in try/finally, moving setIsSending(false) into the finally block to
guarantee the sending state resets when sendMessage rejects. Preserve the
existing response error handling and successful message clearing behavior.
---
Nitpick comments:
In `@src/components/chat/ChatRoomModalContainer.tsx`:
- Around line 102-136: Update the message list rendering around the mapped
message items to use a ul container and an li element for each message instead
of div-only structure, preserving the current visual layout with list-none
styling. Wrap each formatted message timestamp in a time element with dateTime
set to message.createdAt.
- Around line 37-50: Move the Intl.DateTimeFormat instance used by
formatMessageTime to module scope and reuse it for every call, preserving the
existing ko-KR, 24-hour, minute, and Asia/Seoul formatting options.
- Around line 99-101: ChatRoomModalContainer의 messages.map 내부 isMine 판정을 현재 사용자
기준으로 명확히 수정하세요. sender.role과 상대방 역할을 전달하는 participantRole 비교는 제거하고,
ChatMessage.senderId와 현재 사용자 ID를 비교하거나 participantRole을 실제 현재 사용자 역할로 일관되게 변경해
작성자 판정이 오해되지 않도록 하세요.
In `@src/components/estimate/pending/PendingEstimateDetailView.tsx`:
- Line 5: Update the ChatRoomModalContainer imports in
src/components/estimate/pending/PendingEstimateDetailView.tsx#L5-L5 and
src/components/estimate/sent/SentEstimateDetailPage.tsx#L5-L5 to use the named
export from "`@/components/chat`" instead of direct file-path imports.
In `@src/hooks/useChatRoomModalController.ts`:
- Around line 29-39: Optimize mergeMessages and the appendMessages flow to avoid
rebuilding the full message Map and sorting the entire liveMessages collection
on every socket update. Preserve deduplication by message id and
chronological/id ordering, but merge incoming messages by locating their
insertion or replacement positions and only adjusting the affected range; retain
existing behavior for duplicate and already-ordered messages.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bf31040-1288-460d-91c6-845da811ca34
📒 Files selected for processing (6)
src/components/chat/ChatRoomModal.tsxsrc/components/chat/ChatRoomModalContainer.tsxsrc/components/chat/index.tssrc/components/estimate/pending/PendingEstimateDetailView.tsxsrc/components/estimate/sent/SentEstimateDetailPage.tsxsrc/hooks/useChatRoomModalController.ts
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 채팅방 생성/조회 단계와 연결 이후 메시지·소켓 상태를 별도 Hook으로 분리했습니다.
- 모달 open 시 동일 estimate에 대한 채팅방 요청이 중복되지 않도록 방어했습니다.
- 이미 준비된 room은 재사용해 불필요한 API 호출을 줄였습니다.
- REST 메시지와 Socket 실시간 메시지를 ID 기준으로 병합하고 시간순으로 정렬했습니다.
- Socket join 시 missedMessages를 반영하고 추가 누락 가능성이 있으면 REST refetch하도록 구성했습니다.
- 고객/기사 상세 모두 동일한 ChatRoomModalContainer를 재사용합니다.
- 채팅방 준비, 메시지 로딩, 빈 상태, 오류, 재시도 UI가 명확하게 분리되어 있습니다.
- 이전 메시지 pagination을 명시적인 버튼 방식으로 제공했습니다.
- 메시지 발신자 role을 기준으로 내 메시지와 상대 메시지를 구분합니다.
- 전송 disabled 조건을 하나의 변수로 정리해 submit과 button 상태가 일치합니다.
- 사진 첨부, 견적 수정, 견적 확정 액션은 후속 범위로 남겨 현재 PR 범위를 잘 제한했습니다.
- 현재 PR은 open/mergeable 상태이며 변경 파일은 6개입니다.
🔍 확인 및 제안
전체 구조는 잘 잡혀 있고 현재 diff 기준으로 큰 기능 누락은 보이지 않았습니다.
가장 확인하고 싶은 부분은 메시지 전송 상태 복구입니다.
handleSendMessage()에서 isSending=true 설정 후
sendMessage()가 정상 응답을 반환하는 경로에서는 false로 되돌리지만,
예상 밖 Promise rejection이 발생하면 false로 복구되지 않을 수 있습니다.
현재 useChatRoomSocket은 연결 끊김, timeout, empty ack 등을 모두
{ ok: false } 형태로 resolve하도록 구현되어 있어 실제 발생 가능성은 낮아 보입니다.
그래도 비동기 상태는 try/finally로 복구를 보장하면 더 안정적일 것 같습니다.
또 하나는 모달 close/open 사이의 상태 유지 정책입니다.
같은 상세 페이지에서 모달을 닫았다 다시 열면
Hook 인스턴스가 유지되기 때문에 작성 중 입력값과 liveMessages도 유지됩니다.
메시지 캐시 유지에는 장점이 있지만,
입력 중인 draft까지 유지하는 것이 의도된 UX인지만 한 번 확인하면 좋겠습니다.
그 외 채팅방 준비 → 메시지 조회 → 소켓 join → 실시간 수신/전송 흐름은
기존 훅 구조를 잘 재사용해 연결된 것으로 보입니다.
To Reviewer 내용 기준으로
채팅방 생성/조회, REST 메시지 조회, Socket.IO join·수신·전송 연결 흐름을 중점적으로 확인했습니다!
모달이 열리면 estimateId 기준으로 먼저 채팅방을 준비하고,
room이 확보된 이후에만 메시지 조회와 Socket 연결 훅이 실행되는 구조라
단계별 책임이 잘 분리되어 있습니다.
동일한 estimate에 대해서는 Ref와 기존 room을 활용해
중복 채팅방 생성/조회 요청도 방지하고 있습니다.
메시지는 REST 조회 결과와 실시간 Socket 수신 결과를 합치되
message id 기준으로 중복 제거하고 createdAt 순으로 다시 정렬합니다.
Socket join 응답에서 missedMessages가 있으면 즉시 반영하고,
추가 누락 메시지가 있다는 응답이면 REST refetch를 수행해
연결 공백 구간도 보완하도록 구성되어 있습니다.
고객 대기 견적과 기사 보낸 견적 상세도
각각 별도의 채팅 상태를 관리하지 않고
공통 ChatRoomModalContainer를 사용하도록 정리되어 있습니다.
사진 첨부, 견적 수정, 견적 확정 액션을 이번 PR에서 억지로 연결하지 않고
후속 범위로 남긴 것도 현재 작업 목적에 맞는 범위 설정으로 보입니다.
선택적으로 보완하면 좋은 부분은 메시지 전송의 isSending 상태입니다.
현재 Socket 훅에서는 실패도 대부분 { ok: false }로 반환하므로 정상 동작하지만,
예상 밖 reject에도 상태 복구가 보장되도록 try/finally를 사용하면 더 안전할 것 같습니다.
또한 모달을 닫았다 다시 열었을 때 작성 중 입력값을 유지할지 초기화할지는
UX 정책만 한 번 확인하면 좋겠습니다.
2 participants
📋 작업 내용
🔥 변경 사항
ChatRoomModalContainer추가estimateId기준으로 채팅방 생성/조회useChatRoomModalController추가ChatRoomModal의 전송 disabled 조건을 변수로 정리해 중복 조건 제거src/components/chat/index.ts에서ChatRoomModalContainerexport 추가✅ 체크리스트
📷 스크린샷 (선택)
🔗 관련 이슈
Closes #
💬 To Reviewer
Summary by CodeRabbit