[NDGL-118] 내 여행 관련 API 연동#36
Merged
Merged
Conversation
…ningHours 필드로 바꿔주는 확장함수 추가
- 내 여행 전체 일정 수정 - 내 여행 장소 한건 추가 - 내 여행 장소 정보 수정(메모, 비용)
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
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 엔드포인트, 저장소 메서드를 도입하며, 홈 및 여행 기능 전반에서 네비게이션 파라미터(days, itineraryId)를 추가로 전달하는 구조 변경을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Home as Home Screen
participant TravelVM as TravelDetailViewModel
participant Repo as UserTravelRepository
participant PlaceAPI as PlaceRepository
participant EventFlow as Event Flow
User->>Home: 위치 변경 선택 (PlaceDetail 열기)
Home->>TravelVM: 새 위치로 confirmChangePlace 호출
TravelVM->>TravelVM: 선택한 장소 유효성 검증
TravelVM->>Repo: emitChangePlaceEvent(event) 발행
Repo->>EventFlow: ChangePlaceEvent 스트림 전송
EventFlow->>TravelVM: changePlaceEvent 수신
TravelVM->>TravelVM: handleChangePlace 실행
TravelVM->>PlaceAPI: computeRoute로 새 경로 계산
TravelVM->>TravelVM: recalculateTransportSegments 실행
TravelVM->>Repo: updateItinerary로 변경된 일정 저장
Repo-->>TravelVM: 저장 완료
TravelVM->>TravelVM: AnimatePlaceChange 사이드이펙트 발생
TravelVM->>Home: NavigateBack 사이드이펙트 발생
Home->>User: PlaceDetail 닫기 및 변경된 위치로 스크롤
sequenceDiagram
actor User
participant DatePicker as DatePickerScreen
participant DateVM as DatePickerViewModel
participant TravelAPI as TravelApi
participant Repo as UserTravelRepository
participant EventFlow as Event Flow
participant HomeVM as HomeViewModel
User->>DatePicker: 여행 생성 확인
DatePicker->>DateVM: 템플릿에서 여행 생성 요청
DateVM->>TravelAPI: 여행 생성 API 호출
TravelAPI-->>DateVM: userTravelId 반환
DateVM->>Repo: emitTravelCreatedEvent(userTravelId, templateId) 발행
Repo->>EventFlow: TravelCreatedEvent 스트림 전송
EventFlow->>HomeVM: travelCreatedEvent 수신
HomeVM->>HomeVM: loadMyTravel 실행 (새 여행 로드)
HomeVM-->>User: "나의 여행" 섹션 새로고침
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 이 변경사항은 여러 레이어에 걸친 광범위한 수정을 포함합니다: 6개의 새로운 데이터 모델, 2개의 새로운 API 엔드포인트, 이벤트 기반 아키텍처 도입, 9개의 홈 기능 파일에서의 파라미터 전파, 8개의 여행 기능 파일에서의 복잡한 경로 재계산 로직 및 위치 변경 처리. 논리적 밀도가 높고(경로 계산, 교통 수단 재계산, 이벤트 구독) 파일 간 상호작용이 복잡하며, 네비게이션 파라미터 변경이 여러 화면과 뷰모델에 일관되게 전파되어야 합니다. 🚥 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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/navigation/TravelEntry.kt (1)
136-139:⚠️ Potential issue | 🟠 Major대체 장소 상세 이동에서 일정 컨텍스트가 유실됩니다.
Line [139]는
Route.PlaceDetail(googlePlaceId)만 전달해서, 현재PlaceDetail이 가진travelId/day/itineraryId가 끊깁니다. TravelDetail에서 진입한 흐름이라면 이후 일정 관련 액션이 잘못된 컨텍스트로 동작할 수 있습니다.🔧 제안 수정안
PlaceDetailRoute( viewModel = viewModel, navigateBack = { navigator.goBack() }, - navigateToAlternativePlaceDetail = { googlePlaceId -> navigator.navigate(Route.PlaceDetail(googlePlaceId)) }, + navigateToAlternativePlaceDetail = { googlePlaceId -> + navigator.navigate( + Route.PlaceDetail( + googlePlaceId = googlePlaceId, + travelId = route.travelId, + day = route.day, + itineraryId = route.itineraryId, + ), + ) + }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/navigation/TravelEntry.kt` around lines 136 - 139, When navigating to an alternative place detail from PlaceDetailRoute, the code currently calls navigator.navigate(Route.PlaceDetail(googlePlaceId)) which drops the current travel context (travelId/day/itineraryId); update the navigateToAlternativePlaceDetail lambda to include and forward the existing travel context from the current route/state so Route.PlaceDetail is constructed with googlePlaceId plus travelId, day, and itineraryId (or use the existing PlaceDetailRoute's viewModel/currentRoute getters) so downstream itinerary actions keep correct context.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt (1)
760-790:⚠️ Potential issue | 🟠 Major교통 옵션 조회에서 라우트 API가 모드당 2회 호출됩니다.
Line 762-769에서
computeRoute(...)를 호출하지만 결과를 버리고, Line 771 이후 동일 라우트를 다시 호출하고 있습니다. 네트워크 비용/지연이 2배로 늘어납니다.⚡ 제안 diff
val transportOptions = travelModes.map { mode -> async { - computeRoute( - originLatitude = from.placeInfo.latitude, - originLongitude = from.placeInfo.longitude, - destinationLatitude = to.placeInfo.latitude, - destinationLongitude = to.placeInfo.longitude, - newGooglePlaceId = to.placeInfo.googlePlaceId, - travelMode = mode, - ) - - suspendRunCatching { - routeRepository.computeRoute( - originLatitude = from.placeInfo.latitude, - originLongitude = from.placeInfo.longitude, - destinationLatitude = to.placeInfo.latitude, - destinationLongitude = to.placeInfo.longitude, - travelMode = mode, - ) - }.getOrNull()?.let { routeInfo -> - if (routeInfo.distanceMeters > 0) { - TransportSegment( - googlePlaceId = to.placeInfo.googlePlaceId, - type = mode.toTransportTypeFromMode(), - duration = routeInfo.duration.removeSuffix("s").toInt().seconds, - distance = routeInfo.distanceMeters, - ) - } else { - null - } - } + computeRoute( + originLatitude = from.placeInfo.latitude, + originLongitude = from.placeInfo.longitude, + destinationLatitude = to.placeInfo.latitude, + destinationLongitude = to.placeInfo.longitude, + newGooglePlaceId = to.placeInfo.googlePlaceId, + travelMode = mode, + ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt` around lines 760 - 790, The code currently calls computeRoute(...) twice per mode — once at lines where computeRoute(...) is invoked directly and again via routeRepository.computeRoute(...) inside suspendRunCatching — causing duplicated network calls; fix by removing the redundant direct computeRoute(...) call (or instead store its result and reuse it) and use the single routeRepository.computeRoute(...) result to build TransportSegment (referencing transportOptions, travelModes, computeRoute, routeRepository.computeRoute, and TransportSegment) so each travel mode triggers only one route API call.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerViewModel.kt (1)
123-161:⚠️ Potential issue | 🟠 Major
emitTravelCreatedEvent실패 시isLoading이true로 고착될 수 있습니다.
emitTravelCreatedEvent는suspendRunCatching블록 바깥 (onSuccess콜백 내부)에서 호출됩니다. 이 호출이 예외를 던지면onFailure가 실행되지 않아isLoading = false처리가 누락되고 UI가 로딩 상태에 고착됩니다. 또한showDatePickerModal = true나createdTravelId업데이트도 이루어지지 않아 사용자가 다음 화면으로 이동할 수 없게 됩니다.이벤트 발행 실패를 무시하거나(fire-and-forget), 혹은 실패를 처리하려면 아래와 같이
suspendRunCatching으로 래핑하는 것이 안전합니다.🛡️ 제안 수정 (이벤트 발행 예외 격리)
}.onSuccess { response -> // 이벤트 발행 - 홈/내 여행 탭 자동 새로고침 - userTravelRepository.emitTravelCreatedEvent( - TravelCreatedEvent( - userTravelId = response.userTravelId, - templateId = templateId, - ), - ) + suspendRunCatching { + userTravelRepository.emitTravelCreatedEvent( + TravelCreatedEvent( + userTravelId = response.userTravelId, + templateId = templateId, + ), + ) + }.onFailure { e -> + Timber.w(e, "Failed to emit TravelCreatedEvent") + } reduce {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerViewModel.kt` around lines 123 - 161, The emitTravelCreatedEvent call inside createTravelFromTemplate can throw and is currently outside the suspendRunCatching context, which may leave isLoading true and prevent state updates; move or wrap the userTravelRepository.emitTravelCreatedEvent(TravelCreatedEvent(...)) inside the suspendRunCatching/onSuccess flow (or wrap it in its own suspendRunCatching) so any exception is caught and handled, then ensure reduce(...) that sets isLoading = false, showDatePickerModal = true, and createdTravelId = response.userTravelId always runs after emit (or runs even if emit fails) to avoid UI hanging; reference createTravelFromTemplate, suspendRunCatching, onSuccess, userTravelRepository.emitTravelCreatedEvent, and reduce when making the change.
🧹 Nitpick comments (9)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt (1)
239-241: 사진 조회 지연/재시도 값을 하드코딩 대신 상수(또는 설정값)로 분리해 주세요.현재
delay(1000)와repeat(3)가 고정값이라 백엔드 상태 변화 시 조정 비용이 큽니다. 동일 동작을 유지하되 설정 가능하게 분리하고 의도를 주석으로 남겨두는 편이 안전합니다.♻️ 제안 diff
class AddItineraryViewModel `@AssistedInject` constructor( @@ ) : BaseViewModel<AddItineraryState, AddItineraryIntent, AddItinerarySideEffect>( @@ ) { + private companion object { + /** 신규 검색 장소 직후 백엔드 저장 지연을 흡수하기 위한 사진 조회 대기 시간 */ + private const val PLACE_PHOTO_FETCH_DELAY_MS = 1_000L + private const val PLACE_PHOTO_FETCH_RETRY_COUNT = 3 + } + @@ private fun loadPlacePhotos(placeId: String) = viewModelScope.launch { - repeat(3) { - delay(1000) + repeat(PLACE_PHOTO_FETCH_RETRY_COUNT) { + delay(PLACE_PHOTO_FETCH_DELAY_MS) val result = suspendRunCatching { placeRepository.getPlacePhotos(placeId) }Based on learnings: AddItineraryViewModel의 신규 검색 장소 사진 로딩은 1초 지연을 유지하되, 지연값은 설정 가능하고 문서화되어야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt` around lines 239 - 241, Extract the hardcoded retry and delay values in AddItineraryViewModel into named constants (e.g., PHOTO_LOAD_RETRY_COUNT and PHOTO_LOAD_DELAY_MS) or configuration parameters passed into the ViewModel constructor, then replace repeat(3) and delay(1000) with those symbols; keep the existing behavior (default retry count 3 and default delay 1000ms) and add a short comment above the retry loop (around suspendRunCatching { placeRepository.getPlacePhotos(placeId) }) explaining that the delay is intentional to wait for backend processing and that the values are configurable for future tuning.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt (1)
50-54: 변수명firstPlaceInSelectedDay가lastOrNull()사용과 일치하지 않습니다.
lastOrNull()로 마지막 장소를 조회하고 있지만 변수명은 여전히firstPlaceInSelectedDay로 되어 있어 의미가 맞지 않습니다.lastPlaceInSelectedDay등으로 수정하는 것을 권장합니다.♻️ 변수명 수정 제안
- val firstPlaceInSelectedDay = currentDayPlaces?.lastOrNull() + val lastPlaceInSelectedDay = currentDayPlaces?.lastOrNull() - if (firstPlaceInSelectedDay != null) { - return LatLng(firstPlaceInSelectedDay.placeInfo.latitude, firstPlaceInSelectedDay.placeInfo.longitude) + if (lastPlaceInSelectedDay != null) { + return LatLng(lastPlaceInSelectedDay.placeInfo.latitude, lastPlaceInSelectedDay.placeInfo.longitude) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt` around lines 50 - 54, The variable firstPlaceInSelectedDay is misnamed because the code uses lastOrNull(); rename it to lastPlaceInSelectedDay (and update all references in the same scope) so the variable name matches its value, e.g., change declaration val firstPlaceInSelectedDay = currentDayPlaces?.lastOrNull() to val lastPlaceInSelectedDay = currentDayPlaces?.lastOrNull() and update the subsequent null check and return to use lastPlaceInSelectedDay (in TravelDetailContract.kt where this logic occurs).feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportSegment.kt (2)
11-11:duration필드명을estimatedDuration으로 통일하는 것을 권장합니다.Line 11의
duration은 현재 프로젝트의 duration 네이밍 컨벤션과 어긋날 수 있어, 서버 스키마/매핑 일관성을 위해estimatedDuration로 맞추는 편이 안전합니다.Based on learnings: Use the naming convention estimatedDuration for duration-related fields across Kotlin files to align with server API response parameter names and maintain consistency throughout the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportSegment.kt` at line 11, Rename the TransportSegment data field duration to estimatedDuration to match the project naming convention and server schema; update the property declaration (val duration: Duration) to val estimatedDuration: Duration and then refactor all usages within the TransportSegment class and any code referencing TransportSegment.duration (constructors, serializers, mappers, tests) to use TransportSegment.estimatedDuration so mappings and API responses remain consistent.
3-3: feature 계층에서 data 모델 직접 의존은 결합도를 높입니다.
TransportSegment(feature 모델)에서TransportationItem(data 모델)을 직접 참조하면 계층 경계가 흐려집니다. 매핑은 data/domain 경계에서 처리하고, feature에는 feature/domain 모델만 두는 쪽이 유지보수에 유리합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportSegment.kt` at line 3, TransportSegment is directly importing and referencing the data-layer class TransportationItem which increases coupling; remove the import and any direct uses of TransportationItem from TransportSegment and instead accept only feature/domain models (or primitive values) in TransportSegment. Implement a mapper function (e.g., in the data->domain or domain layer) that converts TransportationItem into the domain/feature representation before constructing TransportSegment, and update callers to use that mapper (reference the classes TransportSegment and TransportationItem to locate the code to change).feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt (1)
146-147: 콜백 파라미터 대신 클로저 캡처 사용 — 선택적 개선 사항
_로 무시한 두 번째 파라미터(days)는travel.days와 동일한 값입니다. 현재는 문제없지만,TravelTemplate내부에서 days 계산 방식이 달라질 경우 미묘한 불일치가 발생할 수 있습니다. 콜백 파라미터를 직접 사용하는 편이 더 명확합니다.♻️ 제안: 콜백 파라미터 직접 사용
- onTravelTemplateClick = { travelId, _ -> onTravelClick(travelId, travel.days) }, + onTravelTemplateClick = { travelId, days -> onTravelClick(travelId, days) },🤖 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/main/PopularTravelSection.kt` around lines 146 - 147, 현재 PopularTravelSection에서 onTravelTemplateClick = { travelId, _ -> onTravelClick(travelId, travel.days) }처럼 두번째 파라미터를 무시하고 외부의 travel.days를 캡처하고 있습니다; TravelTemplate 내부에서 전달된 days 파라미터을 직접 사용하도록 콜백 시그니처를 변경하세요: onTravelTemplateClick = { travelId, days -> onTravelClick(travelId, days) }, 참조 대상은 onTravelTemplateClick, onTravelClick, TravelTemplate, travel.days 입니다.data/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddItineraryResponse.kt (1)
22-47:UserTravelTemplateItinerary의 중첩 클래스와 구조 중복
Transportation,PlanBPlace,ItineraryPlace세 클래스가UserTravelTemplateItinerary.kt의 동명 중첩 클래스와 필드 단위까지 동일합니다. API 응답 스키마가 동일하다면 해당 타입들을com.yapp.ndgl.data.travel.model패키지 최상위로 추출하여 양쪽에서 공유하면 유지보수 부담을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddItineraryResponse.kt` around lines 22 - 47, Extract the duplicate nested data classes Transportation, PlanBPlace, and ItineraryPlace from AddItineraryResponse.kt (and the matching nested types in UserTravelTemplateItinerary) into top-level Kotlin data classes in the com.yapp.ndgl.data.travel.model package, then update both AddItineraryResponse and UserTravelTemplateItinerary to reference those shared top-level types instead of defining their own nested classes; ensure the class names and property signatures remain unchanged (Transportation, PlanBPlace, ItineraryPlace) so serialization/compatibility is preserved.data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/PlaceRepository.kt (1)
33-33: 반경 10,000m 매직 넘버 → 상수화 권장하드코딩된
10000.0을 의미 있는 상수로 추출하면 의도가 명확해집니다.♻️ 제안 리팩터
+ companion object { + private const val SEARCH_BIAS_RADIUS_METERS = 10_000.0 + } + suspend fun searchKeyword(keyword: String, representativeLatLng: LatLng): SearchKeywordResponse { ... - val bias = CircularBounds.newInstance(representativeLatLng, 10000.0) + val bias = CircularBounds.newInstance(representativeLatLng, SEARCH_BIAS_RADIUS_METERS)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/PlaceRepository.kt` at line 33, Extract the hardcoded radius 10000.0 used when creating CircularBounds in PlaceRepository (the line assigning bias via CircularBounds.newInstance(representativeLatLng, 10000.0)) into a named constant (e.g., SEARCH_RADIUS_METERS or DEFAULT_PLACE_SEARCH_RADIUS) declared as a private const val near the top of the class or companion object; replace the literal with that constant so the intention is clear and easily configurable.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelViewModel.kt (1)
27-39: 초기 로드와 재조회 로직 중복은 한 경로로 합치는 것을 권장합니다.현재 초기 로드 블록과
refreshTravelData()가 거의 동일해서 추후 수정 시 동기화 누락 위험이 있습니다.♻️ 제안 diff
init { - viewModelScope.launch { - val upcomingDeferred = async { loadUpcomingTravel() } - val listDeferred = async { loadUpcomingTravelList() } - - val upcomingTravel = upcomingDeferred.await() - val upcomingTravels = listDeferred.await() - - reduce { copy(upcomingTravel = upcomingTravel, upcomingTravels = upcomingTravels) } - - if (upcomingTravel == null && upcomingTravels.isEmpty()) { - loadRecommendedTravels() - } - } + refreshTravelData() // 초기 로드 후 이벤트 구독 subscribeToTravelCreatedEvent() }Also applies to: 140-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelViewModel.kt` around lines 27 - 39, The initial-load block in the coroutine duplicates the logic in refreshTravelData(); extract the shared sequence (concurrently call loadUpcomingTravel() and loadUpcomingTravelList(), await both, reduce state, and conditionally call loadRecommendedTravels()) into a single helper method (e.g., refreshTravelData or loadTravelData) and replace the inline block in viewModelScope.launch with a call to that helper so both initial load and refresh use the same implementation; ensure you update calls that currently use the duplicated code to reference the new method and keep the existing functions loadUpcomingTravel(), loadUpcomingTravelList(), and loadRecommendedTravels() intact.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt (1)
31-31:UserTravelRepository와ChangePlaceEvent에 전체 패키지 경로가 사용되고 있습니다.Line 31의
userTravelRepository타입과 Line 126의ChangePlaceEvent가 완전한 qualified name으로 선언되어 있습니다. 파일 상단에 import를 추가하면 가독성이 향상됩니다.♻️ 제안 수정 (import 추가)
파일 상단에 추가:
+import com.yapp.ndgl.data.travel.model.ChangePlaceEvent +import com.yapp.ndgl.data.travel.repository.UserTravelRepository본문 수정:
- private val userTravelRepository: com.yapp.ndgl.data.travel.repository.UserTravelRepository, + private val userTravelRepository: UserTravelRepository,- userTravelRepository.emitChangePlaceEvent( - com.yapp.ndgl.data.travel.model.ChangePlaceEvent( + userTravelRepository.emitChangePlaceEvent( + ChangePlaceEvent(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt` at line 31, The file currently uses fully-qualified names for com.yapp.ndgl.data.travel.repository.UserTravelRepository and com.yapp.ndgl.feature.travel.placedetail.PlaceDetailViewModel.ChangePlaceEvent; add import statements for UserTravelRepository and ChangePlaceEvent at the top of the file and update the constructor parameter and the ChangePlaceEvent references to use the simple class names (UserTravelRepository and ChangePlaceEvent) instead of their full package-qualified names to improve readability.
🤖 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/NDGLInputModal.kt`:
- Around line 60-62: The input modal's internal state textFieldValue (in
NDGLInputModal) is only initialized with remember and doesn't react to external
changes to value, causing UI/state mismatch; fix it by syncing textFieldValue
whenever value changes—for example, keep the existing remember {
mutableStateOf(...) } but add a LaunchedEffect(value) block that sets
textFieldValue = TextFieldValue(value, selection = TextRange(value.length)) (or
use remember(value) to reinitialize), so the displayed text and the external
value used for confirm/enabled logic stay consistent.
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddItineraryRequest.kt`:
- Line 12: AddItineraryRequest.estimatedDuration is declared as String? but
other models (AddItineraryResponse, UpdateItineraryRequest,
UserTravelTemplateItinerary) use Int, causing serialization/runtime errors;
change the property type in AddItineraryRequest (class AddItineraryRequest) from
String? to Int? to match API and other models, update any places constructing
AddItineraryRequest to pass an Int? and adjust JSON serialization annotations if
present to ensure integer serialization.
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/PlaceRepository.kt`:
- Line 37: Remove the leftover commented call `.setCountries(countryCode)` from
PlaceRepository.kt; locate the commented line inside the repository method where
the builder/configuration is constructed (the commented
`.setCountries(countryCode)`), delete that commented-out code so the file
doesn't contain dead/commented code anymore and keep the surrounding code
formatting intact.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlaceInfo.kt`:
- Around line 65-70: Validate the day parameter in toOpeningHours: if day is
zero or negative, return null before computing targetDate; update the
List<String>?.toOpeningHours function to check "day <= 0" and exit early (null)
so plusDays((day - 1).toLong()) is never called for invalid input and avoid
producing an incorrect targetDayOfWeek.
- Line 85: The current return uses openingHour?.substringAfter(":")?.trim(),
which will return the full original string if ':' is missing and the trailing
safe-call is unnecessary; change it to first check for a ':' and only then call
substringAfter and trim (e.g., if openingHour?.contains(":") == true return
openingHour.substringAfter(":").trim() else return null), removing the redundant
safe-call after substringAfter to avoid returning unintended values; apply this
change around the openingHour usage in PlaceInfo.kt (the expression with
substringAfter).
- Around line 5-7: PlaceInfo.kt currently imports java.time.* and mis-handles
the day parameter and an unnecessary safe-call; switch imports to
kotlinx.datetime equivalents (e.g., kotlinx.datetime.LocalDate and
kotlinx.datetime.DayOfWeek) to match the module pattern, add validation inside
the function/method that uses the day parameter (the code that calls
plusDays((day - 1).toLong())) to reject or clamp day <= 0 (e.g., throw
IllegalArgumentException with a clear message or enforce a minimum of 1) so you
never compute a negative offset, and remove the redundant safe-call on
openingHour?.substringAfter(":")?.trim() by using
openingHour?.substringAfter(":").trim() or the correct non-null expression
depending on desired null behavior; reference the PlaceInfo class and the method
where plusDays and openingHour are used to make these changes.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportSegment.kt`:
- Around line 32-35: toTransportationItem() currently sets
TransportationItem.timeMin using duration.inWholeMinutes.toInt(), which
truncates fractional minutes and can yield 0; update the conversion in
TransportSegment.toTransportationItem to enforce a minimum (e.g., Math.ceil
equivalent or max(1, ...)) or apply proper rounding before assigning to
TransportationItem.timeMin, and/or add validation on TransportationItem.timeMin
to reject or clamp values below 1 to guarantee at least 1 minute.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt`:
- Around line 125-135: The call to userTravelRepository.emitChangePlaceEvent can
throw and prevent postSideEffect(PlaceDetailSideEffect.NavigateBack) from
running; wrap the emitChangePlaceEvent call in a try/catch (or
try/catch/finally) around the block in the function containing postSideEffect so
any exception is caught/logged (similar to
DatePickerViewModel.emitTravelCreatedEvent) and ensure
postSideEffect(PlaceDetailSideEffect.NavigateBack) is always invoked (e.g., put
it in finally or after the catch) so the UI always navigates back even if event
emission fails.
- Around line 117-136: In confirmChangePlace(), ensure the confirmation modal is
closed even when null guards cause early returns: invoke reduce {
copy(showChangeModal = false) } before the null checks (or guarantee it's called
in all return paths, e.g., via a try/finally) so that selectedAlternativePlace,
travelId, day, and itineraryId null checks (the existing return@launch points)
cannot leave the modal open; update the logic in confirmChangePlace to call
reduce first, then perform the null guards and the subsequent
userTravelRepository.emitChangePlaceEvent and
postSideEffect(PlaceDetailSideEffect.NavigateBack).
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Around line 120-170: currentItinerary.places.last() can throw when the
itinerary is empty; change the logic in the block around lastPlace /
newTransportSegment / distanceKm / transportation and the startTime computation
to handle the "first place" case: use safe access (e.g., lastOrNull) or explicit
isEmpty() check before calling computeRoute and before computing startTime, set
distanceKm/transportation to null for the first place, and set the new
TravelPlace.startTime to an appropriate initial value (or null) instead of
referencing lastPlace.startTime; update the branches around computeRoute, the
tuple assignment (distanceKm, transportation), and the startTime expression that
uses lastPlace so they all handle empty currentItinerary.places safely.
---
Outside diff comments:
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerViewModel.kt`:
- Around line 123-161: The emitTravelCreatedEvent call inside
createTravelFromTemplate can throw and is currently outside the
suspendRunCatching context, which may leave isLoading true and prevent state
updates; move or wrap the
userTravelRepository.emitTravelCreatedEvent(TravelCreatedEvent(...)) inside the
suspendRunCatching/onSuccess flow (or wrap it in its own suspendRunCatching) so
any exception is caught and handled, then ensure reduce(...) that sets isLoading
= false, showDatePickerModal = true, and createdTravelId = response.userTravelId
always runs after emit (or runs even if emit fails) to avoid UI hanging;
reference createTravelFromTemplate, suspendRunCatching, onSuccess,
userTravelRepository.emitTravelCreatedEvent, and reduce when making the change.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/navigation/TravelEntry.kt`:
- Around line 136-139: When navigating to an alternative place detail from
PlaceDetailRoute, the code currently calls
navigator.navigate(Route.PlaceDetail(googlePlaceId)) which drops the current
travel context (travelId/day/itineraryId); update the
navigateToAlternativePlaceDetail lambda to include and forward the existing
travel context from the current route/state so Route.PlaceDetail is constructed
with googlePlaceId plus travelId, day, and itineraryId (or use the existing
PlaceDetailRoute's viewModel/currentRoute getters) so downstream itinerary
actions keep correct context.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Around line 760-790: The code currently calls computeRoute(...) twice per mode
— once at lines where computeRoute(...) is invoked directly and again via
routeRepository.computeRoute(...) inside suspendRunCatching — causing duplicated
network calls; fix by removing the redundant direct computeRoute(...) call (or
instead store its result and reuse it) and use the single
routeRepository.computeRoute(...) result to build TransportSegment (referencing
transportOptions, travelModes, computeRoute, routeRepository.computeRoute, and
TransportSegment) so each travel mode triggers only one route API call.
---
Nitpick comments:
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddItineraryResponse.kt`:
- Around line 22-47: Extract the duplicate nested data classes Transportation,
PlanBPlace, and ItineraryPlace from AddItineraryResponse.kt (and the matching
nested types in UserTravelTemplateItinerary) into top-level Kotlin data classes
in the com.yapp.ndgl.data.travel.model package, then update both
AddItineraryResponse and UserTravelTemplateItinerary to reference those shared
top-level types instead of defining their own nested classes; ensure the class
names and property signatures remain unchanged (Transportation, PlanBPlace,
ItineraryPlace) so serialization/compatibility is preserved.
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/PlaceRepository.kt`:
- Line 33: Extract the hardcoded radius 10000.0 used when creating
CircularBounds in PlaceRepository (the line assigning bias via
CircularBounds.newInstance(representativeLatLng, 10000.0)) into a named constant
(e.g., SEARCH_RADIUS_METERS or DEFAULT_PLACE_SEARCH_RADIUS) declared as a
private const val near the top of the class or companion object; replace the
literal with that constant so the intention is clear and easily configurable.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt`:
- Around line 146-147: 현재 PopularTravelSection에서 onTravelTemplateClick = {
travelId, _ -> onTravelClick(travelId, travel.days) }처럼 두번째 파라미터를 무시하고 외부의
travel.days를 캡처하고 있습니다; TravelTemplate 내부에서 전달된 days 파라미터을 직접 사용하도록 콜백 시그니처를
변경하세요: onTravelTemplateClick = { travelId, days -> onTravelClick(travelId, days)
}, 참조 대상은 onTravelTemplateClick, onTravelClick, TravelTemplate, travel.days 입니다.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt`:
- Around line 239-241: Extract the hardcoded retry and delay values in
AddItineraryViewModel into named constants (e.g., PHOTO_LOAD_RETRY_COUNT and
PHOTO_LOAD_DELAY_MS) or configuration parameters passed into the ViewModel
constructor, then replace repeat(3) and delay(1000) with those symbols; keep the
existing behavior (default retry count 3 and default delay 1000ms) and add a
short comment above the retry loop (around suspendRunCatching {
placeRepository.getPlacePhotos(placeId) }) explaining that the delay is
intentional to wait for backend processing and that the values are configurable
for future tuning.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportSegment.kt`:
- Line 11: Rename the TransportSegment data field duration to estimatedDuration
to match the project naming convention and server schema; update the property
declaration (val duration: Duration) to val estimatedDuration: Duration and then
refactor all usages within the TransportSegment class and any code referencing
TransportSegment.duration (constructors, serializers, mappers, tests) to use
TransportSegment.estimatedDuration so mappings and API responses remain
consistent.
- Line 3: TransportSegment is directly importing and referencing the data-layer
class TransportationItem which increases coupling; remove the import and any
direct uses of TransportationItem from TransportSegment and instead accept only
feature/domain models (or primitive values) in TransportSegment. Implement a
mapper function (e.g., in the data->domain or domain layer) that converts
TransportationItem into the domain/feature representation before constructing
TransportSegment, and update callers to use that mapper (reference the classes
TransportSegment and TransportationItem to locate the code to change).
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelViewModel.kt`:
- Around line 27-39: The initial-load block in the coroutine duplicates the
logic in refreshTravelData(); extract the shared sequence (concurrently call
loadUpcomingTravel() and loadUpcomingTravelList(), await both, reduce state, and
conditionally call loadRecommendedTravels()) into a single helper method (e.g.,
refreshTravelData or loadTravelData) and replace the inline block in
viewModelScope.launch with a call to that helper so both initial load and
refresh use the same implementation; ensure you update calls that currently use
the duplicated code to reference the new method and keep the existing functions
loadUpcomingTravel(), loadUpcomingTravelList(), and loadRecommendedTravels()
intact.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt`:
- Line 31: The file currently uses fully-qualified names for
com.yapp.ndgl.data.travel.repository.UserTravelRepository and
com.yapp.ndgl.feature.travel.placedetail.PlaceDetailViewModel.ChangePlaceEvent;
add import statements for UserTravelRepository and ChangePlaceEvent at the top
of the file and update the constructor parameter and the ChangePlaceEvent
references to use the simple class names (UserTravelRepository and
ChangePlaceEvent) instead of their full package-qualified names to improve
readability.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt`:
- Around line 50-54: The variable firstPlaceInSelectedDay is misnamed because
the code uses lastOrNull(); rename it to lastPlaceInSelectedDay (and update all
references in the same scope) so the variable name matches its value, e.g.,
change declaration val firstPlaceInSelectedDay = currentDayPlaces?.lastOrNull()
to val lastPlaceInSelectedDay = currentDayPlaces?.lastOrNull() and update the
subsequent null check and return to use lastPlaceInSelectedDay (in
TravelDetailContract.kt where this logic occurs).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLInputModal.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/api/UserTravelApi.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddItineraryRequest.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddItineraryResponse.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/ChangePlaceEvent.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/TransportationItem.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/TravelCreatedEvent.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/UpdateItineraryRequest.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/UpdateTravelPlaceRequest.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/UserTravelTemplateItinerary.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/repository/PlaceRepository.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/repository/UserTravelRepository.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/component/TravelTemplate.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListContract.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListScreen.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/popular/PopularTravelListViewModel.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchContract.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchViewModel.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/datepicker/DatePickerViewModel.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlaceInfo.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/TransportSegment.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelViewModel.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/navigation/TravelEntry.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailContract.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailScreen.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.ktnavigation/src/main/java/com/yapp/ndgl/navigation/Route.kt
Comment on lines
+60
to
62
| var textFieldValue by remember { | ||
| mutableStateOf(TextFieldValue(value, selection = TextRange(value.length))) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "NDGLInputModal.kt" --type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 153
🏁 Script executed:
cat -n "core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLInputModal.kt"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 10320
value 외부 변경이 입력창에 반영되지 않아 상태 불일치가 발생합니다.
Line 60에서 textFieldValue가 remember만으로 초기화되고 외부 value 변경을 감지하는 의존성이 없습니다. 이로 인해 모달이 열린 상태에서 부모 컴포넌트가 value를 변경해도 입력 필드는 이전 값을 유지합니다. 특히 Line 147의 확인 버튼 활성화 조건은 외부 value를 사용하지만 표시되는 텍스트는 textFieldValue이므로, 버튼 상태와 입력 필드 내용이 불일치할 수 있습니다.
🔧 제안 수정
var textFieldValue by remember {
mutableStateOf(TextFieldValue(value, selection = TextRange(value.length)))
}
LaunchedEffect(Unit) {
focusRequester.requestFocus()
}
+
+LaunchedEffect(value) {
+ // 외부 value 변경만 동기화하고, IME 조합 중에는 덮어쓰지 않음
+ if (value != textFieldValue.text && textFieldValue.composition == null) {
+ textFieldValue = textFieldValue.copy(
+ text = value,
+ selection = TextRange(value.length),
+ composition = null,
+ )
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLInputModal.kt`
around lines 60 - 62, The input modal's internal state textFieldValue (in
NDGLInputModal) is only initialized with remember and doesn't react to external
changes to value, causing UI/state mismatch; fix it by syncing textFieldValue
whenever value changes—for example, keep the existing remember {
mutableStateOf(...) } but add a LaunchedEffect(value) block that sets
textFieldValue = TextFieldValue(value, selection = TextRange(value.length)) (or
use remember(value) to reinitialize), so the displayed text and the external
value used for confirm/enabled logic stay consistent.
Contributor
Author
There was a problem hiding this comment.
onValuechange에서 textFieldValue = newValue 하고 있기 때문에 문제가 발생하지는 않았습니다.
There was a problem hiding this comment.
@mj010504님, 맞습니다! onValueChange에서 사용자 입력은 잘 처리되고 있습니다.
다만 제가 지적한 부분은 사용자 입력이 아닌 외부에서 value prop이 변경될 때의 동기화였습니다. PR 설명에서 한글 자음/모음 결합 문제를 해결하셨다고 하셨는데, 이는 올바른 접근입니다. 한글 IME 조합 중에 textFieldValue를 외부 값으로 덮어쓰면 조합이 깨지는 것이 맞습니다.
한 가지 확인할 점이 있습니다:
- Line 102의
onValueChange는textFieldValue.text를 전달하지만 - Line 147의 확인 버튼 활성화 조건은 외부
value를 사용합니다:if (value.isNotEmpty())
현재 구현에서는 모달이 열려있는 동안 외부에서 value를 변경하는 경우가 없다면 문제가 없습니다. 하지만 일관성을 위해 Line 147도 textFieldValue.text.isNotEmpty()를 사용하는 것이 더 안전할 수 있습니다.
모달이 열려있는 동안 외부에서 value를 변경할 가능성이 있나요? 없다면 현재 구현이 적절합니다! 🐰
🧠 Learnings used
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 25
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelContract.kt:52-54
Timestamp: 2026-02-17T22:21:31.141Z
Learning: Use the naming convention estimatedDuration for duration-related fields across Kotlin files to align with server API response parameter names and maintain consistency throughout the codebase. When introducing or renaming duration fields, replace generic names (e.g., duration, totalDuration) with estimatedDuration and update all references (data classes, mappings, JSON parsing) accordingly to avoid mismatch with server payloads.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 27
File: feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt:0-0
Timestamp: 2026-02-18T18:16:40.700Z
Learning: Use the correct Hilt Compose ViewModel import across Kotlin Android files: import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel (from androidx.hilt:hilt-lifecycle-viewmodel-compose:1.3.0). The old import androidx.hilt.navigation.compose.hiltViewModel is deprecated in Hilt 1.3.0. Ensure the hilt-lifecycle-viewmodel-compose dependency is added to Gradle (both Gradle catalog and module dependencies) and update imports accordingly to avoid runtime issues or build failures.
| } | ||
|
|
||
| val openingHour = this.find { it.startsWith(dayOfWeekName) } | ||
| return openingHour?.substringAfter(":")?.trim() |
There was a problem hiding this comment.
substringAfter(":") 구분자 미발견 시 원본 문자열 반환 문제 및 불필요한 ?. 연산자
Kotlin의 substringAfter는 구분자(:)가 없을 경우 빈 문자열이나 null이 아닌 원본 문자열 전체를 반환합니다. 즉, 데이터에 콜론이 없다면 요일명이 포함된 문자열 전체가 반환됩니다. 또한 substringAfter는 String(non-null)을 반환하므로 두 번째 ?.는 불필요합니다.
♻️ 구분자 존재 여부를 확인하여 안전하게 처리
- return openingHour?.substringAfter(":")?.trim()
+ return openingHour
+ ?.takeIf { it.contains(":") }
+ ?.substringAfter(":")
+ ?.trim()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return openingHour?.substringAfter(":")?.trim() | |
| return openingHour | |
| ?.takeIf { it.contains(":") } | |
| ?.substringAfter(":") | |
| ?.trim() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlaceInfo.kt`
at line 85, The current return uses openingHour?.substringAfter(":")?.trim(),
which will return the full original string if ':' is missing and the trailing
safe-call is unnecessary; change it to first check for a ':' and only then call
substringAfter and trim (e.g., if openingHour?.contains(":") == true return
openingHour.substringAfter(":").trim() else return null), removing the redundant
safe-call after substringAfter to avoid returning unintended values; apply this
change around the openingHour usage in PlaceInfo.kt (the expression with
substringAfter).
Contributor
Author
There was a problem hiding this comment.
원본 문자열 전체 반환하는게 의도입니다.
There was a problem hiding this comment.
@mj010504 명확하게 설명해 주셔서 감사합니다! 콜론이 없는 경우에도 원본 문자열 전체를 반환하는 것이 의도된 동작이라는 점 이해했습니다. 영업 시간 데이터 형식이 다양할 수 있으므로, null을 반환하는 것보다 전체 문자열을 표시하는 것이 더 나은 사용자 경험을 제공하는 합리적인 폴백 전략입니다.
🐰
✨ 이해했습니다!
명확한 설계 의도를
기억하겠습니다 ✨
✏️ Learnings added
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 36
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlaceInfo.kt:85-85
Timestamp: 2026-02-25T11:17:43.000Z
Learning: In feature/travel/src/main/java/com/yapp/ndgl/feature/travel/model/PlaceInfo.kt, the toOpeningHours extension function intentionally returns the full original string when the colon delimiter is not found (leveraging substringAfter's default missingDelimiterValue behavior). This design provides a fallback to display the entire opening hours string rather than null when the data format doesn't include a colon separator, improving UX for varied data formats.
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.
🧠 Learnings used
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 17
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt:243-251
Timestamp: 2026-02-11T12:08:02.377Z
Learning: In feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt, the startIndex calculation for the WheelPicker intentionally does NOT subtract visibleItemCount/2 offset. This design choice preserves showing 5 items initially and creates an offset from the initialIndex parameter. Reviewers should not request centering the active item unless the UI design is changed; keep this behavior as the intended UX for this component.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 25
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelContract.kt:52-54
Timestamp: 2026-02-17T22:21:31.141Z
Learning: Use the naming convention estimatedDuration for duration-related fields across Kotlin files to align with server API response parameter names and maintain consistency throughout the codebase. When introducing or renaming duration fields, replace generic names (e.g., duration, totalDuration) with estimatedDuration and update all references (data classes, mappings, JSON parsing) accordingly to avoid mismatch with server payloads.
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 26
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/addplace/AddPlaceViewModel.kt:71-90
Timestamp: 2026-02-18T12:08:18.209Z
Learning: In the travel feature (place photo loading logic in AddPlaceViewModel.kt and AddItineraryViewModel.kt), retain a deliberate 1-second delay before each photo fetch when loading data for newly searched places. This accounts for backend latency needed to save place data before photos are retrievable. Ensure the delay is configurable (e.g., via feature flag or constant) and documented, so it can be adjusted or removed if backend performance improves. This guidance applies to all travel feature view models handling similar photo fetch flows.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 27
File: feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt:0-0
Timestamp: 2026-02-18T18:16:40.700Z
Learning: Use the correct Hilt Compose ViewModel import across Kotlin Android files: import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel (from androidx.hilt:hilt-lifecycle-viewmodel-compose:1.3.0). The old import androidx.hilt.navigation.compose.hiltViewModel is deprecated in Hilt 1.3.0. Ensure the hilt-lifecycle-viewmodel-compose dependency is added to Gradle (both Gradle catalog and module dependencies) and update imports accordingly to avoid runtime issues or build failures.
Comment on lines
+117
to
136
| private fun confirmChangePlace() = viewModelScope.launch { | ||
| val selectedPlace = state.value.selectedAlternativePlace ?: return@launch | ||
| val currentTravelId = travelId ?: return@launch | ||
| val currentDay = day ?: return@launch | ||
| val currentItineraryId = itineraryId ?: return@launch | ||
|
|
||
| reduce { copy(showChangeModal = false) } | ||
|
|
||
| userTravelRepository.emitChangePlaceEvent( | ||
| com.yapp.ndgl.data.travel.model.ChangePlaceEvent( | ||
| travelId = currentTravelId, | ||
| day = currentDay, | ||
| itineraryId = currentItineraryId, | ||
| oldGooglePlaceId = googlePlaceId, | ||
| newGooglePlaceId = selectedPlace.id, | ||
| ), | ||
| ) | ||
|
|
||
| postSideEffect(PlaceDetailSideEffect.NavigateBack) | ||
| } |
There was a problem hiding this comment.
null 가드가 모달 닫기 이전에 위치하여 사용자가 모달에 갇힐 수 있습니다.
travelId, day, itineraryId 중 하나라도 null인 경우 return@launch가 실행되어 reduce { copy(showChangeModal = false) } (Line 123)에 도달하지 못합니다. 이 상태에서는 확인 모달이 닫히지 않고 그대로 남아, 사용자가 UI에서 빠져나갈 수 없게 됩니다.
null 체크를 가져오는 로직과 모달 닫기 순서를 분리하거나, 모달 닫기를 null 체크보다 먼저 실행하도록 변경하는 것을 권장합니다.
🐛 제안 수정 (모달 닫기를 null 가드 전으로 이동)
private fun confirmChangePlace() = viewModelScope.launch {
val selectedPlace = state.value.selectedAlternativePlace ?: return@launch
val currentTravelId = travelId ?: return@launch
val currentDay = day ?: return@launch
val currentItineraryId = itineraryId ?: return@launch
- reduce { copy(showChangeModal = false) }
-
+ reduce { copy(showChangeModal = false) } // null 체크 성공 후 즉시 닫기
+
userTravelRepository.emitChangePlaceEvent(참고: 위 diff만으로는 해결이 안 됩니다 — null 체크 실패 시에도 모달을 닫으려면 아래처럼 null 가드 앞에
reduce를 배치하세요:
private fun confirmChangePlace() = viewModelScope.launch {
+ reduce { copy(showChangeModal = false) }
+
val selectedPlace = state.value.selectedAlternativePlace ?: return@launch
val currentTravelId = travelId ?: return@launch
val currentDay = day ?: return@launch
val currentItineraryId = itineraryId ?: return@launch
- reduce { copy(showChangeModal = false) }
-
userTravelRepository.emitChangePlaceEvent(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private fun confirmChangePlace() = viewModelScope.launch { | |
| val selectedPlace = state.value.selectedAlternativePlace ?: return@launch | |
| val currentTravelId = travelId ?: return@launch | |
| val currentDay = day ?: return@launch | |
| val currentItineraryId = itineraryId ?: return@launch | |
| reduce { copy(showChangeModal = false) } | |
| userTravelRepository.emitChangePlaceEvent( | |
| com.yapp.ndgl.data.travel.model.ChangePlaceEvent( | |
| travelId = currentTravelId, | |
| day = currentDay, | |
| itineraryId = currentItineraryId, | |
| oldGooglePlaceId = googlePlaceId, | |
| newGooglePlaceId = selectedPlace.id, | |
| ), | |
| ) | |
| postSideEffect(PlaceDetailSideEffect.NavigateBack) | |
| } | |
| private fun confirmChangePlace() = viewModelScope.launch { | |
| reduce { copy(showChangeModal = false) } | |
| val selectedPlace = state.value.selectedAlternativePlace ?: return@launch | |
| val currentTravelId = travelId ?: return@launch | |
| val currentDay = day ?: return@launch | |
| val currentItineraryId = itineraryId ?: return@launch | |
| userTravelRepository.emitChangePlaceEvent( | |
| com.yapp.ndgl.data.travel.model.ChangePlaceEvent( | |
| travelId = currentTravelId, | |
| day = currentDay, | |
| itineraryId = currentItineraryId, | |
| oldGooglePlaceId = googlePlaceId, | |
| newGooglePlaceId = selectedPlace.id, | |
| ), | |
| ) | |
| postSideEffect(PlaceDetailSideEffect.NavigateBack) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt`
around lines 117 - 136, In confirmChangePlace(), ensure the confirmation modal
is closed even when null guards cause early returns: invoke reduce {
copy(showChangeModal = false) } before the null checks (or guarantee it's called
in all return paths, e.g., via a try/finally) so that selectedAlternativePlace,
travelId, day, and itineraryId null checks (the existing return@launch points)
cannot leave the modal open; update the logic in confirmChangePlace to call
reduce first, then perform the null guards and the subsequent
userTravelRepository.emitChangePlaceEvent and
postSideEffect(PlaceDetailSideEffect.NavigateBack).
Contributor
Author
There was a problem hiding this comment.
실패 시 로직들은 추후에 정리하겠습니다.
There was a problem hiding this comment.
@mj010504, 알겠습니다! 나중에 실패 처리 로직을 정리하실 때 함께 검토하시면 됩니다.
혹시 이 항목을 추적하기 위해 후속 이슈를 생성하시겠습니까? 원하시면 제가 생성해드릴 수 있습니다.
🧠 Learnings used
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 26
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/addplace/AddPlaceViewModel.kt:71-90
Timestamp: 2026-02-18T12:08:18.209Z
Learning: In the travel feature (place photo loading logic in AddPlaceViewModel.kt and AddItineraryViewModel.kt), retain a deliberate 1-second delay before each photo fetch when loading data for newly searched places. This accounts for backend latency needed to save place data before photos are retrievable. Ensure the delay is configurable (e.g., via feature flag or constant) and documented, so it can be adjusted or removed if backend performance improves. This guidance applies to all travel feature view models handling similar photo fetch flows.
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 17
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt:243-251
Timestamp: 2026-02-11T12:08:02.377Z
Learning: In feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt, the startIndex calculation for the WheelPicker intentionally does NOT subtract visibleItemCount/2 offset. This design choice preserves showing 5 items initially and creates an offset from the initialIndex parameter. Reviewers should not request centering the active item unless the UI design is changed; keep this behavior as the intended UX for this component.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 25
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelContract.kt:52-54
Timestamp: 2026-02-17T22:21:31.141Z
Learning: Use the naming convention estimatedDuration for duration-related fields across Kotlin files to align with server API response parameter names and maintain consistency throughout the codebase. When introducing or renaming duration fields, replace generic names (e.g., duration, totalDuration) with estimatedDuration and update all references (data classes, mappings, JSON parsing) accordingly to avoid mismatch with server payloads.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 27
File: feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt:0-0
Timestamp: 2026-02-18T18:16:40.700Z
Learning: Use the correct Hilt Compose ViewModel import across Kotlin Android files: import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel (from androidx.hilt:hilt-lifecycle-viewmodel-compose:1.3.0). The old import androidx.hilt.navigation.compose.hiltViewModel is deprecated in Hilt 1.3.0. Ensure the hilt-lifecycle-viewmodel-compose dependency is added to Gradle (both Gradle catalog and module dependencies) and update imports accordingly to avoid runtime issues or build failures.
Comment on lines
+125
to
+135
| userTravelRepository.emitChangePlaceEvent( | ||
| com.yapp.ndgl.data.travel.model.ChangePlaceEvent( | ||
| travelId = currentTravelId, | ||
| day = currentDay, | ||
| itineraryId = currentItineraryId, | ||
| oldGooglePlaceId = googlePlaceId, | ||
| newGooglePlaceId = selectedPlace.id, | ||
| ), | ||
| ) | ||
|
|
||
| postSideEffect(PlaceDetailSideEffect.NavigateBack) |
There was a problem hiding this comment.
emitChangePlaceEvent 실패 시 NavigateBack이 실행되지 않아 화면이 고착됩니다.
emitChangePlaceEvent가 예외를 던지면 postSideEffect(PlaceDetailSideEffect.NavigateBack)(Line 135)가 실행되지 않습니다. 이 경우 모달은 이미 닫혀 있지만 PlaceDetail 화면에 그대로 남게 되어 사용자가 정상적인 흐름으로 돌아갈 수 없습니다. DatePickerViewModel의 emitTravelCreatedEvent와 동일한 패턴 문제입니다.
🛡️ 제안 수정 (이벤트 발행 예외 격리 후 항상 NavigateBack 실행)
- userTravelRepository.emitChangePlaceEvent(
- com.yapp.ndgl.data.travel.model.ChangePlaceEvent(
- travelId = currentTravelId,
- day = currentDay,
- itineraryId = currentItineraryId,
- oldGooglePlaceId = googlePlaceId,
- newGooglePlaceId = selectedPlace.id,
- ),
- )
-
- postSideEffect(PlaceDetailSideEffect.NavigateBack)
+ suspendRunCatching {
+ userTravelRepository.emitChangePlaceEvent(
+ ChangePlaceEvent(
+ travelId = currentTravelId,
+ day = currentDay,
+ itineraryId = currentItineraryId,
+ oldGooglePlaceId = googlePlaceId,
+ newGooglePlaceId = selectedPlace.id,
+ ),
+ )
+ }.onFailure { e ->
+ Timber.e(e, "Failed to emit ChangePlaceEvent")
+ }
+
+ postSideEffect(PlaceDetailSideEffect.NavigateBack)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| userTravelRepository.emitChangePlaceEvent( | |
| com.yapp.ndgl.data.travel.model.ChangePlaceEvent( | |
| travelId = currentTravelId, | |
| day = currentDay, | |
| itineraryId = currentItineraryId, | |
| oldGooglePlaceId = googlePlaceId, | |
| newGooglePlaceId = selectedPlace.id, | |
| ), | |
| ) | |
| postSideEffect(PlaceDetailSideEffect.NavigateBack) | |
| suspendRunCatching { | |
| userTravelRepository.emitChangePlaceEvent( | |
| ChangePlaceEvent( | |
| travelId = currentTravelId, | |
| day = currentDay, | |
| itineraryId = currentItineraryId, | |
| oldGooglePlaceId = googlePlaceId, | |
| newGooglePlaceId = selectedPlace.id, | |
| ), | |
| ) | |
| }.onFailure { e -> | |
| Timber.e(e, "Failed to emit ChangePlaceEvent") | |
| } | |
| postSideEffect(PlaceDetailSideEffect.NavigateBack) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt`
around lines 125 - 135, The call to userTravelRepository.emitChangePlaceEvent
can throw and prevent postSideEffect(PlaceDetailSideEffect.NavigateBack) from
running; wrap the emitChangePlaceEvent call in a try/catch (or
try/catch/finally) around the block in the function containing postSideEffect so
any exception is caught/logged (similar to
DatePickerViewModel.emitTravelCreatedEvent) and ensure
postSideEffect(PlaceDetailSideEffect.NavigateBack) is always invoked (e.g., put
it in finally or after the catch) so the UI always navigates back even if event
emission fails.
Contributor
Author
There was a problem hiding this comment.
실패 시 로직이 아직 없음. 우선은 이대로
There was a problem hiding this comment.
@mj010504, 알겠습니다! 나중에 에러 처리 로직을 추가하실 때를 위해 이 내용을 GitHub 이슈로 등록해 드릴까요? 그러면 추후 작업 시 참고하시기 편하실 것 같습니다.
🧠 Learnings used
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 26
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/addplace/AddPlaceViewModel.kt:71-90
Timestamp: 2026-02-18T12:08:18.209Z
Learning: In the travel feature (place photo loading logic in AddPlaceViewModel.kt and AddItineraryViewModel.kt), retain a deliberate 1-second delay before each photo fetch when loading data for newly searched places. This accounts for backend latency needed to save place data before photos are retrievable. Ensure the delay is configurable (e.g., via feature flag or constant) and documented, so it can be adjusted or removed if backend performance improves. This guidance applies to all travel feature view models handling similar photo fetch flows.
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 17
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt:243-251
Timestamp: 2026-02-11T12:08:02.377Z
Learning: In feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt, the startIndex calculation for the WheelPicker intentionally does NOT subtract visibleItemCount/2 offset. This design choice preserves showing 5 items initially and creates an offset from the initialIndex parameter. Reviewers should not request centering the active item unless the UI design is changed; keep this behavior as the intended UX for this component.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 25
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/mytravel/MyTravelContract.kt:52-54
Timestamp: 2026-02-17T22:21:31.141Z
Learning: Use the naming convention estimatedDuration for duration-related fields across Kotlin files to align with server API response parameter names and maintain consistency throughout the codebase. When introducing or renaming duration fields, replace generic names (e.g., duration, totalDuration) with estimatedDuration and update all references (data classes, mappings, JSON parsing) accordingly to avoid mismatch with server payloads.
Learnt from: jihee-dev
Repo: YAPP-Github/27th-App-Team-1-Android PR: 27
File: feature/home/src/main/java/com/yapp/ndgl/feature/home/search/TemplateSearchScreen.kt:0-0
Timestamp: 2026-02-18T18:16:40.700Z
Learning: Use the correct Hilt Compose ViewModel import across Kotlin Android files: import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel (from androidx.hilt:hilt-lifecycle-viewmodel-compose:1.3.0). The old import androidx.hilt.navigation.compose.hiltViewModel is deprecated in Hilt 1.3.0. Ensure the hilt-lifecycle-viewmodel-compose dependency is added to Gradle (both Gradle catalog and module dependencies) and update imports accordingly to avoid runtime issues or build failures.
- AddItineraryRequest 타입 수정 : String? -> Int? - handleAddPlace() : lastPlace가 null 일때 로직 추가 - placeRepository: 주석 제거 - toOpeningHours: day <= 0 체크 추가, java.time.* 의존성 -> kotlinx.datetime으로 수정 - toTransportationItem() 최소값 1분으로 수정
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.
개요
변경사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항