Skip to content

Conversation

@chanho0908
Copy link
Member

@chanho0908 chanho0908 commented Jan 22, 2026

이슈 번호

#34

리뷰/머지 희망 기한 (선택)

2026.01.24

작업내용

온보딩 API 연동 완료

  • 기념일 설정
  • 커플 연결
  • 초대 코드 조회
  • 프로필 등록
  • 온보딩 상태 조회

실제 UI 연동은 로그인/토큰 저장 기능 구현 완료된 뒤에 토큰을 담아서 구현 예정 !

결과물

리뷰어에게 추가로 요구하는 사항 (선택)

OnBoardingStatus, InviteCode 유효성 검증 관련 비즈니스 로직을 캡슐화하고 테스트 코드까지 추가했어 !

아직 구체적인 네트워크 호출 에러처리를 협의하지 않아서 에러처리는 제외하고 구현했고
요건 같이 이야기 해보면 좋을 것 같아서 해커톤 때 이야기해보자 😃

@chanho0908 chanho0908 self-assigned this Jan 22, 2026
@chanho0908 chanho0908 added the Feature Extra attention is needed label Jan 22, 2026
@chanho0908 chanho0908 linked an issue Jan 22, 2026 that may be closed by this pull request
5 tasks
@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

이 PR은 온보딩 기능을 구현하기 위해 네트워크, 데이터, 도메인 계층에 걸쳐 새로운 서비스 및 저장소 인프라를 추가합니다. OnboardingService 인터페이스와 관련 엔드포인트가 정의되고, DefaultOnboardingRepository가 서비스를 데이터 계층과 연결합니다. 도메인 모델(InviteCode, OnboardingStatus)은 입력 검증을 포함하며, DI 모듈(dataModule, repositoryModule, apiServiceModule)이 추가되어 초기화 시 모든 의존성이 로드됩니다. HttpClient에 로깅 기능이 추가되고, 단위 테스트가 도메인 모델의 검증 로직을 검증합니다.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

주요 검토 포인트

✅ 긍정적인 측면

계층 분리가 명확합니다 — 네트워크(OnboardingService), 데이터(DefaultOnboardingRepository), 도메인(모델 및 인터페이스) 계층이 책임에 따라 잘 구분되어 있습니다. 이는 유지보수와 테스트를 용이하게 합니다.

도메인 모델에 검증 로직이 내재되어 있습니다 — InviteCode의 정규식 검증(^[A-Z0-9]+$)과 8자리 길이 체크, OnboardingStatus의 enum 매핑이 초기화 단계에서 이루어져, 잘못된 상태가 시스템 내부로 진입하지 않습니다.

테스트 커버리지가 포함되어 있습니다 — InviteCodeTest와 OnboardingStatusTest가 정상 케이스와 예외 상황을 모두 검증하고 있어, 앞으로의 리팩토링 시에도 안정성을 보장합니다.

🤔 검토 권장 사항

OnboardingService의 POST 메서드들이 String 타입의 요청을 받습니다 — 현재 설계에서 @Body request: String으로 정의되어 있습니다.

`@POST`("onboarding/anniversary")
suspend fun anniversarySetup(`@Body` request: String)

이 방식의 잠재적 문제:

  • 타입 안정성: 클라이언트 측에서 String을 직렬화/역직렬화해야 하므로 오류가 발생하기 쉽습니다
  • 문서화: API 스펙이 명확하지 않으면 요청 본문 형식을 파악하기 어렵습니다

제안: 강타입 요청 객체(예: AnniversarySetupRequest)를 정의하면 어떨까요? 이렇게 하면 직렬화가 자동으로 처리되고, IDE의 자동완성도 지원됩니다.

data class AnniversarySetupRequest(val /* 필드들 */)

`@POST`("onboarding/anniversary")
suspend fun anniversarySetup(`@Body` request: AnniversarySetupRequest)

DefaultOnboardingRepository에서 응답 매핑이 간단합니다 — 현재 InviteCodeResponse.inviteCodeInviteCode 도메인 모델로 변환하는 것이 한 줄입니다.

override suspend fun fetchInviteCode(): InviteCode = 
    service.fetchInviteCode().let { InviteCode(it.inviteCode) }

이는 좋은 설계이지만, 여러 필드를 매핑해야 하는 향후 시나리오에 대비하여 extension 함수나 mapper 유틸리티를 고려할 가치가 있습니다. 현재 상태는 충분히 명확하므로 필수는 아닙니다.

HttpClientProvider의 로깅 설정 — 로깅이 isDebug 조건에 따라 설정되고 있습니다.

val logLevel = if (isDebug) LogLevel.ALL else LogLevel.NONE

좋은 설계입니다만, 프로덕션 환경에서 민감한 정보(토큰, 개인정보)가 로그에 출력될 수 있으니, 필요시 로그 필터링 로직을 추가하는 것을 추천드립니다.

TODO 주석이 남겨져 있습니다// TODO: Bearer token authentication 섹션이 있습니다.

// HttpClient에서 Bearer token authentication 관련 TODO가 주석 처리됨

이는 의도적인 것으로 보이지만, 이 작업의 우선순위와 담당자가 명확하다면 추적 가능한 이슈로 별도 등록하는 것이 좋습니다.

🔍 세부 사항 확인

테스트 클래스의 메서드명이 한글입니다 — 이는 프로젝트의 스타일 가이드에 따른 것으로 보입니다. 팀 내에서 일관성 있게 적용되고 있다면 문제없습니다.

OnboardingStatus.from() 예외 메시지 — 포맷팅이 깔끔합니다.

throw IllegalArgumentException("UNKNOWN_STATUS: %s".format(status))

현재 구현은 좋으나, 향후 디버깅을 위해 더 구체적인 정보(예: 지원되는 상태 목록)를 추가할 수도 있습니다.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
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.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목 '온보딩 api 연결'은 주요 변경사항(온보딩 API 통합)을 명확하고 간결하게 요약하고 있습니다.
Description check ✅ Passed The pull request description is directly related to the changeset, clearly describing the onboarding API integration work (#34) with specific endpoints implemented and business logic validation details.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

} else {
LogLevel.NONE
}
logger = Logger.ANDROID
Copy link
Member Author

Choose a reason for hiding this comment

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

이 설정을 추가하지 않으니까 Logcat에서 로그가 안찍히더라구 !
로그캣과 연동해서 로그가 출력되는걸 확인했는데 혹시 다른 의견 있으면 남겨줘 !

Image

dependencies {
implementation(libs.bundles.ktor)
implementation(libs.ktorfit.lib)
ksp(libs.ktorfit.ksp)
Copy link
Member Author

@chanho0908 chanho0908 Jan 22, 2026

Choose a reason for hiding this comment

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

이 설정이 있어야 createOnboardingService 처럼 Ktrofit이 서비스 인터페이스 프록시를 만들기 때문에 추가했어 !

single<OnboardingService> {
    get<Ktorfit>().createOnboardingService()
}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt`:
- Around line 8-14: DefaultOnboardingRepository::fetchInviteCode currently lets
network exceptions from OnboardingService bubble up; wrap the call to
service.fetchInviteCode() in a try/catch and convert technical exceptions into a
domain-friendly result (e.g. return a Result<InviteCode, DomainError> or throw a
specific domain exception) so callers (UI) can handle errors consistently;
specifically catch Ktor
ResponseException/ClientRequestException/ServerResponseException (or a generic
Exception as fallback), map them to a DomainError (or Result.failure with mapped
error), and only on success construct and return InviteCode(response.inviteCode)
so the repository surface (DefaultOnboardingRepository, fetchInviteCode,
OnBoardingRepository, InviteCode) exposes domain-level errors instead of raw
HTTP exceptions.
🧹 Nitpick comments (4)
domain/src/test/java/com/twix/domain/model/InviteCodeTest.kt (1)

10-42: 테스트 가독성을 위해 Given-When-Then 구분을 통일해 주세요.
현재는 구조가 생략되어 테스트 의도가 덜 선명합니다. 모든 테스트에 동일한 구분을 두면 유지보수가 쉬워집니다. 코딩 가이드라인에 따라 제안드립니다.

🧪 예시 (첫 테스트에 적용)
 `@Test`
 fun `유효한 초대 코드는 정상적으로 생성된다`() {
+    // given
     val inviteCode = InviteCode("AB12CD34")
 
+    // when
+    // then
     assertThat(inviteCode.value).isEqualTo("AB12CD34")
 }
domain/src/test/java/com/twix/domain/model/OnboardingStatusTest.kt (1)

24-28: assertEquals 인자 순서를 관례대로 맞춰주세요.
현재 순서는 실패 메시지에서 expected/actual이 뒤바뀌어 디버깅이 혼란스러울 수 있습니다. 아래처럼 expected → actual 순으로 정리해 주세요.

✅ 제안 수정
-        assertEquals(result, expected)
+        assertEquals(expected, result)
domain/src/main/java/com/twix/domain/repository/OnBoardingRepository.kt (1)

6-15: 요청 타입을 String으로 두면 도메인 의미와 검증이 약해집니다.
왜 문제인가요: 단순 문자열은 필드 의미가 드러나지 않고, 도메인 레벨에서 유효성 검증/불변성 보장이 어려워집니다. 또한 전송 포맷(JSON 등)에 도메인이 끌려갈 수 있습니다.
어떻게 개선할 수 있나요: AnniversarySetupRequest, CoupleConnectionRequest, ProfileSetupRequest 같은 도메인 요청 모델(불변 data class)을 정의하고, data 레이어에서 네트워크 DTO로 매핑하는 구조를 권장합니다. 서버 스펙이 정말 “문자열 본문”인지도 확인해보실까요?

도메인 모듈 리뷰 가이드에 근거해 제안합니다.

data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt (1)

21-31: request 파라미터가 String 타입으로 되어 있습니다.

anniversarySetup, coupleConnection, profileSetup 메서드의 request 파라미터가 모두 String 타입입니다. 이는 JSON 문자열을 직접 전달하는 방식으로 보입니다.

현재 구조가 의도된 것이라면 괜찮지만, 타입 안전성과 유지보수성을 위해 각 요청에 맞는 도메인 모델(예: AnniversarySetupRequest, CoupleConnectionRequest)을 정의하고 Repository에서 직렬화하는 방식도 고려해 볼 수 있습니다. 이렇게 하면 호출하는 쪽에서 잘못된 형식의 요청을 컴파일 타임에 방지할 수 있습니다.

이 부분은 프로젝트의 설계 방향에 따라 결정하시면 됩니다. 현재 단계에서는 API 연결이 주 목적이므로, 추후 리팩토링 시 고려해 주세요.

Comment on lines +8 to +14
class DefaultOnboardingRepository(
private val service: OnboardingService,
) : OnBoardingRepository {
override suspend fun fetchInviteCode(): InviteCode {
val response = service.fetchInviteCode()
return InviteCode(response.inviteCode)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find other Repository implementations to check error handling patterns
fd -e kt -path '*repository*' -type f | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 236


🏁 Script executed:

# Search for error handling patterns in repository classes
rg -n "runCatching|try\s*\{|Result<|catch\s*\(" --type=kotlin -g '*Repository*' -A 2

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Look for domain error model definitions
fd -e kt -type f | xargs grep -l "sealed.*Error\|sealed.*class.*Error\|object.*Error" | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 238


🏁 Script executed:

# Check the full DefaultOnboardingRepository implementation
fd -e kt -path '*DefaultOnboardingRepository*' -type f

Repository: YAPP-Github/Twix-Android

Length of output: 236


🏁 Script executed:

# Find repository files
find . -name "*Repository.kt" | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 218


🏁 Script executed:

# Search for error handling patterns in kotlin files
rg "runCatching|try\s*\{|Result<|catch\s*\(" -A 2 | head -100

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Look for domain error model definitions
rg "sealed.*class|object.*Error|sealed.*Error" --type kotlin | head -30

Repository: YAPP-Github/Twix-Android

Length of output: 276


🏁 Script executed:

# Get the full DefaultOnboardingRepository implementation
cat data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt

Repository: YAPP-Github/Twix-Android

Length of output: 1067


🏁 Script executed:

# Find all Repository implementations to check patterns
find . -name "*Repository.kt" -not -path "*/build/*" | xargs grep -l "class.*Repository"

Repository: YAPP-Github/Twix-Android

Length of output: 144


🏁 Script executed:

# Check the domain repository interface
cat domain/src/main/java/com/twix/domain/repository/OnBoardingRepository.kt

Repository: YAPP-Github/Twix-Android

Length of output: 479


🏁 Script executed:

# Search for Result types and error handling utilities
find . -name "*.kt" -not -path "*/build/*" | xargs grep -l "sealed class Result\|Result<" | head -10

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Look for any error/exception models
find . -name "*Error.kt" -o -name "*Exception.kt" | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Search for OnboardingService implementation
find . -name "*OnboardingService*" -o -name "*Service.kt" | head -20

Repository: YAPP-Github/Twix-Android

Length of output: 142


🏁 Script executed:

# Check ViewModel or UseCase error handling
find . -name "*ViewModel.kt" -o -name "*UseCase.kt" | xargs grep -l "try\|catch\|error\|Error" | head -10

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Look at network layer for error handling
find . -path "*network*" -name "*.kt" | xargs grep -l "Exception\|error\|catch" | head -10

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Check if there are any other data layer operations with error handling
find . -path "*data*" -name "*.kt" | xargs grep -n "try\s*\{" | head -30

Repository: YAPP-Github/Twix-Android

Length of output: 86


🏁 Script executed:

# Check OnboardingService implementation
cat core/network/src/main/java/com/twix/network/service/OnboardingService.kt

Repository: YAPP-Github/Twix-Android

Length of output: 917


🏁 Script executed:

# Search for error handling in data layer more broadly
find . -path "*data*" -name "*.kt" | xargs grep -n "try" | head -30

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Check for error handling in all layers
find . -name "*.kt" -not -path "*/build/*" | xargs grep -n "catch" | head -30

Repository: YAPP-Github/Twix-Android

Length of output: 50


🏁 Script executed:

# Look for network error handling or exception handling
find . -path "*network*" -name "*.kt" | xargs cat | head -100

Repository: YAPP-Github/Twix-Android

Length of output: 2898


네트워크 예외 처리가 필요합니다.

현재 구현에서 OnboardingService를 통한 네트워크 호출 시 발생하는 예외가 그대로 상위로 전파됩니다. HttpClientexpectSuccess = true로 설정되어 있기 때문에 4xx, 5xx 상태 코드에서 예외가 발생합니다.

코딩 가이드라인에서 "에러가 도메인 레벨로 적절히 변환되는가?"를 확인하도록 되어 있는데, 현재는 기술 계층의 예외가 도메인 계층으로 올라가고 있습니다.

프로젝트에서 사용 중인 에러 처리 패턴이 있다면 그에 맞춰 적용하면 됩니다. 예를 들어:

  • 도메인 레벨의 정의된 예외로 변환
  • Result<T> 또는 유사한 래퍼 타입 사용
  • 예외별로 구분된 처리 로직

UI 레이어에서 일관되게 에러를 처리하려면 어디서 예외를 캐치하고 도메인 계층으로 변환할 계획인지 논의해 주세요.

🤖 Prompt for AI Agents
In `@data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt`
around lines 8 - 14, DefaultOnboardingRepository::fetchInviteCode currently lets
network exceptions from OnboardingService bubble up; wrap the call to
service.fetchInviteCode() in a try/catch and convert technical exceptions into a
domain-friendly result (e.g. return a Result<InviteCode, DomainError> or throw a
specific domain exception) so callers (UI) can handle errors consistently;
specifically catch Ktor
ResponseException/ClientRequestException/ServerResponseException (or a generic
Exception as fallback), map them to a DomainError (or Result.failure with mapped
error), and only on success construct and return InviteCode(response.inviteCode)
so the repository surface (DefaultOnboardingRepository, fetchInviteCode,
OnBoardingRepository, InviteCode) exposes domain-level errors instead of raw
HTTP exceptions.

@chanho0908 chanho0908 requested a review from dogmania January 22, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

온보딩 API 연결

2 participants