Skip to content

feat: 마이페이지 - 캐릭터 관리 UI 구현(#13) #31

Open
WAcAW9 wants to merge 13 commits into
developfrom
feature/13-mypage-character-ui
Open

feat: 마이페이지 - 캐릭터 관리 UI 구현(#13) #31
WAcAW9 wants to merge 13 commits into
developfrom
feature/13-mypage-character-ui

Conversation

@WAcAW9

@WAcAW9 WAcAW9 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

📄 작업 내용 요약

  • 마이페이지 > 캐릭터 관리(CharacterManagementScreen) 화면 UI구현
  • 마이페이지 > CharacterManagementScreen 네비게이션 연결
  • 공통 컴포넌트 파라미터 추가 및 조정
    • 버튼 하나 팝업 컴포넌트(OneButtonPopup) title 파라미터 nullable 타입으로 변경
    • 버튼 두개 팝업 컴포넌트(SecondaryButton) iconRes 파라미터 추가

📎 Issue 번호


✅ 작업 목록

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

📝 기타 참고사항

image

Summary by CodeRabbit

  • 새로운 기능

    • 마이페이지에서 캐릭터 관리 화면으로 이동할 수 있으며, 생성 가능한 캐릭터는 최대 5개임을 안내합니다.
    • 캐릭터별 대화 기록 요약을 확인하고, 캐릭터 영구 삭제(삭제 확인 포함)를 진행할 수 있습니다.
    • 메인 삭제 및 캐릭터 추가는 제한 상황에서 안내 팝업이 표시됩니다.
  • UI 개선

    • 보조 버튼에 좌측 아이콘(선택)을 지원하고, 팝업 제목은 필요할 때만 표시됩니다.
    • 마이페이지 편집/삭제 아이콘과 캐릭터 카드 UI가 보강되었습니다.

@WAcAW9 WAcAW9 linked an issue Jul 21, 2026 that may be closed by this pull request
2 tasks
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

마이페이지에 캐릭터 관리 화면을 추가하고, 캐릭터 조회·삭제·추가 제한을 위한 도메인, 저장소, ViewModel, Compose UI 및 내비게이션 연결을 구현했습니다. 공통 버튼과 팝업 컴포넌트도 확장했습니다.

Changes

캐릭터 관리

Layer / File(s) Summary
캐릭터 도메인 및 저장소
core/domain/..., core/data/...
AiCharacter 모델과 저장소 계약을 추가하고, 조회·삭제·추가 가능 여부 구현체를 DI에 등록했습니다.
캐릭터 카드 및 공통 UI
core/designsystem/..., feature/mypage/impl/.../CharacterCard.kt
캐릭터 카드와 편집·삭제 아이콘을 추가하고, 버튼 아이콘 및 선택적 팝업 제목을 지원합니다.
캐릭터 관리 상태 및 이벤트 처리
feature/mypage/impl/.../viewmodel/*
Orbit ViewModel이 목록 로딩, 채팅 기록, 삭제 확인·삭제, 캐릭터 추가 제한 이벤트를 처리합니다.
관리 화면 및 마이페이지 연결
feature/mypage/api/..., feature/mypage/impl/..., app/.../AppScreen.kt
캐릭터 관리 화면과 팝업·프리뷰를 추가하고, 마이페이지 이벤트를 CharacterManagementNavKey 내비게이션에 연결했습니다.

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

Possibly related PRs

Suggested labels: 🧩 Component, ✨ Feature, 🎨 UI/Style

Suggested reviewers: codebidoof

Sequence Diagram(s)

sequenceDiagram
  participant MyPageScreen
  participant AppScreen
  participant CharacterManagementScreen
  participant CharacterManagementViewModel
  participant AICharacterRepository

  MyPageScreen->>AppScreen: 캐릭터 관리 이동 이벤트
  AppScreen->>CharacterManagementScreen: CharacterManagementNavKey
  CharacterManagementScreen->>CharacterManagementViewModel: 화면 상태 수집
  CharacterManagementViewModel->>AICharacterRepository: getCharacters()
  AICharacterRepository-->>CharacterManagementViewModel: 캐릭터 목록
  CharacterManagementViewModel-->>CharacterManagementScreen: 목록 및 사이드이펙트
Loading
🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
모듈 의존성 방향 검증 ⚠️ Warning 변경분은 깨끗하지만 core/domain의 ChatRepository.kt가 androidx.paging.PagingData를 import해 순수 Kotlin 규칙을 위반합니다. ChatRepository에서 PagingData 의존을 제거하고 domain에는 순수 타입만 두세요. 페이징은 data/presentation 계층으로 분리해야 합니다.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Conventional Commits 형식을 따르며 캐릭터 관리 UI 구현과 이슈 번호를 잘 반영합니다.
Linked Issues check ✅ Passed 캐릭터 목록 조회와 화면 구현을 완료해 #13의 직접 요구사항을 충족합니다.
Out of Scope Changes check ✅ Passed 추가된 저장소, 모델, 네비게이션, 아이콘 및 공통 컴포넌트 수정은 캐릭터 관리 UI 구현 범위 안입니다.
Ui 변경 시 스크린샷 첨부 확인 ✅ Passed 변경 파일에 CharacterManagementScreen.kt가 포함되고, PR 설명에 user-attachments 이미지가 첨부돼 스크린샷 확인됨.
✨ 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/13-mypage-character-ui

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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, 시니어 Android 개발자입니다. PR 리뷰 요청해주신 내용 잘 확인했습니다. 새로운 AI 캐릭터 관리 기능을 추가하는 PR로 보이네요. 전반적으로 코드 구조가 깔끔하고 MVI 패턴(Orbit)과 Compose 원칙을 잘 따르고 있습니다. 몇 가지 개선 사항과 미래를 위한 제안을 드립니다.


PR Diff 리뷰

[1. Kotlin 코드 리뷰]

  • Coroutine 사용 시 Dispatcher 명시 여부, viewModelScope/lifecycleScope 오남용:
    • CharacterManagementViewModelloadCharacters()deleteCharacter()는 Orbit의 intent 블록 내에서 suspend 함수인 aiCharacterRepository 메서드를 호출합니다. 현재 AICharacterRepositoryImpl이 임시(in-memory) 구현체이므로 Dispatcher 명시가 필수적이지는 않습니다.
    • 개선 제안 (미래): AICharacterRepositoryImpl이 실제 네트워크 또는 데이터베이스 연동 시에는 withContext(Dispatchers.IO) 등을 사용하여 적절한 Dispatcher를 명시하는 것이 좋습니다. 현재 TODO: API 연동 전 임시 구현체 주석이 있으니, 실제 구현 시 이 부분을 고려해주세요.
  • Flow/StateFlow 사용 시 collect 시점 (repeatOnLifecycle 사용 여부):
    • Orbit의 collectAsStatecollectSideEffect가 Compose lifecycle을 고려하여 내부적으로 처리하고 있으므로 이 부분은 적절합니다.
  • null 안전성 (!! 사용 지양):
    • 코드 전반에 !! 연산자 사용이 보이지 않습니다. null 안전성을 잘 고려하고 있습니다.
    • designsystemOneButtonPopupPopupCard에서 title: String? = null로 변경하여 nullable을 명시적으로 처리한 점은 유연성을 높여 아주 좋습니다. SecondaryButtoniconRes: Int? = null 또한 마찬가지입니다.
  • data class에 불필요한 var 사용 여부:
    • AiCharacterCharacterManagementState 모두 val로만 구성된 data class입니다. 불변성을 유지하고 있어 좋습니다.
  • Hilt DI 시 생성자 주입 원칙 준수 여부:
    • AICharacterRepositoryImplCharacterManagementViewModel 모두 @Inject constructor를 통해 생성자 주입 원칙을 잘 지키고 있습니다.
    • RepositoryModule에서 @Binds를 사용하여 인터페이스-구현체 바인딩을 잘 처리했습니다.
  • 하드코딩된 문자열/매직 넘버:
    • 문제점: CharacterManagementScreenCharacterCard 내의 대부분의 사용자에게 보여지는 텍스트(예: "캐릭터 관리", "생성 가능한 캐릭터 수는 최대 5개입니다.", "채팅 기록 보기", 팝업 메시지 등)가 하드코딩되어 있습니다. AICharacterRepositoryImpl의 임시 데이터도 그렇습니다.
    • 개선 제안:
      • 사용자에게 표시되는 모든 문자열은 strings.xml 리소스 파일로 이동시켜야 합니다. 이는 다국어 지원 및 유지보수성을 위해 필수적입니다.
      • designsystem 컴포넌트(SecondaryButton, PopupCard 등) 및 CharacterCard에서 사용된 dp 값들은 대부분 디자인 시스템의 일관성을 위해 사용된 것으로 보이나, 특정 디자인 토큰을 나타내는 경우 CallTheme.dimens.xxx와 같이 테마에서 관리하는 것을 고려할 수 있습니다. (현재 수준에서는 허용 가능)
    • AICharacterRepositoryImpl의 임시 데이터: 현재는 임시 구현체이므로 하드코딩된 데이터가 허용되지만, TODO 주석과 함께 향후 실제 API 연동 시 제거되어야 함을 명확히 하는 것이 좋습니다.

[2. Jetpack Compose 코드]

  • Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용):
    • CharacterManagementScreen에서 팝업 상태를 remember { mutableStateOf(...) }로 관리하고 있으며, 이는 적절합니다.
    • CharacterCard와 같은 컴포저블은 상태를 호이스팅하여 AiCharacteronClick 람다를 전달받는 형태로 잘 구현되어 있습니다. 불필요한 리컴포지션 유발 요인은 보이지 않습니다.
  • State hoisting 원칙 준수:
    • CharacterManagementScreenviewModel로부터 UI 상태를 받고, 사용자 이벤트를 람다 (onIntent, onBackClick, navigateToAddCharacter)로 상위로 전달하는 등 State hoisting 원칙을 잘 준수하고 있습니다.
    • 팝업 컴포넌트들도 onButtonClick, onDismissRequest 등의 람다를 통해 상태를 외부에서 제어하도록 잘 설계되었습니다.
  • side effect (LaunchedEffect, DisposableEffect) 사용의 적절성:
    • CharacterManagementScreen에서 Orbit의 collectSideEffect를 사용하여 UI 관련 일회성 이벤트를 처리하고 있습니다. MVI 패턴에서 side effect를 적절히 활용한 좋은 예시입니다.
  • UI 상태를 data class로 표현했는지:
    • CharacterManagementStatedata class로 정의되어 UI 상태를 명확하게 표현하고 있습니다. 좋습니다.
  • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지:
    • CharacterManagementState에서 LoadStatus를 사용하여 로딩/에러 상태를 관리하고 있습니다. CharacterManagementViewModel에서도 LoadStatus를 적절히 업데이트하고 있습니다. 좋습니다.

[3. Repository/DataSource 레이어]

  • Retrofit 에러 핸들링 (try-catch, Result 래핑):
    • AICharacterRepositoryImpl은 임시 구현체이므로 Retrofit 에러 핸들링은 현재 없습니다.
    • CharacterManagementViewModel에서는 runCatching 블록을 사용하여 aiCharacterRepository.getCharacters() 호출에 대한 성공/실패를 처리하고 LoadStatus.Error를 업데이트하고 있습니다. 이는 좋은 접근 방식입니다.
    • 개선 제안: deleteCharacter 메서드의 경우, 현재 aiCharacterRepository.deleteCharacter(characterId) 호출에 대한 에러 핸들링이 없습니다. 만약 삭제 작업 중 네트워크 오류나 서버 측 문제 발생 시 이를 처리하여 사용자에게 피드백을 주거나, LoadStatus를 업데이트하는 로직이 필요합니다. runCatching으로 감싸서 에러를 처리하는 것을 권장합니다.
  • 네트워크 응답과 도메인 모델 매핑 분리 여부:
    • 현재 AICharacterRepositoryImpl은 직접 AiCharacter 도메인 모델을 반환합니다. 실제 API 연동 시에는 네트워크 응답을 나타내는 DTO(Data Transfer Object)를 정의하고, Repository 레이어에서 이 DTO를 AiCharacter 도메인 모델로 매핑하는 것이 원칙입니다.
    • 개선 제안 (미래): TODO 주석과 함께 이 부분을 고려해야 함을 명확히 하는 것이 좋습니다.
  • 캐싱 전략 (로컬 DB vs 메모리):
    • AICharacterRepositoryImpl에서 mutableListOf를 사용하여 인메모리 캐싱/저장 역할을 하고 있습니다. 임시 구현체로서는 적절합니다.
    • 개선 제안 (미래): 실제 서비스에서는 로컬 데이터베이스 또는 적절한 네트워크-바운드 리소스 패턴을 고려해야 합니다.

[4. ViewModel]

  • UI 상태와 비즈니스 로직 분리:
    • CharacterManagementViewModelCharacterManagementState를 관리하고, CharacterManagementSideEffect를 통해 UI 관련 이벤트를 발생시킵니다.
    • 캐릭터 삭제 가능 여부 (isMain 확인), 캐릭터 추가 가능 여부 (canAddCharacter() 호출) 등 비즈니스 로직이 ViewModel 내에 잘 구현되어 있습니다. Repository 계층에 위임할 비즈니스 로직(예: 실제 캐릭터 삭제)은 위임하고 있습니다. 좋습니다.
  • Orbit ContainerHost 패턴 준수 여부:
    • CharacterManagementViewModelContainerHost를 구현하고 container 블록 내에서 loadCharacters()를 호출하여 초기 상태를 설정하는 등 Orbit MVI 패턴을 잘 준수하고 있습니다.
  • intent 내부에서 상태 변경(reduce)만 수행하는지:
    • intent 블록 내에서 reduce를 통해 상태를 변경하고, postSideEffect를 통해 일회성 이벤트를 발생시키는 패턴을 잘 따르고 있습니다.
  • postSideEffect 남용 여부:
    • 팝업 표시 (ShowChatHistorySummary, ShowDeleteConfirmDialog, ShowMainCharacterDeleteBlocked, ShowAddCharacterBlocked) 및 화면 이동 (NavigateToAddCharacter)과 같이 UI에서 한 번만 처리되어야 하는 이벤트를 postSideEffect로 처리하고 있습니다. 이는 적절한 사용입니다. 남용으로 보이지 않습니다.
  • blocking 작업을 intent 내부에서 직접 수행하지 않는지:
    • aiCharacterRepository.getCharacters()deleteCharacter()suspend 함수이므로 blocking 작업이 아닙니다. ViewModel에서 이들을 호출하는 것은 문제가 없습니다.

[5. FCM/SSE/실시간 통신 관련 코드]

  • 해당 PR에는 FCM/SSE/실시간 통신 관련 코드가 포함되어 있지 않아 검토 대상이 아닙니다.

최종 결론 및 권장 사항

이 PR은 전반적으로 높은 품질의 코드를 보여주고 있으며, 모범 사례를 잘 따르고 있습니다. 특히 MVI 패턴과 Compose 원칙을 잘 적용했습니다.

반드시 수정/개선 권장 사항:

  1. 하드코딩된 문자열 리소스화: CharacterManagementScreen 및 관련 팝업, CharacterCard에 사용된 모든 사용자 대상 문자열(예: "캐릭터 관리", "생성 가능한 캐릭터 수는 최대 5개입니다.", "채팅 기록 보기", 팝업의 제목/설명/버튼 텍스트 등)을 strings.xml로 이동시켜주세요. CommonTopAppBartitle도 마찬가지입니다.
  2. deleteCharacter 에러 핸들링 추가: CharacterManagementViewModeldeleteCharacter 함수 내에서 aiCharacterRepository.deleteCharacter(characterId) 호출을 runCatching으로 감싸 에러 발생 시 LoadStatus.Error를 업데이트하거나 적절한 SideEffect를 발생시키도록 처리해주세요.

향후 개선을 위한 제안 (TODO 주석 업데이트 또는 추가):

  • API 연동 시 Dispatcher 명시: AICharacterRepositoryImpl에 실제 API 호출 로직이 추가될 때, withContext(Dispatchers.IO)를 사용하여 I/O 작업을 백그라운드 스레드에서 처리하도록 명시해주세요.
  • 네트워크 DTO와 도메인 모델 분리: 실제 API 연동 시, 네트워크 응답 DTO를 정의하고 AICharacterRepositoryImpl에서 이를 AiCharacter 도메인 모델로 매핑하는 로직을 추가해주세요.
  • 캐릭터 추가 로직 보완: aiCharacterRepository.canAddCharacter()의 임시 return false 로직을 실제 백엔드 연동 및 최대 캐릭터 수(5개) 제한을 고려하여 구현해주세요. 클라이언트에서 먼저 최대 개수를 체크하는 로직도 ViewModel/Repository 혹은 UseCase에 포함될 수 있습니다.

이러한 사항들을 반영하면 더욱 견고하고 유지보수가 용이한 코드가 될 것입니다. 수고 많으셨습니다!

@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: 3

🧹 Nitpick comments (1)
core/data/src/main/java/kr/co/call/data/repositoryImpl/AICharacterRepositoryImpl.kt (1)

7-8: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

캐릭터 추가 가능 여부를 false로 고정한 상태를 확인해 주세요.

CharacterManagementViewModel.checkAddCharacter()가 이 값을 그대로 사용하므로 현재 NavigateToAddCharacter는 절대 발생하지 않습니다. UI 차단 팝업만 이번 PR 범위라면 명시해 주고, 24시간 조건부 추가가 요구사항이라면 실제 판정 로직을 구현해야 합니다. 필요하면 조건 판정과 테스트 추가안을 함께 작성할 수 있습니다.

Also applies to: 34-36

🤖 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/AICharacterRepositoryImpl.kt`
around lines 7 - 8, AICharacterRepositoryImpl의 캐릭터 추가 가능 여부가 항상 false가 되지 않도록
요구사항을 확인해 수정하세요. 24시간 조건부 추가가 요구사항이면 해당 판정 로직을 구현하고,
CharacterManagementViewModel.checkAddCharacter()가 이를 사용해 조건 충족 시
NavigateToAddCharacter를 발생시키도록 하세요. 이번 PR 범위가 UI 차단뿐이라면 false 고정을 유지하되 해당 의도를
명시하고 관련 동작을 검증하세요.
🤖 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/di/RepositoryModule.kt`:
- Around line 36-39: Update the bindCharacterRepository binding in
RepositoryModule to apply `@Singleton` so the AICharacterRepositoryImpl instance
and its in-memory mutableList state are shared across injection points.

In
`@feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt`:
- Around line 68-80: CharacterCard의 편집 아이콘 영역에서 동작하지 않는 편집 버튼 표시를 제거하거나 숨기고, 편집
기능을 지원하는 경우에만 onEditClick 콜백과 클릭 처리를 연결하세요. 현재 PR 범위에서 수정 기능이 미완료라면 Icon 및 관련
Box를 노출하지 않도록 변경하세요.

In
`@feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementViewModel.kt`:
- Around line 24-29: Update the ConfirmDeleteCharacter branch in handleIntent
and the deleteCharacter flow to re-fetch the character by ID from the current
State immediately before deletion; if it isMain, block the operation, emit the
existing protection side effect, and return, while preserving normal deletion
for non-main characters.

---

Nitpick comments:
In
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/AICharacterRepositoryImpl.kt`:
- Around line 7-8: AICharacterRepositoryImpl의 캐릭터 추가 가능 여부가 항상 false가 되지 않도록
요구사항을 확인해 수정하세요. 24시간 조건부 추가가 요구사항이면 해당 판정 로직을 구현하고,
CharacterManagementViewModel.checkAddCharacter()가 이를 사용해 조건 충족 시
NavigateToAddCharacter를 발생시키도록 하세요. 이번 PR 범위가 UI 차단뿐이라면 false 고정을 유지하되 해당 의도를
명시하고 관련 동작을 검증하세요.
🪄 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: 45824013-d9a0-4305-9491-f6e23df0ad1a

📥 Commits

Reviewing files that changed from the base of the PR and between 8f30c10 and 5041597.

📒 Files selected for processing (19)
  • 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/AICharacterRepositoryImpl.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/component/button/SecondaryButton.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/component/popup/OneButtonPopup.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/component/popup/PopupCard.kt
  • core/designsystem/src/main/res/drawable/ic_mypage_edit.xml
  • core/designsystem/src/main/res/drawable/ic_mypage_trash.xml
  • core/domain/src/main/java/kr/co/call/domain/model/mypage/AiCharacter.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/AICharacterRepository.kt
  • feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/CharacterManagementScreen.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementIntent.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementSideEffect.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementState.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementViewModel.kt

Comment thread core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
Comment on lines +68 to +80
Box(
modifier = Modifier
.align(Alignment.BottomEnd)
.size(23.dp)
.clip(CircleShape)
.background(CallTheme.colors.white),
contentAlignment = Alignment.Center,
) {
Icon(
painter = painterResource(id = R.drawable.ic_mypage_edit),
contentDescription = "편집하기 버튼",
tint = Color.Unspecified
)

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

동작하지 않는 편집 버튼 표시는 제거하거나 연결하세요.

이 요소는 “편집하기 버튼”으로 안내되지만 클릭 처리와 콜백이 없어 사용자 입력에 반응하지 않습니다. 수정 기능이 준비될 때까지 숨기거나 onEditClick을 연결하세요. 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 `@feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt`
around lines 68 - 80, CharacterCard의 편집 아이콘 영역에서 동작하지 않는 편집 버튼 표시를 제거하거나 숨기고, 편집
기능을 지원하는 경우에만 onEditClick 콜백과 클릭 처리를 연결하세요. 현재 PR 범위에서 수정 기능이 미완료라면 Icon 및 관련
Box를 노출하지 않도록 변경하세요.

@WAcAW9 WAcAW9 self-assigned this Jul 22, 2026
@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.

Actionable comments posted: 2

🤖 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/AICharacterRepositoryImpl.kt`:
- Around line 40-42: Update AICharacterRepositoryImpl.canAddCharacter() to
determine eligibility using the actual character-creation timestamp data or
backend API and return that result instead of always returning false; if
integration is unavailable, remove this placeholder implementation from the
production Hilt binding until a real eligibility check is provided.
- Around line 34-38: Update deleteCharacter in AICharacterRepositoryImpl so the
runCatching block returns Unit rather than removeAll’s Boolean result. Preserve
the removal predicate by character ID, then explicitly complete with Unit to
satisfy the declared Result<Unit> contract.
🪄 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: 28299545-bbc6-4fb3-ae3d-0ed5d81aa2e4

📥 Commits

Reviewing files that changed from the base of the PR and between 5041597 and 0be3423.

📒 Files selected for processing (6)
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/AICharacterRepositoryImpl.kt
  • core/domain/src/main/java/kr/co/call/domain/model/mypage/AiCharacter.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/AICharacterRepository.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (4)
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/domain/src/main/java/kr/co/call/domain/model/mypage/AiCharacter.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementViewModel.kt

Comment on lines +40 to +42
override suspend fun canAddCharacter(): Result<Boolean> {
// TODO: 마지막 캐릭터 생성 시각 기준 24시간 경과 여부를 판단(백엔드)
return runCatching { false }

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

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 5 \
  'canAddCharacter|NavigateToAddCharacter|ShowAddCharacterBlocked' \
  core/data feature

Repository: call-from-ai/CallFromAi_Android

Length of output: 6747


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== repository interface/call sites =="
rg -n -C 3 'fun canAddCharacter\(|suspend fun canAddCharacter|canAddCharacter' --glob '*.kt' .

echo "== repository provider/binding snippets =="
rg -n -C 4 'AICharacterRepository|AIC|RepositoryImpl|Hilt|Provides|Inject' --glob '*.kt' . | sed -n '1,220p'

echo "== module/build dependency hints =="
fd 'build.gradle|build.gradle.kts|settings.gradle|settings.gradle.kts' . -t f -x sh -c 'echo "--- $1"; cat "$1"' sh {}

Repository: call-from-ai/CallFromAi_Android

Length of output: 44761


캐릭터 추가 eligibility 판정한 뒤 반환하도록 수정하세요.

AICharacterRepositoryImpl.canAddCharacter()가 항상 false를 반환하고, 이 구현체가 Hilt Binding을 통해 주입되어 CharacterManagementViewModel.checkAddCharacter()에서 실제 판단 로직 없이 항상 ShowAddCharacterBlocked만 발생시킵니다. 24시간 제한을 실제 데이터/API로 판정하거나, API 연동 전 임시 구현체를 프로덕션 바인딩에서 제외해야 합니다.

🤖 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/AICharacterRepositoryImpl.kt`
around lines 40 - 42, Update AICharacterRepositoryImpl.canAddCharacter() to
determine eligibility using the actual character-creation timestamp data or
backend API and return that result instead of always returning false; if
integration is unavailable, remove this placeholder implementation from the
production Hilt binding until a real eligibility check is provided.

- AICharacterRepositoryImpl의 deleteCharacter Unit 반환
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, 시니어 Android 개발자입니다. PR 리뷰 요청해주셔서 감사합니다.

전반적으로 깔끔하고 신중하게 작성된 코드입니다. 특히 MVI 패턴(Orbit)과 Compose를 효과적으로 활용하여 UI 상태 관리와 비즈니스 로직 분리를 잘 해내셨고, Navigation 및 DI 구조도 모범적으로 구현되었습니다. 신규 기능 추가 시 필요한 모든 레이어(Domain, Data, Feature)를 잘 분배하여 작업하신 점이 인상 깊습니다.

몇 가지 개선할 점과 질문이 있어 코멘트 남깁니다.


General (전반적인 코멘트)

  • 아키텍처 및 패턴 준수: Orbit MVI 패턴과 Clean Architecture 원칙(Domain, Data, Feature 분리)을 잘 준수하고 있습니다. 특히 ViewModel에서 intent 블록 내 reducepostSideEffect의 명확한 분리 사용이 좋습니다.
  • Jetpack Compose: Composable 함수의 재사용성, State Hoisting, UI 상태 관리 (LoadStatus 사용) 등 Compose 모범 사례를 잘 따르고 있습니다. 특히 다양한 팝업 상태에 대한 Preview를 제공한 점이 매우 훌륭합니다.
  • Hilt DI: 생성자 주입 원칙을 잘 지키고 있으며, RepositoryModule에 새로운 Repository 바인딩을 추가하는 방식도 올바릅니다.

Specific Comments & Suggestions

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

--- a/core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
+++ b/core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
@@ -27,6 +29,7 @@ abstract class RepositoryModule {
     ): HomeRepository
     @Binds
+    @Singleton
     abstract fun bindMyPageRepository(
         impl: MyPageRepositoryImpl
     ): MyPageRepository
  • @Singleton 누락: bindMyPageRepository@Singleton 어노테이션이 누락되었습니다. 다른 Repository 바인딩과 일관성을 위해 추가해주시는 것이 좋습니다. AICharacterRepository에는 추가되어 있어 일관성이 필요합니다.
  • 파일 마지막 줄 개행: No newline at end of file 오류가 데이터 모듈의 다른 파일들에서도 보입니다. .editorconfig 설정을 확인하여 파일 끝에 항상 한 줄 개행이 추가되도록 하는 것이 좋습니다.

2. core/data/src/main/java/kr/co/call/data/repositoryImpl/AICharacterRepositoryImpl.kt

class AICharacterRepositoryImpl @Inject constructor() : AICharacterRepository {

    private val aiCharacters = mutableListOf(
        // ...
    )

    override suspend fun getCharacters(): Result<List<AiCharacter>>{
        return runCatching {
            aiCharacters.toList()
        }
    }
    // ...
}
  • TODO 주석: "TODO: API 연동 전 임시 구현체" 주석이 잘 달려있습니다. 실제 API 연동 시 이 부분을 제거하고 네트워크 로직으로 대체하는 것을 잊지 말아 주세요.
  • Result 래핑: runCatching을 사용하여 Result 타입으로 래핑한 것은 좋은 패턴입니다. 실제 네트워크 호출 시에도 이 방식을 유지하는 것이 Retrofit 에러 핸들링 규칙에 부합합니다.
  • 파일 마지막 줄 개행: 이 파일에서도 No newline at end of file이 발생합니다.

3. core/designsystem/src/main/java/kr/co/call/designsystem/component/button/SecondaryButton.kt

// ...
import androidx.compose.ui.res.painterResource
// ...
fun SecondaryButton(
    // ...
    iconRes: Int? = null,
    // ...
) {
    // ...
        if (iconRes != null) {
            Row(verticalAlignment = Alignment.CenterVertically) {
                Icon(
                    painter = painterResource(id = iconRes),
                    contentDescription = null, // TODO: contentDescription 명확화
                    tint = contentTint,
                    modifier = Modifier.size(16.dp),
                )
                Spacer(modifier = Modifier.width(6.dp))
                Text(text = text, style = CallTheme.typography.bodyMediumMedium, color = contentTint)
            }
        } else {
            Text(text = text, style = CallTheme.typography.bodyMediumMedium, color = contentTint)
        }
    // ...
}
  • contentDescription: 아이콘을 사용하는 경우 접근성을 위해 contentDescriptionnull 대신 적절한 문자열로 제공하는 것이 좋습니다. iconRes와 함께 iconContentDescription 파라미터를 추가하여 전달받거나, text를 사용하여 동적으로 설정하는 방법을 고려해볼 수 있습니다.
    • 제안:
      fun SecondaryButton(
          // ...
          iconRes: Int? = null,
          iconContentDescription: String? = null, // 추가
          // ...
      ) {
          // ...
              Icon(
                  painter = painterResource(id = iconRes),
                  contentDescription = iconContentDescription ?: text, // or provide a default for icon-only buttons
                  tint = contentTint,
                  modifier = Modifier.size(16.dp),
              )
          // ...
      }

4. core/domain/src/main/java/kr/co/call/domain/model/mypage/AiCharacter.kt

data class AiCharacter(
    val id: String= "",
    val name: String= "",
    val profileImageUrl: String= "",
    val isMain: Boolean,
    val createdAtLabel: String= "",
    val daysTogetherLabel: String= "",
    val lastConversationLabel: String= "",
    val summary: String= "",
)
  • Default Value: profileImageUrl의 기본값이 ""으로 되어 있습니다. 만약 프로필 이미지가 없을 수도 있다면, null이 더 의미론적일 수 있습니다. (예: val profileImageUrl: String? = null) 이렇게 하면 데이터 계층에서 이미지가 없을 때의 처리가 더 명확해집니다. 현재는 AsyncImageplaceholdererror를 사용하므로 기능적으로는 문제 없지만, 데이터 모델의 정확성을 높일 수 있습니다.
  • 파일 마지막 줄 개행: 이 파일에서도 No newline at end of file이 발생합니다.

5. feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt

--- a/feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt
+++ b/feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt
@@ -4,4 +4,7 @@ import androidx.navigation3.runtime.NavKey
 import kotlinx.serialization.Serializable
 
 @Serializable
-data object MyPageNavKey: NavKey
\ No newline at end of file
+data object MyPageNavKey: NavKey
+
+@Serializable
+data object CharacterManagementNavKey: NavKey
\ No newline at end of file
  • 파일 마지막 줄 개행: 이 파일에서도 No newline at end of file이 발생합니다.

6. feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt

// ...
import kr.co.call.designsystem.R
// ...
@Composable
fun CharacterCard(
    aiCharacter: AiCharacter,
    onChatHistoryClick: () -> Unit,
    onDeleteClick: () -> Unit,
    modifier: Modifier = Modifier,
) {
    // ...
                Box(
                    modifier = Modifier
                        .align(Alignment.BottomEnd)
                        .size(23.dp)
                        .clip(CircleShape)
                        .background(CallTheme.colors.white),
                    contentAlignment = Alignment.Center,
                ) {
                    Icon(
                        painter = painterResource(id = R.drawable.ic_mypage_edit),
                        contentDescription = "편집하기 버튼", // TODO: 하드코딩된 문자열
                        tint = Color.Unspecified
                    )
                }
            }
            // ...
            Text(
                text = "채팅 기록 보기", // TODO: 하드코딩된 문자열
                style = CallTheme.typography.caption,
                color = CallTheme.colors.gray400,
                textDecoration = TextDecoration.Underline,
                modifier = Modifier.clickable(onClick = onChatHistoryClick),
            )
        }
        // ...
        SecondaryButton(
            text = "캐릭터 영구 삭제", // TODO: 하드코딩된 문자열
            onClick = onDeleteClick,
            iconRes = R.drawable.ic_mypage_trash,
            containerColor = CallTheme.colors.mainVariant1,
            modifier = Modifier.height(38.dp)
        )
    // ...
}
  • 하드코딩된 문자열: IconcontentDescription, Text 컴포저블의 텍스트 ("채팅 기록 보기", "캐릭터 영구 삭제") 등 여러 문자열이 하드코딩되어 있습니다. 이는 string resource로 분리하여 관리하는 것이 좋습니다. 다국어 지원, 재사용성, 유지보수 측면에서 유리합니다.
    • ic_mypage_edit contentDescription: 이 아이콘은 캐릭터 편집 화면으로 이동하는 기능을 가리키는 것 같습니다. "편집하기 버튼" 보다는 "AI 캐릭터 프로필 편집"과 같이 좀 더 구체적인 설명이 좋을 것 같습니다. 그리고 위 SecondaryButton 코멘트처럼 string resource로 분리해주세요.
  • 파일 마지막 줄 개행: 이 파일에서도 No newline at end of file이 발생합니다.

7. feature/mypage/impl/src/main/java/kr/co/call/impl/screen/CharacterManagementScreen.kt

@Composable
fun CharacterManagementScreen(
    modifier: Modifier = Modifier,
    viewModel: CharacterManagementViewModel = hiltViewModel(),
    onBackClick: () -> Unit,
    navigateToAddCharacter: () -> Unit,
) {
    val state by viewModel.collectAsState()

    var chatHistoryTarget by remember { mutableStateOf<AiCharacter?>(null) }
    var deleteTarget by remember { mutableStateOf<AiCharacter?>(null) }
    var showMainDeleteBlocked by remember { mutableStateOf(false) }
    var showAddBlocked by remember { mutableStateOf(false) }

    viewModel.collectSideEffect { sideEffect ->
        when (sideEffect) {
            is CharacterManagementSideEffect.ShowChatHistorySummary -> chatHistoryTarget = sideEffect.aiCharacter
            is CharacterManagementSideEffect.ShowDeleteConfirmDialog -> deleteTarget = sideEffect.aiCharacter
            is CharacterManagementSideEffect.ShowMainCharacterDeleteBlocked -> showMainDeleteBlocked = true
            is CharacterManagementSideEffect.ShowAddCharacterBlocked -> showAddBlocked = true
            is CharacterManagementSideEffect.NavigateToAddCharacter -> navigateToAddCharacter()
        }
    }
    // ...
}

@Composable
private fun CharacterManagementScreenContent(
    state: CharacterManagementState,
    onIntent: (CharacterManagementIntent) -> Unit,
    onBackClick: () -> Unit,
    modifier: Modifier = Modifier,
) {
    Column(
        modifier = modifier
            .fillMaxSize()
            .background(CallTheme.colors.background)
            .statusBarsPadding(),
    ) {
        // ...
        Text(
            text = "생성 가능한 캐릭터 수는 최대 5개입니다.", // TODO: 하드코딩된 문자열 & 매직 넘버
            style = CallTheme.typography.caption,
            color = CallTheme.colors.gray400,
        )

        // ...
        SecondaryButton(
            text = "캐릭터 추가하기", // TODO: 하드코딩된 문자열
            onClick = { onIntent(CharacterManagementIntent.ClickAddCharacter) },
            containerColor = CallTheme.colors.mainVariant4,
            contentColor = CallTheme.colors.white,
            modifier = Modifier.padding(bottom = 24.dp)
                .padding(horizontal = 16.dp)
        )
    }
}
  • UI 상태 관리 (remember 변수): chatHistoryTarget, deleteTarget, showMainDeleteBlocked, showAddBlocked 와 같은 팝업 관련 UI 상태를 remember { mutableStateOf(...) } 로 관리하는 것은 Composables 내부에서 UI 상태를 관리하는 일반적인 방법입니다. 다만, 이러한 상태들이 ViewModel의 SideEffect를 통해 변경되는 만큼, ViewModel에서 직접 이 팝업의 show/hide 상태까지 관리하는 CharacterManagementState의 필드로 포함시키는 것도 고려해볼 수 있습니다. 이렇게 하면 Composable은 단순히 ViewModel의 상태를 그리는 역할에 집중하고, 비즈니스 로직에 가까운 팝업 결정 로직은 ViewModel에 더 응집될 수 있습니다.
    • 현재 방식의 장점: Composable 내에서 UI 렌더링에만 직접적으로 관련된 상태를 관리하여 ViewModel의 상태를 간결하게 유지할 수 있습니다.
    • ViewModel에서 관리 시 장점: 팝업 표시/숨김 로직이 ViewModel의 테스트 범위에 포함될 수 있습니다. (예: state.showChatHistoryPopup: AiCharacter?)
    • 결정: 현재 방식도 유효하며, 코드 가독성도 좋으므로 크게 문제되지 않습니다. 팀의 컨벤션에 따라 선택하시면 됩니다.
  • 하드코딩된 문자열 및 매직 넘버:
    • "생성 가능한 캐릭터 수는 최대 5개입니다." 이 문구와 "5"라는 숫자는 string resourceinteger resource로 분리하는 것이 좋습니다.
    • "캐릭터 추가하기" 버튼의 텍스트도 string resource로 분리해야 합니다.
  • 파일 마지막 줄 개행: 이 파일에서도 No newline at end of file이 발생합니다.

8. feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementViewModel.kt

@HiltViewModel
class CharacterManagementViewModel @Inject constructor(
    private val aiCharacterRepository: AICharacterRepository,
) : ViewModel(), ContainerHost<CharacterManagementState, CharacterManagementSideEffect> {

    override val container: Container<CharacterManagementState, CharacterManagementSideEffect> = container(
        initialState = CharacterManagementState()
    ) {
        loadCharacters()
    }
    // ...
    private fun deleteCharacter(characterId: String) = intent {
        // ...
        aiCharacterRepository.deleteCharacter(characterId)
            .onSuccess {
                reduce { state.copy(aiCharacters = state.aiCharacters.filterNot { it.id == characterId }) }
            }
            .onFailure { e ->
                if (e is CancellationException) throw e
                // TODO 삭제 실패 시 처리
            }
    }

    private fun checkAddCharacter() = intent {
        aiCharacterRepository.canAddCharacter()
            .onSuccess { canAdd ->
                if (canAdd) {
                    postSideEffect(CharacterManagementSideEffect.NavigateToAddCharacter)
                } else {
                    postSideEffect(CharacterManagementSideEffect.ShowAddCharacterBlocked)
                }
            }
            .onFailure { e ->
                if (e is CancellationException) throw e
                postSideEffect(CharacterManagementSideEffect.ShowAddCharacterBlocked) // TODO: 에러 처리 개선
            }
    }
}
  • deleteCharacterTODO: 삭제 실패 시 처리에 대한 TODO가 있습니다. 네트워크 에러, 권한 에러 등 다양한 실패 시나리오를 고려하여 사용자에게 피드백을 주거나 재시도할 수 있도록 SideEffect를 통해 적절한 팝업 메시지를 띄우는 것이 좋습니다. (예: postSideEffect(CharacterManagementSideEffect.ShowErrorDialog("캐릭터 삭제에 실패했습니다.")))
  • checkAddCharacteronFailure 처리: aiCharacterRepository.canAddCharacter() 호출 실패 시 (onFailure 블록) 무조건 ShowAddCharacterBlockedpostSideEffect하고 있습니다. canAddCharacter에서 발생할 수 있는 에러는 '추가 불가'와는 다른, '서버 통신 실패' 등의 에러일 수 있습니다. 이 경우 '아직 캐릭터를 추가할 수 없어요'라는 메시지보다는 '캐릭터 추가 가능 여부를 확인할 수 없습니다. 잠시 후 다시 시도해주세요.' 와 같은 일반적인 에러 메시지를 보여주는 SideEffect가 더 적절합니다. CharacterManagementSideEffect.ShowErrorDialog(message: String) 와 같은 범용적인 에러 SideEffect를 추가하여 사용하는 것을 고려해 보세요.
  • Coroutine Dispatcher: ViewModel에서 intent 블록은 기본적으로 Dispatchers.DefaultDispatchers.IO에서 실행될 수 있도록 Orbit 내부적으로 관리됩니다. 명시적으로 withContext(Dispatchers.IO) 등을 사용하지 않았지만, Repository 계층에서 suspend 함수가 네트워크/DB 작업을 처리한다면 해당 레이어에서 적절한 Dispatcher를 사용하고 있을 것이라 기대합니다. 현재 AICharacterRepositoryImpl은 임시 구현체라 큰 문제가 없지만, 실제 네트워크 연동 시에는 데이터 레이어에서 Dispatchers.IO를 명시하는 것이 좋습니다. (이 PR 범위 밖일 수 있습니다.)
  • 파일 마지막 줄 개행: 이 파일에서도 No newline at end of file이 발생합니다.

Navigation 및 DI

  • AppScreen.kt에서 myPageEntrynavigateToCharacterManagement 콜백을 넘겨주고, MyPageScreen에서 이를 호출하는 패턴은 Composable 간의 의존성을 줄이고 State Hoisting 원칙을 잘 지키는 좋은 방법입니다.
  • HiltViewModelAICharacterRepository 인터페이스를 통한 의존성 주입도 올바르게 적용되었습니다.

결론

전반적으로 매우 좋은 PR입니다. 몇 가지 하드코딩된 문자열과 에러 처리의 세부적인 개선, 그리고 파일 끝 개행 문자 문제만 해결하면 될 것 같습니다. MVI 패턴을 효과적으로 사용하고, 각 계층의 역할을 명확히 분리하여 확장성과 유지보수성이 높은 코드를 작성하셨습니다.

수고 많으셨습니다!

@codebidoof codebidoof 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.

롤백만 시켜주세영 나머지는 너무 잘했서여~~~!!!!!!!

},
onBack = {
appNavigator.popBackStack()
})

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.

엔터 한 번 쳐주세요 ㅋㅋ

modifier: Modifier = Modifier,
label: String,
title: String,
title: String? = null,

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.

롤백 부탁드립니다.

modifier: Modifier = Modifier,
label: String,
title: String,
title: String?=null,

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.

롤백 부탁드립니다

- 들여쓰기 적용
- PopupCard/OneButtonPopup 롤백
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요! PR 잘 보았습니다. AI 캐릭터 관리 기능 추가를 위한 전반적인 작업이 잘 진행된 것 같습니다. 신규 기능 추가와 기존 컴포넌트 확장, 그리고 MVI 패턴 적용까지 깔끔하게 작업해 주셨네요. 몇 가지 개선 사항과 궁금한 점들을 정리해서 리뷰 드립니다.


1. Kotlin 코드 (General)

  • Null 안전성 (!! 사용 지양)
    • 코드 전체적으로 !! 사용은 발견되지 않았습니다. null 안전성을 잘 고려하여 개발하신 점 좋습니다.
  • data class에 불필요한 var 사용 여부
    • AiCharacter 데이터 클래스는 모두 val로 선언되어 불변성을 유지하고 있습니다. 좋은 접근입니다.
  • Hilt DI 시 생성자 주입 원칙 준수 여부
    • AICharacterRepositoryImpl, CharacterManagementViewModel 등 Hilt를 사용하는 곳에서 생성자 주입(@Inject constructor()) 원칙을 잘 지키고 있습니다.
  • 하드코딩된 문자열/매직 넘버
    • CharacterManagementScreen.kt 에 "캐릭터 관리", "생성 가능한 캐릭터 수는 최대 5개입니다." 등 여러 사용자 노출 문자열이 하드코딩되어 있습니다. 이는 추후 다국어 지원 및 유지보수를 위해 string.xml 리소스로 분리하는 것이 좋습니다.
    • 마찬가지로 16.dp, 21.dp 등 레이아웃 관련 매직 넘버들이 보입니다. 디자인 시스템 내에 Spacer 값이나 Padding 값을 정의해두고 사용하는 것을 고려해볼 수 있습니다.

2. Jetpack Compose 코드

  • Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용)
    • CharacterCardCharacterManagementScreen에서 Modifier 체이닝 및 remember 사용이 적절해 보입니다. 불필요한 리컴포지션은 크게 우려되지 않습니다.
  • State hoisting 원칙 준수
    • AppScreen에서 myPageEntrynavigateToCharacterManagementonBack 람다를 전달하고, MyPageScreen에서도 onNavigateToCharacterManagement 람다를 받는 등 상태 호이스팅 원칙을 잘 지키고 있습니다.
    • CharacterManagementScreenonIntent 콜백을 통해 ViewModel과 상호작용하며, onBackClick, navigateToAddCharacter 람다를 외부에서 주입받아 처리하고 있습니다. 전반적으로 State hoisting 원칙을 잘 준수하고 있습니다.
  • side effect (LaunchedEffect, DisposableEffect) 사용의 적절성
    • CharacterManagementScreen에서 viewModel.collectSideEffect를 사용하여 CharacterManagementSideEffect를 처리하는 것이 확인됩니다. LaunchedEffect와 유사하게 단발성 이벤트를 처리하는 데 적절하게 사용되었습니다.
  • UI 상태를 data class로 표현했는지
    • CharacterManagementState 데이터 클래스를 통해 UI 상태를 잘 표현하고 있습니다.
    • 다만, CharacterManagementScreen 내부에서 팝업의 가시성을 chatHistoryTarget, deleteTarget, showMainDeleteBlocked, showAddBlocked 등 개별 mutableStateOf로 관리하고 있습니다. 이 부분은 data class PopupState(val type: PopupType, val data: Any?)와 같이 팝업 관련 상태를 하나의 데이터 클래스로 묶어 관리하면, 화면의 UI 상태가 좀 더 응집력 있게 관리될 수 있습니다. (예: CharacterManagementState 내부에 currentPopup: CharacterManagementPopup? 같은 필드 추가)
  • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지
    • CharacterManagementState에서 LoadStatus를 사용하여 로딩 상태를 잘 관리하고 있습니다. 하지만 CharacterManagementScreenContent에서는 state.loadStatus에 따른 UI 변화 (예: 로딩 스피너 표시, 에러 메시지 토스트/스낵바)가 명시적으로 보이지 않습니다. 사용자에게 로딩 중이거나 에러가 발생했음을 알리는 UI 피드백을 추가하는 것을 고려해볼 수 있습니다.

3. Repository/DataSource 레이어

  • Retrofit 에러 핸들링 (try-catch, Result 래핑)
    • AICharacterRepositoryImpl은 현재 임시 구현체(TODO: API 연동 전 임시 구현체)이므로 Retrofit 에러 핸들링은 직접 적용되지 않았습니다. 하지만 runCatching을 사용하여 Result 타입으로 래핑하고 있는 점은 실제 API 연동 시에도 동일한 패턴으로 에러를 처리할 수 있는 기반이 됩니다. 추후 실제 네트워크 통신으로 전환될 때 이 부분을 확인해야 합니다.
  • 네트워크 응답과 도메인 모델 매핑 분리 여부
    • 현재는 임시 구현체라 직접적인 네트워크 응답이 없으므로 매핑 로직은 없습니다. AiCharacter 도메인 모델을 직접 사용하고 있습니다. 추후 API 연동 시 data 모듈 내에 dto를 정의하고 mapper를 통해 AiCharacter로 변환하는 구조를 기대합니다.
  • 캐싱 전략 (로컬 DB vs 메모리)
    • 현재 aiCharactersmutableListOf로 선언하여 인메모리 데이터를 사용하고 있습니다. 이는 임시 구현체로는 괜찮지만, 실제 데이터가 필요한 경우 Room DB와 같은 로컬 캐싱 또는 서버 API를 통한 데이터 영속성 전략이 필요합니다.

4. ViewModel (Orbit)

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

    • CharacterManagementState로 UI 상태를, CharacterManagementViewModelintent 블록으로 비즈니스 로직을 명확하게 분리하여 구현하고 있습니다. 좋은 구조입니다.
  • Orbit ContainerHost 패턴 준수 여부

    • CharacterManagementViewModelContainerHost 인터페이스를 구현하고 container 팩토리 함수를 사용하여 초기화하는 등 Orbit MVI 패턴을 잘 준수하고 있습니다.
  • intent 내부에서 상태 변경(reduce)만 수행하는지

    • intent 블록 내에서 reduce를 통해 상태를 변경하거나 postSideEffect를 통해 단발성 이벤트를 발생시키는 패턴을 따르고 있습니다. 비즈니스 로직과 상태 변경이 분리되어 코드가 명확합니다.
  • postSideEffect 남용 여부

    • postSideEffect는 UI 이벤트 (팝업 표시, 화면 이동)에만 사용되고 있습니다. 남용되지 않고 적절하게 사용되었습니다.
  • blocking 작업을 intent 내부에서 직접 수행하지 않는지

    • aiCharacterRepository 호출과 같은 잠재적인 블로킹 작업은 모두 intent 블록 내부에서 호출되고 있습니다. 이는 Orbit의 intent 블록이 코루틴 스코프를 제공하므로 안전합니다. Dispatcher 명시는 없지만, Orbit의 기본 설정 또는 레포지토리 레이어에서의 Dispatcher 처리에 의존하고 있을 것으로 보입니다.
  • 개선 제안:

    • deleteCharacter 함수 내부에서 targetCharacter?.isMain == true로 다시 메인 캐릭터 여부를 확인하고 ShowMainCharacterDeleteBlocked 사이드 이펙트를 보내는 로직이 있습니다. handleDeleteClick에서 이미 이 검증을 한 번 수행하므로, deleteCharacter는 메인 캐릭터가 아닌 경우에만 호출되도록 handleDeleteClick에서 확실히 분기하거나, deleteCharacter 함수의 인자로 isMain 여부를 넘겨받아 중복 검증을 피할 수 있습니다. (현재도 안전하지만, 약간의 중복이 있습니다.)
    • deleteCharacter 함수에서 onFailureTODO로 처리되어 있습니다. 실제 삭제 실패 시 사용자에게 어떤 방식으로 피드백을 줄지 (예: 토스트 메시지, 에러 팝업) 구현이 필요합니다.

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

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

Design System 관련 (추가)

  • RepositoryModule.kt 에서 bindMyPageRepositorybindCharacterRepository@Singleton 어노테이션이 누락되어 있습니다. Repository 구현체는 주로 싱글톤으로 관리되므로 추가해주시는 것이 좋습니다.
  • SecondaryButton.kticonRes 파라미터가 추가되고 아이콘을 표시하는 로직이 추가되었습니다. tint = Color.Unspecified로 설정하여 아이콘의 원본 색상을 유지하는 것이 좋습니다. ic_mypage_trash의 경우 흰색 아이콘이므로 contentTint를 사용해도 되지만, ic_mypage_edit의 경우 여러 색상을 가지므로 Color.Unspecified가 더 적절합니다. 현재는 ic_mypage_trash가 white이므로 contentTint로 처리해도 문제가 없겠으나, 의도에 따라 명시적으로 Color.Unspecified를 사용하면 좋습니다. (현재 ic_mypage_edit는 XML에 strokeColor가 명시되어 있어 Color.Unspecified가 맞습니다.)

종합 의견

전반적으로 Clean Architecture와 MVI 패턴을 잘 적용하여 체계적으로 코드를 작성해 주셨습니다. 특히 ViewModel에서 Orbit을 활용한 상태 관리와 Side Effect 처리가 인상 깊습니다. 몇 가지 TODO 주석이나 하드코딩된 문자열, LoadStatus에 대한 UI 피드백 등 개선하면 더 완벽해질 수 있는 부분들이 있지만, 기능 구현 자체는 매우 잘 되어 있습니다.

리뷰 내용을 참고하셔서 PR을 마무리해 주시면 좋겠습니다. 수고하셨습니다!

@codebidoof codebidoof 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.

고생했으~~!!

@codebidoof codebidoof 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.

고생했어!! 공용 컴포넌트 쪽 변경사항은 롤백하는 게 좋아보여!

Comment on lines 37 to 88
@Composable
fun SecondaryButton(
modifier: Modifier = Modifier,
text: String,
onClick: () -> Unit,
enabled: Boolean = true,
iconRes: Int? = null,
containerColor: Color = CallTheme.colors.mainVariant4,
contentColor: Color = CallTheme.colors.white,
pressedContainerColor: Color = CallTheme.colors.subPressed3,
) {
val interactionSource = remember { MutableInteractionSource() }
val isPressed by interactionSource.collectIsPressedAsState()

Box(
modifier = modifier
.fillMaxWidth()
.height(51.dp)
.background(
color = when {
!enabled -> containerColor.copy(alpha = 0.38f)
isPressed -> pressedContainerColor
else -> containerColor
},
shape = RoundedCornerShape(10.dp),
)
.clickable(
enabled = enabled,
interactionSource = interactionSource,
indication = ripple(),
onClick = onClick,
),
contentAlignment = Alignment.Center,
) {
Text(
text = text,
style = CallTheme.typography.bodyMediumMedium,
color = if (enabled) contentColor else contentColor.copy(alpha = 0.38f),
)
val contentTint = if (enabled) contentColor else contentColor.copy(alpha = 0.38f)

if (iconRes != null) {
Row(verticalAlignment = Alignment.CenterVertically) {
Icon(
painter = painterResource(id = iconRes),
contentDescription = null,
tint = contentTint,
modifier = Modifier.size(16.dp),
)
Spacer(modifier = Modifier.width(6.dp))
Text(text = text, style = CallTheme.typography.bodyMediumMedium, color = contentTint)
}
} else {
Text(text = text, style = CallTheme.typography.bodyMediumMedium, color = contentTint)
}
}
}

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.

공용 컴포넌트 수정한 이유 있나영

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[마이페이지>캐릭터 관리] 부분에, 아래 이미지와 같이 SecondaryButton 내부, 텍스트 우측에 쓰레기통 아이콘을 추가해야 해서 수정했습니당
확인해보니 다른 페이지에서는 아이콘이 안쓰이던데, 말씀주신것처럼 공통 컴포넌트는 그대로 두고 차라리 feature/mypage/component 쪽에서 새로운 버튼으로 제작하는 편이 나을까요?

image

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.

아! 그런 이유라면 괜찮아 보입니다! 대신에 공용 컴포넌트 수정 시에는 단톡에 말해주는 게 좋을 것 같아요!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

넵 감사합니당!! 최고최고👍

modifier: Modifier = Modifier,
label: String,
title: String,
title: String? = "",

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.

이거 수정한 이유 있나영?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

요건 아래에 PopupCard 컴포넌트 변경한 comment와 동일한 이유입니다! 아래 comment 참고해주세용

Comment on lines 26 to +62
@@ -50,13 +50,16 @@ internal fun PopupCard(
textAlign = TextAlign.Center,
)
Spacer(modifier = Modifier.height(8.dp))
Text(
text = title,
style = CallTheme.typography.bodyLargeBold,
color = CallTheme.colors.black,
textAlign = TextAlign.Center,
modifier = Modifier.fillMaxWidth(),
)
if (title != null) {
Spacer(modifier = Modifier.height(8.dp))
Text(
text = title,
style = CallTheme.typography.bodyLargeBold,
color = CallTheme.colors.black,
textAlign = TextAlign.Center,
modifier = Modifier.fillMaxWidth(),
)
}

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.

이거 수정한 이유 있나영?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

현재 공통 컴포넌트 PopupCard에서는 title을 강제하고 있는데, [마이페이지>캐릭터 관리] 화면에 아래와 같이 title이 없는 팝업 화면이 존재해서 title을 nullable 하게 변경했었습니당! 혹시 이것두 공통에서는 따로 빼서 컴포넌트 제작하는 편이 더 나을까요?

  • image
  • 참고로 PopupCard에서 title은 아래 보이는 이미지 속 굵은 글자("아직 캐릭터를 수정할 수 없어요") 부분입니다!

  • image

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.

이 코멘트 와 동일합니다!

Comment on lines +47 to +48
var chatHistoryTarget by remember { mutableStateOf<AiCharacter?>(null) }
var deleteTarget by remember { mutableStateOf<AiCharacter?>(null) }

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.

이거 널인지 아닌지 여부로 세팅하지 말고 mutableStateOf의 원소 타입을 boolean으로 줘서 true false 기준으로 팝업 띄울지 여부를 분기처리하는 게 더 나아보여!!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

굳굳 바로 적용시켜서 다시 커밋 올려둘게용! 피드백 감사합니당👍

WAcAW9 added 2 commits July 27, 2026 13:32
- popup 컴포넌트에 mutableStateOf 원소 타입을 boolean으로 변경
- 필요한 import문 추가
- 캐릭터 추가하기팝업 "24시간" 강조
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, 시니어 안드로이드 개발자입니다. AI 캐릭터 관리 기능 추가를 위한 PR에 대해 상세한 코드 리뷰를 진행하겠습니다. 전반적으로 Orbit MVI 패턴과 Jetpack Compose의 모범 사례를 잘 적용하고 있어 인상 깊습니다. 몇 가지 개선 사항과 잠재적 문제점을 짚어드리겠습니다.


전반적인 피드백 (General Feedback)

👍 좋은 점 (Good Points)

  • 아키텍처 및 클린 코드: Orbit MVI 패턴을 잘 준수하고 있으며, UI, ViewModel, Repository 간의 관심사 분리가 명확합니다. Repository에서 Result<T>를 사용하여 비동기 작업 결과를 처리하는 방식도 훌륭합니다.
  • Jetpack Compose 모범 사례: State hoisting 원칙, data class를 사용한 UI 상태 관리, remember를 사용한 임시 상태 관리, StateFlowcollectAsState, collectSideEffect를 활용한 생명주기 안전한 데이터 수집, AsyncImage 사용, 그리고 다양한 상태에 대한 프리뷰 제공 등 Compose의 여러 모범 사례를 잘 적용했습니다.
  • Hilt DI: 생성자 주입 원칙을 정확히 준수하고 있습니다.
  • Kotlin Idiomatic: data class에 불필요한 var 사용 없이 val만 사용하고, !! 없이 null 안전성을 잘 확보했습니다.
  • 에러 핸들링: Repository mock에서 runCatching을 사용하고 ViewModel에서 onFailure를 처리하는 등 에러 핸들링의 기초가 잘 마련되어 있습니다.

🤔 개선이 필요한 점 (Areas for Improvement)

  1. 하드코딩된 문자열 (Major): CharacterCard.ktCharacterManagementScreen.kt에 많은 문자열이 하드코딩되어 있습니다. 이는 국제화(i18n) 및 유지보수 측면에서 좋지 않습니다. 모든 사용자에게 노출되는 문자열은 strings.xml 리소스로 분리해야 합니다.

    • 예시: "채팅 기록 보기", "메인", "캐릭터 영구 삭제", "편집하기 버튼", "생성일", "함께한 지", "마지막 대화", "캐릭터 관리", "생성 가능한 캐릭터 수는 최대 5개입니다.", "캐릭터 추가하기", "확인", "삭제", "취소", "현재 메인으로 선택된 연인은 삭제가 불가능합니다.", "아직 캐릭터를 추가할 수 없어요", "새로운 캐릭터를 만든 후", "24시간", "이 지나면 또 다른 캐릭터를 만들 수 있어요".
  2. UI의 로딩/에러 상태 표시 (Minor): CharacterManagementState에서 LoadStatus를 사용하여 로딩/에러 상태를 관리하고 있지만, CharacterManagementScreenContent에서는 현재 로딩 중일 때(예: LoadStatus.Loading) 프로그레스 인디케이터를 보여주거나, 에러 발생 시(예: LoadStatus.Error) 사용자에게 에러 메시지를 표시하는 UI 로직이 없습니다. 사용자 경험을 위해 이 부분을 추가하는 것이 좋습니다.

  3. Repository의 도메인/네트워크 모델 분리 (Future Enhancement): AICharacterRepositoryImpl은 현재 임시 구현체(mock)이므로 AiCharacter 도메인 모델을 직접 반환하고 있습니다. 실제 API 연동 시에는 네트워크 응답을 위한 DTO(Data Transfer Object)를 정의하고, 이를 도메인 모델로 매핑하는 레이어를 Repository 또는 Mapper 클래스에 분리하여 구현해야 합니다.

  4. 캐릭터 삭제 로직의 불필요한 중복 체크 (Minor Refinement): CharacterManagementViewModel.deleteCharacter 내부에서 targetCharacter?.isMain == true 체크가 다시 등장합니다. 이미 handleDeleteClick에서 메인 캐릭터 삭제를 막고 있으므로, 이 체크는 중복될 수 있습니다. deleteCharacter는 메인 캐릭터가 아니라는 전제하에 호출되도록 설계하고, 해당 로직을 제거하여 코드를 더 간결하게 만들 수 있습니다.

  5. canAddCharacter 실패 시 에러 처리 (Minor Refinement): CharacterManagementViewModel.checkAddCharacteronFailure 블록에서 일괄적으로 ShowAddCharacterBlocked SideEffect를 발생시키고 있습니다. canAddCharacter 호출 실패가 캐릭터 추가가 막힌 것과는 다른 종류의 에러일 수 있습니다 (예: 네트워크 오류). 에러의 종류에 따라 사용자에게 더 적절한 피드백을 제공하는 SideEffect를 포스팅하는 것을 고려해 보세요.

  6. deleteCharacter 실패 시 처리 (TODO): deleteCharacter 함수의 onFailure 블록 내 // TODO 삭제 실패 시 처리 주석이 있습니다. 이 부분에 토스트 메시지 표시 또는 에러 팝업을 띄우는 등 실제 사용자에게 피드백을 줄 수 있는 로직을 구현해야 합니다.

  7. 프로필 편집 아이콘 (UI/UX 검토):

    • ic_mypage_edit.xml에는 40dp 크기의 원형 배경과 편집 아이콘이 모두 포함되어 있습니다.
    • CharacterCard에서는 이 아이콘을 23.dp 크기의 Box로 감싸고 Box의 배경색을 CallTheme.colors.white로 설정했습니다.
    • 이로 인해 원래 XML에 정의된 핑크색 원형 배경 (#FFDCE2)이 잘려 보이거나, 흰색 Box에 의해 가려져 디자인 의도와 다르게 보일 수 있습니다. 디자인 스펙을 다시 확인하여, XML 리소스에서 순수 편집 아이콘만 포함하고 배경은 Compose Box에서 처리하거나, 아니면 XML 리소스와 Box의 크기/스타일을 일치시켜야 합니다. 현재 구현으로는 아이콘이 이상하게 렌더링될 가능성이 높습니다.

파일별 상세 리뷰 (Detailed File-by-File Review)

app/src/main/java/kr/co/call/callfromai/AppScreen.kt

  • statusBarsPadding 제거 및 WindowInsets.safeDrawing 사용은 최신 Jetpack Compose UI에 권장되는 방식입니다. 좋습니다.
  • myPageEntrynavigateToCharacterManagement 람다를 추가하여 새 화면으로의 이동을 연결한 방식도 올바릅니다.

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

  • AICharacterRepositoryAICharacterRepositoryImplSingletonComponentSingleton으로 바인딩한 것은 Hilt DI 원칙에 부합합니다.

core/data/src/main/java/kr/co/call/data/repositoryImpl/AICharacterRepositoryImpl.kt (New File)

  • // TODO: API 연동 전 임시 구현체 주석을 통해 현재 mock이라는 것을 명시한 점 좋습니다.
  • runCatching을 사용하여 Result<T>를 반환하는 패턴은 실제 API 연동 시에도 유용합니다.
  • aiCharacters 데이터를 mutableListOf로 관리하여 deleteCharacter 기능을 구현한 점도 mock으로서 충분합니다.
  • 하드코딩된 문자열: mock 데이터 내 createdAtLabel, daysTogetherLabel, lastConversationLabel, summary 등은 임시 데이터이므로 괜찮습니다. 하지만 실제 구현 시에는 적절한 날짜/시간 포매팅 유틸리티를 사용해야 합니다.

core/designsystem/src/main/java/kr/co/call/designsystem/component/button/SecondaryButton.kt

  • iconRes 파라미터를 추가하고 Row 내부에 IconText를 배치하여 버튼에 아이콘을 추가할 수 있게 확장한 것은 좋습니다.
  • contentTintenabled 상태에 따라 조정한 것도 디테일이 살아있습니다.
  • painterResource는 내부적으로 캐싱되므로 Composable 내부에서 사용해도 불필요한 리컴포지션을 유발하지 않습니다.

core/designsystem/src/main/java/kr/co/call/designsystem/component/popup/OneButtonPopup.kt & PopupCard.kt

  • title 파라미터를 nullable(String?)로 변경하고, if (title != null)로 조건부 렌더링을 한 것은 팝업 컴포넌트의 유연성을 높여줍니다. 좋습니다.

core/designsystem/src/main/res/drawable/ic_mypage_edit.xml & ic_mypage_trash.xml (New Files)

  • 새로운 드로어블 리소스 파일입니다.

core/domain/src/main/java/kr/co/call/domain/model/mypage/AiCharacter.kt (New File)

  • data class로 UI 상태를 잘 표현했으며, 모든 필드를 val로 선언하고 기본값을 제공하여 불변성 및 null 안전성을 확보했습니다. 매우 좋습니다.

core/domain/src/main/java/kr/co/call/domain/repository/AICharacterRepository.kt (New File)

  • Repository 인터페이스를 정의하고 suspend 함수로 비동기 작업을 명시하며 Result<T>를 반환하는 것은 Domain 레이어의 역할에 맞습니다.

feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt

  • CharacterManagementNavKey를 추가하여 새로운 내비게이션 경로를 정의한 것은 좋습니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/component/CharacterCard.kt (New File)

  • Compose:
    • aiCharacter, onChatHistoryClick, onDeleteClick 등 필요한 데이터를 파라미터로 받아 State hoisting 원칙을 잘 준수하고 있습니다.
    • AsyncImage를 사용하여 프로필 이미지를 효율적으로 로드하고 placeholder, error 리소스를 지정한 점 좋습니다.
    • MainBadgeInfoColumn 같은 서브 컴포저블로 UI를 분리한 것도 가독성을 높입니다.
    • VerticalDivider 사용도 적절합니다.
  • 하드코딩된 문자열: 위에서 언급했듯이, "채팅 기록 보기", "메인", "생성일", "함께한 지", "마지막 대화", "캐릭터 영구 삭제", "편집하기 버튼" 등 모든 문자열을 strings.xml로 옮겨야 합니다.
  • UI/UX 검토 (프로필 편집 아이콘): 7번 일반 피드백에서 지적했듯이, Box(modifier = Modifier.align(Alignment.BottomEnd).size(23.dp)...)Icon으로 R.drawable.ic_mypage_edit을 사용하는 부분에서 XML 리소스의 디자인과 Composable 코드의 크기/배경 처리 방식이 불일치하여 의도와 다른 렌더링이 발생할 가능성이 높습니다. 디자인 팀과 확인하여 수정하는 것이 필요합니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt

  • CharacterManagementNavKey에 대한 entry를 추가하고 CharacterManagementScreen을 연결한 것은 올바른 내비게이션 구현입니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/screen/CharacterManagementScreen.kt (New File)

  • Compose:
    • viewModel: CharacterManagementViewModel = hiltViewModel()로 ViewModel을 주입하고, state by viewModel.collectAsState()viewModel.collectSideEffect를 사용하여 Orbit MVI 패턴을 정확히 따르고 있습니다.
    • mutableStateOfremember를 사용하여 팝업 가시성 등 UI 전용 상태를 관리하는 것은 적절합니다.
    • CharacterManagementScreenContent로 실제 UI 구현을 분리하여 State hoisting 원칙을 잘 준수하고 있습니다.
    • buildAnnotatedString을 사용하여 팝업 메시지에 부분적으로 스타일을 적용한 점 좋습니다.
    • 다양한 팝업 프리뷰를 제공하여 개발 및 리뷰 편의성을 높인 점은 매우 훌륭합니다.
  • 하드코딩된 문자열: 1번 일반 피드백에서 언급했듯이, 이곳의 모든 사용자용 문자열은 strings.xml로 옮겨야 합니다.
  • 로딩/에러 상태 UI: 2번 일반 피드백에서 언급했듯이, state.loadStatus에 따른 UI (로딩 인디케이터, 에러 메시지)가 현재 구현되어 있지 않습니다. 이 부분을 추가해야 합니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt

  • MyPageSideEffect.NavigateToCharacterManagement를 통해 onNavigateToCharacterManagement()를 호출하도록 연결한 것은 올바른 ViewModel-UI 상호작용 방식입니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementIntent.kt (New File)

  • sealed interface를 사용하여 모든 사용자 액션을 명확하게 정의한 것은 좋습니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementSideEffect.kt (New File)

  • sealed interface를 사용하여 일회성 UI 이벤트(팝업, 내비게이션)를 명확하게 정의했습니다. postSideEffect 사용의 적절성을 잘 준수하고 있습니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementState.kt (New File)

  • data class로 UI 상태를 표현하고 LoadStatus를 포함한 것은 좋습니다. 모든 필드가 val입니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/CharacterManagementViewModel.kt (New File)

  • ViewModel:
    • @HiltViewModel@Inject constructor를 통한 Hilt DI는 올바릅니다.
    • ContainerHostcontainer 초기화 시 loadCharacters()를 호출하여 초기 데이터를 로드하는 방식은 좋습니다.
    • handleIntent에서 사용자 Intent에 따라 적절한 비즈니스 로직 함수를 호출하는 구조는 명확합니다.
    • loadCharacters()에서 LoadStatus.Loading으로 변경하고 onSuccessLoadStatus.Idle, onFailureLoadStatus.Error로 상태를 업데이트하는 것은 좋습니다. CancellationException 처리가 포함된 것도 훌륭합니다.
    • showChatHistory()postSideEffect를 통해 팝업 표시를 요청하는 적절한 방식입니다.
    • handleDeleteClick()에서 메인 캐릭터인 경우와 아닌 경우를 분기하여 다른 SideEffect를 포스팅하는 것은 비즈니스 로직을 ViewModel에서 처리하는 좋은 예시입니다.
    • deleteCharacter():
      • 4번 일반 피드백에서 언급했듯이, targetCharacter?.isMain == true 체크는 handleDeleteClick에서 이미 처리되었으므로 불필요한 중복일 수 있습니다.
      • onSuccessfilterNot을 통해 aiCharacters 상태를 업데이트하는 것은 효율적이고 반응적인 UI를 만드는데 좋습니다.
      • onFailure// TODO 삭제 실패 시 처리 주석에 대한 구현이 필요합니다.
    • checkAddCharacter(): aiCharacterRepository.canAddCharacter() 결과를 바탕으로 NavigateToAddCharacter 또는 ShowAddCharacterBlocked SideEffect를 포스팅하는 것은 좋습니다.
      • 5번 일반 피드백에서 언급했듯이, onFailure 시 무조건 ShowAddCharacterBlocked를 포스팅하는 것은 실제 에러 원인을 가릴 수 있으므로 개선을 고려해볼 수 있습니다.
  • Coroutine Dispatcher: Orbit의 intent 블록은 기본적으로 Dispatchers.Default에서 실행됩니다. Repository의 suspend 함수가 내부적으로 IO Dispatcher를 사용한다면 문제가 없지만, 만약 Repository 구현이 Default에서 많은 연산 작업을 수행한다면 Dispatchers.IO를 명시적으로 사용하도록 고려할 수 있습니다. 현재 mock 구현에서는 큰 문제가 없습니다.
  • ViewModelScope/LifecycleScope 오남용: Orbit 패턴을 사용하므로 viewModelScope.launch 등의 직접적인 사용은 보이지 않습니다. Orbit이 코루틴 생명주기를 관리하므로 이 방식이 올바릅니다.

최종 권고 사항 (Final Recommendations)

아래 Action Item들을 고려하여 PR을 업데이트해 주시면 더욱 견고하고 유지보수하기 좋은 코드가 될 것입니다.

  1. 하드코딩된 문자열 제거: CharacterCard.ktCharacterManagementScreen.kt 내 모든 사용자에게 노출되는 문자열을 strings.xml 리소스로 분리합니다.
  2. 로딩/에러 UI 구현: CharacterManagementScreenContent에서 state.loadStatus 값에 따라 로딩 인디케이터(CircularProgressIndicator)나 에러 메시지(Text)를 표시하는 UI를 추가하여 사용자에게 시각적인 피드백을 제공합니다.
  3. deleteCharacter 실패 시 처리 구현: CharacterManagementViewModeldeleteCharacter 함수 onFailure 블록에 사용자에게 "삭제 실패"를 알리는 로직 (예: 토스트 메시지 또는 팝업)을 추가합니다.
  4. ic_mypage_edit.xml 아이콘 수정 또는 사용 방식 개선: CharacterCard의 프로필 편집 아이콘이 디자인 의도와 다르게 렌더링될 수 있으므로, ic_mypage_edit.xml 리소스를 순수 편집 아이콘으로만 만들거나, Composable의 Box 크기 및 스타일을 XML 디자인에 맞게 조정합니다.
  5. checkAddCharacter 에러 처리 구체화 (선택 사항): canAddCharacter API 호출 실패 시 발생하는 SideEffect를 ShowAddCharacterBlocked 대신, 더 일반적인 에러 메시지(ShowErrorSnackbar(message)) 등으로 변경하여 에러의 원인을 명확히 합니다.

이 PR은 전반적으로 높은 수준의 코드를 보여주고 있습니다. 위의 권고 사항들을 반영하면 더욱 완벽한 기능이 될 것입니다. 수고 많으셨습니다!

@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 (1)
feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt (1)

28-33: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

캐릭터 추가 네비게이션을 실제 목적지에 연결하세요.

feature/mypage/impl/src/main/java/kr/co/call/impl/screen/CharacterManagementScreen.ktNavigateToAddCharacter 발생 시 navigateToAddCharacter()를 호출하지만, 현재 entry에서는 TODO 빈 람다를 전달합니다. 추가가 허용된 사용자도 버튼을 눌러도 아무 화면으로 이동하지 않습니다. 추가 화면이 아직 준비되지 않았다면 해당 액션을 노출하지 않거나 비활성화해야 합니다.

🤖 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/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt`
around lines 28 - 33, CharacterManagementNavKey의 CharacterManagementScreen 호출에서
TODO 빈 람다를 제거하고, navigateToAddCharacter를 실제 캐릭터 추가 목적지로 연결하세요. 추가 화면이 아직 구현되지
않았다면 해당 액션을 노출하지 않거나 비활성화하여 버튼 클릭이 무동작으로 끝나지 않게 하세요.
🧹 Nitpick comments (1)
feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt (1)

18-18: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

onBack의 빈 기본값을 제거하세요.

호출부가 onBack을 생략하면 CharacterManagementScreen, FaqScreen, TermScreen의 뒤로가기 동작이 아무 작업도 하지 않게 됩니다. 실제 호출부가 항상 콜백을 전달하는지 확인하고, 가능하면 필수 파라미터로 변경하세요.

권장 수정
-    onBack: () -> Unit = {},
+    onBack: () -> Unit,
🤖 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/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt`
at line 18, MyPageEntryBuilder의 onBack 파라미터에서 빈 기본값을 제거하고 필수 콜백으로 변경하세요.
CharacterManagementScreen, FaqScreen, TermScreen을 포함한 모든 호출부가 실제 뒤로가기 동작을 수행하는
콜백을 전달하도록 확인하고 누락된 호출부를 수정하세요.
🤖 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
`@feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt`:
- Around line 28-33: CharacterManagementNavKey의 CharacterManagementScreen 호출에서
TODO 빈 람다를 제거하고, navigateToAddCharacter를 실제 캐릭터 추가 목적지로 연결하세요. 추가 화면이 아직 구현되지
않았다면 해당 액션을 노출하지 않거나 비활성화하여 버튼 클릭이 무동작으로 끝나지 않게 하세요.

---

Nitpick comments:
In
`@feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt`:
- Line 18: MyPageEntryBuilder의 onBack 파라미터에서 빈 기본값을 제거하고 필수 콜백으로 변경하세요.
CharacterManagementScreen, FaqScreen, TermScreen을 포함한 모든 호출부가 실제 뒤로가기 동작을 수행하는
콜백을 전달하도록 확인하고 누락된 호출부를 수정하세요.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3190f8d4-b0e4-407f-abe9-2ece062f3639

📥 Commits

Reviewing files that changed from the base of the PR and between 484a244 and 20eb19f.

📒 Files selected for processing (8)
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/component/popup/OneButtonPopup.kt
  • core/designsystem/src/main/java/kr/co/call/designsystem/component/popup/PopupCard.kt
  • feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/CharacterManagementScreen.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt
🚧 Files skipped from review as they are similar to previous changes (6)
  • core/designsystem/src/main/java/kr/co/call/designsystem/component/popup/PopupCard.kt
  • feature/mypage/api/src/main/java/kr/co/call/api/MyPageRoute.kt
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/CharacterManagementScreen.kt

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] 마이페이지 UI 구현 - 캐릭터 관리

2 participants