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:
개요이 변경 사항은 여행 템플릿 검색 기능을 도입하며, 새로운 검색 UI 컴포넌트, API 엔드포인트, 데이터 모델, 뷰모델 및 네비게이션 계층을 포함합니다. TravelContent 데이터 클래스는 HomeState에서 독립적인 모델로 추출되었습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User
participant HomeScreen
participant TemplateSearchScreen
participant TemplateSearchViewModel
participant TravelTemplateRepository
participant TravelTemplateApi
User->>HomeScreen: 검색 아이콘 클릭
HomeScreen->>TemplateSearchScreen: navigateToTemplateSearch()
User->>TemplateSearchScreen: 검색 키워드 입력
TemplateSearchScreen->>TemplateSearchViewModel: UpdateSearchKeyword
TemplateSearchViewModel->>TemplateSearchViewModel: 상태 업데이트
User->>TemplateSearchScreen: 검색 실행
TemplateSearchScreen->>TemplateSearchViewModel: SearchTemplate
TemplateSearchViewModel->>TravelTemplateRepository: searchTravelTemplates(keyword)
TravelTemplateRepository->>TravelTemplateApi: searchTravelTemplates(keyword)
TravelTemplateApi-->>TravelTemplateRepository: SearchTravelTemplates 응답
TravelTemplateRepository-->>TemplateSearchViewModel: 결과 반환
TemplateSearchViewModel->>TemplateSearchViewModel: TravelContent로 매핑
TemplateSearchViewModel->>TemplateSearchViewModel: 상태를 Success로 업데이트
TemplateSearchScreen->>TemplateSearchScreen: 여행 목록 렌더링
User->>TemplateSearchScreen: 여행 항목 클릭
TemplateSearchScreen->>TemplateSearchViewModel: ClickTravelTemplate
TemplateSearchViewModel->>TemplateSearchViewModel: NavigateToFollowTravel 사이드 이펙트 발행
TemplateSearchScreen->>HomeScreen: navigateToFollowTravel(travelId, days)
예상 코드 리뷰 노력🎯 4 (복잡함) | ⏱️ ~60분 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
feature/home/src/main/java/com/yapp/ndgl/feature/home/model/TravelContent.kt (1)
7-17:MyTravelState.RecommendedTravel과 구조적으로 거의 동일
feature/travel모듈의MyTravelState.RecommendedTravel과 모든 필드가 동일하며, 차이는thumbnailUrlvsthumbnail네이밍뿐입니다. 두 모델이 동일한 서버 응답을 표현한다면, 공통 데이터 레이어(:data:travel등)로 이동하는 것을 장기적으로 고려해볼 수 있습니다.🤖 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/TravelContent.kt` around lines 7 - 17, TravelContent duplicates the structure of MyTravelState.RecommendedTravel (same fields except thumbnail vs thumbnailUrl); consolidate by extracting a single shared model in the common data layer (e.g., create a TravelSummary/TravelDto in :data:travel) and update both TravelContent and MyTravelState.RecommendedTravel usages to reference that shared class, or alternatively replace TravelContent with the existing shared type and normalize the thumbnail field name (thumbnailUrl ↔ thumbnail) at mapping boundaries; update all mappers/serializers that construct these types so callers compile against the single shared symbol.core/ui/src/main/res/drawable/img_empty_browser.xml (1)
44-53:strokeAlpha는strokeColor없이는 효과 없음 (Figma 내보내기 아티팩트)Lines 46, 51의
<path>요소에strokeAlpha="0.5"가 지정되어 있으나strokeColor나strokeWidth가 없어 실질적으로 무시됩니다. 디자인 툴 내보내기 시 생성된 불필요한 속성이므로 제거해도 결과물에 영향이 없습니다.🧹 정리 제안
<path android:pathData="M70.44,50.35..." - android:strokeAlpha="0.5" android:fillColor="#F6F6F6" android:fillAlpha="0.5"/> <path android:pathData="M77.11,85.15..." - android:strokeAlpha="0.5" android:fillColor="#F6F6F6" android:fillAlpha="0.5"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/main/res/drawable/img_empty_browser.xml` around lines 44 - 53, Remove the redundant strokeAlpha attributes from the two <path> elements that include android:strokeAlpha="0.5" (identify them by their android:pathData values starting with "M70.44,50.35..." and "M77.11,85.15..."); since there are no strokeColor or strokeWidth attributes, delete android:strokeAlpha="0.5" from those <path> tags to clean up the Figma export artifact without changing rendering.feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchContract.kt (1)
14-18:SearchResult에Loading상태 부재 — 네트워크 요청 중 UI 피드백 확인 필요현재
SearchResult에는Idle,Empty,Success만 존재합니다. 검색 API 호출 중에 로딩 인디케이터를 표시할 수단이 없어 네트워크 지연 시 사용자가 아무 피드백 없이 대기하게 됩니다. 또한 API 실패 시에도 별도Error상태가 없어서onFailure처리 없이 무시됩니다(ViewModel의searchTravelTemplates참고).디자인 의도에 따라 로딩/에러 상태가 불필요할 수 있지만, 추후 UX 개선 시 고려해보시면 좋겠습니다.
🤖 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/search/TemplateSearchContract.kt` around lines 14 - 18, Add explicit Loading and Error states to the sealed interface SearchResult (e.g., data object Loading : SearchResult and data class Error(val throwable: Throwable?, val message: String?) : SearchResult) and update the ViewModel method searchTravelTemplates to emit SearchResult.Loading before starting the network call, emit SearchResult.Success or SearchResult.Empty on success, and emit SearchResult.Error on failure so the UI can show a loading indicator and present error feedback; reference the SearchResult sealed interface and the searchTravelTemplates function to locate where to add the new variants and emissions.feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchViewModel.kt (1)
36-63: 동시 검색 요청 시 이전 코루틴이 취소되지 않아 결과가 뒤바뀔 수 있습니다.사용자가 빠르게 여러 번 검색을 실행하면, 이전 검색 코루틴이 취소되지 않고 병렬 실행됩니다. 느린 요청이 나중에 완료되면 최신 검색 결과를 덮어쓸 수 있습니다. 별도의
Job변수를 두고 새 검색 시 이전Job을 취소하는 방식을 고려해보세요.♻️ Job 기반 취소 처리 제안
+import kotlinx.coroutines.Job + `@HiltViewModel` class TemplateSearchViewModel `@Inject` constructor( private val travelTemplateRepository: TravelTemplateRepository, ) : BaseViewModel<TemplateSearchState, TemplateSearchIntent, TemplateSearchSideEffect>( initialState = TemplateSearchState( searchKeyword = "", searchResult = SearchResult.Idle, ), ) { + private var searchJob: Job? = null + // ... private fun searchTravelTemplates(keyword: String) { - viewModelScope.launch { + searchJob?.cancel() + searchJob = viewModelScope.launch { suspendRunCatching { travelTemplateRepository.searchTravelTemplates(keyword) }🤖 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/search/TemplateSearchViewModel.kt` around lines 36 - 63, The searchTravelTemplates function can start overlapping coroutines; add a Job property (e.g., private var searchJob: Job? = null) in TemplateSearchViewModel, cancel and null out the previous job before launching a new one, then assign searchJob = viewModelScope.launch { ... } around the existing suspendRunCatching block (keeping the existing mapping and reduce logic intact) so that any prior in-flight search is cancelled when a new keyword search starts, preventing stale results from overwriting newer ones.feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt (1)
120-188:InitialEmptyView와EmptyResultView의 중복 구조를 공통 컴포저블로 추출하는 것을 고려해 주세요.두 컴포저블의 레이아웃 구조(Column > Image + Text Column)가 거의 동일하며, 이미지 리소스와 텍스트만 다릅니다. 공통 컴포저블로 추출하면 유지보수가 용이해집니다.
♻️ 공통 빈 상태 뷰 추출 제안
+@Composable +private fun EmptyStateView( + `@DrawableRes` imageRes: Int, + title: String, + description: String, +) { + Column( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 24.dp, vertical = 165.dp), + verticalArrangement = Arrangement.spacedBy(16.dp), + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Image( + painter = painterResource(imageRes), + contentDescription = null, + modifier = Modifier.size(100.dp), + ) + Column( + modifier = Modifier.fillMaxWidth(), + verticalArrangement = Arrangement.spacedBy(4.dp), + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text( + text = title, + color = NDGLTheme.colors.black500, + textAlign = TextAlign.Center, + style = NDGLTheme.typography.subtitleMdSemiBold, + ) + Text( + text = description, + color = NDGLTheme.colors.black400, + textAlign = TextAlign.Center, + style = NDGLTheme.typography.bodyLgRegular, + ) + } + } +}🤖 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/search/TemplateSearchScreen.kt` around lines 120 - 188, Extract the duplicated Column + Image + Text Column structure from InitialEmptyView and EmptyResultView into a reusable composable (e.g., EmptyStateView) that accepts parameters for the image resource id (painterRes), title string-res id and description string-res id; move the common Modifier (.fillMaxWidth().padding(...)), Image modifier (.size(100.dp)), Text styles (NDGLTheme.typography... and NDGLTheme.colors...) and alignment into that composable, then replace InitialEmptyView and EmptyResultView to call EmptyStateView with their respective CoreR.drawable.* and R.string.* resource ids so only the differing resources remain in the callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLSearchNavigationBar.kt`:
- Around line 49-60: NDGLSearchNavigationBar currently sets contentDescription =
null on interactive Icon instances (the back Icon and the search Icon), breaking
TalkBack accessibility; add two optional parameters to NDGLSearchNavigationBar
(e.g., backButtonContentDescription: String? and searchIconContentDescription:
String?) and use those values for the Icon contentDescription properties instead
of null, keeping them optional to preserve callers; update the back button Icon
(the ImageVector.vectorResource with ic_28_chevron_left) and the search Icon
usage to pass the new params, and ensure callers can supply stringResource(...)
values as needed.
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt`:
- Around line 200-202: postNavigateToTravelTemplate currently hardcodes days =
1; change it to use the real template days by either (A) adding a days parameter
to postNavigateToTravelTemplate and passing the TravelContent.days from the
intent handler (update HomeIntent.ClickTravel to carry days and propagate), or
(B) reading the selected TravelContent.days from the ViewModel state inside
postNavigateToTravelTemplate before calling
postSideEffect(HomeSideEffect.NavigateToFollowTravel); ensure the
HomeSideEffect.NavigateToFollowTravel invocation uses that actual days value so
FollowTravelViewModel receives the correct duration (consistent with how
TemplateSearchViewModel and MyTravelViewModel supply template days).
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt`:
- Line 26: Update the deprecated Hilt Compose ViewModel import across the listed
screens: replace any import of
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel with
androidx.hilt.navigation.compose.hiltViewModel in each file (e.g.,
TemplateSearchScreen.kt, HomeScreen.kt, AuthScreen.kt, PlaceDetailScreen.kt,
TravelHelperScreen.kt, TravelDetailScreen.kt, TravelEntry.kt,
FollowTravelScreen.kt, FollowPlaceDetailScreen.kt, MyTravelScreen.kt,
DatePickerScreen.kt) so usages of hiltViewModel() resolve against the
hilt-navigation-compose artifact already declared in Gradle.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchViewModel.kt`:
- Around line 36-63: The searchTravelTemplates function currently only handles
suspendRunCatching.onSuccess, so add an onFailure handler after onSuccess to
handle network/server errors from
travelTemplateRepository.searchTravelTemplates: log the throwable (or pass to
your logger) and call reduce to update searchResult (e.g., SearchResult.Empty or
a new SearchResult.Error) so the UI receives error feedback; ensure you
reference the existing suspendRunCatching, onSuccess block, reduce lambda, and
SearchResult sealed types when implementing this change.
- Around line 66-73: The navigation is hardcoding days=1 in
postNavigateToTravelTemplate causing loss of the actual travel days; update the
intent and call chain so days flows from the search result: add a days parameter
to the ClickTravelTemplate intent (and its handler), change where
ClickTravelTemplate is created in
TemplateSearchViewModel.searchTravelTemplates() to include travel.days (from
TravelTemplateSummary), and then update postNavigateToTravelTemplate(travelId:
Long, days: Int) to forward that days value into
TemplateSearchSideEffect.NavigateToFollowTravel(days = days) instead of 1;
adjust any callers to pass the new days argument.
---
Nitpick comments:
In `@core/ui/src/main/res/drawable/img_empty_browser.xml`:
- Around line 44-53: Remove the redundant strokeAlpha attributes from the two
<path> elements that include android:strokeAlpha="0.5" (identify them by their
android:pathData values starting with "M70.44,50.35..." and "M77.11,85.15...");
since there are no strokeColor or strokeWidth attributes, delete
android:strokeAlpha="0.5" from those <path> tags to clean up the Figma export
artifact without changing rendering.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/model/TravelContent.kt`:
- Around line 7-17: TravelContent duplicates the structure of
MyTravelState.RecommendedTravel (same fields except thumbnail vs thumbnailUrl);
consolidate by extracting a single shared model in the common data layer (e.g.,
create a TravelSummary/TravelDto in :data:travel) and update both TravelContent
and MyTravelState.RecommendedTravel usages to reference that shared class, or
alternatively replace TravelContent with the existing shared type and normalize
the thumbnail field name (thumbnailUrl ↔ thumbnail) at mapping boundaries;
update all mappers/serializers that construct these types so callers compile
against the single shared symbol.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchContract.kt`:
- Around line 14-18: Add explicit Loading and Error states to the sealed
interface SearchResult (e.g., data object Loading : SearchResult and data class
Error(val throwable: Throwable?, val message: String?) : SearchResult) and
update the ViewModel method searchTravelTemplates to emit SearchResult.Loading
before starting the network call, emit SearchResult.Success or
SearchResult.Empty on success, and emit SearchResult.Error on failure so the UI
can show a loading indicator and present error feedback; reference the
SearchResult sealed interface and the searchTravelTemplates function to locate
where to add the new variants and emissions.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt`:
- Around line 120-188: Extract the duplicated Column + Image + Text Column
structure from InitialEmptyView and EmptyResultView into a reusable composable
(e.g., EmptyStateView) that accepts parameters for the image resource id
(painterRes), title string-res id and description string-res id; move the common
Modifier (.fillMaxWidth().padding(...)), Image modifier (.size(100.dp)), Text
styles (NDGLTheme.typography... and NDGLTheme.colors...) and alignment into that
composable, then replace InitialEmptyView and EmptyResultView to call
EmptyStateView with their respective CoreR.drawable.* and R.string.* resource
ids so only the differing resources remain in the callers.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchViewModel.kt`:
- Around line 36-63: The searchTravelTemplates function can start overlapping
coroutines; add a Job property (e.g., private var searchJob: Job? = null) in
TemplateSearchViewModel, cancel and null out the previous job before launching a
new one, then assign searchJob = viewModelScope.launch { ... } around the
existing suspendRunCatching block (keeping the existing mapping and reduce logic
intact) so that any prior in-flight search is cancelled when a new keyword
search starts, preventing stale results from overwriting newer ones.
jihee-dev
force-pushed
the
feature/NDGL-101/impl-search-travel-template
branch
2 times, most recently
from
bf13d61 to
9de5bc3
Compare
jihee-dev
force-pushed
the
feature/NDGL-101/impl-search-travel-template
branch
from
9de5bc3 to
fa9ba69
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-101 여행 템플릿 검색 기능 구현
연관 문서
디자인
스크린샷 (Optional)
변경사항
/api/v1/travel-templates/searchAPI 추가테스트 체크 리스트
Summary by CodeRabbit
Release Notes
New Features
UI Improvements