Merged
Conversation
- 내 여행에 장소 추가 - PlanB 장소로 변경 - 교통수단 변경 - 장소 삭제
Contributor
Author
|
@coderabbitai review @coderabbitai why you skip the review? |
|
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새로운 NDGLSnackbar UI 컴포넌트를 도입하고, AddPlaceEvent 데이터 모델에서 4개 필드(websiteUrl, rating, userRatingCount, estimatedDuration)를 제거했으며, TravelDetail 화면에 스낵바 알림 기능을 통합하여 여행 항목 추가, 변경, 삭제 시 사용자 피드백을 제공합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Screen as TravelDetailScreen
participant ViewModel as TravelDetailViewModel
participant SnackbarHost as SnackbarHost
User->>ViewModel: 여행 항목 추가/변경/삭제<br/>(Add/Change/Delete Action)
ViewModel->>ViewModel: 여행 상태 업데이트
ViewModel->>ViewModel: ShowSnackbar(message) 생성
ViewModel-->>Screen: SideEffect 방출
Screen->>Screen: ShowSnackbar 감지
Screen->>SnackbarHost: snackbarHostState.showSnackbar(message)
SnackbarHost->>SnackbarHost: NDGLSnackbar 렌더링
SnackbarHost-->>User: 스낵바 표시<br/>(검은 배경, 흰 텍스트)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt (1)
164-169: 스낵바 문구 하드코딩 대신 메시지 키 기반으로 분리하는 편이 안전합니다.Line 165-168처럼 계약 계층에 문자열을 직접 두면 i18n 확장 시 ViewModel/Contract까지 수정 범위가 커집니다.
ShowSnackbar에는 키(enum/sealed value)만 전달하고, 실제 문자열은 UI에서stringResource로 매핑하는 구조를 권장합니다.예시 리팩터링
sealed interface TravelDetailSideEffect : UiSideEffect { @@ - data class ShowSnackbar(val message: String) : TravelDetailSideEffect + data class ShowSnackbar(val messageKey: SnackbarMessageKey) : TravelDetailSideEffect - companion object { - const val SNACKBAR_ADDED_TO_MY_TRAVEL = "내 여행에 추가되었습니다" - const val SNACKBAR_PLACE_CHANGED = "장소가 변경되었습니다" - const val SNACKBAR_TRANSPORT_CHANGED = "교통수단이 변경되었습니다" - const val SNACKBAR_PLACE_DELETED = "장소가 삭제되었습니다" - } + enum class SnackbarMessageKey { + ADDED_TO_MY_TRAVEL, + PLACE_CHANGED, + TRANSPORT_CHANGED, + PLACE_DELETED, + } }🤖 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 164 - 169, The companion object in TravelDetailContract currently exposes hardcoded snackbar messages (SNACKBAR_ADDED_TO_MY_TRAVEL, SNACKBAR_PLACE_CHANGED, SNACKBAR_TRANSPORT_CHANGED, SNACKBAR_PLACE_DELETED); change these to message keys (e.g., an enum or sealed class like SnackbarMessageKey) and update any usages (including the ShowSnackbar event) to pass only the key/value type instead of raw strings so the UI layer can call stringResource/res.getString to map keys to localized text; ensure TravelDetailContract.Companion no longer contains literal Korean strings and that ViewModel/events use the new SnackbarMessageKey type.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt (1)
157-157:estimatedDuration = 60은 상수화해서 의도를 드러내는 편이 좋습니다.비즈니스 기본값이라면 명명된 상수로 분리해 추후 정책 변경 시 리스크를 줄여주세요.
🤖 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` at line 157, The literal 60 assigned to estimatedDuration should be extracted to a named constant to make the business default explicit and easier to change; create a top-level or companion object constant (e.g. DEFAULT_ESTIMATED_DURATION_MINUTES or ESTIMATED_DURATION_DEFAULT) and replace the inline value in TravelDetailViewModel where estimatedDuration = 60 is set with that constant, and ensure any related documentation or usages (e.g., in methods inside TravelDetailViewModel) reference the new constant.
🤖 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/traveldetail/TravelDetailViewModel.kt`:
- Around line 152-154: The calculation uses
lastPlace.placeInfo.estimatedDuration which ignores user edits; change it to use
the user-adjusted duration (lastPlace.duration, which maps to
userData.estimatedDuration) when computing the next start time so the expression
becomes lastPlace.startTime + lastPlace.duration +
(newTransportSegment?.duration ?: 0.hours); update the code in
TravelDetailViewModel where this calculation occurs (reference symbols:
lastPlace, placeInfo.estimatedDuration, lastPlace.duration,
newTransportSegment?.duration).
---
Nitpick comments:
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt`:
- Around line 164-169: The companion object in TravelDetailContract currently
exposes hardcoded snackbar messages (SNACKBAR_ADDED_TO_MY_TRAVEL,
SNACKBAR_PLACE_CHANGED, SNACKBAR_TRANSPORT_CHANGED, SNACKBAR_PLACE_DELETED);
change these to message keys (e.g., an enum or sealed class like
SnackbarMessageKey) and update any usages (including the ShowSnackbar event) to
pass only the key/value type instead of raw strings so the UI layer can call
stringResource/res.getString to map keys to localized text; ensure
TravelDetailContract.Companion no longer contains literal Korean strings and
that ViewModel/events use the new SnackbarMessageKey type.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Line 157: The literal 60 assigned to estimatedDuration should be extracted to
a named constant to make the business default explicit and easier to change;
create a top-level or companion object constant (e.g.
DEFAULT_ESTIMATED_DURATION_MINUTES or ESTIMATED_DURATION_DEFAULT) and replace
the inline value in TravelDetailViewModel where estimatedDuration = 60 is set
with that constant, and ensure any related documentation or usages (e.g., in
methods inside TravelDetailViewModel) reference the new constant.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLSnackbar.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddPlaceEvent.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.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.kt
💤 Files with no reviewable changes (2)
- data/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddPlaceEvent.kt
- feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt
✅ Actions performedReview triggered.
|
|
CodeRabbit은 증분 리뷰 시스템으로 작동하여 이미 리뷰된 커밋은 다시 리뷰하지 않습니다. 자동 리뷰가 일시 중지되었거나 새로운 커밋이 아직 리뷰되지 않은 경우 리뷰가 건너뛰어질 수 있습니다. 지금 전체 PR에 대한 리뷰를 시작하겠습니다. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt (1)
152-154:⚠️ Potential issue | 🟠 Major시작시간 계산에 사용자 수정 체류시간이 반영되지 않습니다.
Line 152에서
lastPlace.placeInfo.estimatedDuration을 사용하면, 사용자가 수정한 체류시간(lastPlace.duration)이 무시됩니다. 동일한 계산 문제가 Line 183에도 남아 있습니다.수정 제안
- lastPlace.startTime + lastPlace.placeInfo.estimatedDuration + + lastPlace.startTime + lastPlace.duration + (newTransportSegment?.duration ?: 0.hours) ) .parseDurationToTimeString() @@ - startTime = (lastPlace?.startTime ?: Itinerary.DEFAULT_START_TIME.hours) + - (lastPlace?.placeInfo?.estimatedDuration ?: 1.hours) + (newTransportSegment?.duration ?: 0.hours), + startTime = (lastPlace?.startTime ?: Itinerary.DEFAULT_START_TIME.hours) + + (lastPlace?.duration ?: 1.hours) + (newTransportSegment?.duration ?: 0.hours),🤖 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 152 - 154, The start-time calculation is using the original estimated duration (lastPlace.placeInfo.estimatedDuration) which ignores user-edited stay time; update the logic in TravelDetailViewModel (locations around the existing expressions at line ~152 and ~183) to use lastPlace.duration (the possibly modified stay length) when computing startTime: replace lastPlace.placeInfo.estimatedDuration with lastPlace.duration and ensure the addition still includes the newTransportSegment?.duration fallback as before so the corrected start-time uses the user-adjusted duration.
🧹 Nitpick comments (2)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt (1)
184-184:106.dp매직 넘버는 상수로 분리해두는 편이 안전합니다.하단 CTA 높이나 간격이 바뀔 때 스낵바 위치 수정 누락을 줄일 수 있습니다.
리팩터 예시
+private val SNACKBAR_BOTTOM_PADDING = 106.dp @@ - NDGLSnackbar(modifier = Modifier.padding(bottom = 106.dp), snackbarData = data) + NDGLSnackbar(modifier = Modifier.padding(bottom = SNACKBAR_BOTTOM_PADDING), snackbarData = data)🤖 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/TravelDetailScreen.kt` at line 184, Replace the magic literal 106.dp used in the NDGLSnackbar call with a named constant to avoid hard-to-maintain magic numbers; update the call in TravelDetailScreen where NDGLSnackbar(modifier = Modifier.padding(bottom = 106.dp), ...) is used to reference a clearly named constant (e.g., SNACKBAR_BOTTOM_PADDING_DP) and declare that constant in a single place (either a top-level val in the same file or inside the TravelDetailScreen companion/object) so future CTA height/spacing changes only require updating the constant.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt (1)
162-169: 스낵바 문구는 문자열 상수보다 “메시지 키” 전달 구조가 더 확장성 있습니다.현재는 계약 레이어에 문구가 고정되어 있어 문구 변경/다국어 대응 시 수정 범위가 커집니다.
ShowSnackbar(messageKey: SnackbarMessage)형태로 전달하고, 실제 문자열은 UI에서stringResource로 매핑하는 방식을 권장합니다.🤖 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 162 - 169, The side-effect currently embeds hardcoded Korean strings in TravelDetailContract (companion object SNACKBAR_ADDED_TO_MY_TRAVEL, SNACKBAR_PLACE_CHANGED, SNACKBAR_TRANSPORT_CHANGED, SNACKBAR_PLACE_DELETED) and ShowSnackbar(val message: String) — change ShowSnackbar to carry a message key type (e.g., sealed class or enum named SnackbarMessage) instead of raw String, replace the companion string constants with that SnackbarMessage members, update any emitters that create TravelDetailContract.ShowSnackbar to use the new SnackbarMessage values, and move actual string resolution to the UI layer (map SnackbarMessage -> stringResource there) so text changes and localization are handled in the view.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Around line 152-154: The start-time calculation is using the original
estimated duration (lastPlace.placeInfo.estimatedDuration) which ignores
user-edited stay time; update the logic in TravelDetailViewModel (locations
around the existing expressions at line ~152 and ~183) to use lastPlace.duration
(the possibly modified stay length) when computing startTime: replace
lastPlace.placeInfo.estimatedDuration with lastPlace.duration and ensure the
addition still includes the newTransportSegment?.duration fallback as before so
the corrected start-time uses the user-adjusted duration.
---
Nitpick comments:
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt`:
- Around line 162-169: The side-effect currently embeds hardcoded Korean strings
in TravelDetailContract (companion object SNACKBAR_ADDED_TO_MY_TRAVEL,
SNACKBAR_PLACE_CHANGED, SNACKBAR_TRANSPORT_CHANGED, SNACKBAR_PLACE_DELETED) and
ShowSnackbar(val message: String) — change ShowSnackbar to carry a message key
type (e.g., sealed class or enum named SnackbarMessage) instead of raw String,
replace the companion string constants with that SnackbarMessage members, update
any emitters that create TravelDetailContract.ShowSnackbar to use the new
SnackbarMessage values, and move actual string resolution to the UI layer (map
SnackbarMessage -> stringResource there) so text changes and localization are
handled in the view.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt`:
- Line 184: Replace the magic literal 106.dp used in the NDGLSnackbar call with
a named constant to avoid hard-to-maintain magic numbers; update the call in
TravelDetailScreen where NDGLSnackbar(modifier = Modifier.padding(bottom =
106.dp), ...) is used to reference a clearly named constant (e.g.,
SNACKBAR_BOTTOM_PADDING_DP) and declare that constant in a single place (either
a top-level val in the same file or inside the TravelDetailScreen
companion/object) so future CTA height/spacing changes only require updating the
constant.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLSnackbar.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddPlaceEvent.ktfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.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.kt
💤 Files with no reviewable changes (2)
- data/travel/src/main/java/com/yapp/ndgl/data/travel/model/AddPlaceEvent.kt
- feature/travel/src/main/java/com/yapp/ndgl/feature/travel/additinerary/AddItineraryViewModel.kt
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
릴리스 노트