Skip to content

[Feat]: UI 채팅 목록 화면 구현 및 "채팅 목록 -> 채팅방 내비게이션 로직" 구현(#6)#22

Merged
codebidoof merged 20 commits into
developfrom
feature/chatlist-ui
Jul 18, 2026
Merged

[Feat]: UI 채팅 목록 화면 구현 및 "채팅 목록 -> 채팅방 내비게이션 로직" 구현(#6)#22
codebidoof merged 20 commits into
developfrom
feature/chatlist-ui

Conversation

@codebidoof

@codebidoof codebidoof commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

📄 작업 내용 요약

  • 채팅 목록 화면 구현
  • 채팅 목록 -> 채팅방 내비게이션 로직 구현

📎 Issue 번호


✅ 작업 목록

  • 기능 구현
  • 코드 리뷰 반영
  • 테스트 코드 작성
  • 문서 업데이트

📝 기타 참고사항

Summary by CodeRabbit

  • 새 기능

    • 채팅 목록에서 채팅방으로 이동할 수 있습니다.
    • 채팅 목록과 채팅방 화면의 기본 구조를 추가했습니다.
    • 채팅 목록 로딩 상태와 스켈레톤 UI를 제공합니다.
    • 채팅 항목을 좌우로 밀어 알림 설정 및 삭제 메뉴를 확인할 수 있습니다.
    • 읽지 않은 메시지 수와 채팅방 정보를 표시합니다.
    • 채팅 관련 아이콘과 UI 요소를 추가했습니다.
  • 개선

    • 채팅 목록 탐색 및 뒤로가기 동작을 지원합니다.
    • 클릭 시 리플 효과가 없는 UI 동작을 제공합니다.

- 프로필, 이름, 메시지, 읽지 않은 메시지 수 등을 포함하는 ChatSummary 도메인 모델 추가
- ChattingScreen을 ChatListScreen으로 변경하고 빈 ChatScreen 스텁 추가
- 스와이프 제스처를 지원하는 ChatListItem 컴포넌트 추가
- 스와이프 상호작용을 위한 FrontRow 및 BackgroundRow 컴포넌트 추가
- SwipeableBox 및 UnReadMessageCountBox 유틸리티 컴포넌트 추가
- ic_chat_bell 및 ic_chat_pin 드로어블 리소스 추가
- 이름 변경에 맞춰 ChattingEntryBuilder가 ChatListScreen을 참조하도록 수정
- 채팅 목록 스켈레톤 애니메이션을 위한 LoadingColumn 컴포넌트 추가
- 클릭 시 리플 효과를 제거하는 noRippleClickable Modifier 추가
- 로딩 상태를 위한 shimmer 애니메이션 기반 skeleton Modifier 추가
- 새로운 로딩 컴포넌트를 적용하도록 ChatListScreen 업데이트
- FrontRow.kt: 메인 캐릭터 아이콘(`ic_chat_pin`)이 관리자(`isManager`)가 아닐 때만 노출되도록 조건식 수정
- FrontRow.kt: 읽지 않은 메시지 수(`UnReadMessageCountBox`) 상단의 불필요한 4.dp 간격(`Spacer`) 제거
- `ChatSummary` 데이터 클래스의 모든 프로퍼티에 기본값 추가 (매니저 채팅방 대응 등)
- `ChatListScreen`의 메인 컨테이너에 `statusBarsPadding()`을 적용하여 상태 표시줄 겹침 방지
- `ChatListScreen`에서 매니저용 `FrontRow` 호출 시 `ChatSummary` 기본값을 활용하도록 코드 간소화
- `core/domain` 모듈 내 미사용 `.gitkeep` 파일 삭제
- ChattingRoute.kt: 채팅방 이동을 위한 `ChatRoomNavKey` 및 `ChatListNavKey` 정의
- ChatSummary.kt: `chatRoomId` 필드 타입을 `Int`에서 `Long`으로 변경
- ChatRoomScreen.kt: 채팅방 화면 구성을 위한 `ChatRoomScreen` 컴포저블 신규 생성
- ChattingEntryBuilder.kt: `chattingEntry`에 `ChatRoomNavKey` 진입점 추가 및 화면 이동을 위한 콜백 파라미터 정의
- ChatListScreen.kt: 채팅 아이템 클릭 이벤트를 처리할 수 있도록 `onChatRoomClick` 콜백 구현 및 데이터 바인딩
- AppScreen.kt: `AppNavigator`를 통해 실제 채팅방 이동(`navigate`) 및 뒤로가기(`popBackStack`) 로직 연결
- AppNavigator.kt: 불필요한 주석 제거 및 내비게이션 관련 TODO 문구 정리
- ChatListIntent: 채팅방 클릭 액션을 정의하는 ClickChatRoom 추가
- ChatListSideEffect: 화면 이동 처리를 위한 NavigateToChatRoom 추가
- ChatListViewModel: ClickChatRoom 인텐트 수신 시 NavigateToChatRoom 사이드 이펙트를 방출하도록 구현
- ChatListScreen: collectSideEffect를 통해 채팅방 이동 로직을 처리하고, 클릭 이벤트를 ViewModel로 전달하도록 수정
- ChattingRoute: 사용하지 않는 ChatListNavKey 제거
- ChatListScreen.kt: ViewModel의 상태 구독(state) 로직에 설명 주석 추가
- ChatListScreen.kt: SideEffect 수신 및 처리 로직에 설명 주석 추가
@codebidoof codebidoof linked an issue Jul 17, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b8a4ea9f-786c-47c6-b754-a8f4dbf52ed5

📥 Commits

Reviewing files that changed from the base of the PR and between 24ee11c and a71929b.

📒 Files selected for processing (2)
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt

📝 Walkthrough

Walkthrough

채팅 요약·메시지 도메인 모델과 저장소 구현, 채팅 목록 UI 및 상태 관리, 채팅방 진입 라우팅, Compose 디자인 컴포넌트와 채팅방 기반 구조가 추가되었습니다.

Changes

채팅 기능

Layer / File(s) Summary
채팅 계약과 목록 데이터
core/domain/model/chatting/*, core/domain/repository/ChatRepository.kt, core/data/repositoryImpl/ChatRepositoryImpl.kt, core/data/di/RepositoryModule.kt
채팅 메시지·요약 모델과 저장소 계약이 추가되고, 하드코딩된 목록 조회 구현 및 Hilt 바인딩이 구성되었습니다.
채팅 목록 UI와 디자인 구성
core/designsystem/..., feature/chatting/impl/component/*, core/designsystem/res/drawable/ic_chat_*.xml
스켈레톤·무리플 클릭 Modifier, 채팅 아이콘, 목록 행, 읽지 않은 메시지 배지 및 스와이프 UI가 추가되었습니다.
채팅 목록 상태와 화면 흐름
feature/chatting/impl/viewmodel/ChatList*.kt, feature/chatting/impl/screen/ChatListScreen.kt
목록 상태·Intent·SideEffect와 ViewModel이 추가되었고, 로딩·오류·목록 상태별 렌더링 및 채팅방 이동 이벤트가 연결되었습니다.
메시지 UI와 채팅방 상태 기반
feature/chatting/impl/model/*, feature/chatting/impl/mapper/*, feature/chatting/impl/viewmodel/ChatRoom*.kt, feature/chatting/impl/screen/ChatRoomScreen.kt, feature/chatting/impl/component/ChatRoomTopBar.kt
서버·로컬 메시지 UI 모델과 변환 함수, 채팅방 상태·Intent·SideEffect·ViewModel 및 채팅방 화면 진입점이 추가되었습니다.
채팅 라우팅 통합
feature/chatting/api/ChattingRoute.kt, feature/chatting/impl/entry/ChattingEntryBuilder.kt, app/src/main/java/kr/co/call/callfromai/{AppScreen,AppNavigator}.kt
채팅방 내비게이션 키와 목록·채팅방 엔트리가 추가되고 앱 내 이동 및 뒤로가기가 연결되었습니다.
프로젝트 설정 지원
.gitignore
IDE 설정 및 하위 경로의 빌드 디렉터리 무시 규칙이 갱신되었습니다.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ChatListScreen
  participant ChatListViewModel
  participant ChatRepository
  participant AppScreen
  participant ChatRoomScreen
  User->>ChatListScreen: 채팅방 항목 클릭
  ChatListScreen->>ChatListViewModel: ClickChatRoom(roomId, name)
  ChatListViewModel->>ChatRepository: getChatList()
  ChatListViewModel-->>ChatListScreen: NavigateToChatRoom(roomId, name)
  ChatListScreen->>AppScreen: 채팅방 이동 콜백
  AppScreen->>ChatRoomScreen: ChatRoomNavKey로 roomId, name 전달
Loading

Suggested reviewers: jiwonlee42

🚥 Pre-merge checks | ✅ 2 | ❌ 5

❌ Failed checks (5 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning 제목은 PR 내용과 관련은 있지만 [Feat]:를 사용해 Conventional Commits 형식(feat: 설명)을 따르지 않습니다. feat: 채팅 목록 화면 및 채팅방 내비게이션 구현(#6)처럼 'type: 설명' 형식으로 수정하세요.
Linked Issues check ⚠️ Warning 채팅 목록은 구현됐지만 채팅방 화면/TopBar는 본문이 비어 있어 채팅방 컴포넌트 구현이 완료되지 않았습니다. ChatRoomScreen과 관련 컴포넌트에 실제 UI 렌더링과 동작을 추가하고, TODO/빈 구현을 제거하세요.
Out of Scope Changes check ⚠️ Warning .gitignore 변경은 #6의 채팅 목록/채팅방 구현과 무관한 부수 수정입니다. 해당 변경을 별도 PR로 분리하거나 제거하고, 채팅 기능 관련 수정만 유지하세요.
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Ui 변경 시 스크린샷 첨부 확인 ⚠️ Warning ChatListScreen/ChatRoomScreen 같은 Compose 화면 파일이 포함됐지만 PR 설명에는 스크린샷·녹화 첨부가 없습니다. UI 변경 화면의 스크린샷 또는 짧은 녹화/GIF 링크를 PR 설명에 첨부하세요.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
모듈 의존성 방향 검증 ✅ Passed feature/*/impl의 교차 impl 직접 import나 core/data 구현체 참조가 없고, core/domain에도 android/androidx import가 없습니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/chatlist-ui

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, 시니어 Android 개발자로서 코드리뷰를 진행하겠습니다. 전반적으로 MVI 아키텍처와 Compose, Hilt 등 최신 기술 스택을 잘 활용하고 계시며, 각 레이어의 책임 분리도 명확하게 잘 되어 있습니다. 특히 Compose 컴포넌트의 재사용성, 성능(recomposition 방지), 접근성 고려 등 많은 부분에서 좋은 관행을 따르고 있습니다. 몇 가지 개선 사항과 TODO 항목에 대한 제안을 드립니다.


1. AppNavigator.kt & AppScreen.kt

  • 변경 내용: 내비게이션 설명 주석 업데이트 및 ChattingEntryBuildernavigateToChatRoomonBack 람다 전달.
  • 리뷰:
    • AppNavigator.kt: 주석 수정은 명확성을 높여줍니다. TODO는 계속 남아있으나, 점진적으로 추가될 부분으로 이해합니다.
    • AppScreen.kt: navigateToChatRoomonBack 람다를 chattingEntry에 전달하는 방식은 State Hoisting 원칙에 부합하며, 하위 컴포넌트가 내비게이션 로직에 직접 의존하지 않고 상위에서 관리하게 하여 결합도를 낮추는 좋은 패턴입니다. appNavigator.navigateappNavigator.popBackStack의 사용도 적절합니다.

2. core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt

  • 변경 내용: abstract class로 변경 후 ChatRepositoryChatRepositoryImpl@Binds로 연결.
  • 리뷰:
    • Hilt DI 원칙 준수: @Binds를 사용하여 인터페이스와 구현체를 연결하고 @Singleton 스코프를 부여한 것은 Hilt DI의 권장 방식입니다. 아주 좋습니다.

3. core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt (신규)

  • 변경 내용: ChatRepository 구현체. 더미 데이터를 반환하며 delay를 사용.
  • 리뷰:
    • Hilt DI: @Inject constructor()로 의존성 주입이 준비되어 있습니다.
    • Retrofit 에러 핸들링 (Rule 3): runCatching 블록으로 Result를 반환하는 것은 Result 래핑 원칙에 잘 맞습니다. onFailure 시나리오에 대한 TODO가 있으나, API 연동 시 실제 에러를 어떻게 처리할지 정의하고 Result.failure()에 전달해야 합니다. (예: Result.failure(ApiException("네트워크 오류")))
    • 네트워크 응답과 도메인 모델 매핑 (Rule 3): 현재는 더미 데이터이므로 매핑 로직은 없지만, ChatSummary 도메인 모델을 바로 반환하는 것은 좋습니다. 실제 API 연동 시에는 DTO를 ChatSummary로 변환하는 매핑 로직이 이 Repository 레이어에 추가되어야 합니다.
    • 캐싱 전략 (Rule 3): 아직 캐싱은 없지만, 현재 단계에서는 문제가 없습니다.
    • Coroutine 사용 시 Dispatcher 명시 (Rule 1): delay는 suspend 함수이므로 자동으로 적절한 Dispatcher에서 실행됩니다. 현재로서는 명시적인 withContext가 필요하지 않아 보입니다.
    • 하드코딩된 문자열/매직 넘버 (Rule 1):
      • 더미 데이터 내부의 문자열은 현재 단계에서는 허용 가능합니다.
      • delay(1000.milliseconds)는 Kotlin Duration을 사용하여 명시적인 시간을 표현하는 좋은 방법입니다.

4. core/designsystem/src/main/java/kr/co/call/designsystem/modifier/noRippleClickable.kt (신규)

  • 변경 내용: 리플 효과 없는 클릭 가능한 Modifier 확장 함수.
  • 리뷰:
    • Jetpack Compose (Rule 2): composed를 사용하여 Modifier를 재사용 가능하게 만든 점, remember { MutableInteractionSource() }를 사용하여 불필요한 객체 생성을 막고, indication = null을 통해 리플 효과를 제거한 점 모두 아주 훌륭합니다.

5. core/designsystem/src/main/java/kr/co/call/designsystem/modifier/skeleton.kt (신규)

  • 변경 내용: 스켈레톤 로딩 효과를 적용하는 Modifier 확장 함수.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • composed, rememberInfiniteTransition, animateFloat, drawWithCache 등의 사용이 적절하여 불필요한 recomposition을 유발하지 않도록 잘 설계되었습니다. 특히 drawWithCache는 브러쉬 같은 객체를 캐싱하여 드로잉 성능을 최적화하는 데 좋습니다.
      • if (!isLoading) return@composed this와 같이 early exit를 통해 isLoadingfalse일 때 불필요한 애니메이션 및 드로잉 로직 실행을 막는 점도 좋습니다.
      • 하드코딩된 문자열/매직 넘버 (Rule 1): durationMillis: Int = 1000은 매직 넘버입니다. 디자인 시스템 내의 AnimationConstants 등으로 분리하면 더 좋습니다.
    • Preview: SkeletonPreview를 통해 사용법과 동작을 명확히 보여주는 점 좋습니다.

6. core/designsystem/src/main/res/drawable/*.xml, .png (신규)

  • 변경 내용: 새로운 아이콘 및 이미지 리소스 추가.
  • 리뷰: 디자인 리소스는 문제없어 보입니다. 벡터 에셋의 tint = Color.Unspecified 사용은 해당 아이콘의 원본 색상을 유지하기 위한 의도라면 적절합니다.

7. core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatMessage.kt (신규)

  • 변경 내용: ChatMessageList, ChatMessage 데이터 클래스 및 SenderType, MessageType Enum 클래스 정의.
  • 리뷰:
    • null 안전성 (Rule 1): nextCursor: Long?, photoUrl: String = "", content: String = "" 등 null 가능성 또는 기본값을 적절히 처리하여 !! 사용을 지양한 점 좋습니다.
    • data class에 불필요한 var 사용 여부 (Rule 1): 모든 프로퍼티가 val로 선언되어 **불변성(Immutability)**을 지킨 점 아주 좋습니다.
    • Enum 클래스의 fromApiValue 컴패니언 객체 함수는 API 응답 값을 안전하게 파싱하고 UNKNOWN 값으로 폴백 처리하는 좋은 패턴입니다.

8. core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatSummary.kt (신규)

  • 변경 내용: ChatSummary 데이터 클래스 정의.
  • 리뷰:
    • data class에 불필요한 var 사용 여부 (Rule 1): 모든 프로퍼티가 val로 선언되어 불변성을 지킨 점 아주 좋습니다.
    • null 안전성 (Rule 1): 기본값을 잘 제공하고 있습니다.
    • 하드코딩된 문자열/매직 넘버 (Rule 1):
      • chatRoomId = -1은 매니저 챗방을 의미하는 매직 넘버로 보입니다. const val MANAGER_CHAT_ROOM_ID = -1L와 같이 상수로 정의하여 사용하는 것을 권장합니다.
      • unReadMessageCount: String = "0": 만약 이 값이 항상 숫자를 나타내고 UI에서 숫자로 처리될 예정이라면 (실제로 FrontRow에서 toIntOrNull()을 사용하고 있습니다), 도메인 모델에서도 Int 타입으로 변경하는 것을 고려해볼 수 있습니다. 이렇게 하면 불필요한 타입 변환을 줄이고 타입 안전성을 높일 수 있습니다. (예: unReadMessageCount: Int = 0)

9. core/domain/src/main/java/kr/co/call/domain/repository/ChatRepository.kt (신규)

  • 변경 내용: ChatRepository 인터페이스 정의.
  • 리뷰: suspend fun getChatList(): Result<List<ChatSummary>>와 같이 Result 타입을 반환하여 Repository 계층에서 에러를 캡슐화하는 좋은 패턴입니다.

10. feature/chatting/api/src/main/java/kr/co/call/api/ChattingRoute.kt

  • 변경 내용: ChatRoomNavKey 데이터 클래스 추가.
  • 리뷰: @Serializable 어노테이션을 사용하여 내비게이션 인자를 안전하게 전달하는 좋은 방법입니다. roomIdname을 함께 전달하는 것도 적절합니다.

11. feature/chatting/impl/src/main/java/kr/co/call/impl/component/BackgroundRow.kt (신규)

  • 변경 내용: 스와이프 액션의 배경으로 표시되는 컴포넌트.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • onAlarmClick, onDeleteClick과 같이 콜백을 받아 상위 컴포넌트로 이벤트를 전달하는 방식(State Hoisting)이 좋습니다.
      • contentDescription: 아이콘에 접근성 설명을 제공한 점 좋습니다.
      • 하드코딩된 문자열/매직 넘버 (Rule 1):
        • size(204.dp, 102.dp)와 같은 고정된 Dp 값들은 디자인 시스템의 Dimens 객체 등을 통해 관리하면 일관성을 유지하기 좋습니다.
        • "목록에서\n지우기" 텍스트는 strings.xml 리소스로 분리해야 합니다.

12. feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt (신규)

  • 변경 내용: 채팅 목록의 개별 아이템 컴포넌트, SwipeableBox 사용.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • remember { AnchoredDraggableState(...) }를 사용하여 스와이프 상태를 recomposition으로부터 안전하게 유지하는 점 좋습니다.
      • chatSummary라는 UI 상태(도메인 모델)를 data class로 받아 처리하는 점 좋습니다.
      • 콜백(onItemClick, onAlarmClick, onDeleteClick)을 통해 이벤트를 상위로 전달하는 State Hoisting 원칙을 잘 지키고 있습니다.

13. feature/chatting/impl/src/main/java/kr/co/call/impl/component/FrontRow.kt (신규)

  • 변경 내용: 채팅 목록 아이템의 전경(실제 내용) 컴포넌트.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • contentDescription: 이미지 및 아이콘에 접근성 설명을 제공한 점 좋습니다.
      • if (chatSummary.isMainCharacter && !isManager)와 같은 조건부 UI 렌더링은 명확합니다.
      • if ((chatSummary.unReadMessageCount.toIntOrNull() ?: 0) > 0): unReadMessageCountString일 때 toIntOrNull()을 사용하여 안전하게 숫자로 변환하는 점 좋습니다. 다만, 위에서 언급했듯이 도메인 모델에서 Int 타입으로 변경을 고려해볼 수 있습니다.
      • chatSummary를 data class로 받아 처리하는 점 좋습니다.
      • 하드코딩된 문자열/매직 넘버 (Rule 1):
        • height(102.dp), padding(horizontal = 20.dp), size(52.dp), padding(horizontal = 12.dp), size(8.dp), height(6.dp), height(16.dp) 등 대부분의 Dimension 값들이 하드코딩되어 있습니다. 디자인 시스템의 Dimens나 테마 확장 객체를 통해 관리하는 것을 강력히 권장합니다.
        • chatSummary.name.take(1)은 프로필 이미지 없는 경우의 Fallback 로직인데, 괜찮습니다.

14. feature/chatting/impl/src/main/java/kr/co/call/impl/component/LoadingColumn.kt (신규)

  • 변경 내용: 스켈레톤 로딩 상태를 보여주는 컴포넌트.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • repeat(5): 로딩 아이템의 개수가 매직 넘버입니다. const val로 추출하거나, Modifier.placeholder 라이브러리를 사용해 실제 콘텐츠의 개수를 반영하는 것이 더 유연할 수 있습니다.
      • clip(RoundedCornerShape(8.dp)): Dimension 매직 넘버입니다.

15. feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt (신규)

  • 변경 내용: 스와이프 가능한 컨테이너 컴포넌트.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • AnchoredDraggableState, DraggableAnchors, anchoredDraggable의 사용이 매우 적절합니다.
      • LaunchedEffect(Unit)를 사용하여 컴포지션 진입 시 한 번만 앵커를 업데이트하는 점 좋습니다.
      • swipeWidthPxLocalDensity.current를 통해 Dp에서 픽셀로 변환하는 점 좋습니다.
      • offsetX.isNaN() 체크를 통해 안전하게 offset 값을 사용하는 점 좋습니다.
      • backgroundAlpha 계산을 통해 배경 콘텐츠가 스와이프 정도에 따라 투명도가 변하는 효과를 구현한 점 좋습니다.
      • BoxScope.()backgroundContent에 제공하여 배경 컴포넌트가 Modifier.align을 사용할 수 있도록 한 점도 유연성을 높입니다.

16. feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt (신규)

  • 변경 내용: 읽지 않은 메시지 수를 표시하는 뱃지 컴포넌트.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • size(20.dp), RoundedCornerShape(3.dp) 등의 Dimension 매직 넘버를 디자인 시스템으로 분리하는 것을 권장합니다.
      • CallTheme.colors.subRed 등 테마 색상을 사용한 점 좋습니다.

17. feature/chatting/impl/src/main/java/kr/co/call/impl/entry/ChattingEntryBuilder.kt

  • 변경 내용: ChatListScreenChatRoomScreen에 대한 엔트리 추가, navigateToChatRoomonBack 람다 전달.
  • 리뷰:
    • entry<ChatRoomNavKey> { key -> ... }에서 key 객체를 통해 roomIdname을 안전하게 추출하여 ChatRoomScreen에 전달하는 점 좋습니다.
    • navigateToChatRoomonBack 람다를 통해 내비게이션 이벤트를 상위 AppScreen에서 처리하도록 위임한 점도 State Hoisting 원칙에 맞게 잘 구현되었습니다.

18. feature/chatting/impl/src/main/java/kr/co/call/impl/mapper/UiModelMapper.kt (신규)

  • 변경 내용: 도메인 모델(ChatMessage)을 UI 모델(ChatItemUiModel)로 매핑하는 객체.
  • 리뷰:
    • 네트워크 응답과 도메인 모델 매핑 (Rule 3): 도메인 모델을 UI 모델로 매핑하는 UiModelMapper를 별도로 둔 것은 책임 분리가 명확하여 좋습니다.
    • timeFormatterobject 내부에서 한 번만 초기화하여 사용하는 점 좋습니다.
    • ChatMessage.toUiModel() 확장 함수는 명확하고 재사용성이 높습니다. photoUrl.ifEmpty { null }과 같이 null 안전성을 고려한 점 좋습니다.
    • createSendingMessage를 통해 전송 중인 메시지 UI 모델을 생성하는 로직은 실제 채팅 앱에서 매우 유용합니다. UUID.randomUUID().toString()을 사용하여 clientId를 생성하는 것도 좋습니다.
    • clientId = "server-$chatMessageId"의 "server-" 접두사는 내부적인 구분자로 괜찮아 보이지만, 만약 chatMessageId 자체가 고유하다면 접두사 없이 바로 사용하는 것도 고려해볼 수 있습니다.

19. feature/chatting/impl/src/main/java/kr/co/call/impl/model/ChatItemUiModel.kt (신규)

  • 변경 내용: 채팅 말풍선 UI 모델 및 SendStatus Enum 정의.
  • 리뷰:
    • UI 상태를 data class로 표현했는지 (Rule 2): ChatItemUiModel은 UI 상태를 나타내는 data class로 적절하게 정의되었습니다.
    • data class에 불필요한 var 사용 여부 (Rule 1): 모든 프로퍼티가 val로 선언되어 불변성을 지킨 점 아주 좋습니다.
    • null 안전성 (Rule 1): chatMessageId: Long?, photoUrl: String?과 같이 nullable 타입을 적절히 사용하여 !! 사용을 지양한 점 좋습니다.

20. feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt (신규)

  • 변경 내용: 채팅 목록 화면의 Compose 구현.
  • 리뷰:
    • Jetpack Compose (Rule 2):
      • statusBarsPadding()을 사용하여 시스템 UI 인셋을 적절히 처리한 점 좋습니다.
      • LazyColumn을 사용하여 효율적인 목록 렌더링을 구현하고, key 파라미터에 chatRoomId를 사용하여 불필요한 recomposition을 최소화한 점 아주 좋습니다.
      • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지 (Rule 2): LoadStatus enum (도메인 레이어)을 사용하여 로딩/에러/아이들 상태를 명확하게 관리하는 점 훌륭합니다.
        • LoadStatus.Loading일 때 LoadingColumn을 보여주는 점 좋습니다.
        • LoadStatus.Error에 대한 TODO가 있으나, 에러 메시지 표시, 재시도 버튼 등의 UI 처리를 추가해야 합니다.
      • onChatRoomClick 콜백을 통해 내비게이션 이벤트를 상위로 전달하는 State Hoisting 원칙을 잘 지키고 있습니다.
      • ChatSummary라는 UI 상태(도메인 모델)를 data class로 받아 처리하는 점 좋습니다.
      • 하드코딩된 문자열/매직 넘버 (Rule 1):
        • "채팅" 타이틀 텍스트와 매니저 채팅의 이름/내용 ("전화왔어 매니저", "수현님, 반가워요! 👋🏻 오늘은 어떤 이야기를")은 strings.xml 리소스로 분리해야 합니다.
        • height(16.dp), height(20.dp), padding(start = 16.dp), verticalArrangement = Arrangement.spacedBy(4.dp) 등의 Dimension 매직 넘버를 디자인 시스템으로 분리하는 것을 권장합니다.
    • ViewModel 연동:
      • hiltViewModel()로 ViewModel을 주입받는 점 좋습니다.
      • viewModel.collectAsState().value로 UI 상태를 관찰하고, viewModel.collectSideEffect로 Side Effect를 수신하는 Orbit MVI 패턴을 잘 따르고 있습니다.
      • onIntent(ChatListIntent.ClickChatRoom(...))를 통해 사용자 입력을 ViewModel의 intent로 전달하는 점 좋습니다.

21. feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatRoomScreen.kt (신규)

  • 변경 내용: 채팅방 화면의 플레이스홀더.
  • 리뷰: 플레이스홀더이므로 구체적인 리뷰는 어렵습니다. roomIdname 인자를 잘 받고 있는 점은 확인됩니다.

22. feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListIntent.kt (신규)

  • 변경 내용: ChatListIntent sealed interface 정의.
  • 리뷰: ViewModel (Rule 4): Sealed interface를 사용하여 의도를 명확하게 표현하고, ClickChatRoom 데이터 클래스로 필요한 정보를 담아 전달하는 Orbit MVI Intent 패턴을 잘 따르고 있습니다.

23. feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListSideEffect.kt (신규)

  • 변경 내용: ChatListSideEffect sealed interface 정의.
  • 리뷰: ViewModel (Rule 4): Sealed interface를 사용하여 상태 변경과는 별개의 일회성 이벤트를 표현하는 Orbit MVI Side Effect 패턴을 잘 따르고 있습니다. NavigateToChatRoom 데이터 클래스로 필요한 정보를 담아 전달하는 점 좋습니다.

24. feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListState.kt (신규)

  • 변경 내용: ChatListState 데이터 클래스 정의.
  • 리뷰: ViewModel (Rule 4): UI 상태를 data class로 표현하고 chatList를 불변 리스트로 유지하는 점 좋습니다. LoadStatus를 사용하여 로딩 상태를 관리하는 점도 좋습니다.

25. feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt (신규)

  • 변경 내용: ChatListScreen의 ViewModel 구현. Orbit MVI 사용.
  • 리뷰:
    • ViewModel (Rule 4):
      • @HiltViewModel@Inject constructor(private val chatRepository: ChatRepository)를 통해 Hilt DI 원칙을 준수하고 있습니다.
      • Orbit ContainerHost 패턴 준수 여부: ContainerHost<ChatListState, ChatListSideEffect>를 상속받아 Orbit MVI 패턴을 정확히 구현하고 있습니다. container 초기화도 올바릅니다.
      • init { loadChatList() }: ViewModel 초기화 시 데이터를 로드하는 좋은 패턴입니다.
      • intent 내부에서 상태 변경(reduce)만 수행하는지: loadChatList()emitNavigateToChatRoom() 함수 내의 intent 블록에서 reduce는 오직 상태 변경만을 수행하고, postSideEffect는 일회성 이벤트를 발행하는 등 역할이 명확하게 분리되어 있습니다. 아주 훌륭합니다.
      • postSideEffect 남용 여부: postSideEffectNavigateToChatRoom과 같은 일회성 내비게이션 이벤트에만 사용되어 남용되지 않고 적절하게 사용되었습니다.
      • blocking 작업을 intent 내부에서 직접 수행하지 않는지: chatRepository.getChatList()suspend 함수이므로 비동기적으로 실행됩니다. intent 블록 내에서 UI 스레드를 블로킹하는 작업은 보이지 않습니다. 좋습니다.
      • onFailure 시나리오에 대한 TODO가 있으니, 실제 API 연동 시 에러 처리 로직(LoadStatus.Error로 상태 변경 등)을 추가해야 합니다.

26. feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomIntent.kt, ChatRoomSideEffect.kt, ChatRoomUiState.kt (신규)

  • 변경 내용: 채팅방 ViewModel 관련 플레이스홀더 인터페이스/데이터 클래스.
  • 리뷰: 플레이스홀더이므로 구체적인 리뷰는 어렵습니다. 이름과 구조는 Orbit MVI 패턴에 맞게 잘 정의되어 있습니다.

종합 의견 및 개선 권고 사항

전반적으로 이 PR은 매우 높은 품질의 코드와 모범 사례를 따르고 있습니다. 특히 MVI 패턴, Hilt DI, Jetpack Compose의 성능 및 접근성 고려가 돋보입니다. 몇 가지 개선 권고 사항은 다음과 같습니다.

  1. 하드코딩된 문자열 외부화 (P0):
    • 사용자에게 보여지는 모든 문자열 (예: "채팅", "목록에서 지우기", "전화왔어 매니저", "수현님, 반가워요! 👋🏻 오늘은 어떤 이야기를")은 res/values/strings.xml로 옮겨야 합니다. 이는 다국어 지원 및 유지보수성 측면에서 필수적입니다.
  2. 매직 넘버 (Dimension) 외부화 (P1):
    • dp 단위로 하드코딩된 Dimension 값들 (예: 102.dp, 20.dp, 52.dp, 4.dp, 8.dp 등)은 core/designsystem 모듈 내에 Dimens 객체를 만들거나 CallTheme를 확장하여 관리하는 것을 권장합니다. 이는 디자인 일관성과 변경 용이성을 크게 향상시킵니다.
  3. ChatSummary unReadMessageCount 타입 변경 검토 (P2):
    • ChatSummary.unReadMessageCount가 현재 String 타입이나, 실제 사용처에서 toIntOrNull()로 변환하여 숫자로 활용되고 있습니다. 만약 항상 숫자를 나타낸다면, 도메인 모델에서 Int 타입으로 변경하여 타입 안전성을 높이는 것을 고려해 보세요.
  4. 매직 넘버 (Const Val) 추출 (P2):
    • ChatSummary.chatRoomId = -1L과 같은 특정 의미를 가지는 매직 넘버는 const val 상수로 정의하여 사용하는 것이 좋습니다. (예: const val MANAGER_CHAT_ROOM_ID = -1L)
    • LoadingColumnrepeat(5) 또한 const val로 추출하는 것이 좋습니다.
  5. 에러 핸들링 구체화 (P1):
    • ChatListViewModel.loadChatList().onFailureChatListScreenContentLoadStatus.Error TODO를 해결해야 합니다. 사용자에게 에러 상황을 명확히 알리고 (예: SnackBar, Toast 또는 에러 메시지 UI), 필요한 경우 재시도 액션을 제공해야 합니다.
  6. TODO 처리:
    • 코드 내에 남아있는 TODO 주석들을 확인하고, 이 PR의 범위 내에서 처리할 수 있는 것은 처리하고, 아닌 것은 다음 PR로 계획을 세워야 합니다.

위 권고 사항들을 반영하면 더욱 견고하고 유지보수하기 좋은 코드가 될 것입니다. 좋은 코드 잘 봤습니다!

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, 시니어 Android 개발자입니다. PR 잘 봤습니다. 전반적으로 코드 구조와 Jetpack Compose, Orbit MVI 패턴을 잘 적용하려는 노력이 돋보입니다. 특히 LoadStatus를 사용하여 로딩 상태를 관리하고, LazyColumn에 key를 지정한 점, runCatching으로 Result를 반환하여 에러 핸들링을 시작한 점은 매우 좋습니다.

몇 가지 개선 사항과 피드백을 드리니 참고하여 반영해주시면 좋겠습니다.


General Feedback

  • 하드코딩된 문자열 (High Priority)
    • 여러 Composable 함수에서 사용자에게 보이는 문자열(예: "알람 설정 버튼", "목록에서 지우기", "채팅", "전화왔어 매니저")이 하드코딩되어 있습니다. 이는 다국어 지원 및 유지보수 측면에서 좋지 않습니다.
    • 권장사항: 모든 사용자 대상 문자열을 strings.xml 리소스 파일로 추출하여 사용해주세요.
  • UI Dimension 관리 (Medium Priority)
    • Compose 코드 내에서 102.dp, 20.dp, 4.dp 등 UI 레이아웃 관련 매직 넘버가 많이 사용되고 있습니다.
    • 권장사항: 디자인 시스템 모듈 내에 Dimens.kt와 같은 파일을 만들거나, CallTheme에 확장 프로퍼티로 정의하여 일관된 디자인 시스템을 구축하는 것이 좋습니다. 예를 들어 CallTheme.dimens.paddingMedium 등으로 사용하는 방식입니다.
  • AppNavigator TODO (Medium Priority)
    • AppNavigator.kt 파일에 여전히 // TODO: 내비게이션 이동이 필요할 때 메서드를 작성 주석이 남아있습니다. chattingEntry에서 navigateToChatRoom을 추가하신 것처럼, 앞으로 필요한 내비게이션 함수들을 이곳에 추가하며 TODO를 해결해나가면 좋겠습니다.

Kotlin Code Review

  1. Coroutine 사용 시 Dispatcher 명시 여부, viewModelScope/lifecycleScope 오남용

    • ChatRepositoryImpl.kt에서 delay()를 사용한 부분은 Mocking이므로 괜찮습니다. suspend 함수이므로 호출하는 쪽(ViewModel)에서 Dispatcher를 관리하게 됩니다.
    • ChatListViewModel.kt에서 intent 블록 내에서 chatRepository.getChatList()를 호출하는 것은 Orbit의 viewModelScope 내에서 실행되므로 적절합니다.
    • 현재까지는 Dispatcher 관련하여 큰 문제는 없어 보입니다.
  2. Flow/StateFlow 사용 시 collect 시점 (repeatOnLifecycle 사용 여부)

    • ChatListScreen.kt에서 viewModel.collectAsState().valueviewModel.collectSideEffect를 사용하여 Orbit의 상태와 SideEffect를 수집하고 있습니다. Orbit Compose 통합은 Compose의 Lifecycle을 자동으로 따르므로 repeatOnLifecycle을 명시적으로 사용할 필요는 없습니다. 잘 사용하고 있습니다.
  3. null 안전성 (!! 사용 지양)

    • UiModelMapper.kt에서 photoUrl.ifEmpty { null }과 같이 null 대신 빈 문자열을 처리하는 방식이 좋습니다.
    • ChatListScreen.kt에서 chatSummary.unReadMessageCount.toIntOrNull() ?: 0와 같이 안전한 타입 변환을 사용한 점도 좋습니다.
    • 전체적으로 !! 사용을 지양하고 안전하게 코드를 작성하고 있어 훌륭합니다.
  4. data class에 불필요한 var 사용 여부

    • 새로 추가된 모든 data class (ChatMessage, ChatSummary, ChatRoomNavKey, ChatItemUiModel, ChatListState, ChatRoomUiState)의 필드가 모두 val로 선언되어 있습니다. 이는 데이터 클래스의 불변성을 유지하는 좋은 접근 방식입니다.
  5. Hilt DI 시 생성자 주입 원칙 준수 여부

    • ChatRepositoryImplChatListViewModel 모두 @Inject constructor()를 통해 생성자 주입을 하고 있습니다.
    • RepositoryModule.kt에서 ChatRepository 인터페이스를 ChatRepositoryImpl 구현체와 바인딩하기 위해 @Binds를 사용한 것도 올바른 Hilt DI 방식입니다.
  6. 하드코딩된 문자열/매직 넘버

    • 앞서 General Feedback에서 언급했듯이, 사용자 대상 문자열은 리소스 파일로 분리하고, UI Dimension은 디자인 시스템을 통해 관리하는 것을 강력히 권장합니다.
    • ChatRepositoryImpl.kt에 Mock 데이터로 하드코딩된 문자열과 숫자는 실제 API 연동 시점에 사라질 것이므로 현재로서는 허용 가능합니다. TODO: 추후에 api 연동 주석이 잘 달려 있습니다.
    • LoadingColumn.ktrepeat(5)와 같은 반복 횟수도 매직 넘버에 해당합니다. 로딩 아이템 개수가 고정이라면 상수로 정의하거나, 동적으로 계산하는 방식을 고려해볼 수 있습니다.

Jetpack Compose Code Review

  1. Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용)

    • noRippleClickable.kt에서 remember { MutableInteractionSource() }를 사용하여 리컴포지션 시마다 새로운 InteractionSource가 생성되는 것을 방지한 점이 좋습니다. composed modifier를 사용하여 Context를 가져오는 방식도 올바릅니다.
    • skeleton.kt에서 rememberInfiniteTransition을 사용하여 애니메이션 상태를 효율적으로 관리하고 있습니다. drawWithCache를 사용하여 그리기 작업을 최적화한 점도 훌륭합니다.
    • ChatListItem.kt에서 remember { AnchoredDraggableState(...) }를 사용하여 스와이프 상태가 리컴포지션 시에도 유지되도록 했습니다.
    • ChatListScreen.ktLazyColumn에서 key = { index -> state.chatList[index].chatRoomId }를 사용하여 리스트 아이템의 효율적인 리컴포지션을 보장하는 점은 매우 좋습니다.
  2. State hoisting 원칙 준수

    • BackgroundRow, ChatListItem, FrontRow, UnReadMessageCountBox, SwipeableBox 등 대부분의 Composable 함수에서 필요한 상태나 이벤트를 파라미터로 받아 상위 Composable에서 관리하도록 (onAlarmClick, onDeleteClick, chatSummary, swipeState 등) State hoisting 원칙을 잘 준수하고 있습니다.
    • ChatListItem에서 AnchoredDraggableState를 내부적으로 remember하는 것은 이 컴포넌트의 특정 UI 상태에 해당하므로 허용 가능합니다. 다만, 만약 스와이프 상태를 외부에서 제어해야 하는 상황이 생긴다면 이 또한 hoisting 대상이 될 수 있습니다.
  3. side effect (LaunchedEffect, DisposableEffect) 사용의 적절성

    • SwipeableBox.kt에서 LaunchedEffect(Unit)를 사용하여 swipeState.updateAnchors를 컴포지션 초기에 한 번만 호출하도록 한 점이 적절합니다.
  4. UI 상태를 data class로 표현했는지

    • ChatListStateChatItemUiModel 모두 data class로 UI 상태를 명확하게 표현하고 있어 좋습니다.
  5. 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지

    • ChatListState에서 LoadStatus enum class를 사용하여 로딩, 에러, 유휴 상태를 명확하게 관리하고 있습니다.
    • ChatListScreen.kt에서 when (state.status)를 통해 각 상태에 따라 다른 UI(로딩 스켈레톤, 실제 콘텐츠 등)를 표시하는 방식은 매우 훌륭합니다.

Repository/DataSource 레이어

  1. Retrofit 에러 핸들링 (try-catch, Result 래핑)

    • ChatRepositoryImpl.kt에서 runCatching을 사용하여 Result<List<ChatSummary>>를 반환하는 패턴은 네트워크 통신 시 발생할 수 있는 예외를 안전하게 처리하고 UI 레이어로 전파하기 위한 좋은 접근 방식입니다.
    • onFailure 블록 내부에서 에러를 적절히 로깅하거나 다른 에러 핸들링 로직을 추가해야 합니다.
  2. 네트워크 응답과 도메인 모델 매핑 분리 여부

    • 현재는 Mock 데이터를 사용하고 있어 DTO(Data Transfer Object)와 도메인 모델 간의 매핑 로직은 없지만, core:domain에 도메인 모델이 정의되어 있고 core:data에 Repository 구현체가 있는 구조 자체는 매핑 계층을 분리하기에 적합합니다. 실제 API 연동 시 DTO를 정의하고 매핑 로직을 ChatRepositoryImpl 내부에 구현하게 될 것입니다.
    • UiModelMapper.kt를 통해 도메인 모델을 UI 모델로 변환하는 계층이 분리된 점은 훌륭합니다.
  3. 캐싱 전략 (로컬 DB vs 메모리)

    • 이번 PR에서는 캐싱 전략이 구현되지 않았습니다. 초기 기능 개발 단계에서는 흔한 일입니다. 향후 요구사항에 따라 적절한 캐싱 전략(로컬 DB, 메모리 캐시 등)을 도입하는 것을 고려해볼 수 있습니다.

ViewModel Review

  1. UI 상태와 비즈니스 로직 분리

    • ChatListViewModel.kt에서 loadChatList() 메서드가 chatRepository를 통해 데이터를 가져오고, 그 결과를 reduce를 통해 UI 상태(ChatListState)에 반영하는 구조는 UI 상태와 비즈니스 로직이 잘 분리되어 있음을 보여줍니다.
  2. Orbit ContainerHost 패턴 준수 여부

    • ChatListViewModelContainerHost 인터페이스를 구현하고 container를 초기화하는 등 Orbit MVI 패턴을 정확히 따르고 있습니다.
  3. intent 내부에서 상태 변경(reduce)만 수행하는지

    • loadChatList() intent 내부에서 reduce를 사용하여 상태를 변경하고, chatRepository.getChatList()와 같은 비동기 작업은 intent 블록 내에서 suspend 함수 호출로 처리하여 상태 변경 로직과 비동기 작업을 적절히 분리하고 있습니다.
  4. postSideEffect 남용 여부

    • emitNavigateToChatRoom 함수에서 postSideEffect를 사용하여 일회성 이벤트인 화면 전환을 처리하는 것은 Side Effect의 올바른 사용 예시입니다. 남용으로 보이지 않습니다.
  5. blocking 작업을 intent 내부에서 직접 수행하지 않는지

    • chatRepository.getChatList()suspend 함수이므로 blocking 작업이 아니며, delay() 또한 coroutine의 non-blocking 지연입니다. 따라서 intent 내부에서 blocking 작업을 직접 수행하지 않는 원칙을 잘 지키고 있습니다.

FCM/SSE/실시간 통신 관련 코드

  • 해당 PR에는 FCM/SSE/실시간 통신 관련 코드가 포함되어 있지 않습니다.

추가 제안사항:

  • ChatRoomScreen.ktChatScreen.kt가 현재 비어있지만, 실제 구현 시에는 이 PR에서 보여준 좋은 패턴들(LoadStatus, State Hoisting, ViewModel/Orbit 등)을 동일하게 적용해주시면 좋겠습니다.
  • UiModelMapper.kttimeFormatterDateTimeFormatter.ofPattern("HH:mm")으로 하드코딩되어 있는데, 날짜/시간 형식도 국제화의 대상이 될 수 있습니다. 필요하다면 이 또한 리소스(예: string.xml에 패턴 문자열 저장)를 통해 관리하는 것을 고려해볼 수 있습니다.

전반적으로 매우 깔끔하고 모범적인 코드를 작성해주셨습니다. 위 피드백을 반영하여 더욱 견고하고 유지보수하기 쉬운 코드를 만들어주세요! 수고 많으셨습니다.

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요! GitHub PR 리뷰 요청 주셔서 감사합니다. 시니어 Android 개발자의 관점에서 PR diff를 꼼꼼히 살펴보았습니다. 전반적으로 Jetpack Compose, Hilt, Orbit MVI 패턴을 잘 적용하여 현대적인 아키텍처를 구성하려는 노력이 돋보입니다. 특히 Result 타입과 LoadStatus를 활용한 상태 관리는 매우 훌륭합니다.

몇 가지 개선점을 제안하며 코드 리뷰를 시작하겠습니다.


PR 종합 의견

이번 PR은 채팅 기능의 핵심 UI, 도메인 모델, 데이터 레이어, 그리고 ViewModel을 성공적으로 도입했습니다. Jetpack Compose의 효율적인 recomposition을 위한 LazyColumn key 사용, remember를 통한 상태 관리, LaunchedEffect의 적절한 활용 등 컴포즈 모범 사례를 잘 따르고 있습니다. Orbit MVI 패턴 역시 ViewModel에서 명확하게 적용되었습니다.

다만, 대부분의 UI 컴포넌트에서 하드코딩된 문자열매직 넘버가 많이 발견됩니다. 이는 향후 다국어 지원, 디자인 시스템의 일관성 유지, 그리고 코드 변경 용이성 측면에서 개선이 필요합니다. 또한, 데이터 타입의 일관성에러 핸들링 부분도 보완하면 더욱 견고한 코드가 될 것입니다.


세부 코드 리뷰

1. Kotlin 코드 리뷰

  • Coroutine 사용 시 Dispatcher 명시 여부, viewModelScope/lifecycleScope 오남용:

    • ChatListViewModel에서 intent { ... } 블록 내에서 suspend 함수인 chatRepository.getChatList()를 호출하는 방식은 적절합니다. Orbit의 intentviewModelScope 내에서 실행되며, 내부적으로 적절한 Dispatcher를 사용합니다. chatRepository 구현체 (향후 Retrofit 연동 시) 내부에서 withContext(Dispatchers.IO) 등으로 I/O 작업을 명시적으로 처리한다면 더욱 좋습니다. 현재로서는 문제가 없습니다.
  • Flow/StateFlow 사용 시 collect 시점 (repeatOnLifecycle 사용 여부):

    • ChatListScreen에서 viewModel.collectAsState().valueviewModel.collectSideEffect { ... }를 사용하고 있습니다. Compose의 collectAsState는 자동으로 repeatOnLifecycle을 적용하여 컴포저블의 생명주기에 맞춰 Flow 수집을 시작/중지하므로, 이 부분은 올바르게 사용되었습니다.
  • null 안전성 (!! 사용 지양):

    • 코드에서 !! 연산자는 발견되지 않았습니다. photoUrl.ifEmpty { null }, chatMessageId: Long? 등 nullable 타입을 명시적으로 선언하고 toIntOrNull() ?: 0와 같이 안전한 방식으로 null/empty 값을 처리하는 모습이 좋습니다.
    • 개선 권고: ChatSummary.unReadMessageCount는 현재 String 타입입니다. 메시지 "개수"를 의미하므로 Int 타입으로 변경하는 것이 더 적절합니다. UI에 표시할 때만 String으로 변환하여 사용하는 것이 데이터의 무결성을 높이고, 계산이나 비교 등에서 발생할 수 있는 오류를 줄일 수 있습니다.
      // domain/model/chatting/ChatSummary.kt
      data class ChatSummary(
          // ...
          val unReadMessageCount: Int = 0, // String -> Int 변경
          // ...
      )
      
      // impl/component/FrontRow.kt
      // 변경 전: if ((chatSummary.unReadMessageCount.toIntOrNull() ?: 0) > 0) { ... }
      // 변경 후: if (chatSummary.unReadMessageCount > 0) { ... }
      
      // impl/component/UnReadMessageCountBox.kt
      // 변경 전: unReadMessageCount: String = "0"
      // 변경 후: unReadMessageCount: Int = 0
      // Text(text = unReadMessageCount.toString(), ...)
  • data class에 불필요한 var 사용 여부:

    • 모든 data class (예: ChatMessageList, ChatMessage, ChatSummary, ChatListState, ChatItemUiModel)에서 모든 필드를 val로 선언하여 불변성을 유지하고 있습니다. 매우 좋은 구현입니다.
  • Hilt DI 시 생성자 주입 원칙 준수 여부:

    • RepositoryModule에서 @Binds@Singleton을 사용하여 인터페이스와 구현체를 바인딩하는 방식은 Hilt의 모범 사례를 따릅니다.
    • ChatRepositoryImplChatListViewModel 모두 @Inject constructor()를 사용하여 의존성을 주입받고 있으며, ChatListViewModel@HiltViewModel을 사용하고 있습니다. Hilt DI 원칙을 잘 준수하고 있습니다.
  • 하드코딩된 문자열/매직 넘버:

    • 하드코딩된 문자열:
      • BackgroundRow.kt: "알람 설정 버튼", "목록에서\n지우기"
      • FrontRow.kt: "매니저", "메인 캐릭터", "전화왔어 매니저", "수현님, 반가워요! 👋🏻 오늘은 어떤 이야기를"
      • ChatListScreen.kt: "채팅", "전화왔어 매니저", "수현님, 반가워요! 👋🏻 오늘은 어떤 이야기를"
      • 개선 권고: 모든 사용자에게 보이는 문자열(UI 텍스트, 접근성 contentDescription 등)은 res/values/strings.xml에 String Resource로 정의하여 사용해야 합니다. 이는 다국어 지원, 텍스트 변경의 용이성, 그리고 코드와 UI 텍스트의 분리를 위해 필수적입니다.
    • 매직 넘버:
      • ChatRepositoryImpl.kt: delay(1000.milliseconds) (현재는 TODO이므로 임시로 허용)
      • ChatSummary.kt: chatRoomId = -1 (매니저 채팅방을 위한 특수 값), unReadMessageCount = "0" (타입 개선과 함께 수정 필요)
      • skeleton.kt: durationMillis: Int = 1000, alpha = 0.3f, alpha = 0.6f
      • BackgroundRow.kt: 204.dp, 102.dp
      • FrontRow.kt: 52.dp, 102.dp, 20.dp, 12.dp, 8.dp, 6.dp, 16.dp, chatSummary.name.take(1)1
      • LoadingColumn.kt: repeat(5)5, 4.dp, 102.dp, 8.dp
      • SwipeableBox.kt: swipeWidth: Dp = 204.dp
      • UnReadMessageCountBox.kt: 3.dp, 20.dp
      • ChatListScreen.kt: 16.dp, 20.dp, 4.dp
      • 개선 권고: UI 요소의 크기(DP), 간격(DP), 알파 값, 반복 횟수 등은 디자인 시스템의 일부로 간주될 수 있습니다. 재사용 가능성이 높거나 디자인 가이드에 정의된 값이라면, DesignSystem 모듈 내의 Dimens.ktTheme.ktval 상수로 정의하여 사용하는 것이 좋습니다. 특정 컴포넌트에만 사용되는 값이라면 해당 컴포넌트 파일 상단에 private const val로 정의하여 가독성을 높일 수 있습니다. chatSummary.name.take(1)1은 글자 수 제한을 의미하므로 상수로 명시하는 것이 좋습니다.

2. Jetpack Compose 코드 확인

  • Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용):

    • remember { AnchoredDraggableState(...) }와 같은 remember 사용은 컴포저블의 상태를 효율적으로 관리하여 불필요한 recomposition을 방지합니다.
    • LazyColumn에서 key = { index -> state.chatList[index].chatRoomId }를 사용하여 각 아이템에 고유한 키를 부여한 것은 매우 훌륭합니다. 이는 LazyColumn의 성능 최적화에 필수적이며, 아이템의 추가/삭제/이동 시 올바른 recomposition을 보장합니다.
  • State hoisting 원칙 준수:

    • ChatListScreen에서 stateonIntent 콜백을 ChatListScreenContent로 호이스팅하고, ChatListItem, BackgroundRow, FrontRow 등 하위 컴포넌트에서도 필요한 데이터와 이벤트를 파라미터로 전달하는 등 State hoisting 원칙을 잘 준수하고 있습니다. 이는 컴포저블의 재사용성과 테스트 용이성을 높이는 좋은 접근 방식입니다.
  • side effect (LaunchedEffect, DisposableEffect) 사용의 적절성:

    • SwipeableBox에서 LaunchedEffect(Unit)를 사용하여 AnchoredDraggableStateupdateAnchors를 한 번만 호출하도록 한 것은 Side Effect를 적절하게 다룬 좋은 예시입니다. LocalDensity.current에 의존하여 계산된 값을 한 번 설정하는 데 사용되었습니다.
    • ChatListScreen에서 collectSideEffect를 통해 내비게이션과 같은 일회성 이벤트를 처리하는 방식 또한 Side Effect를 올바르게 사용한 것입니다.
  • UI 상태를 data class로 표현했는지:

    • ChatListStateChatItemUiModel 모두 data class로 정의되어 UI 상태를 명확하게 표현하고 있습니다. 이는 상태 관리를 용이하게 하고, 가독성을 높입니다.
  • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지:

    • ChatListState에서 LoadStatus (Idle, Loading, Error) enum을 사용하여 로딩/에러 상태를 체계적으로 관리하고 있습니다. ChatListScreenContent에서 when (state.status)를 통해 각 상태에 따라 다른 UI를 보여주는 방식은 매우 바람직합니다.

3. Repository/DataSource 레이어 확인

  • Retrofit 에러 핸들링 (try-catch, Result 래핑):

    • ChatRepositoryImpl에서 runCatching { ... }를 사용하여 Result<List<ChatSummary>>를 반환하는 것은 Retrofit 연동 시 발생할 수 있는 네트워크 오류 등을 처리하기 위한 좋은 패턴입니다.
    • ChatListViewModel에서 이 Result.fold(onSuccess = { ... }, onFailure = { ... })로 소비하는 방식 또한 올바른 에러 핸들링 접근입니다.
    • 개선 권고: ChatListViewModelonFailure 블록은 현재 비어 있습니다. 실제 에러 발생 시 UI에 적절한 에러 메시지를 표시하거나, 에러 상태를 LoadStatus.Error(exception)와 같이 업데이트하여 ChatListScreenContent에서 에러 UI를 렌더링하도록 처리해야 합니다. 사용자에게 에러 상황을 인지시키고 재시도 등의 옵션을 제공하는 것이 중요합니다.
      // impl/viewmodel/ChatListViewModel.kt
      onFailure = { throwable ->
          reduce {
              state.copy(
                  status = LoadStatus.Error(throwable), // LoadStatus에 throwable을 전달할 수 있도록 확장
                  chatList = emptyList() // 에러 시 목록 비우기 또는 이전 상태 유지 선택
              )
          }
          // 또는 postSideEffect(ChatListSideEffect.ShowErrorToast("에러 발생: ${throwable.localizedMessage}"))
      }
      LoadStatus가 현재 LoadStatus.Error만 있고, 에러 상세 정보(예외 객체)를 담을 수 없으므로 LoadStatus를 Sealed Interface 또는 Data Class로 확장하는 것을 고려할 수 있습니다.
      // domain/util/LoadStatus.kt
      sealed interface LoadStatus {
          data object Idle : LoadStatus
          data object Loading : LoadStatus
          data class Error(val throwable: Throwable? = null) : LoadStatus
      }
  • 네트워크 응답과 도메인 모델 매핑 분리 여부:

    • 현재는 ChatRepositoryImpl에서 하드코딩된 ChatSummary (도메인 모델)를 직접 반환하고 있습니다.
    • UiModelMapper.kt를 통해 ChatMessage (도메인 모델)를 ChatItemUiModel (UI 모델)로 매핑하는 구조는 이미 잘 분리되어 있습니다.
    • 개선 권고: 향후 Retrofit을 연동할 때, 네트워크 응답을 위한 별도의 DTO(Data Transfer Object)를 정의하고, ChatRepositoryImpl 내부 또는 별도의 매퍼 클래스에서 DTO를 도메인 모델(ChatSummary, ChatMessage)로 변환하는 로직을 구현해야 합니다. 이는 데이터 소스 계층과 도메인 계층의 독립성을 유지하는 데 필수적입니다.
  • 캐싱 전략 (로컬 DB vs 메모리):

    • 현재 PR에서는 캐싱 전략이 포함되어 있지 않습니다. 이는 초기 구현 단계에서 자연스러운 부분입니다.
    • 추후 고려 사항: 실시간 통신이나 오프라인 지원, 성능 최적화가 필요할 경우 로컬 데이터베이스(Room) 또는 메모리 캐싱 전략 도입을 고려할 수 있습니다.

4. ViewModel 확인

  • UI 상태와 비즈니스 로직 분리:

    • ChatListViewModel은 UI 상태(ChatListState)를 관리하고, 채팅 목록 로드와 같은 비즈니스 로직을 처리하며, UI 이벤트를 처리(handleIntent)하여 UI 상태를 변경하거나 Side Effect를 발행합니다. 이는 UI와 비즈니스 로직의 좋은 분리입니다.
  • Orbit ContainerHost 패턴 준수 여부:

    • ChatListViewModelContainerHost 인터페이스를 구현하고 container를 초기화하며, intent { reduce { ... } }postSideEffect { ... }를 사용하는 등 Orbit MVI 패턴을 정확하게 따르고 있습니다. 매우 모범적인 구현입니다.
  • intent 내부에서 상태 변경(reduce)만 수행하는지:

    • loadChatList() 함수 내에서 reduce는 오직 상태 변경(state.copy)에만 사용되고, 비즈니스 로직(chatRepository.getChatList())은 reduce 외부에서 (하지만 intent 블록 내에서) 수행됩니다. 이는 Orbit의 intentreduce의 역할을 올바르게 분리한 것입니다.
  • postSideEffect 남용 여부:

    • postSideEffect는 내비게이션과 같은 일회성 이벤트를 처리하는 데 사용됩니다 (ChatListSideEffect.NavigateToChatRoom). 이는 postSideEffect의 올바른 사용 사례이며, 남용이라고 볼 수 없습니다.
  • blocking 작업을 intent 내부에서 직접 수행하지 않는지:

    • chatRepository.getChatList()suspend 함수이므로 intent 블록 내에서 호출되어도 스레드를 블로킹하지 않습니다. 따라서 Blocking 작업 관련 문제는 없습니다.

5. FCM/SSE/실시간 통신 관련 코드 확인

  • 해당 PR에는 FCM/SSE/실시간 통신 관련 코드가 포함되어 있지 않습니다.

정리 및 최종 의견

이번 PR은 기술적으로 매우 잘 구현된 상태이며, 현대 Android 개발 스택과 아키텍처 패턴을 모범적으로 적용하고 있습니다. 특히 Jetpack Compose와 Orbit MVI의 활용이 인상 깊습니다.

가장 시급한 개선 사항은 하드코딩된 문자열과 매직 넘버를 String Resource 및 상수로 분리하는 것입니다. 그 다음으로는 unReadMessageCount데이터 타입 개선에러 핸들링을 좀 더 견고하게 만드는 것을 추천합니다.

이러한 개선 사항들을 반영한다면, 더욱 유지보수하기 좋고 확장 가능한 코드가 될 것입니다. 수고 많으셨습니다!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (6)
core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatSummary.kt (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

표시용 시간 문자열을 도메인 모델에서 분리해 주세요.

whenSubmitted"오후 3:21", "어제" 같은 로케일·상대시간 문자열을 저장하면 도메인 계층이 한국어 UI 포맷에 결합되고, 다국어·시간대·정렬 처리가 어려워집니다. 원시 timestamp를 보관한 뒤 UI 계층에서 포맷하세요. 현재 ChatRepositoryImpl.kt Line 22, Line 31, Line 40, Line 49에서도 같은 형식이 직접 주입됩니다.

🤖 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 `@core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatSummary.kt` at
line 20, ChatSummary의 whenSubmitted를 로케일·상대시간 표시 문자열 대신 원시 timestamp 타입으로 변경하고,
ChatRepositoryImpl의 해당 생성 지점들에서도 포맷된 문자열이 아닌 timestamp 값을 전달하세요. 표시용 시간 포맷팅은 도메인
계층에서 제거해 UI 계층이 담당하도록 유지하세요.
feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt (1)

20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

함수명을 lower camelCase로 바꾸세요.

UnReadMessageCountBox는 함수명 규칙에 맞지 않습니다. unreadMessageCountBox로 변경하고 FrontRow.kt의 호출도 함께 갱신하세요.

As per path instructions, “함수/변수 camelCase.”

🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt`
around lines 20 - 22, Rename the composable function UnReadMessageCountBox to
unreadMessageCountBox to follow lower camelCase naming, and update its
invocation in FrontRow.kt accordingly. Ensure all references use the new
function name.

Source: Path instructions

feature/chatting/impl/src/main/java/kr/co/call/impl/component/FrontRow.kt (1)

35-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

재사용 컴포저블의 Modifier 계약을 통일하세요.

모든 재사용 컴포저블은 호출자가 전달하는 Modifier를 첫 인자로 받아야 하며, 기본값을 두지 않아야 합니다.

  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/FrontRow.kt#L35-L40: modifier 기본값을 제거하고 모든 호출 지점에서 명시적으로 전달하세요.
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt#L19-L22: 첫 인자로 기본값 없는 modifier: Modifier를 추가해 루트 Box에 연결하세요.

As per path instructions, “Modifier 파라미터 위치(파라미터 앞쪽) 및 기본값 없이 전달 여부.”

🤖 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 `@feature/chatting/impl/src/main/java/kr/co/call/impl/component/FrontRow.kt`
around lines 35 - 40, 재사용 컴포저블의 Modifier 계약을 통일하세요. FrontRow의 modifier를 첫 번째
파라미터로 유지하되 기본값을 제거하고 모든 호출부에서 명시적으로 전달하세요. UnReadMessageCountBox에는 기본값 없는
modifier: Modifier를 첫 번째 파라미터로 추가하고 루트 Box에 연결하세요. 변경 대상은 FrontRow.kt 35-40행과
UnReadMessageCountBox.kt 19-22행입니다.

Source: Path instructions

feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeState.kt (1)

3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

enum 상수명을 UPPER_SNAKE_CASE로 변경하세요.

ClosedOpened는 프로젝트 규칙의 상수 명명 규칙을 따르지 않습니다. CLOSED, OPENED으로 변경하고 SwipeableBox.kt의 앵커 참조도 함께 갱신하세요.

수정 예시
 enum class SwipeState {
-    Closed,
-    Opened
+    CLOSED,
+    OPENED,
 }

As per path instructions, 상수 UPPER_SNAKE_CASE.

🤖 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 `@feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeState.kt`
around lines 3 - 5, SwipeState enum constants currently use nonstandard casing.
Rename Closed and Opened to CLOSED and OPENED, and update all corresponding
anchor references in SwipeableBox.kt while preserving the existing state
behavior.

Source: Path instructions

feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt (1)

39-46: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

상태와 SideEffect를 수명주기 인지 방식으로 수집하세요.

현재 구현은 경로 규칙의 collectAsStateWithLifecycleLaunchedEffect + collect 패턴을 따르지 않습니다. 상태는 lifecycle-aware API로, SideEffect는 안정적인 key를 둔 LaunchedEffect에서 sideEffectFlow를 수집하도록 교체하세요.

As per path instructions, **/*Screen.kt: "Orbit MVI Container의 state는 collectAsStateWithLifecycle로, sideEffect는 LaunchedEffect + collect로 구독하는지."

🤖 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 `@feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt`
around lines 39 - 46, Update ChatListScreen’s state collection to use
collectAsStateWithLifecycle, and replace viewModel.collectSideEffect with a
LaunchedEffect using a stable key that collects viewModel.sideEffectFlow.
Preserve the existing NavigateToChatRoom handling and callback arguments inside
the flow collector.

Source: Path instructions

feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt (1)

12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

ViewModel이 Repository 대신 UseCase에 의존하도록 변경하세요.

ChatListViewModelChatRepository를 직접 주입·호출합니다. 목록 조회 UseCase를 core:domain에 두고 ViewModel은 상태 전환과 SideEffect 조정만 담당하게 분리하세요.

As per path instructions, **/*ViewModel.kt: "비즈니스 로직이 UseCase(core:domain) 호출로 위임되고 ViewModel에 과도하게 몰려있지 않은지 확인."

Also applies to: 33-45

🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt`
around lines 12 - 14, ChatListViewModel의 ChatRepository 직접 의존성을 제거하고 목록 조회 전용
UseCase를 core:domain에 추가해 주입·호출하도록 변경하세요. ViewModel은 UseCase 결과를 바탕으로
ChatListState 전환과 ChatListSideEffect 조정만 담당하게 하며, 관련 조회 비즈니스 로직은 UseCase로 이동하세요.

Source: Path instructions

🤖 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
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt`:
- Around line 14-15: Update getChatList so CancellationException from delay is
rethrown rather than captured as Result.failure, while continuing to wrap
ordinary exceptions in the returned Result. Keep the existing delay and
repository behavior unchanged.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt`:
- Around line 21-35: The modifier parameter is currently only applied to the
BackgroundRow child component instead of the SwipeableBox parent component,
which prevents the parent layout, accessibility, and click area from being
applied correctly. Pass the modifier directly to the SwipeableBox component in
the ChatListItem function to ensure the entire swipeable item receives the
intended layout and interaction handling from its parent.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt`:
- Around line 37-49: Update the LaunchedEffect in the SwipeableBox anchor-update
flow to key on swipeWidthPx in addition to its current trigger, so anchors are
recalculated whenever the swipe width or density-derived pixel value changes.
Keep the existing Closed and Opened anchor definitions unchanged.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt`:
- Around line 25-35: Update the unread-count container in the composable
containing the Box and Text so counts of three digits or more are not clipped.
Replace the fixed 20.dp sizing with a minimum size and horizontal padding that
allow the content to expand, or constrain the displayed count to the established
99+ representation while preserving the existing styling and alignment.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt`:
- Line 132: Remove the unsafe cast from the onChatRoomClick argument in the
Preview and provide a two-parameter no-op lambda that accepts the chat room ID
and name.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatRoomScreen.kt`:
- Around line 9-24: ChatRoomScreen과 ChatRoomScreenContent를 구현해 빈 화면을 제거하세요.
ChatRoomScreen에서 roomId와 name을 진입 Intent로 전달하고 Orbit MVI Container의 state는
collectAsStateWithLifecycle로, sideEffect는 LaunchedEffect에서 collect로 구독하세요. 수집한
state와 방 정보를 ChatRoomScreenContent에 전달해 실제 UI를 렌더링하고, 뒤로가기 side effect 및 앱 내
뒤로가기를 onBack에 연결하세요.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt`:
- Around line 42-44: Update ChatListViewModel.kt lines 42-44 in the onFailure
callback to store the failure message in LoadStatus.Error. Update
ChatListScreen.kt lines 82-85 to render an error message and provide retry
behavior that re-runs the chat-list load operation.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomViewModel.kt`:
- Around line 10-15: ChatRoomViewModel의 container 초기화에서 initialState의 TODO()를
제거하고 ChatRoomUiState의 기본 초기값을 사용하도록 변경하세요. 생성 시 예외가 발생하지 않도록 하며, 초기 데이터 로딩이
필요하다면 별도의 intent 블록에서 처리하세요.

---

Nitpick comments:
In `@core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatSummary.kt`:
- Line 20: ChatSummary의 whenSubmitted를 로케일·상대시간 표시 문자열 대신 원시 timestamp 타입으로
변경하고, ChatRepositoryImpl의 해당 생성 지점들에서도 포맷된 문자열이 아닌 timestamp 값을 전달하세요. 표시용 시간
포맷팅은 도메인 계층에서 제거해 UI 계층이 담당하도록 유지하세요.

In `@feature/chatting/impl/src/main/java/kr/co/call/impl/component/FrontRow.kt`:
- Around line 35-40: 재사용 컴포저블의 Modifier 계약을 통일하세요. FrontRow의 modifier를 첫 번째
파라미터로 유지하되 기본값을 제거하고 모든 호출부에서 명시적으로 전달하세요. UnReadMessageCountBox에는 기본값 없는
modifier: Modifier를 첫 번째 파라미터로 추가하고 루트 Box에 연결하세요. 변경 대상은 FrontRow.kt 35-40행과
UnReadMessageCountBox.kt 19-22행입니다.

In `@feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeState.kt`:
- Around line 3-5: SwipeState enum constants currently use nonstandard casing.
Rename Closed and Opened to CLOSED and OPENED, and update all corresponding
anchor references in SwipeableBox.kt while preserving the existing state
behavior.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt`:
- Around line 20-22: Rename the composable function UnReadMessageCountBox to
unreadMessageCountBox to follow lower camelCase naming, and update its
invocation in FrontRow.kt accordingly. Ensure all references use the new
function name.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt`:
- Around line 39-46: Update ChatListScreen’s state collection to use
collectAsStateWithLifecycle, and replace viewModel.collectSideEffect with a
LaunchedEffect using a stable key that collects viewModel.sideEffectFlow.
Preserve the existing NavigateToChatRoom handling and callback arguments inside
the flow collector.

In
`@feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt`:
- Around line 12-14: ChatListViewModel의 ChatRepository 직접 의존성을 제거하고 목록 조회 전용
UseCase를 core:domain에 추가해 주입·호출하도록 변경하세요. ViewModel은 UseCase 결과를 바탕으로
ChatListState 전환과 ChatListSideEffect 조정만 담당하게 하며, 관련 조회 비즈니스 로직은 UseCase로 이동하세요.
🪄 Autofix (Beta)

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 83ac8cac-112a-4a04-8701-a5a7d10073f6

📥 Commits

Reviewing files that changed from the base of the PR and between 148b3eb and af8f858.

⛔ Files ignored due to path filters (1)
  • core/designsystem/src/main/res/drawable/ic_chat_manager.png is excluded by !**/*.png
📒 Files selected for processing (44)
  • app/src/main/java/kr/co/call/callfromai/AppNavigator.kt
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/modifier/noRippleClickable.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/modifier/skeleton.kt
  • core/designsystem/src/main/res/drawable/ic_chat_back.xml
  • core/designsystem/src/main/res/drawable/ic_chat_bell.xml
  • core/designsystem/src/main/res/drawable/ic_chat_bell_off.xml
  • core/designsystem/src/main/res/drawable/ic_chat_call.xml
  • core/designsystem/src/main/res/drawable/ic_chat_pin.xml
  • core/domain/src/main/java/kr/co/call/domain/model/chatting/.gitkeep
  • core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatMessage.kt
  • core/domain/src/main/java/kr/co/call/domain/model/chatting/ChatSummary.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/ChatRepository.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/gitkeep
  • feature/chatting/api/src/main/java/kr/co/call/api/ChattingRoute.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/.gitkeep
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/BackgroundRow.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatRoomTopBar.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/FrontRow.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/LoadingColumn.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeState.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/entry/.gitkeep
  • feature/chatting/impl/src/main/java/kr/co/call/impl/entry/ChattingEntryBuilder.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/mapper/UiModelMapper.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/model/ChatItemUiModel.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/.gitkeep
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatRoomScreen.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatScreen.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChattingScreen.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/.gitkeep
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListIntent.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListSideEffect.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListState.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomIntent.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomSideEffect.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomUiState.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomViewModel.kt
💤 Files with no reviewable changes (2)
  • core/domain/src/main/java/kr/co/call/domain/repository/gitkeep
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChattingScreen.kt

Comment on lines +14 to +15
override suspend fun getChatList(): Result<List<ChatSummary>> = runCatching {
delay(1000.milliseconds)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt' \
             'core/domain/**' \
             'core/data/**' | sed 's#^\./##' | sort

printf '\n--- ChatRepositoryImpl.kt ---\n'
cat -n core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt

printf '\n--- Search for getChatList usages ---\n'
rg -n "getChatList\(" core -S

printf '\n--- Search for runCatching + delay patterns ---\n'
rg -n "runCatching|CancellationException|delay\(1000\.milliseconds\)" core/data -S

Repository: call-from-ai/CallFromAi_Android

Length of output: 4380


코루틴 취소 예외를 재전파하세요. core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt:14-15runCatchingdelay(1000.milliseconds)가 던지는 CancellationException까지 Result.failure로 바꿉니다. 화면 이탈이나 상위 작업 취소가 실패로 처리되지 않도록, 취소 예외는 그대로 던지고 일반 예외만 감싸세요.

🤖 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 `@core/data/src/main/java/kr/co/call/data/repositoryImpl/ChatRepositoryImpl.kt`
around lines 14 - 15, Update getChatList so CancellationException from delay is
rethrown rather than captured as Result.failure, while continuing to wrap
ordinary exceptions in the returned Result. Keep the existing delay and
repository behavior unchanged.

Comment on lines +21 to +35
SwipeableBox(
swipeState = state,
backgroundContent = {
BackgroundRow(
modifier = modifier,
isAlarmEnabled = chatSummary.isAlarmEnabled,
onAlarmClick = onAlarmClick,
onDeleteClick = onDeleteClick
)
},
) {
FrontRow(
chatSummary = chatSummary,
onClick = onItemClick
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "[1] Locate the target file and related SwipeableBox definition"
git ls-files | rg 'feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem\.kt|SwipeableBox'

echo
echo "[2] Show the target file with line numbers"
cat -n feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt

echo
echo "[3] Search for SwipeableBox declaration and usages"
rg -n "fun\s+SwipeableBox|class\s+SwipeableBox|SwipeableBox\(" -S .

Repository: call-from-ai/CallFromAi_Android

Length of output: 2631


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "[1] Show SwipeableBox file with line numbers"
cat -n feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt

echo
echo "[2] Show any modifier-related call sites in the component package"
rg -n "modifier\s*=|Modifier\." feature/chatting/impl/src/main/java/kr/co/call/impl/component -S

Repository: call-from-ai/CallFromAi_Android

Length of output: 5179


modifierSwipeableBox에 전달하세요. feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt:21-35 현재 modifierBackgroundRow에만 붙어 있어, 부모가 기대한 전체 항목의 레이아웃/접근성/클릭 영역에 적용되지 않습니다.

수정 예시
     SwipeableBox(
+        modifier = modifier,
         swipeState = state,
         backgroundContent = {
             BackgroundRow(
-                modifier = modifier,
                 isAlarmEnabled = chatSummary.isAlarmEnabled,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SwipeableBox(
swipeState = state,
backgroundContent = {
BackgroundRow(
modifier = modifier,
isAlarmEnabled = chatSummary.isAlarmEnabled,
onAlarmClick = onAlarmClick,
onDeleteClick = onDeleteClick
)
},
) {
FrontRow(
chatSummary = chatSummary,
onClick = onItemClick
)
SwipeableBox(
modifier = modifier,
swipeState = state,
backgroundContent = {
BackgroundRow(
isAlarmEnabled = chatSummary.isAlarmEnabled,
onAlarmClick = onAlarmClick,
onDeleteClick = onDeleteClick
)
},
) {
FrontRow(
chatSummary = chatSummary,
onClick = onItemClick
)
🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt`
around lines 21 - 35, The modifier parameter is currently only applied to the
BackgroundRow child component instead of the SwipeableBox parent component,
which prevents the parent layout, accessibility, and click area from being
applied correctly. Pass the modifier directly to the SwipeableBox component in
the ChatListItem function to ensure the entire swipeable item receives the
intended layout and interaction handling from its parent.

Comment on lines +37 to +49
val swipeWidthPx = with(LocalDensity.current) { swipeWidth.toPx() }
val offsetX = swipeState.offset
val clampedOffset = if (offsetX.isNaN()) 0 else offsetX.roundToInt()
val backgroundAlpha = if (swipeWidthPx == 0f) 0f else (-clampedOffset / swipeWidthPx).coerceIn(0f, 1f)

LaunchedEffect(Unit) {
swipeState.updateAnchors(
DraggableAnchors {
SwipeState.Closed at 0f
SwipeState.Opened at -swipeWidthPx
}
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

ast-grep outline feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt --view expanded

Repository: call-from-ai/CallFromAi_Android

Length of output: 263


🏁 Script executed:

sed -n '1,220p' feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt

Repository: call-from-ai/CallFromAi_Android

Length of output: 2532


LaunchedEffect 키에 swipeWidthPx를 포함하세요.
지금은 밀도나 swipeWidth가 바뀌어도 앵커가 다시 계산되지 않아, 열림 위치와 배경 알파가 이전 값에 맞춰질 수 있습니다.

수정 예시
-    LaunchedEffect(Unit) {
+    LaunchedEffect(swipeState, swipeWidthPx) {
         swipeState.updateAnchors(
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val swipeWidthPx = with(LocalDensity.current) { swipeWidth.toPx() }
val offsetX = swipeState.offset
val clampedOffset = if (offsetX.isNaN()) 0 else offsetX.roundToInt()
val backgroundAlpha = if (swipeWidthPx == 0f) 0f else (-clampedOffset / swipeWidthPx).coerceIn(0f, 1f)
LaunchedEffect(Unit) {
swipeState.updateAnchors(
DraggableAnchors {
SwipeState.Closed at 0f
SwipeState.Opened at -swipeWidthPx
}
)
}
val swipeWidthPx = with(LocalDensity.current) { swipeWidth.toPx() }
val offsetX = swipeState.offset
val clampedOffset = if (offsetX.isNaN()) 0 else offsetX.roundToInt()
val backgroundAlpha = if (swipeWidthPx == 0f) 0f else (-clampedOffset / swipeWidthPx).coerceIn(0f, 1f)
LaunchedEffect(swipeState, swipeWidthPx) {
swipeState.updateAnchors(
DraggableAnchors {
SwipeState.Closed at 0f
SwipeState.Opened at -swipeWidthPx
}
)
}
🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt`
around lines 37 - 49, Update the LaunchedEffect in the SwipeableBox
anchor-update flow to key on swipeWidthPx in addition to its current trigger, so
anchors are recalculated whenever the swipe width or density-derived pixel value
changes. Keep the existing Closed and Opened anchor definitions unchanged.

Comment on lines +25 to +35
Box(
modifier = Modifier
.size(20.dp)
.background(color = CallTheme.colors.subRed, shape = shape),
contentAlignment = Alignment.Center,
) {
Text(
text = unReadMessageCount,
style = CallTheme.typography.bodySmallBold,
color = CallTheme.colors.white,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

세 자릿수 이상의 읽지 않은 메시지 수가 잘립니다.

20.dp 고정 폭에서는 "100" 이상을 온전히 표시할 수 없습니다. 최소 크기와 가로 패딩을 사용해 내용에 맞게 늘어나게 하거나, 표시 값을 99+로 제한하세요.

🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/component/UnReadMessageCountBox.kt`
around lines 25 - 35, Update the unread-count container in the composable
containing the Box and Text so counts of three digits or more are not clipped.
Replace the fixed 20.dp sizing with a minimum size and horizontal padding that
allow the content to expand, or constrain the displayed count to the established
99+ representation while preserving the existing styling and alignment.

private fun ChatListScreenPreview() {
CallFromAiTheme {
ChatListScreenContent(
onChatRoomClick = {} as (Long, String) -> Unit,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt"

echo "== file outline =="
ast-grep outline "$FILE" --view expanded || true

echo
echo "== relevant lines around 120-145 =="
sed -n '120,145p' "$FILE" | cat -n

Repository: call-from-ai/CallFromAi_Android

Length of output: 1327


Preview의 unsafe cast를 제거하세요.
{}(Long, String) -> Unit으로 캐스팅하면 Preview에서 ClassCastException이 납니다. onChatRoomClick = { _, _ -> },로 바꾸면 됩니다.

🤖 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 `@feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt`
at line 132, Remove the unsafe cast from the onChatRoomClick argument in the
Preview and provide a two-parameter no-op lambda that accepts the chat room ID
and name.

Source: Path instructions

Comment on lines +9 to +24
fun ChatRoomScreen(
roomId: Long,
name: String,
modifier: Modifier = Modifier,
viewModel: ChatRoomViewModel = hiltViewModel(),
onBack: () -> Unit = {},
) {

}

@Composable
fun ChatRoomScreenContent(

) {

} No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

채팅방 라우트가 빈 화면으로 종료됩니다.

전달받은 roomId, name, onBack을 사용하지 않고 어떤 UI도 렌더링하지 않습니다. 현재 목록에서 방을 선택하면 빈 화면으로 이동하며 앱 내 뒤로가기 처리도 연결되지 않습니다. 진입 Intent에 방 정보를 전달하고, lifecycle-aware state 및 side effect를 구독한 뒤 실제 콘텐츠와 onBack 처리를 연결하세요.

As per path instructions, Orbit MVI Container의 state는 collectAsStateWithLifecycle로, sideEffect는 LaunchedEffect + collect로 구독하는지.

🧰 Tools
🪛 detekt (1.23.8)

[warning] 15-17: This empty block of code can be removed.

(detekt.empty-blocks.EmptyFunctionBlock)


[warning] 22-24: This empty block of code can be removed.

(detekt.empty-blocks.EmptyFunctionBlock)

🤖 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 `@feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatRoomScreen.kt`
around lines 9 - 24, ChatRoomScreen과 ChatRoomScreenContent를 구현해 빈 화면을 제거하세요.
ChatRoomScreen에서 roomId와 name을 진입 Intent로 전달하고 Orbit MVI Container의 state는
collectAsStateWithLifecycle로, sideEffect는 LaunchedEffect에서 collect로 구독하세요. 수집한
state와 방 정보를 ChatRoomScreenContent에 전달해 실제 UI를 렌더링하고, 뒤로가기 side effect 및 앱 내
뒤로가기를 onBack에 연결하세요.

Source: Path instructions

Comment on lines +42 to +44
onFailure = {

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target files around the cited lines
sed -n '1,160p' feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt
printf '\n----\n'
sed -n '1,180p' feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt

Repository: call-from-ai/CallFromAi_Android

Length of output: 7135


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect state and status definitions used by ChatListViewModel / ChatListScreen
sed -n '1,200p' feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListState.kt
printf '\n----\n'
sed -n '1,200p' core/domain/src/main/java/kr/co/call/domain/util/LoadStatus.kt
printf '\n----\n'
sed -n '1,200p' core/domain/src/main/java/kr/co/call/domain/repository/ChatRepository.kt

Repository: call-from-ai/CallFromAi_Android

Length of output: 951


채팅 목록 실패를 상태로 올려야 합니다.

onFailure가 비어 있어 실패 시 LoadStatus.Error로 전환되지 않고 Loading 상태가 계속 남습니다. 화면 쪽도 LoadStatus.Error 분기가 비어 있어 오류 안내나 재시도가 없습니다.

  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt#L42-L44: 실패 메시지를 담아 LoadStatus.Error(...)로 전환하세요.
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt#L82-L85: 오류 UI와 재시도 동작을 추가하세요.
📍 Affects 2 files
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt#L42-L44 (this comment)
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatListScreen.kt#L82-L85
🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatListViewModel.kt`
around lines 42 - 44, Update ChatListViewModel.kt lines 42-44 in the onFailure
callback to store the failure message in LoadStatus.Error. Update
ChatListScreen.kt lines 82-85 to render an error message and provide retry
behavior that re-runs the chat-list load operation.

Comment on lines +10 to +15
class ChatRoomViewModel @Inject constructor(
private val chatRepository: ChatRepository
): ViewModel(), ContainerHost<ChatRoomUiState, ChatRoomSideEffect> {
override val container: Container<ChatRoomUiState, ChatRoomSideEffect> = container(
initialState = TODO()
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate relevant files
fd -a 'ChatRoomViewModel.kt|ChatRoomScreen.kt' .

# Show file sizes
for f in $(fd -a 'ChatRoomViewModel.kt|ChatRoomScreen.kt' .); do
  echo "== $f =="
  wc -l "$f"
done

# Print relevant sections with line numbers
for f in $(fd -a 'ChatRoomViewModel.kt|ChatRoomScreen.kt' .); do
  echo "===== $f ====="
  cat -n "$f" | sed -n '1,220p'
done

Repository: call-from-ai/CallFromAi_Android

Length of output: 2386


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for Hilt annotations and ViewModel usage related to chat room
rg -n --hidden --glob '!**/build/**' --glob '!**/.gradle/**' '`@HiltViewModel`|hiltViewModel\(|ContainerHost<ChatRoomUiState, ChatRoomSideEffect>|TODO\(\)|initialState\s*=' .

Repository: call-from-ai/CallFromAi_Android

Length of output: 1046


initialState의 TODO()를 실제 초기 상태로 교체하세요. ChatRoomViewModel은 생성 시 TODO() 때문에 바로 예외가 발생합니다. ChatRoomUiState의 기본값으로 바꾸고, 필요하면 초기 데이터 로딩은 intent {} 안에서 처리하세요.

🤖 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
`@feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomViewModel.kt`
around lines 10 - 15, ChatRoomViewModel의 container 초기화에서 initialState의 TODO()를
제거하고 ChatRoomUiState의 기본 초기값을 사용하도록 변경하세요. 생성 시 예외가 발생하지 않도록 하며, 초기 데이터 로딩이
필요하다면 별도의 intent 블록에서 처리하세요.

- SwipeState.kt: Enum 상수를 Kotlin 명명 규칙에 맞춰 Closed, Opened에서 대문자 형태인 CLOSED, OPENED로 변경
- ChatListItem.kt, SwipeableBox.kt: 변경된 SwipeState enum 상수를 반영하여 참조 코드 수정
- .gitignore: IDE 설정 파일인 .idea/planningMode.xml 추가 배제 설정 정의
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

Gemini API 응답을 읽지 못했습니다.

{
  "error": {
    "code": 503,
    "message": "This model is currently experiencing high demand. Spikes in demand are usually temporary. Please try again later.",
    "status": "UNAVAILABLE"
  }
}

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/src/main/java/kr/co/call/callfromai/AppScreen.kt (2)

72-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

PR 설명에 변경 화면의 스크린샷 또는 녹화를 첨부하세요.

AppScreen의 BottomBar 레이아웃과 채팅방 이동 흐름이 변경됐지만, 제공된 PR 설명에는 시각 검증 자료가 없습니다. As per coding guidelines, “Jetpack Compose 화면 파일이 변경된 경우, PR 설명에 스크린샷 또는 화면 녹화가 첨부되었는지 확인하고 없으면 경고한다.”

🤖 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 `@app/src/main/java/kr/co/call/callfromai/AppScreen.kt` around lines 72 - 101,
AppScreen의 MainBottomBar 레이아웃과 chattingEntry의 ChatRoomNavKey 이동 흐름 변경을 검증할 수 있도록
PR 설명에 해당 화면의 스크린샷 또는 화면 녹화를 첨부하세요.

Source: Coding guidelines


68-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

remember 키에 density도 포함하세요.
LocalDensity가 바뀌면 px 값이 같아도 이전 dp 값이 재사용될 수 있습니다.

🤖 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 `@app/src/main/java/kr/co/call/callfromai/AppScreen.kt` around lines 68 - 70,
Update the remember key in the bottomBarPadding calculation to include density
alongside bottomBarHeightPx, ensuring the dp value is recomputed when
LocalDensity changes.
🤖 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.

Outside diff comments:
In `@app/src/main/java/kr/co/call/callfromai/AppScreen.kt`:
- Around line 72-101: AppScreen의 MainBottomBar 레이아웃과 chattingEntry의
ChatRoomNavKey 이동 흐름 변경을 검증할 수 있도록 PR 설명에 해당 화면의 스크린샷 또는 화면 녹화를 첨부하세요.
- Around line 68-70: Update the remember key in the bottomBarPadding calculation
to include density alongside bottomBarHeightPx, ensuring the dp value is
recomputed when LocalDensity changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: efb3b2be-704c-417d-a611-b8212ba1482f

📥 Commits

Reviewing files that changed from the base of the PR and between af8f858 and 1d6f448.

📒 Files selected for processing (8)
  • .gitignore
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeState.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/screen/ChatRoomScreen.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (4)
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeState.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/ChatListItem.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/viewmodel/ChatRoomViewModel.kt
  • feature/chatting/impl/src/main/java/kr/co/call/impl/component/SwipeableBox.kt

@JiwonLee42 JiwonLee42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수 고 했 어요 ,

- .idea 디렉토리 내 개별 파일 설정을 `/.idea/` 전체 디렉토리 무시로 통합
- `/build` 경로를 `**/build/`로 변경하여 모든 하위 모듈의 빌드 결과물이 제외되도록 수정
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요! PR 잘 올려주셨습니다. 이번 PR은 채팅 기능의 초기 구조와 UI 컴포넌트를 도입하는 중요한 작업이네요. 전반적으로 Orbit MVI 패턴과 Jetpack Compose의 모범 사례를 잘 따르려고 노력하신 점이 좋습니다. 새로운 기능이 추가될 때마다 체계적으로 접근하는 모습이 인상 깊습니다.

몇 가지 개선점과 질문 사항을 정리했으니, 확인하시고 반영해주시면 감사하겠습니다.


🔍 전반적인 코드 품질 및 변경 사항

  • 긍정적:

    • core/data, core/domain, feature/chatting/api, feature/chatting/impl 등 모듈 분리가 잘 되어 있어 책임 분할이 명확합니다.
    • Jetpack Compose 컴포넌트들이 component 패키지에 잘 정리되어 있습니다.
    • Orbit MVI 아키텍처를 도입하여 UI 상태 관리와 단방향 데이터 흐름을 잘 구현했습니다.
    • LoadStatus를 사용하여 로딩/에러 상태를 명확하게 관리하는 점이 좋습니다.
    • noRippleClickable, skeleton과 같은 유용한 Compose Modifier 확장 함수들을 추가하여 재활용성을 높였습니다.
    • Repository 레이어에서 Result 래핑을 통해 에러 핸들링 기반을 마련했습니다.
    • Compose에서 rememberLaunchedEffect를 적절히 사용하여 불필요한 recomposition을 줄이려는 노력이 보입니다.
    • data class의 필드를 대부분 val로 선언하여 불변성을 유지하는 점이 좋습니다.
    • Hilt DI 원칙을 잘 준수하고 있습니다.
  • 개선 필요 사항:

    • 하드코딩된 문자열/매직 넘버: UI 컴포넌트 곳곳에 하드코딩된 dp 값, 색상, 그리고 특정 상수 값(chatRoomId = -1)이 보입니다. 이들은 디자인 시스템의 DimensConstants 객체로 추출하여 관리하는 것이 좋습니다.
    • .gitignore 변경: /build 대신 **/build/로 변경한 것은 좋습니다. 하지만 .idea 디렉토리 전체를 무시하는 것은 팀 전체의 IDE 설정 동기화에 영향을 줄 수 있으므로, 어떤 파일들을 무시할지 좀 더 신중하게 결정하는 것이 좋습니다. 일반적으로 workspace.xml, modules.xml 등 사용자별 설정 파일만 무시하고 나머지는 포함하는 경우가 많습니다.
    • 파일 끝 개행(\ No newline at end of file): 몇몇 새 파일들(ChatRepositoryImpl.kt, ChatMessage.kt, ChatSummary.kt, ChattingRoute.kt 등)에 파일 끝 개행이 누락되어 있습니다. 통일된 코드 스타일을 위해 추가해주시는 것이 좋습니다.

🎯 Kotlin 코드 리뷰

  1. Coroutine 사용 시 Dispatcher 명시 여부, viewModelScope/lifecycleScope 오남용

    • ChatListViewModel에서 intent 블록 내에서 chatRepository.getChatList()를 호출하고 있는데, intent 블록은 기본적으로 Dispatchers.Default에서 실행됩니다. Repository 내부의 delay(1000.milliseconds)suspend 함수이므로 Dispatchers.Default에서 안전하게 실행됩니다.
    • 만약 chatRepository.getChatList() 내부에 I/O 작업(예: Retrofit 네트워크 호출, DB 접근)이 포함될 경우, ChatRepositoryImpl 내부 또는 ChatRepository 인터페이스의 구현체에서 withContext(Dispatchers.IO)를 사용하여 명시적으로 I/O Dispatcher를 지정하는 것이 좋습니다. 현재는 delay만 있어 큰 문제는 없지만, 미래를 대비해 명심하면 좋습니다.
  2. Flow/StateFlow 사용 시 collect 시점 (repeatOnLifecycle 사용 여부)

    • 이번 PR에는 Flow/StateFlow를 직접 collect하는 코드는 없지만, Orbit의 collectAsStatecollectSideEffect를 사용하고 계십니다. Orbit 내부적으로 repeatOnLifecycle을 활용하여 수명 주기를 인식하는 방식으로 동작하므로, 올바르게 사용하고 계십니다.
  3. null 안전성 (!! 사용 지양)

    • 새로 추가된 코드에서 !! 사용은 발견되지 않았습니다. 좋은 접근 방식입니다.
  4. data class에 불필요한 var 사용 여부

    • ChatSummary, ChatMessage, ChatMessageList, ChatItemUiModel 모두 val로 선언되어 있어 불변성을 잘 유지하고 있습니다. 훌륭합니다.
  5. Hilt DI 시 생성자 주입 원칙 준수 여부

    • ChatRepositoryImplChatListViewModel 모두 @Inject constructor()를 사용하여 생성자 주입 원칙을 잘 준수하고 있습니다. RepositoryModule에서 @Binds를 통해 인터페이스와 구현체를 연결하는 것도 올바른 Hilt 사용법입니다.
  6. 하드코딩된 문자열/매직 넘버

    • ChatRepositoryImpl.kt: delay(1000.milliseconds) 이 값은 로딩 스켈레톤 테스트를 위한 임시 값일 텐데, 추후 API 연동 시 실제 네트워크 지연 시간을 반영하거나, 테스트를 위한 별도의 DI 모듈로 분리하는 것을 고려해볼 수 있습니다.
    • ChatSummary.kt: chatRoomId = -1 이 값은 매니저 챗방을 나타내는 매직 넘버로 보입니다. companion object 등에 const val MANAGER_CHAT_ROOM_ID = -1L 과 같이 상수화하여 사용하는 것이 가독성과 유지보수성에 좋습니다.
    • ChatSummary.kt: unReadMessageCount: String = "0" unReadMessageCount는 일반적으로 Int 타입으로 관리하고, UI에 표시할 때만 String으로 포매팅하는 것이 좋습니다. 현재 FrontRow에서 toIntOrNull()을 사용하여 0보다 큰지 확인하는데, Int 타입이면 더 안전하게 처리할 수 있습니다.
    • UiModelMapper.kt: timeFormatter는 클래스 레벨에서 정의되어 재활용되고 있어 좋습니다.
    • FrontRow.kt, UnReadMessageCountBox.kt, LoadingColumn.kt, ChatListScreen.kt: 102.dp, 52.dp, 20.dp, 8.dp, 6.dp, 16.dp, 4.dp, 3.dp, 8.dp 등 많은 dp 값들이 하드코딩되어 있습니다. 이러한 값들은 디자인 시스템의 Dimens 객체(예: CallTheme.dimens.paddingSmall)로 정의하여 사용하면 일관된 UI를 유지하고 쉽게 변경할 수 있습니다.

🎨 Jetpack Compose 코드 리뷰

  1. Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용)

    • noRippleClickableskeleton Modifier에서 rememberrememberInfiniteTransition을 사용하여 상태를 기억하고 애니메이션 상태를 관리하는 방식은 올바릅니다.
    • ChatListItem에서 AnchoredDraggableStateremember로 감싸는 것도 좋습니다.
    • LazyColumn에서 key = { index -> state.chatList[index].chatRoomId }를 명시하여 아이템 재활용 시 올바른 recomposition을 유도하는 점도 훌륭합니다.
    • SwipeableBoxLaunchedEffect(Unit)를 통한 앵커 업데이트도 적절합니다.
  2. State hoisting 원칙 준수

    • ChatListScreenContentstateonIntent를 인자로 받아 내부적으로 상태를 직접 변경하지 않고 상위 컴포넌트(ViewModel)로 이벤트를 전달하는 방식으로 State Hoisting 원칙을 잘 준수하고 있습니다.
    • ChatListItemchatSummaryonClick, onAlarmClick, onDeleteClick 콜백을 받아 올바르게 구현되어 있습니다.
  3. side effect (LaunchedEffect, DisposableEffect) 사용의 적절성

    • SwipeableBox에서 LaunchedEffect(Unit)를 사용하여 초기 앵커를 설정하는 것은 적절한 side effect 사용입니다. Unit 키는 해당 Effect가 컴포넌트의 초기 생명 주기 동안 한 번만 실행되도록 보장합니다.
  4. UI 상태를 data class로 표현했는지

    • ChatListState는 UI 상태를 나타내는 data class로 잘 정의되어 있습니다.
    • ChatItemUiModel 또한 채팅 말풍선의 UI 표현을 위한 data class로 적절합니다.
  5. 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지

    • ChatListState에서 status: LoadStatus를 사용하여 로딩, 에러, 유휴 상태를 명확하게 관리하고 있습니다. ChatListScreen에서도 이 LoadStatus에 따라 LoadingColumn을 표시하는 등 적절하게 UI를 분기하고 있습니다. 훌륭합니다.

💾 Repository/DataSource 레이어 리뷰

  1. Retrofit 에러 핸들링 (try-catch, Result 래핑)

    • ChatRepositoryImpl에서 runCatching을 사용하여 Result 타입으로 래핑하는 것은 좋은 에러 핸들링 패턴입니다.
    • 하지만 ChatListViewModelchatRepository.getChatList().fold(...)에서 onFailure = { /* TODO: 에러 처리 */ } 블록이 비어있습니다. 실제 API 연동 시 네트워크 에러, 서버 에러 등에 대한 처리 로직(예: 사용자에게 에러 메시지 표시, 재시도 버튼 제공)을 추가해야 합니다. 현재 LoadStatus.Error가 있지만, 이 상태로 전환만 하고 아무것도 하지 않으면 사용자 경험이 저하될 수 있습니다.
  2. 네트워크 응답과 도메인 모델 매핑 분리 여부

    • 현재 ChatRepositoryImpl은 하드코딩된 데이터를 직접 도메인 모델(ChatSummary)로 생성하고 있습니다. TODO: 추후에 api 연동 명시된 대로, 실제 API 응답 모델(DTO)이 생기면 이 DTO를 도메인 모델로 변환하는 매핑 로직을 ChatRepositoryImpl 내부 또는 별도의 Mapper 객체에서 처리하도록 분리하는 것이 좋습니다.
  3. 캐싱 전략 (로컬 DB vs 메모리)

    • 현재는 하드코딩된 데이터를 사용하고 있어 캐싱 전략이 적용되지 않았습니다. 추후 API 연동 시 데이터의 특성(자주 변경되는지, 오프라인 지원 필요한지 등)에 따라 적절한 캐싱 전략(Room DB, DataStore, 메모리 캐시 등)을 고려해야 합니다.

🚀 ViewModel 리뷰

  1. UI 상태와 비즈니스 로직 분리

    • ChatListViewModelChatListState를 관리하고, chatRepository를 통해 데이터를 가져오는 비즈니스 로직을 위임하는 등 역할 분리가 잘 되어 있습니다.
  2. Orbit ContainerHost 패턴 준수 여부

    • ChatListViewModelContainerHost 인터페이스를 구현하고 container를 통해 initialState를 설정하는 등 Orbit 패턴을 잘 준수하고 있습니다.
  3. intent 내부에서 상태 변경(reduce)만 수행하는지

    • loadChatList에서 reduce를 통해 statuschatList를 업데이트하는 것은 올바른 상태 변경입니다.
    • emitNavigateToChatRoom에서 postSideEffect를 사용하는 것도 단일성 이벤트 전달에 적합합니다.
  4. postSideEffect 남용 여부

    • ChatListSideEffect.NavigateToChatRoom과 같이 UI에서 한 번만 처리해야 하는 이벤트(예: 내비게이션, 토스트 메시지, 다이얼로그)에 postSideEffect를 사용하는 것은 좋은 사용례입니다. 남용되고 있지 않습니다.
  5. blocking 작업을 intent 내부에서 직접 수행하지 않는지

    • chatRepository.getChatList()suspend 함수이므로 intent 내부에서 호출하더라도 코루틴 스케줄링에 의해 메인 스레드를 블로킹하지 않습니다. 문제가 없습니다.

💡 기타 제안 사항

  • AppNavigator.kt: 주석 // TODO: 모듈 간의 내비게이션 이동이 필요할 때 메서드를 작성 이 부분은 이제 navigateToChatRoom과 같이 구체적인 함수가 추가되었으므로, 해당 TODO를 구체화하거나 제거하는 것이 좋겠습니다. 또한, backStack += key처럼 직접 backStack을 조작하는 대신 NavControllernavigatepopBackStack을 사용하는 것이 더 안전하고 권장됩니다. 현재 사용하고 있는 androidx.navigation3 라이브러리의 특성을 다시 한번 확인해보시면 좋을 것 같습니다.
  • AppScreen.kt: chattingEntrynavigateToChatRoomonBack을 파라미터로 넘겨주는 방식은 좋습니다. 이렇게 함으로써 ChattingFeatureAppNavigator에 대한 직접적인 의존성 없이 독립적으로 내비게이션 이벤트를 발생시킬 수 있습니다.
  • ChatRoomScreen.kt: 현재는 빈 화면이지만, roomIdname을 인자로 받는 등 기본적인 구조가 잘 잡혀 있습니다. ViewModel의 역할도 추가될 예정이겠네요.
  • feature/chatting/impl/screen/ChatScreen.kt: 이 파일은 비어있고 사용되지 않는 것 같습니다. 불필요한 파일은 삭제하는 것이 좋습니다.
  • FrontRow.kt: isManager 인자를 받아서 매니저일 경우 이미지와 텍스트 스타일을 다르게 처리하는 방식은 좋습니다. 다만, 매니저용 ChatSummary를 직접 만들어서 FrontRow에 넘기는 것보다는, ChatSummary 자체에 매니저 여부를 나타내는 필드를 추가하고 FrontRow에서는 해당 필드를 참조하여 처리하는 것이 더 도메인 모델에 가까울 수 있습니다. (현재 isManager: Boolean 파라미터가 있지만, chatSummary.chatRoomId == -1 등으로 내부적으로 판단하게 하거나 ChatSummaryisManagerChat 같은 플래그를 두는 방식도 고려해볼 수 있습니다.)

전반적으로 이 PR은 채팅 기능의 탄탄한 기반을 잘 다지고 있습니다. 위에 언급된 사항들을 참고하여 더욱 완성도 높은 코드를 만들어 주시면 감사하겠습니다. 수고 많으셨습니다!

@codebidoof
codebidoof merged commit 1af5339 into develop Jul 18, 2026
1 of 2 checks passed
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요! 시니어 안드로이드 개발자로서 이 PR을 리뷰하겠습니다. 전반적으로 코드 품질이 매우 좋고, Jetpack Compose, Hilt, Orbit MVI 패턴을 잘 적용하고 계신 것 같습니다. 특히 도메인 레이어와 UI 레이어의 분리, LoadStatus를 사용한 상태 관리 등이 인상 깊습니다.

몇 가지 개선할 점과 질문 사항이 있으니 아래 내용을 참고해주세요.


코드 리뷰 (Code Review)

1. Kotlin 코드 리뷰

  • Coroutine 사용 시 Dispatcher 명시 여부, viewModelScope/lifecycleScope 오남용:
    • ChatListViewModel에서 chatRepository.getChatList() 호출 시 명시적인 Dispatcher 전환은 보이지 않습니다. 하지만 Repository 레이어에서 suspend 함수로 제공하므로, Repository 내부에서 적절한 Dispatcher (예: Dispatchers.IO)를 사용하여 네트워크 호출을 처리한다면 ViewModel에서는 별도로 명시하지 않아도 무방합니다. 현재 ChatRepositoryImpldelay만 사용하고 있으므로, 추후 API 연동 시 이 점을 확인해주시기 바랍니다.
  • Flow/StateFlow 사용 시 collect 시점 (repeatOnLifecycle 사용 여부):
    • Orbit MVI의 collectAsState()collectSideEffect()를 사용하고 있어 repeatOnLifecycle 등의 LifeCycle-aware 컬렉션이 Orbit 라이브러리 내부적으로 처리됩니다. 이는 올바른 접근 방식입니다.
  • null 안전성 (!! 사용 지양):
    • 전반적으로 !! 사용은 보이지 않습니다. photoUrl.ifEmpty { null }과 같이 null 안전성을 고려한 코드가 잘 작성되어 있습니다.
  • data class에 불필요한 var 사용 여부:
    • 모든 data classval을 사용하고 있어 불필요한 var 사용은 없습니다. 매우 좋습니다.
  • Hilt DI 시 생성자 주입 원칙 준수 여부:
    • ChatRepositoryImpl, ChatListViewModel 등 Hilt를 사용하는 클래스에서 @Inject constructor()를 사용하여 생성자 주입 원칙을 잘 준수하고 있습니다. RepositoryModule에서의 @Binds 설정도 올바릅니다.
  • 하드코딩된 문자열/매직 넘버:
    • ChatListScreen.kt: "채팅" 이라는 제목이 하드코딩되어 있습니다. string resource로 분리하는 것이 좋습니다.
    • LoadingColumn.kt: repeat(5)는 로딩 스켈레톤 아이템의 수를 의미하는 매직 넘버입니다. 상수로 분리하거나, 디자인 가이드에 따라 적절한 값으로 설정하도록 internal const val 등으로 관리하는 것이 좋습니다.
    • SwipeableBox.kt: swipeWidth: Dp = 204.dp 또한 매직 넘버입니다. 디자인 시스템 내의 상수로 관리하거나, Dimen 리소스로 분리하는 것이 좋습니다.
    • UiModelMapper.kt: timeFormatter = DateTimeFormatter.ofPattern("HH:mm")에서 "HH:mm" 패턴이 하드코딩되어 있습니다. 날짜/시간 포맷은 지역화나 변경의 가능성이 있으므로 상수로 분리하는 것을 권장합니다.
    • ChatSummary.kt: unReadMessageCountString 타입으로 되어있고, FrontRow.kt에서 toIntOrNull()을 사용하여 숫자로 변환합니다. 만약 이 값이 항상 숫자로만 표현된다면 Int 타입으로 변경하여 불필요한 변환 로직을 제거하고 타입 안전성을 높이는 것을 제안합니다. (예: unReadMessageCount: Int = 0). 만약 "99+"와 같은 비수치적 표현이 포함될 수 있다면 현재 String 타입이 적절합니다. 의도를 명확히 해주세요.

2. Jetpack Compose 코드 확인

  • Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용):
    • ChatListItem에서 AnchoredDraggableStateremember로 감싸 불필요한 재구성을 방지하고 있습니다. 좋습니다.
    • ChatListScreenLazyColumn에서 key = { index -> state.chatList[index].chatRoomId }를 사용하여 아이템의 고유성을 보장하고 불필요한 재구성을 줄였습니다. 매우 좋은 접근입니다.
    • noRippleClickable Modifier에서 remember { MutableInteractionSource() }를 사용하여 Modifier 재구성을 효율적으로 처리하고 있습니다.
  • State hoisting 원칙 준수:
    • ChatListItem, BackgroundRow, FrontRow 등 여러 컴포저블에서 onClick, onAlarmClick, onDeleteClick과 같은 이벤트를 람다로 받아 상위 컴포저블로 호이스팅하고 있습니다. ChatListScreen에서 onChatRoomClick을 ViewModel의 handleIntent로 전달하는 방식도 State Hoisting 원칙에 잘 부합합니다.
  • side effect (LaunchedEffect, DisposableEffect) 사용의 적절성:
    • SwipeableBox에서 LaunchedEffect(Unit)를 사용하여 초기 한 번만 DraggableAnchors를 업데이트하고 있습니다. 이는 적절한 side effect 사용입니다.
  • UI 상태를 data class로 표현했는지:
    • ChatListState (data class)를 사용하여 UI 상태를 잘 표현하고 있습니다.
    • ChatItemUiModel (data class)도 채팅 말풍선 UI 상태를 상세하게 표현하고 있습니다.
  • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지:
    • ChatListState에서 LoadStatus (LoadStatus.Idle, LoadStatus.Loading, LoadStatus.Error)를 사용하여 UI의 로딩/에러 상태를 명확하게 관리하고 있습니다. 이 접근 방식은 매우 좋습니다. ChatListScreen에서도 when(state.status)로 분기하여 각 상태에 맞는 UI (로딩 스켈레톤)를 보여주고 있습니다.

3. Repository/DataSource 레이어 확인

  • Retrofit 에러 핸들링 (try-catch, Result 래핑):
    • ChatRepositoryImpl.kt에서 runCatching { ... }를 사용하여 Result<T> 타입으로 결과를 래핑하고 있습니다. 이는 에러를 도메인 레이어까지 전파하는 좋은 방법입니다. 추후 Retrofit 연동 시에도 이 패턴을 유지하는 것이 중요합니다.
  • 네트워크 응답과 도메인 모델 매핑 분리 여부:
    • 현재는 delay를 통해 더미 데이터를 직접 ChatSummary 도메인 모델 리스트로 반환하고 있어 매핑 로직이 분리될 필요가 없습니다. 하지만, 추후 실제 네트워크 API 연동 시 ApiResponse와 같은 네트워크 응답 모델을 별도로 정의하고 이를 도메인 모델 ChatSummary로 변환하는 매핑 로직을 분리하는 것을 권장합니다 (예: data 모듈 내 mapper 패키지). 현재 UiModelMapper가 UI 모델 매핑을 처리하듯이, DataModelMapper 같은 역할을 추가할 수 있습니다.
  • 캐싱 전략 (로컬 DB vs 메모리):
    • 현재 PR에서는 캐싱 전략이 명시적으로 구현되어 있지 않습니다. 요구사항에 따라 캐싱이 필요하다면, ChatRepository 내부에서 LocalDataSourceRemoteDataSource를 조합하여 캐싱 전략을 구현해야 합니다.

4. ViewModel 확인

  • UI 상태와 비즈니스 로직 분리:
    • ChatListViewModelChatListState를 관리하고 ChatRepository를 통해 데이터를 가져오는 역할을 수행하며, ChatListIntent를 통해 UI 이벤트를 처리합니다. 비즈니스 로직과 UI 상태 관리가 잘 분리되어 있습니다.
  • Orbit ContainerHost 패턴 준수 여부:
    • ChatListViewModelContainerHost<ChatListState, ChatListSideEffect>를 구현하고 container()를 사용하여 Orbit MVI 패턴을 잘 준수하고 있습니다.
  • intent 내부에서 상태 변경(reduce)만 수행하는지:
    • loadChatList() 함수 내 intent 블록에서 reduce { ... }를 통해 상태 변경만 수행하고, 네트워크 호출과 같은 부수 효과는 reduce 블록 외부에서 chatRepository.getChatList().fold { ... } 형태로 처리합니다. 이는 Orbit MVI 패턴에 부합합니다.
  • postSideEffect 남용 여부:
    • emitNavigateToChatRoom() 함수에서 postSideEffect를 사용하여 NavigateToChatRoom과 같은 일회성 이벤트를 잘 처리하고 있습니다. 남용된 부분은 보이지 않습니다.
  • blocking 작업을 intent 내부에서 직접 수행하지 않는지:
    • chatRepository.getChatList()suspend 함수이므로 intent 내부에서 직접 blocking 작업을 수행하지 않습니다. 이는 코루틴 및 Orbit 사용 규칙에 맞습니다.
    • ChatListViewModelloadChatList()에서 chatRepository.getChatList().foldonFailure 블록이 현재 비어있습니다. 실제 에러 발생 시 사용자에게 에러 메시지를 보여주거나, 에러 상태로 UI를 업데이트하는 reduce 호출 등을 추가하는 것이 좋습니다.

5. FCM/SSE/실시간 통신 관련 코드 확인

  • 현재 PR에는 FCM/SSE/실시간 통신 관련 코드가 포함되어 있지 않습니다.

전반적인 의견 및 개선 제안

  1. 파일 끝 개행 문자 (Newline at end of file): .kt 파일들 및 .xml 파일 끝에 개행 문자가 없는 경우가 많습니다. 이는 Git diff나 일부 에디터에서 문제를 일으킬 수 있으므로, 모든 파일 끝에 개행 문자를 추가하는 것을 권장합니다.
  2. UiModelMapper의 시간 로직:
    • LocalDateTime.now()UiModelMapper.createSendingMessage 내부에서 직접 사용하는 것은 테스트 용이성을 저해할 수 있습니다. LocalDateTime.now() 대신 Clock 인터페이스 등을 주입받아 사용하거나, 시간을 인자로 넘겨주는 방식으로 개선하면 테스트하기 더 쉬워집니다.
  3. 하드코딩된 Manager Chat Data: ChatListScreen에서 매니저 채팅 아이템의 데이터 (name, content)가 하드코딩되어 있습니다. 이 데이터가 변경될 가능성이 있다면, ChatListViewModel에서 ChatSummary 형태로 제공하도록 하는 것이 좋습니다.
  4. ChatRoomScreen ViewModel: ChatRoomScreen의 ViewModel이 주석 처리되어 있습니다. 추후 구현 시 ChatListViewModel과 마찬가지로 Hilt와 Orbit 패턴을 잘 적용해주시기 바랍니다.

전반적으로 매우 좋은 PR이며, 제시된 개선점들은 코드의 견고함과 유지보수성을 더욱 높이는 데 도움이 될 것입니다. 수고하셨습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] 채팅 목록 화면 및 채팅방 컴포넌트 구현

2 participants