[NDGL-88] 홈 화면 API 연결#21
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough기존 홈 화면의 여행 데이터 모델을 여행 프로그램 기반으로 재구조화하고, 로컬 모의 데이터에서 실제 API 호출로 전환했습니다. 네트워크 계층 인프라를 개선하고, 새로운 직렬화 및 저장소 계층을 추가하며, UI 컴포넌트를 새로운 데이터 모델에 맞춰 업데이트했습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/api/TravelTemplateApi.kt`:
- Around line 9-18: The getRecommendTravelTemplates method in TravelTemplateApi
is using the same endpoint as getPopularTravelTemplates, causing ambiguity;
update the annotation on getRecommendTravelTemplates to a distinct path (e.g.,
change `@GET`("/api/v1/travel-templates/popular") to
`@GET`("/api/v1/travel-templates/recommend")) or alternatively add a
distinguishing query parameter to the getRecommendTravelTemplates signature so
the backend can differentiate requests; locate TravelTemplateApi and update the
`@GET` annotation for the getRecommendTravelTemplates method accordingly.
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/TravelProgramRepository.kt`:
- Around line 8-9: TravelProgramRepository is missing the `@Singleton` annotation
while UserTravelRepository is annotated; add `@Singleton` to the
TravelProgramRepository class declaration (above the class) so the class
TravelProgramRepository (the `@Inject` constructor with travelProgramApi:
TravelProgramApi) is provided as a singleton by the DI container, matching
UserTravelRepository's lifecycle.
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt`:
- Around line 66-70: Scaffold의 contentPadding에서 하단에 하드코딩된 100.dp만 사용해
innerPadding이 무시되고 있으니 HomeScreen의 Scaffold 호출부에서 contentPadding의 bottom 값을
innerPadding.calculateBottomPadding()와 기존 100.dp를 더한 값으로 변경하세요 (즉 contentPadding
= PaddingValues(top = innerPadding.calculateTopPadding() + 20.dp, bottom =
innerPadding.calculateBottomPadding() + 100.dp)). 이렇게 하면 시스템 내비게이션 바나 향후
BottomBar 추가 시 하단 여백이 올바르게 반영됩니다.
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt`:
- Around line 163-171: The loadRecommendedTravel function uses
suspendRunCatching(...).onSuccess but omits .onFailure, so failures are ignored;
update loadRecommendedTravel to chain .onFailure after suspendRunCatching {
travelTemplateRepository.getRecommendTravelTemplates() } and handle errors (at
minimum log the exception via your logger or viewModelScope logger and
optionally update UI state), referencing the existing symbols
travelTemplateRepository.getRecommendTravelTemplates(), suspendRunCatching, and
reduce { copy(recommendedContents = ...) } so that errors are reported similarly
to loadMyTravel/loadPopularTemplates.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt`:
- Around line 159-172: The UI is currently showing the raw country ISO code
(travel.country) alongside its flag emoji in PopularTravelSection Row; replace
that ISO code with a human-readable country name (or mirror the
RecommendedContentSection/ CountryChip behavior by showing travel.city) to keep
UX consistent. Update the Text that now uses travel.country to instead call a
utility that converts the ISO code to a localized country name (e.g.,
getDisplayCountry(travel.country) or a countryNameMap) or simply use travel.city
if that matches other sections; locate the Row/Text using travel.country and
toFlagEmoji() in PopularTravelSection and change the second Text's source
accordingly.
In `@lint.xml`:
- Around line 3-5: Remove the global opt-in for InternalSerializationApi from
the UnsafeOptInUsageError rule in lint.xml by editing the <option name="opt-in"
value="..."> entry so it no longer includes
kotlinx.serialization.InternalSerializationApi; leave
kotlinx.serialization.ExperimentalSerializationApi only if you truly need it,
and instead of a global lint opt-in add
`@OptIn`(ExperimentalSerializationApi::class) directly on the specific
classes/functions that require it to minimize scope and ensure
InternalSerializationApi usage will be detected.
🧹 Nitpick comments (14)
data/travel/src/main/java/com/yapp/ndgl/data/travel/model/PlaceCategory.kt (1)
6-25:@SerialName값이 enum 상수명과 동일하여 불필요합니다.
kotlinx.serialization은 기본적으로 enum 상수의 이름을 직렬화 이름으로 사용합니다.@SerialName("AIRPORT")등은 상수명과 동일하므로 제거해도 동작이 같습니다. 향후 API 응답의 값이 달라질 가능성을 대비한 것이라면 유지해도 무방합니다.data/travel/src/main/java/com/yapp/ndgl/data/travel/model/ProgramType.kt (1)
6-13: API에서 새로운 프로그램 타입이 추가될 경우 역직렬화 실패 가능성을 고려해 주세요.현재
YOUTUBE와TV두 가지만 정의되어 있습니다. 서버에서 새로운 타입(예:PODCAST)을 반환하면kotlinx.serialization이 예외를 발생시킵니다. 필요에 따라coerceInputValues = true설정과 함께 기본값 처리를 추가하거나,UNKNOWN상수를 두는 것을 고려해 볼 수 있습니다.PlaceCategory에도 동일하게 적용됩니다.data/core/src/main/java/com/yapp/ndgl/data/core/serializer/LocalDateSerializer.kt (1)
12-24:DateTimeFormatter.ISO_LOCAL_DATE사용을 고려해 주세요.
"yyyy-MM-dd"패턴은DateTimeFormatter.ISO_LOCAL_DATE와 동일합니다. 표준 상수를 사용하면 의도가 더 명확해지고 커스텀 포매터 생성을 생략할 수 있습니다.♻️ 제안하는 변경
object LocalDateSerializer : KSerializer<LocalDate> { - private val formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd") + private val formatter = DateTimeFormatter.ISO_LOCAL_DATE override val descriptor: SerialDescriptor =feature/home/src/main/java/com/yapp/ndgl/feature/home/util/ResourceUtil.kt (1)
16-19:YOUTUBE와TV가 동일한 아이콘(ic_20_video)에 매핑되고 있습니다.현재 두
ProgramType모두 같은 아이콘을 사용하고 있습니다. 의도된 것이라면 괜찮지만, 향후 TV 전용 아이콘이 추가될 예정이라면 TODO 주석을 남겨두는 것이 좋겠습니다.data/core/src/main/java/com/yapp/ndgl/data/core/di/NetworkModule.kt (1)
67-70: 로깅 인터셉터 순서를 확인하세요.
httpLoggingInterceptor가NDGLInterceptor보다 먼저 추가되어 있어, 로깅 시점에NDGLInterceptor가 추가하는 헤더(인증 토큰 등)가 로그에 포함되지 않습니다. 디버깅 시 완전한 요청/응답을 확인하려면 로깅 인터셉터를 마지막에 추가하는 것이 일반적입니다.인증 토큰 노출 방지가 목적이라면 현재 순서가 맞지만, 그렇지 않다면 순서를 변경하세요.
제안
val builder = OkHttpClient.Builder() - .addInterceptor(httpLoggingInterceptor) .addInterceptor(interceptor) .authenticator(authenticator) + .addInterceptor(httpLoggingInterceptor)feature/home/src/main/java/com/yapp/ndgl/feature/home/main/UpcomingTravelCardSection.kt (2)
149-151:DateTimeFormatter를remember로 감싸세요.
DateTimeFormatter.ofPattern()이 Composable 함수 내에서 매 recomposition마다 새로 생성됩니다.remember를 사용하여 불필요한 재할당을 방지하세요. Line 219-221의 동일한 패턴에도 적용됩니다.제안
- val dateFormatter = DateTimeFormatter.ofPattern( - stringResource(R.string.home_my_travel_card_date_format), - ) + val pattern = stringResource(R.string.home_my_travel_card_date_format) + val dateFormatter = remember(pattern) { + DateTimeFormatter.ofPattern(pattern) + }
267-268: 카테고리별 아이콘 매핑이 하드코딩되어 있습니다.FIXME 주석이 있으나,
PlaceCategory에 대한toDisplayRes()처럼 아이콘 매핑 유틸도 함께 구현하면 좋겠습니다. 이 작업을 별도 이슈로 트래킹하시겠습니까?feature/home/src/main/java/com/yapp/ndgl/feature/home/main/RecommendedContentSection.kt (1)
130-134:home_popular_travel_nights_days리소스 이름 확인 필요.추천 콘텐츠 섹션에서
R.string.home_popular_travel_nights_days를 사용하고 있는데, 리소스 이름에 "popular"가 포함되어 있어 의미적으로 맞지 않습니다.PopularTravelSection에서도 동일한 리소스를 사용하므로 공용 리소스명(예:home_common_nights_days)으로 변경하면 가독성이 향상됩니다.data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/TravelTemplateRepository.kt (1)
10-25: Repository에 인터페이스 추상화 부재.
TravelTemplateRepository가 구체 클래스로 직접 주입되고 있어 테스트 시 모킹이 어렵습니다. 인터페이스를 분리하고 Hilt@Binds로 바인딩하면 테스트 용이성과 의존성 역전 원칙 준수에 도움이 됩니다. 다른 repository(TravelProgramRepository,UserTravelRepository)에도 동일하게 적용할 수 있습니다.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt (1)
65-99: LazyColumn의 조건부 item 렌더링 패턴이 일관되지 않습니다.Line 81의
PopularTravelSection은item { }내부에서if체크를 하고 있어 조건이false일 때도 빈 아이템이 생성됩니다. 반면 Line 91의RecommendedContentSection은if로item { }블록 자체를 감싸서 조건이false이면 아이템이 생성되지 않습니다.
PopularTravelSection도 동일한 패턴으로 통일하는 것을 권장합니다.♻️ 제안하는 수정
- item { - if (state.filteredPopularTravels.isNotEmpty()) { - PopularTravelSection( - tabs = state.travelProgramTabs, - selectedTabIndex = state.popularTravelSelectedTabIndex, - travels = state.filteredPopularTravels, - onTabSelected = onTabSelected, - ) - } - } + if (state.filteredPopularTravels.isNotEmpty()) { + item { + PopularTravelSection( + tabs = state.travelProgramTabs, + selectedTabIndex = state.popularTravelSelectedTabIndex, + travels = state.filteredPopularTravels, + onTabSelected = onTabSelected, + ) + } + }feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt (4)
39-41: 세션 초기화 실패 시 사용자에게 빈 화면이 영구적으로 노출됩니다.
initSession실패 시loadHomeContents()가 호출되지 않아 UI가 초기 상태(빈 화면 또는 로딩 상태)에 머무릅니다. FIXME가 이미 남겨져 있지만, 최소한 에러 상태로 전환하거나 재시도 로직을 추가하는 것을 권장합니다.이 부분에 대한 에러 상태 처리 구현을 도와드릴까요? 또는 별도 이슈로 트래킹하시겠습니까?
95-97:loadMyTravel실패 시 상태가 갱신되지 않습니다.
onFailure에서 로깅만 하고 상태를 업데이트하지 않아,myTravel이 초기 상태에 머무릅니다.HomeState의myTravel초기값이 로딩 상태라면 사용자에게 무한 로딩으로 보일 수 있습니다. 실패 시에도MyTravel.None등으로 상태를 전환하는 것이 좋습니다.제안
.onFailure { Timber.e("Failed to load upcoming travel: $it") + reduce { copy(myTravel = HomeState.MyTravel.None) } }
101-110: 에러 로깅에Timber.d(디버그) 대신Timber.e(에러)를 사용해야 합니다.
loadMyTravel에서는Timber.e를 사용하는 반면, 이 메서드와loadPopularTemplatesByPrograms내부(Line 107, 116, 123)에서는Timber.d를 사용하고 있습니다. API 호출 실패는 에러 레벨로 로깅해야 프로덕션에서 문제를 추적할 수 있습니다.제안
- Timber.d("fail to load popular $it") + Timber.e("fail to load popular $it")
112-161:loadPopularTemplatesByPrograms에서 불필요한viewModelScope.launch사용.이 함수는 이미
loadPopularTemplates()의viewModelScope.launch내부에서 호출됩니다. 내부에서 다시viewModelScope.launch를 사용하면 호출자와 독립적인 코루틴이 생성되어, 호출자 코루틴이 완료되어도 내부 작업이 아직 진행 중일 수 있습니다.coroutineScope { }사용 또는suspend fun으로 변경하면 구조화된 동시성(structured concurrency)을 유지할 수 있습니다.제안
- private fun loadPopularTemplatesByPrograms(programs: List<TravelProgram>) { - viewModelScope.launch { + private suspend fun loadPopularTemplatesByPrograms(programs: List<TravelProgram>) { + coroutineScope { val allTemplateDeferred = async {
jihee-dev
force-pushed
the
feature/NDGL-88/impl-home-api
branch
from
0aee3aa to
ac20eec
Compare
jihee-dev
force-pushed
the
feature/NDGL-88/impl-home-api
branch
2 times, most recently
from
decc74a to
b5a6455
Compare
jihee-dev
force-pushed
the
feature/NDGL-88/impl-home-api
branch
from
b5a6455 to
5f21cb0
Compare
1 participant
NDGL-88 홈 화면 API 연결
연관 문서
디자인
변경사항
홈 화면 API 연동
데이터 모델 재구성
네트워크 모듈 리팩토링
UI 업데이트
인프라/설정
테스트 체크 리스트
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항