-
Notifications
You must be signed in to change notification settings - Fork 0
[feature/#34] 편지 보내기 기능 구현 #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough위로 보내기(편지) 플로우가 새 도메인/데이터/네트워크 모듈로 추가되고 Result 흐름이 Fairy/Letter/Result로 재구성되었습니다. 네비게이션 시그니처, Koin DI 모듈, Gradle 설정, ViewModel 및 화면(페어리·편지·결과)과 관련 DTO/서비스/리포지토리가 추가/갱신되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant Chat as ChattingScreen
participant Nav as MainNavigator
participant Fairy as FairyScreen
participant LetterS as LetterScreen
participant LVM as LetterViewModel
participant Repo as LetterRepository
participant RDS as LetterRemoteDataSource
participant API as LetterApiService
participant Net as EmotiaNetwork
participant Result as ResultScreen
U->>Chat: 요정 선택
Chat->>Nav: navigateToFairy(id,name,image,silhouette,chatRoomId)
Nav->>Fairy: FairyMain(...)
U->>Fairy: 화면 탭(편지 이동)
Fairy->>Nav: navigateToLetter(id,name,image,chatRoomId)
Nav->>LetterS: Letter(...)
U->>LetterS: 내용 입력 후 전송
LetterS->>LVM: Intent.SendLetter
LVM->>Repo: sendLetter(chatRoomId,fairyId,contents)
Repo->>RDS: sendLetter(...)
RDS->>API: sendLetter(request)
API->>Net: POST /api/v1/letters
Net-->>API: SendLetterResponse
API-->>RDS: SendLetterResponse
RDS-->>Repo: SendLetterResponse
Repo-->>LVM: Result<Letter>
alt 성공
LVM->>LetterS: SideEffect.NavigateToResult(letter)
LetterS->>Nav: navigateToLetterResult(fairyName,contents)
Nav->>Result: Result(...)
else 실패
LVM->>LetterS: SideEffect.ShowError(message)
LetterS->>U: 에러 표시(미구현)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
♻️ Duplicate comments (1)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyRoute.kt (1)
21-23: Result의 contents 직렬화 크기 고려
- 긴 본문을 직접 직렬화해 라우팅하면 환경에 따라 제약이 있을 수 있습니다. MainNavigator 코멘트와 동일 제언이므로, 이 라우트에 대해서도 식별자 기반 전달(또는 SavedStateHandle) 대안 검토를 권장합니다.
🧹 Nitpick comments (42)
core/domain/letter/build.gradle.kts (2)
6-6: namespace 설정 스타일 통일 제안다른 모듈과 일관성을 위해 block 형태를 권장합니다. 동작 차이는 없으나 가독성이 좋습니다.
-android.namespace = "com.nexters.emotia.core.domain.letter" +android { + namespace = "com.nexters.emotia.core.domain.letter" +}
11-11: core/domain/letter 모듈에서 Koin DI 의존성 제거 권장스크립트(
rg -n -C2 --glob 'core/domain/letter/**' -e 'org\.koin' -e 'Koin' -e 'module\s*\(') 실행 결과 해당 모듈 내에 Koin 관련 참조가 전혀 발견되지 않았습니다.
Clean Architecture 관점에서 도메인 계층은 순수 모델/규약만 포함하고 외부 프레임워크 의존성을 제거하는 것이 유지보수성과 재사용성 측면에서 유리하므로, 아래와 같이libs.koin.core의존성을 제거할 것을 권장드립니다.적용 예시:
kotlin { sourceSets { commonMain.dependencies { - implementation(libs.koin.core) } } }core/network/src/commonMain/kotlin/com/nexters/emotia/network/dto/request/SendLetterRequest.kt (1)
7-9: 백엔드 스키마 명세 일치 여부 확인 및 소소한 견고성 보강 제안
- 서버가 snake_case를 사용한다면 명시적 @SerialName을 붙여 스키마 변경(케이싱 차이)에 강하게 만들 수 있습니다.
- ID 스키마가 Long일 가능성이 있으면(특히 DB PK 직결) Int → Long 전환을 검토해주세요. 현재 네트워크/리모트/레포 계층 모두 일관 타입이어야 합니다.
예시(스키마가 camelCase 그대로라면 굳이 추가할 필요는 없습니다):
package com.nexters.emotia.network.dto.request import kotlinx.serialization.Serializable +import kotlinx.serialization.SerialName @Serializable data class SendLetterRequest( - val chatRoomId: Int, - val fairyId: Int, - val contents: String, + @SerialName("chatRoomId") val chatRoomId: Int, + @SerialName("fairyId") val fairyId: Int, + @SerialName("contents") val contents: String, )feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterState.kt (2)
9-10: 에러 표현을 문자열 대신 타입화(예: UiText/UiMessage)로 개선 고려문자열 기반 에러는 현지화/메시지 재사용/복구 액션 제공이 어렵습니다. KMP/Compose 리소스 전략에 맞춰 에러를 래핑한 값 객체를 도입하면 이점이 큽니다.
예시:
-data class LetterState( +data class LetterState( val fairyId: Int = 0, val fairyName: String = "", val fairyImage: String = "", val chatRoomId: Int = 0, val contents: String = "", val isLoading: Boolean = false, - val error: String? = null, + val error: UiMessage? = null, )추가(별도 파일):
@Immutable data class UiMessage( val key: String, // 에러 코드 or 문자열 키 val args: List<String> = emptyList(), val cause: Throwable? = null, )또는 Compose Multiplatform에서 사용하는 UiText 패턴(리소스/직접 문자열 혼용)을 적용할 수 있습니다.
8-8: 텍스트 입력 지연(작성자 코멘트) 개선 힌트포커스 변경 시 커서/선택 상태 때문에 지연이 보인다면 UI 레벨에서 TextFieldValue를 사용해 selection/ime 상태를 함께 관리하는 방법도 있습니다. 상태 폭발을 피하려면 VM에는 String, 화면 내부 상태로 TextFieldValue를 유지하는 절충안을 추천합니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairyIntent.kt (1)
10-12: Intent와 SideEffect의 네이밍 충돌 가능성 — 분리/정규화 제안
NavigateToOnBoarding,NavigateToChatting가 Intent와 SideEffect 양쪽에 동일 명칭으로 존재하면 로그/분기 추적 시 혼동을 야기합니다. 이벤트(사용자 액션)를 표현하는 Intent는RequestNavigateToOnBoarding/RequestNavigateToChatting또는OnClickOnBoarding처럼 “요청/액션”으로, 실제 내비 이동은 SideEffect로 구분하는 네이밍을 권장합니다. 또한 Fairy → Letter → Result 플로우라면NavigateToLetterIntent도 함께 두어 일관성을 맞추는 것이 유지보수에 유리합니다.core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt (3)
94-102: placeholder 조건 중복 — 단순화 권장
OutlinedTextField는 기본적으로 값이 비어있을 때만placeholder슬롯을 그립니다.value.isEmpty()체크는 중복이므로 제거 가능합니다.- placeholder = { - if (placeholder.isNotEmpty() && value.isEmpty()) { - Text( - text = placeholder, - fontSize = 14.sp, - color = state.getPlaceholderColor() - ) - } - }, + placeholder = { + if (placeholder.isNotEmpty()) { + Text( + text = placeholder, + fontSize = 14.sp, + color = state.getPlaceholderColor() + ) + } + },
52-56: 색상 헬퍼 단순화 가능
getTextColor,getPlaceholderColor는 상태와 무관하게 항상 동일한 색을 반환합니다. 상태 인지 필요가 없다면 일반 함수로 변경하거나 상수/로컬 값으로 인라인하여 의존성을 줄일 수 있습니다. 다만 추후 상태별 커스터마이즈 계획이 있다면 현 구조 유지도 타당합니다.Also applies to: 58-62
65-71: 입력 UX 확장 옵션 제안(minLines/keyboardOptions/supportingText 등)멀티라인 특성상 최소 줄수, 키보드 액션, 글자수 카운트/제한, 스크롤 동작 등의 확장 파라미터를 노출하면 재사용성이 크게 향상됩니다. API 안정성 관점에서 별도 PR로 점진 적용을 제안드립니다.
Also applies to: 84-118
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/mapper/LetterMapper.kt (1)
6-6: 모듈 외부 노출 최소화 — 가시성을 internal로 좁히는 것을 고려매퍼는 data 모듈 내부 구현 세부에 가깝습니다. 외부 API 표면적 축소와 바이너리 안정성을 위해
internal가시성을 권장합니다.-fun SendLetterResponse.toDomain(): Letter { +internal fun SendLetterResponse.toDomain(): Letter {core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/di/DataLetterModule.kt (1)
10-14: Koin DSL 간소화 제안 — singleOf/bind 사용으로 보일러플레이트 축소동일 동작이지만 선언을 단순화할 수 있습니다.
-val dataLetterModule = module { - single<LetterApiService> { LetterApiService(get()) } - single<LetterRemoteDataSource> { LetterRemoteDataSourceImpl(get()) } - single<LetterRepository> { LetterRepositoryImpl(get()) } -} +val dataLetterModule = module { + singleOf(::LetterApiService) + singleOf(::LetterRemoteDataSourceImpl) { bind<LetterRemoteDataSource>() } + singleOf(::LetterRepositoryImpl) { bind<LetterRepository>() } +}core/domain/letter/src/commonMain/kotlin/com/nexters/emotia/core/domain/letter/repository/LetterRepository.kt (3)
5-11: Result 반환 시 실패 모델/취소 전파 기준을 명확히 합의해 주세요
- 서버의 논리적 실패(success=false, 4xx/5xx, 비즈니스 에러 코드 등)를
Result.failure로 매핑할지, 도메인 전용 에러(예:sealed interface LetterError)로 감쌀지 기준이 필요합니다.- 코루틴
CancellationException은 잡지 말고 상위로 전파해야 합니다(UX에서 재시도/중단 구분 필요).- 호출부(ViewModel)의 상태머신(Orbit 등)에서 어떤 에러를 사용자 메시지로 변환할지 일관성을 확보하세요.
5-11: 계약(KDoc)으로 파라미터 제약과 실패 시맨틱을 문서화해 주세요호출 측 실수를 줄이기 위해
chatRoomId/fairyId의 유효 범위,contents의 최소/최대 길이, 공백-only 금지 여부 등을 KDoc에 남기는 것을 권장합니다.아래처럼 간단한 KDoc을 추가하는 수준만으로도 큰 도움이 됩니다.
interface LetterRepository { + /** + * 요정에게 편지를 전송합니다. + * + * @param chatRoomId 유효한 채팅방 ID (양수 권장) + * @param fairyId 대상 요정 ID (양수 권장) + * @param contents 편지 본문. 공백-only 금지, 최대 길이 정책 명시 필요(e.g. 1..2000) + * @return 성공 시 생성된 [Letter], 실패 시 네트워크/비즈니스 에러를 포함한 [Result.failure] + * + * 실패 규칙: + * - HTTP 4xx/5xx 및 API success=false → failure로 매핑 + * - [CancellationException]은 전파(잡지 않음) + */ suspend fun sendLetter( chatRoomId: Int, fairyId: Int, contents: String ): Result<Letter> }
5-11: 리포지토리 단위 테스트 케이스 제안
- 성공: 정상 응답 →
Result.success(Letter)매핑- 비즈니스 실패: API success=false or 에러코드 →
Result.failure(DomainError.Business(...))- 네트워크/직렬화 실패 →
Result.failure(DomainError.Network(...))- 취소 → 전파 확인(잡히지 않음)
필요하시면 테스트 스캐폴딩을 생성해 드릴게요.
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/navigation/ChattingNavigation.kt (2)
12-14: 람다 단순 포워딩은 제거 가능합니다동일 시그니처를 그대로 전달하므로 축약할 수 있습니다.
- ChattingScreen( - onNavigateToFairy = { id, name, image, silhouetteImage, chatRoomId -> - onNavigateToFairy(id, name, image, silhouetteImage, chatRoomId) - } - ) + ChattingScreen( + onNavigateToFairy = onNavigateToFairy + )
8-9: 콜백 시그니처 가독성·안전성 향상: typealias(또는 DTO) 도입 제안튜플 형태
(Int, String, String, String, Int)는 순서 실수가 발생하기 쉽습니다. 간단히 typealias를 쓰거나,FairyNavArgs데이터 클래스로 의도를 명확히 하세요.-fun NavGraphBuilder.chattingScreen( - onNavigateToFairy: (Int, String, String, String, Int) -> Unit, +fun NavGraphBuilder.chattingScreen( + onNavigateToFairy: FairyNav, ) {추가 코드(동일 파일 상단 등 적절한 위치):
typealias FairyNav = (fairyId: Int, fairyName: String, fairyImage: String, silhouetteImage: String, chatRoomId: Int) -> Unitfeature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterSideEffect.kt (1)
5-9: NavigateToResult에 전체 Letter 객체를 싣는 대신 최소 페이로드로 전환을 고려해 주세요SideEffect는 “이동” 트리거 성격이 강해, 큰 도메인 객체 전달보다
letterId등 최소 정보만 넘기고, 화면 진입 시 조회하거나 Shared VM/SavedState로 전달하는 편이 결합도를 낮춥니다.
변경 파급이 크면 보류 가능하나, 장기적으로 내비게이션 계층이 도메인 모델에 직접 의존하지 않도록 하는 것을 권장합니다.core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSource.kt (1)
5-11: 데이터소스 계약에 KDoc 추가 및 DTO 의존 경계 점검
- 현재 인터페이스가 네트워크 DTO(
SendLetterResponse)에 직접 의존합니다. data 계층이 network 모듈에 의존하는 설계라면 OK지만, 향후 교체 가능성을 고려하면 data 내부 DTO(또는 mapper 반환)로 추상화하는 선택지도 있습니다.- 최소한 실패 시맨틱과 파라미터 제약을 KDoc으로 문서화해 주세요.
interface LetterRemoteDataSource { - suspend fun sendLetter( + /** + * 원격 API로 편지를 전송합니다. 네트워크/직렬화 예외는 상위로 전파합니다. + * @throws IOException, SerializationException 등 + */ + suspend fun sendLetter( chatRoomId: Int, fairyId: Int, contents: String ): SendLetterResponse }core/network/src/commonMain/kotlin/com/nexters/emotia/network/service/LetterApiService.kt (1)
15-22: 엔드포인트 경로 상수화로 드리프트 방지경로 문자열을 상수로 추출하면 변경/검색이 용이합니다. 모듈 공통 prefix(
api/v1) 정책이 있다면 재사용하세요.class LetterApiService( private val network: EmotiaNetwork, ) { + private companion object { + const val PATH_SEND_LETTER = "api/v1/letters" + } suspend fun sendLetter( chatRoomId: Int, fairyId: Int, contents: String ): SendLetterResponse { return network.post( - path = "api/v1/letters", + path = PATH_SEND_LETTER, body = SendLetterRequest( chatRoomId = chatRoomId, fairyId = fairyId, contents = contents ) ) } }core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSourceImpl.kt (1)
6-9: 가시성 축소로 모듈 경계 안정화 제안 (internal 권장)이 구현체는 DI 모듈 내부에서만 필요합니다. 외부 노출을 줄이기 위해
internal로 가시성을 낮추면 API 표면을 작게 유지할 수 있습니다.적용 예시:
-class LetterRemoteDataSourceImpl( +internal class LetterRemoteDataSourceImpl( private val apiService: LetterApiService, ) : LetterRemoteDataSource {feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyViewModel.kt (1)
33-39: 네이밍 일관성: OnBoarding → Onboarding 권장프로젝트 전반의 표기와 용어집에 맞춰
Onboarding으로 통일하는 것을 권장합니다. 이 변경은FairySideEffect와 사용처 전반에 ripple이 있으므로 일괄 치환 시 주의해 주세요.본 파일 내 변경 예:
- postSideEffect(FairySideEffect.NavigateToOnBoarding) + postSideEffect(FairySideEffect.NavigateToOnboarding)SideEffect 정의 파일 수정은 별도 코멘트에 포함했습니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairySideEffect.kt (2)
3-7: 네이밍 일관성 정리: OnBoarding → Onboarding영문 표기 통일을 위해
NavigateToOnBoarding를NavigateToOnboarding으로 변경하는 것을 권장합니다. 사용처(FairyViewModel 등)도 함께 치환해 주세요.-sealed interface FairySideEffect { - data object NavigateToOnBoarding : FairySideEffect - data object NavigateToChatting : FairySideEffect - data class ShowError(val message: String) : FairySideEffect -} +sealed interface FairySideEffect { + data object NavigateToOnboarding : FairySideEffect + data object NavigateToChatting : FairySideEffect + data class ShowError(val message: String) : FairySideEffect +}
6-6: 에러 메시지 String 직접 전달 대신 타입화 고려
ShowError(String)는 i18n/분류/로깅에 취약합니다. 메시지와 원인(cause)을 분리하거나, 도메인/프리젠테이션 에러 타입을 도입하는 것을 권장합니다.경량 개선 예:
- data class ShowError(val message: String) : FairySideEffect + data class ShowError( + val message: String? = null, + val cause: Throwable? = null + ) : FairySideEffect혹은 코드/리소스 키 기반의 에러 타입(예:
sealed interface UiError { data object Network : UiError; data class Business(val code: String) : UiError })로 확장성을 확보할 수 있습니다.core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/repository/LetterRepositoryImpl.kt (3)
20-22: 에러 메시지 null/공백 안전성 보강
response.error가 null/공백일 수 있다면 현재 예외 메시지는"API 요청 실패: null"이 됩니다. 안전하게 디폴트 메시지를 제공해 주세요.- if (!response.success) { - throw IllegalStateException("API 요청 실패: ${response.error}") - } + if (!response.success) { + val message = response.error?.takeIf { it.isNotBlank() } ?: "API 요청 실패" + throw IllegalStateException(message) + }
25-27: println 대신 멀티플랫폼 로그 추상화 사용 권장
println은 플랫폼·환경에 따라 노이즈가 되거나 유실될 수 있습니다. Napier/Kermit 등 멀티플랫폼 로거 사용을 권장합니다.예시(Napier):
+import io.github.aakira.napier.Napier ... - }.onFailure { exception -> - println("편지 전송 실패: ${exception.message}") - } + }.onFailure { exception -> + Napier.e("편지 전송 실패", exception) + }의존성 추가가 부담된다면 최소한 공용 로깅 헬퍼를 모듈에 두고 교체 가능하도록 추상화하는 것도 방법입니다.
12-28: sendLetter 오류 분류 및 단위 테스트 추가 필요현재
LetterRepositoryImpl.sendLetter는 비즈니스 실패(success=false)와 네트워크/예외 발생을 모두Result.failure로만 처리하고 있습니다.
이를 분리하면 UI 표시 메시지와 리트라이 정책을 보다 명확하게 설계할 수 있습니다. 또한, 해당 로직에 대한 단위 테스트가 아직 존재하지 않아 안정성이 부족합니다.수정 및 검토가 필요한 위치:
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/repository/LetterRepositoryImpl.kt
• 비즈니스 실패 시 별도 예외(혹은 커스텀 에러 타입) 던지기
• 네트워크/전송 예외와 분리된 처리 로직 추가- 단위 테스트 파일 (예:
LetterRepositoryImplTest.kt) 추가
• 성공 케이스:success=true→ 도메인 매핑(toDomain()) 검증
• 비즈니스 실패 케이스:success=false+error메시지 보존 검증
• 전송 예외 케이스:remoteDataSource.sendLetter예외 발생 시Result.failure전파 및 로깅 호출 검증feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt (1)
73-76: onNavigateToFairy 호출부 전수 확인 완료 — 파라미터명 일관성 개선 권장모든 호출부에서 확장된 시그니처(
fairyId, fairyName, fairyImage, silhouetteImage, chatRoomId)가 올바르게 반영된 것을 확인했습니다.
다만, FairyRoute의 필드 네이밍(imageUrl,silhouetteImageUrl)과 일관성을 위해 파라미터명을 아래와 같이 바꾸시면 가독성과 유지보수성에 도움이 될 것 같습니다.•
fairyImage→fairyImageUrl
•silhouetteImage→silhouetteImageUrl— 적용 위치
- feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt (정의부)
- feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/navigation/ChattingNavigation.kt
- feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavHost.kt
feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavigator.kt (1)
51-56: navigateToLetterResult에서 contents 직접 전달 시 경로 길이/인코딩 취약성 검토
- 현재
navigateToLetterResult(fairyName: String, contents: String)는navController.navigateTo(FairyRoute.Result(fairyName, contents))로 다이렉트 전달하고 있어, 매우 긴 텍스트나 특수 문자(줄바꿈, 공백, UTF-8 심볼 등)가 URI 경로 직렬화 과정에서 잘리거나 인코딩 오류를 일으킬 수 있습니다.gradle/libs.versions.toml에서androidx-navigation-compose가version.ref = "androidx-navigation"으로 관리되고 있음을 확인했습니다(버전 명시 필요) .- 코드 내 별도 길이 제한이나 밸리데이션 로직이 발견되지 않습니다.
대안(선택적 리팩터링 권장)
- SavedStateHandle 혹은 Shared ViewModel/Repository에
Letter엔티티 전체를 저장한 뒤, 라우팅에는 해당 엔티티의 식별자(ID)만 전달.- 대상 화면에서 ID로 조회해 필요한 데이터를 불러오는 방식을 도입하여 URI 길이·인코딩 이슈를 회피할 수 있습니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.kt (1)
16-29: FairyMain 경로에서 silhouetteImageUrl 미전달 확인 및 정리 권고
FairyRoute.FairyMain에는 silhouetteImageUrl 필드가 선언되어 있으나,
–feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.kt의composable<FairyRoute.FairyMain>블록에서FairyScreen호출 시 전달되지 않음
FairyScreen의 초기화 로직(LaunchedEffect) 또한FairyIntent.InitializeFairy(fairyId, fairyName, fairyImage)만 호출하여,
–feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairyIntent.kt의InitializeFairy생성자(val fairySilhouetteImage: String) 파라미터에 silhouetteImageUrl이 아닌 imageUrl이 전달되고 있음Optional Refactor 제안
- 일관성 유지 및 향후 silhouette 이미지 사용을 위해
FairyNavigation.kt에서FairyScreen호출 시silhouetteImageUrl = data.silhouetteImageUrl파라미터 추가FairyScreen시그니처 및LaunchedEffect호출부에 silhouetteImageUrl 인자 반영FairyIntent.InitializeFairy호출 시에도 silhouetteImageUrl 전달- 만약 silhouetteImageUrl이 실제로 사용되지 않는다면,
FairyRoute,FairyIntent등 선언부에서 해당 필드를 제거하여 불필요한 코드를 정리feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterViewModel.kt (2)
49-55: 중복 전송 방지 가드 추가 제안UI에서 버튼을
isLoading으로 비활성화하고 있지만, 빠른 연속 탭/재진입 상황을 완전히 막지 못할 수 있습니다.sendLetter()진입 초기에state.isLoading가드를 두면 안전합니다.적용 diff:
private fun sendLetter() = intent { - val currentState = state + val currentState = state + + if (currentState.isLoading) return@intent if (currentState.contents.isBlank()) return@intent
67-76: 에러 메시지 null 안전 및 UX 보강
exception.message가 null인 경우"…: null"이 노출됩니다. 기본 문구로 폴백해 주세요.적용 diff:
- .onFailure { exception -> - val errorMessage = "편지 전송에 실패했습니다: ${exception.message}" + .onFailure { exception -> + val detail = exception.message?.takeIf { it.isNotBlank() } + val errorMessage = detail?.let { "편지 전송에 실패했습니다: $it" } + ?: "편지 전송에 실패했습니다. 잠시 후 다시 시도해 주세요." reduce { state.copy( isLoading = false, error = errorMessage ) } postSideEffect(LetterSideEffect.ShowError(errorMessage)) }feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/result/ResultScreen.kt (4)
80-84: 장식용 Divider 이미지 접근성 개선시각적 구분자 이미지는 보조기기에 읽힐 필요가 없습니다.
contentDescription = null로 설정해 스크린리더 노이즈를 줄여 주세요.적용 diff:
- Image( - painter = painterResource(Res.drawable.img_divider_above), - contentDescription = "Divider", - ) + Image( + painter = painterResource(Res.drawable.img_divider_above), + contentDescription = null, + ) ... - Image( - painter = painterResource(Res.drawable.img_divider_below), - contentDescription = "Divider", - ) + Image( + painter = painterResource(Res.drawable.img_divider_below), + contentDescription = null, + )Also applies to: 96-99
87-92: 긴 본문(contents) 처리 보강 제안긴 텍스트가 작은 단말에서 카드 영역을 넘어설 수 있습니다. 최대 줄수/오버플로우 또는 내부 스크롤을 고려해 주세요.
예:
maxLines = 8+overflow = TextOverflow.Ellipsis, 또는verticalScroll(rememberScrollState())를 카드 내부 Column에 적용
63-76: 하드코딩 문자열 리소스화 제안문구들이 코드에 직접 박혀 있어 다국어/카피 수정이 어렵습니다. Compose Multiplatform 리소스(
Res.string.*,stringResource(...))로 이전을 권장합니다.Also applies to: 103-108, 114-117, 121-126
104-108: 문구 교정(사소하지만 깔끔하게)
"마음 속"→"마음속"이 맞춤법상 권장 표기입니다.적용 diff:
- text = "- 너의 마음 속 ${fairyName}으로부터", + text = "- 너의 마음속 ${fairyName}으로부터",feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (4)
124-129: 배경 이미지는 장식용 — 스크린리더 제외배경은 정보 전달 요소가 아니므로
contentDescription = null로 설정해 주세요.적용 diff:
- Image( + Image( painter = painterResource(Res.drawable.img_letter_background), - contentDescription = "Background Image", + contentDescription = null, modifier = Modifier.fillMaxWidth(), contentScale = ContentScale.FillWidth )
131-147: 이미지 contentDescription 개선
"$fairyImage image"는 URL/경로가 노출될 수 있고 접근성에도 부적절합니다. 이름 기반 설명으로 바꾸거나 장식이라면null을 사용하세요.적용 diff(의미 있는 대체텍스트):
- contentDescription = "$fairyImage image", + contentDescription = "${fairyName} 이미지",장식 처리시:
- contentDescription = "$fairyImage image", + contentDescription = null,
81-95: 에러 사이드이펙트 처리 비워짐
LetterSideEffect.ShowError가 TODO 상태입니다. 최소한 스낵바/토스트 등으로 사용자에게 오류를 노출해 주세요. 필요 시 간단한 헬퍼를 함께 드릴 수 있습니다.
100-115: 키보드 표시 감지 로직 단순화 제안IME 높이 변화를 감지해 스크롤을 애니메이션하는 로직이 현재 “버벅임”의 원인일 수 있습니다(작성자 코멘트와 부합).
imePadding()+ 포커스 시BringIntoViewRequester를 사용하는 방식으로 전환하면 부하와 깜빡임을 줄일 수 있습니다.feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt (3)
87-91: 탭 차단 로직과 전체 클릭 네비게이션(LGTM, 사소한 접근성 보완 제안)
- 확장 애니메이션 중 Canvas로 터치를 차단하는 구조는 합리적입니다.
- 화면 전체 클릭으로 Letter로 이동하는 패턴도 의도대로 동작합니다.
사소한 보완:
- 전체 클릭 영역에
indication = null을 명시하거나, 세맨틱 역할(버튼)을 부여하면 접근성이 좋아집니다.Also applies to: 132-157
93-98: 배경 이미지는 장식용 — contentDescription 제거배경 이미지는 정보를 담지 않으므로 스크린리더 대상에서 제외해 주세요.
적용 diff:
Image( painter = painterResource(Res.drawable.img_result_background), - contentDescription = "Background Image", + contentDescription = null, modifier = Modifier.fillMaxHeight(), contentScale = ContentScale.Crop )
68-76: 에러 사이드이펙트 TODO 처리
FairySideEffect.ShowError처리부가 비어 있습니다. 최소 알림 노출 또는 로깅을 추가해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
core/designsystem/src/commonMain/composeResources/drawable/img_letter_background.pngis excluded by!**/*.png
📒 Files selected for processing (40)
composeApp/build.gradle.kts(1 hunks)composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt(2 hunks)core/data/letter/build.gradle.kts(1 hunks)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSource.kt(1 hunks)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSourceImpl.kt(1 hunks)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/di/DataLetterModule.kt(1 hunks)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/mapper/LetterMapper.kt(1 hunks)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/repository/LetterRepositoryImpl.kt(1 hunks)core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt(1 hunks)core/domain/letter/build.gradle.kts(1 hunks)core/domain/letter/src/commonMain/kotlin/com/nexters/emotia/core/domain/letter/entity/Letter.kt(1 hunks)core/domain/letter/src/commonMain/kotlin/com/nexters/emotia/core/domain/letter/repository/LetterRepository.kt(1 hunks)core/network/src/commonMain/kotlin/com/nexters/emotia/network/dto/request/SendLetterRequest.kt(1 hunks)core/network/src/commonMain/kotlin/com/nexters/emotia/network/dto/response/SendLetterResponse.kt(1 hunks)core/network/src/commonMain/kotlin/com/nexters/emotia/network/service/LetterApiService.kt(1 hunks)feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt(2 hunks)feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/navigation/ChattingNavigation.kt(1 hunks)feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavHost.kt(3 hunks)feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavigator.kt(2 hunks)feature/result/build.gradle.kts(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/ResultViewModel.kt(0 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/contract/ResultIntent.kt(0 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/contract/ResultSideEffect.kt(0 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/di/ResultModule.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt(6 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyViewModel.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairyIntent.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairySideEffect.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairyState.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterViewModel.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterIntent.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterSideEffect.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterState.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyRoute.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/ResultNavigation.kt(0 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/ResultRoute.kt(0 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/result/ResultScreen.kt(1 hunks)settings.gradle.kts(1 hunks)
💤 Files with no reviewable changes (5)
- feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/ResultViewModel.kt
- feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/ResultNavigation.kt
- feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/ResultRoute.kt
- feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/contract/ResultIntent.kt
- feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/contract/ResultSideEffect.kt
🧰 Additional context used
📓 Path-based instructions (4)
**/build.gradle*
📄 CodeRabbit inference engine (CLAUDE.md)
Use libs.* references for dependency declarations
Files:
core/domain/letter/build.gradle.ktscomposeApp/build.gradle.ktscore/data/letter/build.gradle.ktsfeature/result/build.gradle.kts
composeApp/build.gradle*
📄 CodeRabbit inference engine (CLAUDE.md)
composeApp/build.gradle*: Common dependencies in commonMain.dependencies
Platform-specific dependencies in respective androidMain/iosMain blocks
Static iOS frameworks for better performance
Resource exclusions configured for Android packaging
Files:
composeApp/build.gradle.kts
composeApp/src/{commonMain,androidMain,iosMain}/kotlin/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
composeApp/src/{commonMain,androidMain,iosMain}/kotlin/**/*.kt: Follow expect/actual pattern for platform-specific code
Use @composable functions with @Preview annotations
Leverage Compose resources via Res.drawable.* and Res.string.*
Use expect/actual for platform-specific APIs (camera, notifications, etc.)
Use Compose Resources for strings, images, and other assets
Files:
composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt
composeApp/src/commonMain/kotlin/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
composeApp/src/commonMain/kotlin/**/*.kt: Keep business logic, data models, and network calls in commonMain
Implement navigation logic in common code when possible
Use common state management patterns (ViewModel, StateFlow)
Files:
composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt
🧠 Learnings (9)
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/build.gradle* : Platform-specific dependencies in respective androidMain/iosMain blocks
Applied to files:
core/domain/letter/build.gradle.ktscomposeApp/build.gradle.ktscore/data/letter/build.gradle.kts
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/{commonMain,androidMain,iosMain}/kotlin/**/*.kt : Follow expect/actual pattern for platform-specific code
Applied to files:
core/domain/letter/build.gradle.kts
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/{commonMain,androidMain,iosMain}/kotlin/**/*.kt : Use Compose Resources for strings, images, and other assets
Applied to files:
core/domain/letter/build.gradle.ktscomposeApp/build.gradle.kts
📚 Learning: 2025-08-10T08:22:45.334Z
Learnt from: sxunea
PR: Nexters/team-ace-client#22
File: feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt:26-31
Timestamp: 2025-08-10T08:22:45.334Z
Learning: In Kotlin Multiplatform (KMP) projects, `org.koin.compose.viewmodel.koinViewModel()` from Koin is now compatible with commonMain source sets and can be used across all platforms, not just Android.
Applied to files:
core/domain/letter/build.gradle.kts
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/build.gradle* : Common dependencies in commonMain.dependencies
Applied to files:
composeApp/build.gradle.kts
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/commonMain/kotlin/**/*.kt : Keep business logic, data models, and network calls in commonMain
Applied to files:
composeApp/build.gradle.kts
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/commonMain/kotlin/**/*.kt : Implement navigation logic in common code when possible
Applied to files:
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.ktfeature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/navigation/ChattingNavigation.ktfeature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavigator.ktfeature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavHost.kt
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/commonMain/kotlin/**/*.kt : Use common state management patterns (ViewModel, StateFlow)
Applied to files:
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/{commonMain,androidMain,iosMain}/kotlin/**/*.kt : Leverage Compose resources via Res.drawable.* and Res.string.*
Applied to files:
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt
🧬 Code graph analysis (10)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.kt (3)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt (1)
FairyScreen(50-159)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (1)
LetterScreen(55-236)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/result/ResultScreen.kt (1)
ResultScreen(33-128)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairyIntent.kt (1)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/contract/ResultIntent.kt (2)
fairyId(3-13)fairyId(4-8)
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt (1)
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/theme/Theme.kt (1)
EmotiaTheme(13-28)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (2)
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt (1)
EmotiaMultiLineTextField(64-119)core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaButton.kt (1)
EmotiaButton(16-42)
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/di/DataLetterModule.kt (1)
core/network/src/commonMain/kotlin/com/nexters/emotia/network/EmotiaNetwork.kt (1)
get(87-91)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/result/ResultScreen.kt (2)
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaButton.kt (1)
EmotiaButton(16-42)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/ResultScreen.kt (1)
ResultScreen(46-143)
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/navigation/ChattingNavigation.kt (1)
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt (1)
ChattingScreen(71-422)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterSideEffect.kt (1)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/contract/ResultSideEffect.kt (1)
message(3-7)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterViewModel.kt (5)
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSource.kt (2)
sendLetter(5-11)sendLetter(6-10)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSourceImpl.kt (1)
sendLetter(10-20)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/repository/LetterRepositoryImpl.kt (1)
sendLetter(12-28)core/domain/letter/src/commonMain/kotlin/com/nexters/emotia/core/domain/letter/repository/LetterRepository.kt (2)
sendLetter(5-11)sendLetter(6-10)core/network/src/commonMain/kotlin/com/nexters/emotia/network/service/LetterApiService.kt (1)
sendLetter(10-23)
feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavHost.kt (1)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.kt (1)
fairyScreen(10-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Firebase App Distribution
- GitHub Check: build
🔇 Additional comments (33)
core/domain/letter/build.gradle.kts (1)
1-4: libs. 의존성/플러그인 참조 방식 준수 — 좋습니다*
- plugins와 commonMain.dependencies에서 모두 libs 카탈로그를 사용하고 있어 가이드에 부합합니다.
Also applies to: 10-12
core/network/src/commonMain/kotlin/com/nexters/emotia/network/dto/request/SendLetterRequest.kt (1)
5-10: 간결한 요청 DTO 정의 — 직관적입니다@serializable 적용과 필드 구조 모두 명확합니다. 멀티라인 contents도 JSON 직렬화에 문제 없습니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterState.kt (1)
3-11: UI 상태 모델 구성 적절
- 필수 식별자(fairyId, chatRoomId)와 UI 제어 플래그(isLoading, error) 기본값 구성이 합리적입니다.
- 멀티라인 입력(contents) 상태 보관도 명확합니다.
core/network/src/commonMain/kotlin/com/nexters/emotia/network/dto/response/SendLetterResponse.kt (1)
5-10: 응답 DTO 스키마 명확 — 직렬화 준비 OKsuccess/data/error 삼분 구조가 이해하기 쉽고, @serializable 적용도 적절합니다.
Also applies to: 12-18
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/contract/FairyState.kt (2)
3-11: 요정 실루엣 이미지 필드 추가 적절FairyState로의 명명/패키지 이동과 fairySilhouetteImage 추가가 UI 요구사항에 부합해 보입니다. 기본값 구성도 일관적입니다.
1-11: ResultState 참조 제거 및 FairyState 적용 완료 확인
feature/result디렉토리 전체에서ResultState에 대한 참조는 전혀 발견되지 않았으며, 오직FairyState만 정상적으로 조회되었습니다.
네비게이션, ViewModel, 화면, DI 등 모든 관련 영역에서ResultState → FairyState전환이 누락 없이 완료된 것으로 확인됩니다.core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt (1)
105-116: cursorColor 파라미터 호환성 점검(Compose M3 버전별 차이 존재)일부 Compose 버전에서는
OutlinedTextFieldDefaults.colors가cursorColor가 아닌cursorBrush만 지원합니다. 멀티플랫폼/버전 호환성을 위해 브러시 기반 파라미터 사용으로 전환하는 것을 권장합니다. 적용 전 컴파일 확인 바랍니다.@@ - colors = OutlinedTextFieldDefaults.colors( + colors = OutlinedTextFieldDefaults.colors( focusedTextColor = state.getTextColor(), unfocusedTextColor = state.getTextColor(), disabledTextColor = state.getTextColor(), focusedBorderColor = state.getBorderColor(), unfocusedBorderColor = state.getBorderColor(), disabledBorderColor = state.getBorderColor(), focusedContainerColor = Color.Transparent, unfocusedContainerColor = Color.Transparent, disabledContainerColor = Color.Transparent, - cursorColor = colors.primaryLight + cursorBrush = SolidColor(colors.primaryLight) ),추가 import가 필요합니다:
import androidx.compose.ui.graphics.SolidColorAlso applies to: 1-19
composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt (2)
4-4: dataLetterModule DI 포함: 적절한 위치에 잘 추가됨네트워크/플랫폼 → 데이터 계층 → 도메인/피처 순으로 include가 이어지는 현재 순서 내에서
dataLetterModule추가가 자연스럽습니다. 상위 의존성(coreNetworkModule) 이후에 배치된 점도 적절합니다.Also applies to: 18-18
18-18: 레터 도메인 DI 모듈 확인 및 등록 완료
- settings.gradle.kts에
:core:data:letter모듈이 포함되어 있습니다 (라인 39)composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt에서
import com.nexters.emotia.core.data.letter.di.dataLetterModule(라인 4)dataLetterModule이includes(...)목록에 정상 등록되어 있습니다 (라인 18)core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/di/DataLetterModule.kt에val dataLetterModule = module { … }선언이 있어 심볼 경로·가시성에도 문제가 없습니다따라서 별도 추가 작업 없이 현 구성 유지하시면 됩니다.
composeApp/build.gradle.kts (1)
36-36: 설정 및 DI 구성 확인 완료
settings.gradle.kts(39행)에:core:data:letter모듈이 포함되어 있고,
composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt(4행·18행)에서dataLetterModule이 정상 임포트·등록된 것을 확인했습니다.
추가 조치 불필요합니다.feature/result/build.gradle.kts (1)
11-11: 검증 완료: core/domain/letter 모듈이 settings.gradle.kts에 포함되어 있습니다.settings.gradle.kts 44번째 줄에
include(":core:domain:letter")가 있어, 신규 도메인 모듈이 정상적으로 포함된 것을 확인했습니다. 추가 작업은 필요 없습니다.settings.gradle.kts (1)
39-39: 새 서브프로젝트 include 정합성 확인 — 빌드 그래프 확장 OK
:core:data:letter,:core:domain:letter추가가 다른 모듈 의존과 일관되며, 도메인↔데이터 계층 분리에도 부합합니다. 별도 조치 없이 빌드 설정 측면에서는 합리적으로 보입니다.Also applies to: 44-44
core/domain/letter/src/commonMain/kotlin/com/nexters/emotia/core/domain/letter/entity/Letter.kt (1)
3-8: 도메인 모델 단순·명료 — 현 시점 요구에 적합필수 필드를 non-null로 강제한 선택이 명확합니다. 이후 식별자 강화(예:
@JvmInline value class FairyId(val value: Int)), 이미지 URL 유효성(별도 VO) 등은 실제 사용처가 늘어날 때 단계적으로 고려해도 충분합니다.core/data/letter/build.gradle.kts (2)
1-5: 플러그인 선언 방식 적절 — 버전 카탈로그(alias) 활용 OK
libs.plugins.*alias를 사용해 플러그인을 일관되게 선언했습니다. 사내 가이드(versions catalog 기반) 준수로 보입니다.
11-29: 소스셋별 의존 배치 적절 — 플랫폼 전용 드라이버 분리 OK
androidMain: OkHttp 드라이버iosMain: Darwin 드라이버commonMain: 공통 Ktor/Serialization/Koin/Datetime 및 내부 모듈장기 학습 메모(“플랫폼 전용 의존은 각 androidMain/iosMain 블록에 배치”)와도 일치합니다. 추가 조치는 없습니다.
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/di/DataLetterModule.kt (2)
10-14: 의존 그래프 구성 명료 — API→DS→Repo 단일톤 체인 적합
LetterApiService → LetterRemoteDataSource → LetterRepository의 단일톤 바인딩 계층이 데이터 흐름에 적합합니다. 현재 스코프로 충분합니다.
10-14: dataLetterModule DI 및 Gradle 의존성 반영 확인 완료– composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt
•import com.nexters.emotia.core.data.letter.di.dataLetterModule확인
• DI 모듈 리스트에dataLetterModule등록(18번째 줄) 확인– composeApp/build.gradle.kts
•implementation(projects.core.data.letter)의존성 추가(36번째 줄) 확인위 사항으로 앱 DI와 Gradle 의존성 모두 정상 반영되었으니, 해당 리뷰 코멘트는 해결되었습니다.
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/navigation/ChattingNavigation.kt (1)
7-16: commonMain에서 내비게이션을 구성한 점 좋습니다공통 코드에서 내비게이션 API를 노출하려는 팀 컨벤션(저장된 러닝)에 부합합니다. 유지보수성 측면에서 👍
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterSideEffect.kt (1)
7-7: data object 사용 가능 확인 완료
gradle/libs.versions.toml의[versions]섹션에서kotlin = "2.2.0"로 선언되어 있으며,org.jetbrains.kotlin.multiplatform등 모든 Kotlin 플러그인(plugin)에 동일 버전이 참조되고 있습니다.
따라서 iOS/JS를 포함한 모든 빌드 대상에서 Kotlin 2.2.0을 사용하므로data object기능을 안전하게 사용할 수 있습니다.core/network/src/commonMain/kotlin/com/nexters/emotia/network/service/LetterApiService.kt (1)
10-14: contents 검증/정규화 위치를 결정해 주세요현재 서비스는 입력을 있는 그대로 전송합니다. 다음 옵션 중 하나를 팀 컨벤션으로 확정하는 것이 좋습니다.
- ViewModel/Domain에서
require(contents.isNotBlank())및 길이 제한(예: ≤2000) 검증 후 서비스 호출- Repository에서
.trim()등 경량 정규화 수행- 서버가 길이/금칙어를 검증하고 클라이언트는 즉시 반영 가능한 에러 코드로 처리
어느 계층에서 무엇을 보장할지 합의 부탁드립니다.
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/datasource/LetterRemoteDataSourceImpl.kt (1)
10-20: 굿: API 위임 레이어가 단순·명확합니다Repository에서의 도메인 매핑/에러 처리와 역할이 잘 분리되어 있습니다. 현재 수준으로 충분히 유지보수하기 좋아 보입니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyViewModel.kt (2)
19-23: State 필드명 검증 필요:fairyImagevsfairySilhouetteImageAI Summary 기준
FairyState에fairySilhouetteImage가 추가/이관된 것으로 보이는데, 본 파일에서는fairyImage를 세팅하고 있습니다. 컴파일 타임에 잡히겠지만 네이밍이 불일치할 가능성이 큽니다. 실제 정의에 맞춰 필드명을 통일해 주세요.확인 후 필요한 경우 다음과 같이 변경하세요:
- fairyImage = event.fairySilhouetteImage + fairySilhouetteImage = event.fairySilhouetteImage
27-31: 애니메이션 종료 처리 의도 확인 요청
StartExpandAnimation에서isExpanding = true만 설정되고 false로 되돌리는 경로가 없습니다. 애니메이션 완료 콜백이나 별도 Intent(StopExpandAnimation)로 원복하는 설계가 맞는지 확인 부탁드립니다.feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/di/ResultModule.kt (2)
9-10: LGTM: ViewModel 바인딩 분리로 책임이 명확해졌습니다
FairyViewModel,LetterViewModel를 각각 바인딩한 점 좋아요.LetterViewModel(get())의 의존성도 DI 그래프에 잘 연결되어 있다면 문제 없겠습니다.
9-10: 문제 없음:viewModelDSL 및LetterRepositoryDI 바인딩이 올바르게 설정되어 있습니다
- 공용 소스셋인
commonMain에서 이미 OnBoardingModule, ChattingModule, ResultModule 등 다수의 모듈에서
import org.koin.core.module.dsl.viewModel을 사용 중이며, Koin 멀티플랫폼 공용 DSL 지원이 정상 작동합니다.core/data/letter/di/DataLetterModule.kt에서
single<LetterRepository> { LetterRepositoryImpl(get()) }로 구현체를 바인딩하고 있으며,
composeApp/src/commonMain/kotlin/com/nexters/emotia/di/Modules.kt에서
dataLetterModule이featureModule(→ResultModule 포함)보다 앞서 로드되어LetterViewModel(get())의존성이 충족됩니다.따라서 추가 검토나 수정 없이 이대로 머지하셔도 무방합니다.
feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavigator.kt (1)
32-40: 이전 플로우 관련 잔여 코드 없음 확인 완료
ResultRoute,resultScreen,navigateToResult(패턴이 Kotlin 파일 전역에서 검색되지 않아(검색 결과 0건), 구 플로우의 흔적은 제거된 것이 확인되었습니다.
이대로 병합 가능합니다.feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/navigation/FairyNavigation.kt (3)
10-15: NavGraphBuilder.fairyScreen 진입점 구성 적절
- 온보딩/채팅/편지/결과로 이어지는 콜백 인터페이스가 명확하며, 상위(NavHost)에서 주입하기에 깔끔합니다.
31-44: Letter 경로 연동 OK — chatRoomId 전달 유지 확인됨
- LetterScreen에 chatRoomId를 정확히 전달하고, 결과 내비게이션과 채팅 복귀 콜백도 명확합니다.
46-55: Result 경로 연동 OK
- 입력 파라미터를 그대로 위임하고, 홈(온보딩) 복귀 콜백만 노출하는 단방향 구성이 간결합니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/contract/LetterIntent.kt (1)
3-13: Kotlin 2.2.0 및 Compose Multiplatform 1.8.2 호환성 확인
gradle/libs.versions.toml에서 Kotlin 버전이 2.2.0으로 설정되어 있습니다.- Compose Multiplatform 플러그인 버전은 1.8.2이며, Compose Compiler Gradle plugin도 Kotlin MPP plugin 버전(2.2.0)과 일치하도록 적용되어 있습니다.
- Compose Multiplatform 1.8.2는 Kotlin 2.1.0 이상에서만 동작하며, 최신 Kotlin 버전과 항상 호환됩니다(jetbrains.com).
따라서
data object사용을 위한 최소 요구 Kotlin 버전(1.9+)을 충족하며, 추가 조치 없이 현재 구조를 유지하셔도 무방합니다.feature/main/src/commonMain/kotlin/com/nexters/emotia/feature/main/MainNavHost.kt (3)
21-22: NavHost 배경 지정(LGTM)검정 배경 적용이 디자인 시스템(colors.black)과 일관되며, 시작 지점에서 깜빡임을 줄이는 데도 도움이 됩니다.
36-49: fairyScreen 콜백 매핑(LGTM)
- Letter/Result 경로 콜백이
MainNavigator의 대응 메서드에 올바르게 위임됩니다.- 온보딩/채팅 복귀 경로도 누락 없이 연결되어 있습니다.
31-33:FairyRoute정의 경로 및 파라미터 정합성 수동 확인 요청
FairyRoute.kt파일을 스크립트로 찾지 못해, 아래 사항을 직접 확인해 주세요:
FairyRoute객체(또는 클래스)가 정의된 파일 경로- 해당 파일에서 경로 템플릿에 사용된 파라미터 순서(예:
id,name,image,silhouetteImage,chatRoomId)가
MainNavigator.navigateToFairy(...)호출부와 완전히 일치하는지파일을 열어 파라미터 순서 및 타입을 검토하신 후, 불일치 시 런타임 네비게이션 오류가 발생할 수 있으니 맞춰주시면 감사하겠습니다.
| alias(libs.plugins.kotlinxSerialization) | ||
| } | ||
|
|
||
| android.namespace = "com.nexters.emotia.core.data.letter" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AGP DSL 사용 방식 이슈 가능성 — android { namespace = ... } 블록으로 수정 필요
현재 android.namespace = "..." 형태는 KTS에서 확장 접근이 보장되지 않아 빌드 실패 위험이 있습니다. 일반적으로 아래와 같이 android 블록 내에서 설정합니다.
-android.namespace = "com.nexters.emotia.core.data.letter"
+android {
+ namespace = "com.nexters.emotia.core.data.letter"
+}📝 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.
| android.namespace = "com.nexters.emotia.core.data.letter" | |
| android { | |
| namespace = "com.nexters.emotia.core.data.letter" | |
| } |
🤖 Prompt for AI Agents
In core/data/letter/build.gradle.kts around line 7, the namespace is being set
via a top-level property access (`android.namespace =
"com.nexters.emotia.core.data.letter"`) which can fail in Kotlin DSL; move this
setting into the android block by adding or updating the existing android {
namespace = "com.nexters.emotia.core.data.letter" } so the namespace is
configured via the DSL block rather than top-level assignment.
| fun SendLetterResponse.toDomain(): Letter { | ||
| return Letter( | ||
| fairyId = data?.fairyId ?: 0, | ||
| name = data?.name ?: "", | ||
| image = data?.image ?: "", | ||
| contents = data?.contents ?: "" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
무음 기본값(0/빈 문자열)으로의 매핑이 서버/데이터 오류를 숨깁니다 — 실패-우선 전략으로 전환 권장
data 혹은 그 내부 필드가 null인 경우에도 0/""로 치환되어 도메인으로 전달됩니다. 이러면 UI/후속 로직이 “정상 응답”으로 오인할 위험이 큽니다. 최소한 필수 키(fairyId, contents, name 등)는 즉시 실패(예외)로 전환하여 Repository 단의 Result로 전파하는 편이 안전합니다. 이미지 URL처럼 선택적일 수 있는 필드는 orEmpty()로 허용하는 타협이 가능합니다.
아래처럼 requireNotNull을 사용해 fail-fast로 바꾸는 것을 제안합니다(Repository가 runCatching { ... }로 감싸고 있다면 자연스럽게 Result.failure로 귀결됩니다).
-fun SendLetterResponse.toDomain(): Letter {
- return Letter(
- fairyId = data?.fairyId ?: 0,
- name = data?.name ?: "",
- image = data?.image ?: "",
- contents = data?.contents ?: ""
- )
-}
+fun SendLetterResponse.toDomain(): Letter {
+ val d = requireNotNull(data) { "SendLetterResponse.data is null" }
+ return Letter(
+ fairyId = d.fairyId,
+ name = d.name,
+ image = d.image.orEmpty(),
+ contents = d.contents,
+ )
+}📝 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.
| fun SendLetterResponse.toDomain(): Letter { | |
| return Letter( | |
| fairyId = data?.fairyId ?: 0, | |
| name = data?.name ?: "", | |
| image = data?.image ?: "", | |
| contents = data?.contents ?: "" | |
| ) | |
| fun SendLetterResponse.toDomain(): Letter { | |
| val d = requireNotNull(data) { "SendLetterResponse.data is null" } | |
| return Letter( | |
| fairyId = d.fairyId, | |
| name = d.name, | |
| image = d.image.orEmpty(), | |
| contents = d.contents, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/mapper/LetterMapper.kt
around lines 6 to 12, the mapper currently substitutes null required fields with
silent defaults (0/""), hiding server/data errors; change to fail-fast by using
requireNotNull (or explicit checks) for mandatory fields (e.g., fairyId, name,
contents) so a NullPointerException/IllegalArgumentException is thrown and can
be propagated as a failed Result from the Repository, while allowing optional
fields like image to use safe defaults (orEmpty()).
| internal fun getMultiLineTextFieldState( | ||
| enabled: Boolean, | ||
| isFocused: Boolean, | ||
| hasText: Boolean, | ||
| ): MultiLineTextFieldState { | ||
| return when { | ||
| !enabled -> MultiLineTextFieldState.DISABLED | ||
| isFocused && hasText -> MultiLineTextFieldState.TYPING | ||
| isFocused && !hasText -> MultiLineTextFieldState.FOCUSED | ||
| !isFocused && hasText -> MultiLineTextFieldState.TYPED | ||
| else -> MultiLineTextFieldState.FOCUSED | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
상태 머신 로직 오류: 포커스 없음 + 빈 텍스트가 FOCUSED로 분류됨
주석 정의에 따르면 FOCUSED는 “포커스 있고 비어있음”인데, 현재 else 분기(포커스 없음 + 비어있음)에서 FOCUSED를 반환합니다. 이는 스타일/상태 표현이 뒤바뀌는 버그로, 기본(Idle) 상태를 별도로 두거나 매핑을 수정해야 합니다. 아래처럼 IDLE 상태를 추가하고 매핑/색상 정책을 조정하는 것을 권장합니다.
@@
-internal enum class MultiLineTextFieldState {
- DISABLED, // 비활성화 상태
- FOCUSED, // 포커스 있고 비어있음 (입력 중)
- TYPING, // 포커스 있고 텍스트 있음 (타이핑 중)
- TYPED // 포커스 없고 텍스트 있음 (입력 완료)
-}
+internal enum class MultiLineTextFieldState {
+ IDLE, // 포커스 없고 비어있음 (기본)
+ DISABLED, // 비활성화 상태
+ FOCUSED, // 포커스 있고 비어있음 (입력 중)
+ TYPING, // 포커스 있고 텍스트 있음 (타이핑 중)
+ TYPED // 포커스 없고 텍스트 있음 (입력 완료)
+}
@@
internal fun getMultiLineTextFieldState(
enabled: Boolean,
isFocused: Boolean,
hasText: Boolean,
): MultiLineTextFieldState {
return when {
!enabled -> MultiLineTextFieldState.DISABLED
isFocused && hasText -> MultiLineTextFieldState.TYPING
isFocused && !hasText -> MultiLineTextFieldState.FOCUSED
!isFocused && hasText -> MultiLineTextFieldState.TYPED
- else -> MultiLineTextFieldState.FOCUSED
+ else -> MultiLineTextFieldState.IDLE
}
}
@@
internal fun MultiLineTextFieldState.getBorderColor(): Color {
val colors = LocalEmotiaColors.current
return when (this) {
- MultiLineTextFieldState.DISABLED -> colors.lightGray
+ MultiLineTextFieldState.IDLE -> colors.lightGray
+ MultiLineTextFieldState.DISABLED -> colors.lightGray
MultiLineTextFieldState.FOCUSED -> colors.primaryLight
MultiLineTextFieldState.TYPING -> colors.primaryLight
MultiLineTextFieldState.TYPED -> colors.primaryLight
}
}필요 시 IDLE 프리뷰도 추가하면 디자이너/QA 확인이 수월합니다.
Also applies to: 41-50
🤖 Prompt for AI Agents
In
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt
around lines 27-39 (and also apply same fix to 41-50), the state machine wrongly
maps the case "not focused && empty" to FOCUSED; change the mapping to return a
new IDLE (or EMPTY/INACTIVE) state for the default/no-focus+no-text case and
update the enum MultiLineTextFieldState to include IDLE, then adjust all
callers/styles/previews (including any color/style policies and previews) to
handle IDLE instead of treating empty+no-focus as FOCUSED. Ensure the when
branches explicitly cover !enabled -> DISABLED, isFocused && hasText -> TYPING,
isFocused && !hasText -> FOCUSED, !isFocused && hasText -> TYPED, else -> IDLE
and update tests/previews accordingly.
| data class SendLetterResponse( | ||
| val success: Boolean, | ||
| val data: LetterData?, | ||
| val error: String?, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
널 조합 불변식 보강 및 역직렬화 회복력 향상 제안
- 관례상 success == true ⇒ data != null이 보장되어야 합니다. 맵퍼에서 이 불변식을 확인하고 아닐 경우 에러로 처리하는지 점검 부탁드립니다.
- 서버가 특정 필드를 누락해도 크래시하지 않도록 기본값을 추가해 역직렬화 회복력을 높일 수 있습니다.
예시:
@Serializable
data class SendLetterResponse(
- val success: Boolean,
- val data: LetterData?,
- val error: String?,
+ val success: Boolean = false,
+ val data: LetterData? = null,
+ val error: String? = null,
)
@Serializable
data class LetterData(
- val fairyId: Int,
- val name: String,
- val image: String,
- val contents: String,
+ val fairyId: Int = 0,
+ val name: String = "",
+ val image: String = "",
+ val contents: String = "",
)맵핑 검증 스크립트(데이터 계층에서 null/data 불일치 처리 확인):
🏁 Script executed:
#!/bin/bash
# SendLetterResponse -> Domain 매핑에서 null 체크/불변식 확인
rg -n -C2 --glob 'core/data/letter/**' -e 'SendLetterResponse' -e '\.toDomain\(' -e 'success' -e 'data\s*!='Length of output: 5180
🏁 Script executed:
#!/bin/bash
# Locate SendLetterResponse and LetterData definitions to inspect default values and @Serializable annotations
rg -n -C3 'data class (SendLetterResponse|LetterData)' -g 'core/network/src/commonMain/kotlin/com/nexters/emotia/network/dto/response/*.kt'Length of output: 1806
🏁 Script executed:
#!/bin/bash
# 출력: SendLetterResponse.toDomain 구현 내용 확인
sed -n '1,200p' core/data/letter/src/commonMain/kotlin/com/nexters/emotia/core/data/letter/mapper/LetterMapper.ktLength of output: 508
SendLetterResponse.success==true 인 경우 data null 불변식 검증 추가 필요
현재 매핑 흐름을 살펴본 결과,
- Repository(
LetterRepositoryImpl.kt라인 19~23)에서response.success == false인 경우 예외를 던지지만 success == true인 경우toDomain()호출로 바로 넘어가며,- Mapper(
LetterMapper.kt라인 6~12)에서는data?.fairyId ?: 0식으로 널을 기본값으로 대체해 버려 불변식(success==true ⇒ data != null)을 검증하지 않습니다.
따라서 success==true인데 data가 null인 예외 케이스를 놓치지 않도록, Mapper나 Repository 단계에서 명시적으로 검증하고 에러 처리 로직을 추가해야 합니다.
수정 예시(Mapper 기준):
fun SendLetterResponse.toDomain(): Letter {
- return Letter(
- fairyId = data?.fairyId ?: 0,
- name = data?.name ?: "",
- image = data?.image ?: "",
- contents= data?.contents ?: ""
- )
+ require(success) { "API 성공 응답이 아닙니다: error=$error" }
+ val nonNullData = data
+ ?: throw IllegalStateException("success==true 이지만 data가 null 입니다")
+ return Letter(
+ fairyId = nonNullData.fairyId,
+ name = nonNullData.name,
+ image = nonNullData.image,
+ contents = nonNullData.contents
+ )
}— 또는 Repository에서 response.data == null 검증 후 예외 처리하는 방식으로도 좋습니다.
@serializable DTO 기본값 추가 고려 (optional)
현재 DTO(SendLetterResponse.kt, LetterData)에 생성자 기본값이 없어, 서버가 필드를 아예 누락할 경우 디직렬라이제이션 예외가 발생할 수 있습니다. 필드 누락에도 크래시를 방지하려면 다음과 같이 기본값을 추가할 수 있습니다:
@Serializable
data class SendLetterResponse(
- val success: Boolean,
- val data: LetterData?,
+ val success: Boolean = false,
+ val data: LetterData? = null,
val error: String? = null,
)
@Serializable
data class LetterData(
- val fairyId: Int,
- val name: String,
- val image: String,
- val contents: String,
+ val fairyId: Int = 0,
+ val name: String = "",
+ val image: String = "",
+ val contents: String = "",
)- 장점: 서버 스키마 변경·필드 누락 시에도 기본값으로 복원 가능
- 단점: 필드 누락을 조용히 넘어갈 수 있으므로, 디버깅 시 주의 요망
📝 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.
| data class SendLetterResponse( | |
| val success: Boolean, | |
| val data: LetterData?, | |
| val error: String?, | |
| ) | |
| fun SendLetterResponse.toDomain(): Letter { | |
| require(success) { "API 성공 응답이 아닙니다: error=$error" } | |
| val nonNullData = data | |
| ?: throw IllegalStateException("success==true 이지만 data가 null 입니다") | |
| return Letter( | |
| fairyId = nonNullData.fairyId, | |
| name = nonNullData.name, | |
| image = nonNullData.image, | |
| contents = nonNullData.contents | |
| ) | |
| } |
| onNavigateToFairy( | ||
| selectedFairy.id, | ||
| selectedFairy.name, | ||
| selectedFairy.silhouetteImage | ||
| selectedFairy.image, | ||
| selectedFairy.silhouetteImage, | ||
| uiState.roomId?.toInt() ?: 0 | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
chatRoomId를 0으로 폴백하고 내비게이션하는 것은 위험합니다
- chatRoomId가 null일 때 0을 넘기면, 서버/다음 화면에서 잘못된 방으로 전송되거나 실패할 수 있습니다. 이 케이스에서는 내비게이션을 중단하거나 에러를 노출하는 편이 안전합니다.
다음과 같이 null인 경우 내비게이션을 중단하도록 가드를 추가하는 것을 권장합니다.
- onNavigateToFairy(
- selectedFairy.id,
- selectedFairy.name,
- selectedFairy.image,
- selectedFairy.silhouetteImage,
- uiState.roomId?.toInt() ?: 0
- )
+ val chatRoomId = uiState.roomId?.toInt()
+ ?: return@LaunchedEffect // 채팅방 ID가 없으면 내비게이션 중단
+ onNavigateToFairy(
+ selectedFairy.id,
+ selectedFairy.name,
+ selectedFairy.image,
+ selectedFairy.silhouetteImage,
+ chatRoomId
+ )chatRoomId의 원 타입(예: Long? / String?)과 범위가 일관적인지 점검해 주세요. Long → Int 변환시 오버플로우 가능성도 배제할 수 없습니다.
🏁 Script executed:
#!/bin/bash
# roomId 선언부 및 타입 추적
rg -nP --type=kt 'roomId\s*[:=]' -C3
rg -nP --type=kt 'chatRoomId\s*[:=]' -C3Length of output: 135
roomId의 실제 선언부와 타입을 파악하기 위해 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
# Kotlin UI State에서 roomId 타입 확인
rg -n 'class.*UiState' -C1 -g '*.kt'
rg -n 'val.*roomId' -C3 -g '*.kt'
rg -n 'uiState\.roomId' -C3 -g '*.kt'Length of output: 5197
onNavigateToFairy의 파라미터 타입(특히 roomId)을 정확히 파악하기 위해 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
# onNavigateToFairy 함수/람다 시그니처 조회
rg -n 'onNavigateToFairy' -C3 -g '*.kt'Length of output: 4470
String? → Int 변환 시 Null 및 숫자 포맷 에러 방지
- ChattingState에서
roomId는String?타입이며, onNavigateToFairy의 마지막 파라미터는Int를 요구합니다.- 따라서
toInt()호출에서NumberFormatException발생 가능성뿐만 아니라,?: 0처럼 0으로 폴백할 경우 잘못된 방으로 내비게이션이 일어날 위험이 있습니다. toIntOrNull()을 사용하고,null일 때는 내비게이션을 중단하거나 오류를 노출하는 가드문을 추가하세요.
- onNavigateToFairy(
- selectedFairy.id,
- selectedFairy.name,
- selectedFairy.image,
- selectedFairy.silhouetteImage,
- uiState.roomId?.toInt() ?: 0
- )
+ val chatRoomId = uiState.roomId
+ ?.toIntOrNull()
+ ?: return@LaunchedEffect // roomId가 없거나 숫자 변환에 실패하면 중단
+ onNavigateToFairy(
+ selectedFairy.id,
+ selectedFairy.name,
+ selectedFairy.image,
+ selectedFairy.silhouetteImage,
+ chatRoomId
+ )📝 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.
| onNavigateToFairy( | |
| selectedFairy.id, | |
| selectedFairy.name, | |
| selectedFairy.silhouetteImage | |
| selectedFairy.image, | |
| selectedFairy.silhouetteImage, | |
| uiState.roomId?.toInt() ?: 0 | |
| ) | |
| // roomId가 없거나 숫자 변환에 실패하면 중단 | |
| val chatRoomId = uiState.roomId | |
| ?.toIntOrNull() | |
| ?: return@LaunchedEffect | |
| onNavigateToFairy( | |
| selectedFairy.id, | |
| selectedFairy.name, | |
| selectedFairy.image, | |
| selectedFairy.silhouetteImage, | |
| chatRoomId | |
| ) |
🤖 Prompt for AI Agents
In
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt
around lines 127 to 133, avoid using toInt() with a fallback 0 for
uiState.roomId (String?) because it can throw NumberFormatException or navigate
to an incorrect room; replace the direct toInt() call with toIntOrNull() and add
a guard: if the parsed Int is null, do not call onNavigateToFairy (instead
return early, log or show an error/Toast), otherwise pass the parsed Int to
onNavigateToFairy so navigation only occurs with a valid room id.
| @@ -0,0 +1,237 @@ | |||
| package com.nexters.emotia.feature.result.letter | |||
|
|
|||
| import EmotiaMultiLineTextField | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
잘못된 임포트로 인한 컴파일 오류
import EmotiaMultiLineTextField는 패키지명이 없어 실패합니다. 디자인시스템 컴포넌트의 정식 패키지로 임포트해 주세요.
적용 diff:
-import EmotiaMultiLineTextField
+import com.nexters.emotia.core.designsystem.component.EmotiaMultiLineTextField📝 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.
| import EmotiaMultiLineTextField | |
| import com.nexters.emotia.core.designsystem.component.EmotiaMultiLineTextField |
🤖 Prompt for AI Agents
In
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt
around line 3, the top-level import "import EmotiaMultiLineTextField" is invalid
and causes a compile error; replace it with the component's fully-qualified
package import from the design system (for example use the project's actual
package such as
com.nexters.emotia.designsystem.components.EmotiaMultiLineTextField or the
correct package path used in this repo) so the symbol resolves.
| Column( | ||
| modifier = modifier | ||
| .fillMaxSize() | ||
| .verticalScroll(scrollState), | ||
| horizontalAlignment = Alignment.CenterHorizontally | ||
| ) { | ||
| Box { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
verticalScroll + weight 혼용으로 인한 레이아웃 비의도 동작
Column(...).verticalScroll(scrollState)의 자식에서 Modifier.weight(1f)는 효과가 없습니다(스크롤 컨테이너에서는 무한 제약으로 weight가 무력화). 현재 입력영역이 의도대로 확장/수축하지 않을 수 있습니다.
권장:
- 루트
Column에서verticalScroll제거. - 필요 시 카드 내부에만 스크롤을 적용하거나, 텍스트필드 자체 스크롤에 맡깁니다.
- 키보드 대응은
Modifier.imePadding()또는windowInsetsPadding(WindowInsets.ime)로 처리.
최소 변경 diff:
- Column(
- modifier = modifier
- .fillMaxSize()
- .verticalScroll(scrollState),
+ Column(
+ modifier = modifier
+ .fillMaxSize(),
horizontalAlignment = Alignment.CenterHorizontally
) {추가로, 키보드 표시 시 하단 여백 보정이 필요하다면 다음을 바깥 Column 또는 하단 버튼 영역에 적용해 주세요(참고용):
modifier = modifier
.fillMaxSize()
.imePadding()Also applies to: 149-157, 170-184
🤖 Prompt for AI Agents
In
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt
around lines 117-123 (also check 149-157 and 170-184), the root Column uses
.verticalScroll(...) while its children rely on Modifier.weight(1f), which is
ignored inside an unbounded scroll container; remove the verticalScroll from the
root Column, move any scrolling to the specific card or the TextField (or rely
on the TextField's own scrolling) so weighted children can size properly, and
add keyboard handling via Modifier.imePadding() or
windowInsetsPadding(WindowInsets.ime) on the outer Column or the bottom button
area to adjust for the IME; make minimal changes by deleting the root
verticalScroll and applying scroll/ime padding only where needed.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt
Outdated
Show resolved
Hide resolved
| fun handleIntent(intent: LetterIntent) = intent { | ||
| when (intent) { | ||
| is LetterIntent.InitializeFairy -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Orbit intent {}가 파라미터 이름에 의해 섀도잉되어 컴파일 실패 가능
fun handleIntent(intent: LetterIntent) = intent { ... }에서 파라미터 이름 intent가 Orbit의 intent {} 확장함수를 가립니다. 현재 코드는 파라미터(타입: LetterIntent)를 함수처럼 호출하려 하므로 컴파일 에러가 납니다. 파라미터명을 변경해 주세요.
적용 diff:
- fun handleIntent(intent: LetterIntent) = intent {
- when (intent) {
+ fun handleIntent(action: LetterIntent) = intent {
+ when (action) {
- is LetterIntent.InitializeFairy -> {
+ is LetterIntent.InitializeFairy -> {
...
- is LetterIntent.UpdateContents -> {
+ is LetterIntent.UpdateContents -> {
...
- is LetterIntent.SendLetter -> {
+ is LetterIntent.SendLetter -> {
...
- is LetterIntent.ClearError -> {
+ is LetterIntent.ClearError -> {
...
}
}
}🤖 Prompt for AI Agents
In
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterViewModel.kt
around lines 18-20, the function parameter named "intent" shadows Orbit's intent
{ } extension causing a compile error because the parameter is treated like a
function; rename the parameter (e.g., to "action" or "letterIntent") and update
all references inside the function body to use the new name so you can call
Orbit's intent { ... } block normally.
| @Serializable | ||
| sealed interface FairyRoute : Route { | ||
| @Serializable | ||
| data class FairyMain( | ||
| val id: Int, | ||
| val name: String, | ||
| val imageUrl: String, | ||
| val silhouetteImageUrl: String, | ||
| val chatRoomId: Int, | ||
| ) : FairyRoute | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
타입 안전한 시리얼라이저블 라우트 정의, 좋습니다
- @serializable + sealed interface 구성으로 타입 안전성과 유지 보수성이 확보되었습니다.
- 사소하지만 imageUrl/silhouetteImageUrl를 fairyImageUrl/fairySilhouetteImageUrl처럼 도메인 용어를 앞에 두는 네이밍도 고려해볼 만합니다.
- chatRoomId의 원 타입이 Long일 가능성이 있으면, Long 채택 또는 전 구간 Int로의 표준화를 검토해 주세요.
🏁 Script executed:
#!/bin/bash
# chatRoomId 타입 일관성 점검
rg -nP --type=kt '\bchatRoomId\s*:\s*(Int|Long)' -nLength of output: 80
전체 코드베이스에서 chatRoomId 선언 및 사용 위치를 재확인합니다.
🏁 Script executed:
#!/bin/bash
# chatRoomId 사용 일관성 점검: 모든 .kt 파일에서 검색
rg -n 'chatRoomId' -g '*.kt'Length of output: 6871
chatRoomId 타입 일관성 검토 필요
-
@serializable + sealed interface 구성으로 타입 안전성과 유지 보수성 확보되었음.
-
사소한 네이밍 제안:
imageUrl/silhouetteImageUrl대신fairyImageUrl/fairySilhouetteImageUrl처럼 도메인 용어를 앞에 두는 것도 고려해 보세요. -
현재 Feature·ViewModel·DTO 등 대부분의 Kotlin 코드에서
chatRoomId를Int로 사용하고 있으나,core/network/service/ChatApiService.kt의 API 메소드 시그니처에서는String으로 선언되어 있습니다. 이 불일치가 잠재적 버그 원인이 될 수 있으니, 아래 중 하나를 선택해 적용해 주세요.
• API 서비스 메소드의chatRoomId파라미터를Int로 변경
•String사용 사유(서버 스펙 등)를 코드에 주석으로 명시하고, 호출 지점에서 변환 로직(예:toString(),toInt())을 일관되게 적용 -
점검 대상 위치
• core/network/src/commonMain/kotlin/com/nexters/emotia/network/service/ChatApiService.kt:- suspend fun getFairies(chatRoomId: String): GetFairiesResponse + suspend fun getFairies(chatRoomId: Int): GetFairiesResponse
📝 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.
| @Serializable | |
| sealed interface FairyRoute : Route { | |
| @Serializable | |
| data class FairyMain( | |
| val id: Int, | |
| val name: String, | |
| val imageUrl: String, | |
| val silhouetteImageUrl: String, | |
| val chatRoomId: Int, | |
| ) : FairyRoute | |
| suspend fun getFairies(chatRoomId: Int): GetFairiesResponse |
작업 내용
이건 꼭 봐주세요
Summary by CodeRabbit