[NDGL-68] 따라가기 여행 템플릿 관련 API 연동#23
Conversation
- 영향 받은 화면 : PlaceDetail,TravelDetail, FollowTravel
|
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이 PR은 여행 템플릿 데이터 로딩 기능을 추가하고, 데이터 모델들을 공유 모델 패키지로 이동하며, UI 레이아웃을 Scaffold 기반으로 리팩토링하고, 네비게이션에 days 파라미터를 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as FollowTravelScreen
participant VM as FollowTravelViewModel
participant Repo as TravelTemplateRepository
participant API as TravelTemplateApi
participant Data as Backend
UI->>VM: 초기화 (travelId, days)
activate VM
VM->>VM: loadTravelTemplateItinerary() 시작
loop 각 day (1..days)
VM->>Repo: getTravelTemplateItinerary(travelId, day)
activate Repo
Repo->>API: getTravelTemplateItinerary(travelId, day)
activate API
API->>Data: GET /api/v1/travel-templates/{id}/itinerary?day={day}
Data-->>API: TravelTemplateItinerary
API-->>Repo: BaseResponse<TravelTemplateItinerary>
deactivate API
Repo-->>VM: TravelTemplateItinerary
deactivate Repo
VM->>VM: Itinerary로 변환 및 상태 업데이트
end
par 병렬 요청
VM->>Repo: getTravelTemplateContentInfo(travelId)
activate Repo
Repo->>API: getTravelTemplateContentInfo(travelId)
activate API
API->>Data: GET /api/v1/travel-templates/{id}/content-card
Data-->>API: TravelTemplateContentInfo
API-->>Repo: BaseResponse<TravelTemplateContentInfo>
deactivate API
Repo-->>VM: TravelTemplateContentInfo
deactivate Repo
VM->>VM: ContentInfo로 변환 및 상태 업데이트
end
deactivate VM
VM-->>UI: 상태 업데이트 (itineraries, contentInfo)
activate UI
UI->>UI: 탭 및 장소 정보 렌더링
deactivate UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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
🤖 Fix all issues with AI agents
In `@core/util/src/main/java/com/yapp/ndgl/core/util/CountryCodeUtil.kt`:
- Around line 5-9: The String.toCountryName() function can throw
IllformedLocaleException when setRegion() receives lowercase or invalid input;
to fix it, normalize the receiver by trimming and converting to uppercase (e.g.,
call uppercase()) before passing to Locale.Builder().setRegion(), and wrap the
Locale.Builder()/setRegion()/build() and locale.displayCountry access in a
try-catch that catches IllformedLocaleException (and any other relevant
exceptions) and returns an empty string on error; ensure you still return "" for
non-2-character inputs and reference the String.toCountryName() function and
Locale.Builder().setRegion() locations when making the change.
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/model/TravelTemplateItinerary.kt`:
- Line 16: Rename the misspelled property travlerTips to travelerTips in the
TravelTemplateItinerary data class (TravelTemplateItinerary.kt); update any
references/usages and constructor parameters to the new name, and if this class
is serialized/deserialized ensure you add or update `@SerialName` or migration
handling if other modules expect the old name so deserialization remains
compatible.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerScreen.kt`:
- Around line 133-156: The button is currently marked ACTIVE whenever
state.isDateSelected is true, even if state.isInsufficientDuration is also true;
update the NDGLCTAButton status logic so the button is ACTIVE only when
isDateSelected is true AND isInsufficientDuration is false (otherwise use
DISABLED). Locate the NDGLCTAButton instantiation in DatePickerScreen and change
the status expression to combine DatePickerState.isDateSelected and
!DatePickerState.isInsufficientDuration; also ensure clickCompleteButton is not
triggered when the button is disabled (rely on the button component to ignore
clicks when status is DISABLED or guard the handler if needed).
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlacePhoto.kt`:
- Around line 8-9: The computed property aspectRatio in PlacePhoto (val
aspectRatio: Float get() = width.toFloat() / height.toFloat()) can produce
Infinity or NaN when height is 0 or non‑positive; update the getter to
defensively handle height <= 0 by returning a safe finite positive fallback (for
example 1f or max(1f, width.toFloat())) and/or clamp the result to a finite
positive value so Modifier.aspectRatio() always receives a valid positive float;
change only the aspectRatio getter in PlacePhoto.kt to perform the check and
return the safe value.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/travel/TravelViewModel.kt`:
- Around line 23-25: In clickTravel, stop ignoring the travelId and remove the
hardcoded postSideEffect call; call
postSideEffect(TravelSideEffect.NavigateToFollowTravel(travelId.toLong(),
<followId>)) using the actual travelId passed to clickTravel and convert it with
.toLong(), or alternatively change TravelIntent.ClickTravel.travelId in
TravelContract.kt from Int to Long so types align; update any callers of
ClickTravel accordingly to keep types consistent and ensure
NavigateToFollowTravel receives the correct long travelId instead of the
hardcoded (2, 1).
🧹 Nitpick comments (14)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoRow.kt (2)
42-44:clip(CircleShape)이 trailing 아이콘에 불필요해 보입니다.배경(background)이 없는 상태에서
clip(CircleShape)만 적용하면, 아이콘의 시각적 콘텐츠가 원형으로 잘릴 수 있습니다. chevron 아이콘의 경우 모서리 부분이 잘릴 수 있습니다. 의도된 동작이 아니라면 제거를 권장합니다.제안
modifier = Modifier - .size(24.dp) - .clip(CircleShape), + .size(24.dp),
27-50:content가 남은 공간을 채우지 않아 chevron이 오른쪽 끝에 배치되지 않을 수 있습니다.현재
Row에horizontalArrangement = spacedBy(8.dp)만 적용되어 있어, content 영역이 좁을 경우 trailing chevron 아이콘이 Row 끝이 아닌 content 바로 옆에 위치합니다. 호출부에서Modifier.weight(1f)를 content에 적용해야 올바르게 정렬됩니다. 만약 chevron을 항상 오른쪽 끝에 두는 것이 의도라면,Spacer(Modifier.weight(1f))를 content와 chevron 사이에 추가하거나Arrangement.SpaceBetween활용을 고려해 보세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerScreen.kt (1)
113-158: 중첩된 Column 단순화 고려외부 Column(Line 113-158)은
padding(top = 24.dp, bottom = 16.dp)만 적용하고, 내부 Column(Line 118-157)이 실제 콘텐츠를 담고 있습니다. 두 Column 모두fillMaxSize()를 사용하며, 하나로 합쳐도 동일한 레이아웃을 구현할 수 있습니다.단일 Column로 병합하는 예시
- Column( - modifier = Modifier - .fillMaxSize() - .padding(top = 24.dp, bottom = 16.dp), - ) { - Column( - modifier = Modifier - .fillMaxSize() - .padding(horizontal = 24.dp), - ) { + Column( + modifier = Modifier + .fillMaxSize() + .padding(top = 24.dp, bottom = 16.dp) + .padding(horizontal = 24.dp), + ) { CalendarView( ... ) ... NDGLCTAButton( ... ) - } - } + }feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlaceType.kt (1)
20-26:RESTAURANT과CAFE가 동일한 색상(etcOrange)을 사용합니다.의도된 디자인이라면 무시해주세요. 만약 두 타입의 시각적 구분이 필요하다면 별도 색상 할당을 고려해보세요.
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PriceRange.kt (1)
7-8:formattedPriceRange에서endPrice의 통화 기호가 누락되어 있습니다.현재
"€5~15"형태로 출력되는데,startPrice와endPrice의 통화가 다를 경우 오해의 소지가 있습니다. 동일 통화가 보장된다면 현재 형태도 괜찮지만, 방어적으로endPrice.symbol도 포함하거나 통화 일치 여부를 검증하는 것을 고려해보세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/OpeningHours.kt (1)
13-27: 파싱 로직은 잘 동작하지만, 입력 형식에 대한 암묵적 의존이 있습니다.
", "와"~"구분자에 의존하는 파싱이므로, 서버 응답 형식이 변경되면 조용히 빈periods를 반환할 수 있습니다. 현 단계에서는 문제없지만, 추후 API 연동 시 구조화된 JSON 응답(배열 형태)으로 대체하는 것이 더 안정적입니다.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.kt (1)
99-114:openingHours와estimatedDuration모두 동일한ic_24_clock아이콘 사용두 행이 연속으로 표시될 때 동일한 시계 아이콘이 반복되어 사용자가 구분하기 어려울 수 있습니다.
estimatedDuration에 다른 아이콘(예: 타이머/모래시계)을 사용하는 것을 고려해 보세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailScreen.kt (1)
254-264: 사진 분할 로직이 매 recomposition마다 재계산됩니다
foldIndexed와mutableListOf를 사용한 좌/우 사진 분할이 composable 내부에서 매번 실행됩니다.remember로 감싸거나partition사용을 권장합니다.♻️ partition 사용 제안
- val (leftPhotos, rightPhotos) = state.photos.foldIndexed( - initial = mutableListOf<PlacePhoto>() to mutableListOf<PlacePhoto>(), - ) { index, lists, photo -> - if (index % 2 == 0) { - lists.first.add(photo) - } else { - lists.second.add(photo) - } - lists - } + val leftPhotos = state.photos.filterIndexed { index, _ -> index % 2 == 0 } + val rightPhotos = state.photos.filterIndexed { index, _ -> index % 2 != 0 }feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportType.kt (1)
15-17:else분기에서 TRANSIT, BUS, TRAIN 등 모든 교통수단이 CAR로 매핑됩니다.
TransportCategory에는 DRIVING, TRANSIT, BICYCLING, TAXI, TWO_WHEELER, FERRY, FLIGHT 등 7개 카테고리가 있지만, WALKING 외에는 전부CAR로 폴백됩니다.TransportType에 이미BUS,TRAIN이 정의되어 있으므로 최소한TRANSIT매핑은 추가하는 것이 좋겠습니다.제안
internal fun TransportCategory.toTransportType(): TransportType = when (this) { TransportCategory.WALKING -> TransportType.WALK - else -> TransportType.CAR // FIXME: 교통수단 변경 + TransportCategory.TRANSIT -> TransportType.TRAIN + TransportCategory.DRIVING, TransportCategory.TAXI -> TransportType.CAR + else -> TransportType.CAR // FIXME: BICYCLING, TWO_WHEELER, FERRY, FLIGHT 매핑 추가 }feature/travel/src/main/java/com/yapp/ndgl/feature/travel/travel/TravelContract.kt (1)
11-18:travelId타입 불일치: Intent는Int, SideEffect는Long
ClickTravel.travelId는Int(Line 12)인 반면,NavigateToFollowTravel.travelId는Long(Line 17)입니다.Route.FollowTravel과FollowTravelViewModel이 모두Long을 사용하고 있으므로, Intent 쪽도Long으로 맞추면 향후 실제 travelId를 전달할 때 타입 변환 누락 위험을 방지할 수 있습니다.제안
sealed interface TravelIntent : UiIntent { - data class ClickTravel(val travelId: Int) : TravelIntent + data class ClickTravel(val travelId: Long) : TravelIntent data class ClickTravelDetail(val travelId: Int) : TravelIntent }feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelViewModel.kt (2)
107-121:days,countryCode등 이름 섀도잉으로 인한 가독성 저하
TravelTemplateContentInfo.toContentInfo()는 ViewModel의 멤버 함수이면서 동시에TravelTemplateContentInfo의 확장 함수입니다. Line 112의days는 Kotlin 스코핑 규칙에 따라 확장 리시버(TravelTemplateContentInfo.days)를 참조하지만, ViewModel의 생성자 파라미터days와 이름이 같아 혼동될 수 있습니다.명시적으로 작성하면 의도가 더 명확해집니다:
제안
- private fun TravelTemplateContentInfo.toContentInfo(): ContentInfo = ContentInfo( - country = countryCode.toCountryName(), - city = city, - budgetPerPerson = Budget(budgetPerPerson ?: 0), - nights = nights, - days = days, + private fun TravelTemplateContentInfo.toContentInfo(): ContentInfo = ContentInfo( + country = this.countryCode.toCountryName(), + city = this.city, + budgetPerPerson = Budget(this.budgetPerPerson ?: 0), + nights = this.nights, + days = this.days,
46-48: 에러 처리 미구현 (FIXME/TODO)
loadTravelTemplateItinerary(Line 47)와loadTravelTemplateContentInfo(Line 64) 모두onFailure에서 에러를 무시하고 있습니다. 현재 상태에서는 API 실패 시 빈 화면이 표시되며 사용자에게 어떤 피드백도 없습니다. 로딩/에러 상태를FollowTravelState에 추가하여 UI에서 적절히 처리할 수 있도록 하는 것을 권장합니다.로딩/에러 상태 관리를 포함한 구현을 생성해 드릴까요?
Also applies to: 63-65
navigation/src/main/java/com/yapp/ndgl/navigation/Route.kt (1)
17-24:travelId타입 불일치:FollowTravel은Long,TravelDetail은Int
FollowTravel.travelId는Long으로 업데이트되었지만,TravelDetail.travelId는 여전히Int입니다. 동일한 travel ID가 두 Route에서 사용될 수 있다면, 타입을 통일하는 것이 안전합니다. 서버에서Long범위의 ID를 반환할 경우TravelDetail에서 잘릴 수 있습니다.제안
`@Serializable` - data class TravelDetail(val travelId: Int) : Route + data class TravelDetail(val travelId: Long) : Routefeature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelScreen.kt (1)
150-167: 동일한 itinerary 접근 패턴 중복Line 156의
state.itineraries.getOrNull(state.selectedDay - 1)과 Line 169의 동일한 접근이 반복됩니다.LazyListScope레벨에서 한 번만 계산하여 map과 places 모두에 사용할 수 있습니다. 다만 서로 다른item블록에서 참조하므로 현재 구조도 문제없습니다.
| if (state.isInsufficientDuration) { | ||
| Spacer(Modifier.height(24.dp)) | ||
| Text( | ||
| stringResource( | ||
| R.string.date_picker_error_insufficient, | ||
| ), | ||
| color = NDGLTheme.colors.red500, | ||
| style = NDGLTheme.typography.bodySmMedium, | ||
| ) | ||
| } | ||
| Spacer(Modifier.weight(1f)) | ||
| NDGLCTAButton( | ||
| modifier = Modifier | ||
| .fillMaxWidth(), | ||
| type = NDGLCTAButtonAttr.Type.PRIMARY, | ||
| size = NDGLCTAButtonAttr.Size.LARGE, | ||
| status = if (state.isDateSelected) { | ||
| NDGLCTAButtonAttr.Status.ACTIVE | ||
| } else { | ||
| NDGLCTAButtonAttr.Status.DISABLED | ||
| }, | ||
| label = stringResource(R.string.date_picker_complete), | ||
| onClick = clickCompleteButton, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*DatePickerViewModel*" -o -name "*DatePickerState*" | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 172
🏁 Script executed:
fd "DatePickerViewModel" -t fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 170
🏁 Script executed:
fd "DatePickerState" -t fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
cat ./feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerViewModel.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 3957
🏁 Script executed:
fd "DatePickerState" -t f --search-path feature/travelRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
find ./feature/travel -name "*.kt" | xargs grep -l "data class DatePickerState\|class DatePickerState"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 171
🏁 Script executed:
cat ./feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerContract.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1806
버튼 상태 로직 수정 필요
DatePickerState의 계산 속성들을 확인한 결과, 리뷰 지적이 타당합니다:
isDateSelected: startDate와 endDate가 모두 null이 아닐 때 trueisInsufficientDuration: startDate와 endDate가 모두 null이 아니면서 기간이 tripDays 미만일 때 true
따라서 사용자가 날짜를 선택했지만 기간이 부족한 경우(isDateSelected == true && isInsufficientDuration == true)가 실제로 발생할 수 있으며, 현재 코드는 이 경우 에러 메시지와 함께 활성화된 버튼을 표시합니다.
버튼 비활성화 조건에 isInsufficientDuration을 추가하여 수정해야 합니다:
수정사항
status = if (state.isDateSelected) {
- NDGLCTAButtonAttr.Status.ACTIVE
+ if (state.isInsufficientDuration) {
+ NDGLCTAButtonAttr.Status.DISABLED
+ } else {
+ NDGLCTAButtonAttr.Status.ACTIVE
+ }
} else {
NDGLCTAButtonAttr.Status.DISABLED
},🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerScreen.kt`
around lines 133 - 156, The button is currently marked ACTIVE whenever
state.isDateSelected is true, even if state.isInsufficientDuration is also true;
update the NDGLCTAButton status logic so the button is ACTIVE only when
isDateSelected is true AND isInsufficientDuration is false (otherwise use
DISABLED). Locate the NDGLCTAButton instantiation in DatePickerScreen and change
the status expression to combine DatePickerState.isDateSelected and
!DatePickerState.isInsufficientDuration; also ensure clickCompleteButton is not
triggered when the button is disabled (rely on the button component to ignore
clicks when status is DISABLED or guard the handler if needed).
There was a problem hiding this comment.
날짜가 부족할 때 비활성화되는 것은 기획에 해당하지 않음
There was a problem hiding this comment.
@mj010504, 이해했습니다! 날짜가 부족할 때에도 버튼이 활성화되어 사용자가 진행할 수 있도록 하는 것이 의도된 기획이군요. 에러 메시지를 표시하면서도 사용자가 계속 진행할 수 있게 하는 UX 디자인으로 확인했습니다. 명확히 알려주셔서 감사합니다!
✏️ Learnings added
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 23
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerScreen.kt:133-156
Timestamp: 2026-02-16T12:26:32.687Z
Learning: DatePickerScreen에서 날짜 기간이 부족할 때(isInsufficientDuration == true) 에러 메시지를 표시하지만, 버튼은 비활성화하지 않고 사용자가 계속 진행할 수 있도록 하는 것이 제품 기획입니다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
mj010504
force-pushed
the
feature/NDGL-68
branch
from
5bb4123 to
846981b
Compare
1 participant
개요
변경사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항