[NDGL-112] 장소 즐겨찾기 관련 API 연동#35
Merged
Merged
Conversation
Walkthrough여행 앱에 장소 북마크 기능을 추가하는 변경사항입니다. 새로운 API 엔드포인트 세 개를 정의하고, 대응하는 데이터 모델과 저장소 메서드를 추가했습니다. UI 계층에서는 추천 및 북마크된 장소를 동적으로 로드하고 북마크 상태를 관리하도록 업데이트했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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: 2
🧹 Nitpick comments (3)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryContract.kt (1)
57-62:thumbnail기본값 추가 고려nullable로 바뀐 건 좋습니다. 선택적 필드라면 기본값을
null로 두면 호출부에서 불필요한 전달을 줄일 수 있습니다.♻️ 제안 변경
data class SelectablePlace( val googlePlaceId: String, val name: String, val placeType: PlaceType = PlaceType.ATTRACTION, - val thumbnail: String?, + val thumbnail: String? = null, )🤖 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/AddItineraryContract.kt` around lines 57 - 62, SelectablePlace의 thumbnail이 nullable로 변경된 것은 적절하나 기본값을 지정하지 않아 호출부에서 불필요하게 전달해야 할 수 있으므로 thumbnail의 기본값을 null로 설정해 호출 시 생략 가능하도록 변경하세요; 데이터 클래스 SelectablePlace의 프로퍼티 thumbnail에 기본값 null을 부여하고(생성자 시 기본 파라미터) 관련 생성 호출부가 문제 없이 컴파일되는지(필요시 호출부에서 인자 제거)만 확인하면 됩니다.data/travel/src/main/java/com/yapp/ndgl/data/travel/api/PlaceApi.kt (1)
33-33: 사소한 포맷팅:@Query("size")뒤에 공백 누락다른
@Query어노테이션들과 일관성을 위해 공백을 추가해주세요.💅 수정 제안
- `@Query`("size")size: Int? = null, + `@Query`("size") size: Int? = null,🤖 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/api/PlaceApi.kt` at line 33, In PlaceApi.kt update the parameter declaration `@Query("size")size: Int? = null` to match other parameters by inserting a space after the annotation so it reads `@Query("size") size: Int? = null`; this is a purely formatting change to keep `@Query` annotations consistent across the interface.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt (1)
90-108:Timber.d로 에러를 로깅하고 있지만, 에러 수준이 적절하지 않을 수 있습니다.API 호출 실패는
Timber.d(debug) 보다Timber.e(error) 또는Timber.w(warning)가 더 적합합니다. 디버그 레벨은 프로덕션 빌드에서 필터링될 수 있습니다.♻️ 수정 제안
}.onFailure { // FIXME: Handle Error - Timber.d("${it.message} $it") + Timber.e(it, "Failed to load bookmarked places") }🤖 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 90 - 108, The onFailure block in loadBookmarkedPlaces currently logs failures with Timber.d which is too low-severity for API call errors; change the logging to Timber.e (or Timber.w if you prefer warning-level) and pass the throwable so the stacktrace is recorded (e.g., replace the Timber.d("${it.message} $it") in the onFailure of loadBookmarkedPlaces with a Timber.e call that includes the throwable and a short context message).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt`:
- Around line 308-339: When placeRepository.getPlace(checkedPlaceId) returns
null in AddItineraryViewModel (the checkedPlaceId branch), don’t silently
navigate back; instead emit an error side effect before returning. Update the
branch that checks if (placeDetail != null) so the else path calls
postSideEffect with an error/feedback action (e.g.,
AddItinerarySideEffect.ShowError or ShowToast) describing the failure, and only
navigate back after user-visible feedback if appropriate; reference the existing
postSideEffect and AddItinerarySideEffect types and ensure
userTravelRepository.emitAddPlaceEvent is only called when placeDetail is
non-null.
- Around line 357-383: bookmarkPlace calls placeRepository using the passed
placeId but updates UI state using state.value.selectedPlaceDetail, which can
mismatch; ensure consistency by checking
selectedPlaceDetail?.placeInfo?.googlePlaceId against the placeId before
mutating state: call placeRepository.bookmarkPlace/unBookmarkPlace with the
given placeId as you do, but only run the reduce { copy(selectedPlaceDetail =
...) } branch when selectedPlaceDetail?.placeInfo?.googlePlaceId == placeId
(otherwise skip the local selectedPlaceDetail update); use the existing symbols
bookmarkPlace, selectedPlaceDetail, placeInfo.googlePlaceId,
placeRepository.bookmarkPlace/unBookmarkPlace and still call
loadBookmarkedPlaces() after success.
---
Nitpick comments:
In `@data/travel/src/main/java/com/yapp/ndgl/data/travel/api/PlaceApi.kt`:
- Line 33: In PlaceApi.kt update the parameter declaration `@Query("size")size:
Int? = null` to match other parameters by inserting a space after the annotation
so it reads `@Query("size") size: Int? = null`; this is a purely formatting
change to keep `@Query` annotations consistent across the interface.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryContract.kt`:
- Around line 57-62: SelectablePlace의 thumbnail이 nullable로 변경된 것은 적절하나 기본값을 지정하지
않아 호출부에서 불필요하게 전달해야 할 수 있으므로 thumbnail의 기본값을 null로 설정해 호출 시 생략 가능하도록 변경하세요; 데이터
클래스 SelectablePlace의 프로퍼티 thumbnail에 기본값 null을 부여하고(생성자 시 기본 파라미터) 관련 생성 호출부가 문제
없이 컴파일되는지(필요시 호출부에서 인자 제거)만 확인하면 됩니다.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt`:
- Around line 90-108: The onFailure block in loadBookmarkedPlaces currently logs
failures with Timber.d which is too low-severity for API call errors; change the
logging to Timber.e (or Timber.w if you prefer warning-level) and pass the
throwable so the stacktrace is recorded (e.g., replace the
Timber.d("${it.message} $it") in the onFailure of loadBookmarkedPlaces with a
Timber.e call that includes the throwable and a short context message).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
data/travel/src/main/java/com/yapp/ndgl/data/travel/api/PlaceApi.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/GetBookmarkedPlacesResponse.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/repository/PlaceRepository.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryContract.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryScreen.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/component/AddItineraryBottomSheet.kt
Comment on lines
+308
to
+339
| // AddItineraryBottomSheet에서 체크박스로 선택한 장소가 있는 경우 | ||
| if (checkedPlaceId != null) { | ||
| // 장소 상세 정보 가져오기 | ||
| val placeDetail = suspendRunCatching { | ||
| placeRepository.getPlace(checkedPlaceId) | ||
| }.getOrNull() | ||
|
|
||
| if (placeDetail != null) { | ||
| userTravelRepository.emitAddPlaceEvent( | ||
| AddPlaceEvent( | ||
| travelId = travelId, | ||
| day = day, | ||
| googlePlaceId = placeDetail.place.id, | ||
| name = placeDetail.place.name, | ||
| latitude = placeDetail.place.location.latitude, | ||
| longitude = placeDetail.place.location.longitude, | ||
| thumbnail = placeDetail.place.thumbnail, | ||
| placeType = placeDetail.place.category, | ||
| address = placeDetail.place.formattedAddress, | ||
| phoneNumber = placeDetail.place.nationalPhoneNumber | ||
| ?: placeDetail.place.internationalPhoneNumber, | ||
| googleMapsUri = placeDetail.place.googleMapsUri, | ||
| websiteUrl = placeDetail.place.websiteUri, | ||
| rating = placeDetail.place.rating, | ||
| userRatingCount = placeDetail.place.userRatingCount, | ||
| estimatedDuration = 60, // 기본값 60분 | ||
| ), | ||
| ) | ||
| postSideEffect(AddItinerarySideEffect.NavigateBack) | ||
| return@launch | ||
| } | ||
| } |
There was a problem hiding this comment.
checkedPlaceId 경로에서 장소 상세 조회 실패 시 사용자 피드백 없이 뒤로 이동
placeRepository.getPlace(checkedPlaceId)가 실패하면 (getOrNull() → null), 사용자가 장소를 체크하고 추가 버튼을 눌렀음에도 아무런 피드백 없이 뒤로 이동합니다. 최소한 에러 사이드이펙트를 발생시키거나 토스트 메시지를 보여주는 것이 좋겠습니다.
🤖 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 308 - 339, When placeRepository.getPlace(checkedPlaceId) returns
null in AddItineraryViewModel (the checkedPlaceId branch), don’t silently
navigate back; instead emit an error side effect before returning. Update the
branch that checks if (placeDetail != null) so the else path calls
postSideEffect with an error/feedback action (e.g.,
AddItinerarySideEffect.ShowError or ShowToast) describing the failure, and only
navigate back after user-visible feedback if appropriate; reference the existing
postSideEffect and AddItinerarySideEffect types and ensure
userTravelRepository.emitAddPlaceEvent is only called when placeDetail is
non-null.
There was a problem hiding this comment.
@mj010504, 알겠습니다! 나중에 수정하시는군요.
이 내용을 추적하기 위해 GitHub 이슈를 생성해드릴까요? 생성 시 PR과 댓글 링크를 포함하여 나중에 참고하기 쉽게 해드리겠습니다.
🧠 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.
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
릴리스 노트
새로운 기능
개선 사항