[NDGL-107] 인기 여행 따라가기 페이지 구현#28
Merged
Merged
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:
워크스루인기 여행 목록 기능을 도입하면서 TravelProgramTab 데이터 모델을 홈 계약에서 별도 모델 파일로 추출했습니다. 새로운 화면, 뷰모델, 계약을 추가하고 네비게이션 인프라를 확장하며 기존 홈 및 여행 화면을 업데이트하여 이 기능을 통합했습니다. Changes
시퀀스 다이어그램sequenceDiagram
actor User
participant HomeScreen
participant HomeViewModel
participant Repository as TravelProgram/<br>TemplateRepository
participant PopularTravelListScreen
participant PopularTravelListViewModel
User->>HomeScreen: "더보기" 클릭
HomeScreen->>HomeViewModel: ClickTravelMore
HomeViewModel->>HomeViewModel: postNavigateToTravelMore()
HomeViewModel-->>HomeScreen: NavigateToTravelMore
HomeScreen->>User: 인기 여행 목록으로 이동
User->>PopularTravelListScreen: 화면 표시
PopularTravelListScreen->>PopularTravelListViewModel: 초기화
PopularTravelListViewModel->>Repository: 모든 프로그램 조회
PopularTravelListViewModel->>Repository: 각 프로그램별 템플릿 조회
PopularTravelListViewModel->>Repository: 인기 템플릿 조회 (모두)
Repository-->>PopularTravelListViewModel: 데이터 반환
PopularTravelListViewModel->>PopularTravelListViewModel: 상태 업데이트
PopularTravelListViewModel-->>PopularTravelListScreen: 상태 제공
PopularTravelListScreen->>User: UI 렌더링
User->>PopularTravelListScreen: 탭 선택 또는 여행 클릭
PopularTravelListScreen->>PopularTravelListViewModel: SelectPopularTravelTab/<br>ClickTravel
PopularTravelListViewModel-->>PopularTravelListScreen: 네비게이션 사이드 이펙트
PopularTravelListScreen->>User: 해당 화면으로 이동
sequenceDiagram
participant HomeViewModel
participant PopularTravelListViewModel
participant State as PopularTravelListState
participant Repository
HomeViewModel->>PopularTravelListViewModel: 초기화
PopularTravelListViewModel->>Repository: 프로그램 목록 로드
PopularTravelListViewModel->>Repository: 각 프로그램 템플릿 로드 (병렬)
PopularTravelListViewModel->>Repository: 전체 인기 템플릿 로드 (병렬)
Repository-->>PopularTravelListViewModel: TravelProgram 리스트
Repository-->>PopularTravelListViewModel: TravelTemplate 맵
Repository-->>PopularTravelListViewModel: TravelTemplate 리스트
PopularTravelListViewModel->>State: travelProgramTabs 설정<br>(All + Custom 탭)
PopularTravelListViewModel->>State: allPopularTravels 설정
PopularTravelListViewModel->>State: popularTravelsByProgram 설정
PopularTravelListViewModel->>State: selectedProgramTravels<br>(lazy 계산)
State-->>PopularTravelListViewModel: 상태 준비 완료
PopularTravelListViewModel-->>HomeViewModel: UI 렌더링 준비
코드 리뷰 노력 추정🎯 4 (Complex) | ⏱️ ~45 분 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
jihee-dev
force-pushed
the
feature/NDGL-107/impl-follow-travel
branch
from
b07c8ba to
7a3610b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListScreen.kt (2)
101-119: [선택] 탭 목록 매핑 결과를remember로 메모이제이션 고려
state.travelProgramTabs.map { ... }.toPersistentList()가 리컴포지션마다 실행됩니다.travelProgramTabs가 자주 변경되지 않는 경우,remember(state.travelProgramTabs) { ... }로 감싸면 불필요한 재계산을 줄일 수 있습니다.♻️ 수정 제안
+ val tabs = remember(state.travelProgramTabs) { + state.travelProgramTabs.map { tab -> + when (tab) { + TravelProgramTab.All -> NDGLChipTabAttr.Tab(tag = "All", name = "전체") + is TravelProgramTab.Custom -> NDGLChipTabAttr.Tab( + tag = tab.programId.toString(), + name = tab.name, + leadingIcon = tab.type.toIconRes(), + ) + } + }.toPersistentList() + } NDGLChipTab( - tabs = state.travelProgramTabs.map { tab -> - when (tab) { - TravelProgramTab.All -> NDGLChipTabAttr.Tab( - tag = "All", - name = "전체", - ) - is TravelProgramTab.Custom -> NDGLChipTabAttr.Tab( - tag = tab.programId.toString(), - name = tab.name, - leadingIcon = tab.type.toIconRes(), - ) - } - }.toPersistentList(), + tabs = tabs,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListScreen.kt` around lines 101 - 119, The mapping of state.travelProgramTabs into the NDGLChipTab tabs list is recomputed on every recomposition; wrap the mapping in a remember keyed on state.travelProgramTabs to memoize it (e.g., compute a rememberedTabs = remember(state.travelProgramTabs) { state.travelProgramTabs.map { ... }.toPersistentList() } and pass rememberedTabs to NDGLChipTab) so NDGLChipTab receives a stable list and avoids unnecessary recalculation; update usage of NDGLChipTab to use this remembered list while keeping selectedIndex and onTabSelected unchanged.
75-93: [선택]Scaffold에fillMaxSize추가 고려
MyTravelScreen(Line 87)은Scaffold에modifier = Modifier.fillMaxSize()를 명시하지만,PopularTravelListScreen의Scaffold에는 해당 modifier가 없어 일관성이 다소 떨어집니다. 실제 동작에는 영향 없을 수 있으나, 명시적으로 추가하면 코드 의도가 명확해집니다.♻️ 수정 제안
Scaffold( + modifier = Modifier.fillMaxSize(), topBar = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListScreen.kt` around lines 75 - 93, Add an explicit full-size modifier to the Scaffold in PopularTravelListScreen to match MyTravelScreen: update the Scaffold call inside PopularTravelListScreen to include modifier = Modifier.fillMaxSize() (same modifier pattern used in MyTravelScreen) so the intent is explicit and consistent across screens while keeping existing topBar, NDGLNavigationBar, and trailingContents unchanged.feature/home/src/main/java/com/yapp/ndgl/feature/home/model/TravelProgramTab.kt (1)
5-13: [선택] Compose 안정성 어노테이션 추가 고려
HomeContract.kt의MyTravelsealed interface처럼@Stable/@Immutable어노테이션을 명시적으로 추가하면 코드베이스 내 일관성을 높일 수 있습니다.Long,String,enum으로 구성된 필드는 Compose 컴파일러가 안정성을 자동 추론할 수 있으나, 명시적 어노테이션은 의도를 명확히 전달합니다.♻️ 수정 제안
+import androidx.compose.runtime.Immutable +import androidx.compose.runtime.Stable +@Stable sealed interface TravelProgramTab { + `@Immutable` data object All : TravelProgramTab + `@Immutable` data class Custom( val programId: Long, val name: String, val type: ProgramType, ) : TravelProgramTab }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/model/TravelProgramTab.kt` around lines 5 - 13, Add explicit Compose stability annotations to the TravelProgramTab sealed type to match HomeContract.kt's MyTravel: annotate the sealed interface TravelProgramTab with `@Immutable` (or `@Stable`) and annotate the Custom data class and the All object accordingly, and add the necessary import from androidx.compose.runtime (e.g., androidx.compose.runtime.Immutable). This makes the intent explicit for TravelProgramTab, All, and Custom (programId, name, type) without changing runtime behavior.feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListViewModel.kt (2)
34-44: 에러 발생 시 사용자 피드백 부재
getAllPrograms()실패 시Timber.d로 로깅만 하고 사용자에게 에러 상태를 전달하지 않습니다. 개별 프로그램 템플릿 조회(line 49, 53-55)도getOrNull()로 실패를 무시하여 빈 목록만 표시됩니다. FIXME 주석이 있으니 인지하고 계신 것 같지만, 에러 뷰 구현이 후속 작업으로 예정되어 있는지 확인해 주세요.이 이슈를 추적하기 위한 별도 이슈를 생성해 드릴까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListViewModel.kt` around lines 34 - 44, In loadPopularTemplates(), failure from travelProgramRepository.getAllPrograms() is currently only logged; update loadPopularTemplates to surface errors to the UI by exposing an error state (e.g., LiveData/StateFlow like popularTemplatesError or uiState) and set it in the onFailure block with the exception details; likewise update loadPopularTemplatesByPrograms (which currently calls getOrNull() and silently drops failures) to propagate errors upward instead of swallowing them—return a Result or throw/emit an error so the caller can update the same error state and trigger the error view; ensure UI observes this error state to display a user-facing error view or retry action.
34-106:HomeViewModel과의 코드 중복 — 공통 로직 추출을 권장합니다.
loadPopularTemplates(),loadPopularTemplatesByPrograms(),toTravelContent()매핑이HomeViewModel과 거의 동일하게 구현되어 있습니다. 두 ViewModel의 핵심 로직은 동일하며, 현재는 컬렉션 타입(ListvsImmutableList)과 페이지네이션 처리(MAX_POPULAR_TRAVEL_COUNT적용 여부)만 다릅니다. 추후 유지보수 시 한쪽만 수정하고 다른 쪽을 놓치는 문제를 방지하기 위해 공통 UseCase나 헬퍼로 추출하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListViewModel.kt` around lines 34 - 106, Duplicate logic in loadPopularTemplates, loadPopularTemplatesByPrograms, and toTravelContent should be extracted into a shared use-case/helper to avoid drift with HomeViewModel; create a single FetchPopularTravelTemplatesUseCase (or helper) that accepts travelProgramRepository and travelTemplateRepository and returns a data class containing tabs, allPopularTravels (ImmutableList<TravelContent>), and popularTravelsByProgram (ImmutableMap<Long, ImmutableList<TravelContent>>) while preserving existing null/empty handling and optional MAX_POPULAR_TRAVEL_COUNT behavior; move the TravelTemplateSummary -> TravelContent mapping into a shared mapper used by the use-case; then update PopularTravelListViewModel.loadPopularTemplates and loadPopularTemplatesByPrograms to call the new use-case and reduce() with the returned data, and remove the duplicated toTravelContent implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt`:
- Around line 48-50: CI is failing due to missing trailing commas on call sites;
add a trailing comma before the closing parenthesis for the lambda argument at
onTravelMoreClick (the call invoking
viewModel.onIntent(HomeIntent.ClickTravelMore)) and the two other reported call
sites (lines referenced in the comment) so each multi-line argument list ends
with a comma before the ) — locate the lambda argument assignments (e.g.,
onTravelMoreClick = { ... }) and insert a comma after the closing brace in each
call site.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/navigation/HomeEntry.kt`:
- Around line 22-25: Add the missing trailing comma after the lambda assigned to
navigateToPopularTravelList to satisfy the TrailingCommaOnCallSite lint rule:
locate the call site where navigateToPopularTravelList = {
navigator.navigate(Route.PopularTravelList) } is passed (in HomeEntry or the
composable invocation) and insert a comma immediately after the closing brace of
that lambda so the call's argument list ends with a trailing comma.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListContract.kt`:
- Around line 26-28: The Kotlin call sites for
popularTravelsByProgram.getOrDefault(...) and persistentListOf(...) are missing
trailing commas causing the TrailingCommaOnCallSite/DeclarationSite CI errors;
update the calls referenced (the branches handling TravelProgramTab.Custom and
similar uses in PopularTravelListContract) to include trailing commas after the
last argument so the call sites end with a trailing comma (e.g., in the
getOrDefault invocation and the persistentListOf invocation used as its default)
to satisfy the lint rule.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListScreen.kt`:
- Around line 59-63: The call handling
PopularTravelListSideEffect.NavigateToFollowTravel is missing a trailing comma
after the last argument in navigateToFollowTravel(sideEffect.travelId,
sideEffect.days); update the invocation inside the when branch that matches
PopularTravelListSideEffect.NavigateToFollowTravel to add a trailing comma after
sideEffect.days so the call becomes multiline with a trailing comma to satisfy
the TrailingCommaOnCallSite lint rule.
- Around line 95-100: The Column(...) call is missing a trailing comma after its
last argument which triggers the TrailingCommaOnCallSite lint; update the Column
invocation (the call that passes modifier =
Modifier.fillMaxSize().padding(innerPadding) and verticalArrangement =
Arrangement.spacedBy(12.dp)) to include a trailing comma after the final
parameter so the call site ends with a comma and the Kotlin formatter/linter
will accept the change.
- Around line 104-107: Replace the hardcoded Korean label "전체" in the
TravelProgramTab.All -> NDGLChipTabAttr.Tab invocation with a string resource:
add <string name="home_popular_travel_tab_all">전체</string> to res/strings.xml
and use R.string.home_popular_travel_tab_all (similar to the existing use of
R.string.home_popular_travel_section_title) when constructing the Tab's name
property to ensure consistent localization.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListViewModel.kt`:
- Around line 27-28: The CI error is caused by missing trailing commas at call
sites; update the PopularTravelListViewModel constructor invocation (the
BaseViewModel<PopularTravelListState, PopularTravelListIntent,
PopularTravelListSideEffect>( initialState = PopularTravelListState() ) call) to
include a trailing comma after the last argument, and apply the same fix to the
other multiline call/block referenced around lines 125-130 (ensure trailing
commas where parameters/arguments are split across lines) so the Kotlin
TrailingCommaOnCallSite rule is satisfied.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/navigation/TravelEntry.kt`:
- Around line 42-44: Add the missing trailing comma after the lambda assigned to
navigateToPopularTravelList to satisfy the TrailingCommaOnCallSite lint rule;
locate the property/argument named navigateToPopularTravelList (the block
containing navigator.navigate(Route.PopularTravelList)) and insert a comma after
the closing brace of that lambda.
---
Nitpick comments:
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/model/TravelProgramTab.kt`:
- Around line 5-13: Add explicit Compose stability annotations to the
TravelProgramTab sealed type to match HomeContract.kt's MyTravel: annotate the
sealed interface TravelProgramTab with `@Immutable` (or `@Stable`) and annotate the
Custom data class and the All object accordingly, and add the necessary import
from androidx.compose.runtime (e.g., androidx.compose.runtime.Immutable). This
makes the intent explicit for TravelProgramTab, All, and Custom (programId,
name, type) without changing runtime behavior.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListScreen.kt`:
- Around line 101-119: The mapping of state.travelProgramTabs into the
NDGLChipTab tabs list is recomputed on every recomposition; wrap the mapping in
a remember keyed on state.travelProgramTabs to memoize it (e.g., compute a
rememberedTabs = remember(state.travelProgramTabs) { state.travelProgramTabs.map
{ ... }.toPersistentList() } and pass rememberedTabs to NDGLChipTab) so
NDGLChipTab receives a stable list and avoids unnecessary recalculation; update
usage of NDGLChipTab to use this remembered list while keeping selectedIndex and
onTabSelected unchanged.
- Around line 75-93: Add an explicit full-size modifier to the Scaffold in
PopularTravelListScreen to match MyTravelScreen: update the Scaffold call inside
PopularTravelListScreen to include modifier = Modifier.fillMaxSize() (same
modifier pattern used in MyTravelScreen) so the intent is explicit and
consistent across screens while keeping existing topBar, NDGLNavigationBar, and
trailingContents unchanged.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListViewModel.kt`:
- Around line 34-44: In loadPopularTemplates(), failure from
travelProgramRepository.getAllPrograms() is currently only logged; update
loadPopularTemplates to surface errors to the UI by exposing an error state
(e.g., LiveData/StateFlow like popularTemplatesError or uiState) and set it in
the onFailure block with the exception details; likewise update
loadPopularTemplatesByPrograms (which currently calls getOrNull() and silently
drops failures) to propagate errors upward instead of swallowing them—return a
Result or throw/emit an error so the caller can update the same error state and
trigger the error view; ensure UI observes this error state to display a
user-facing error view or retry action.
- Around line 34-106: Duplicate logic in loadPopularTemplates,
loadPopularTemplatesByPrograms, and toTravelContent should be extracted into a
shared use-case/helper to avoid drift with HomeViewModel; create a single
FetchPopularTravelTemplatesUseCase (or helper) that accepts
travelProgramRepository and travelTemplateRepository and returns a data class
containing tabs, allPopularTravels (ImmutableList<TravelContent>), and
popularTravelsByProgram (ImmutableMap<Long, ImmutableList<TravelContent>>) while
preserving existing null/empty handling and optional MAX_POPULAR_TRAVEL_COUNT
behavior; move the TravelTemplateSummary -> TravelContent mapping into a shared
mapper used by the use-case; then update
PopularTravelListViewModel.loadPopularTemplates and
loadPopularTemplatesByPrograms to call the new use-case and reduce() with the
returned data, and remove the duplicated toTravelContent implementation.
7a3610b to
a5c160d
Compare
jihee-dev
force-pushed
the
feature/NDGL-107/impl-follow-travel
branch
from
a5c160d to
814799c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NDGL-107 인기 여행 따라가기 페이지 구현
연관 문서
디자인
스크린샷 (Optional)
변경사항
테스트 체크 리스트
Summary by CodeRabbit
릴리스 노트