[NDGL-137] 콘텐츠 추천 화면 UI/UX 제작#45
Closed
mj010504 wants to merge 5 commits into
Closed
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough이 PR은 YouTube 콘텐츠의 메타데이터를 조회하고 여행 관련 추천을 제출할 수 있는 새로운 feature 모듈을 추가합니다. 데이터 계층의 API 통합부터 완전한 UI 화면까지, 설정 메뉴와의 네비게이션 통합을 포함합니다. Changes콘텐츠 추천 기능
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ReasonSection.kt (1)
32-51: ⚡ Quick win추천 이유 입력 필드에 최대 길이 제한을 고려하세요.
서버 API에 추천 이유 길이 제한이 있다면,
BasicTextField에maxLength제약을 추가하여 사용자가 입력 중에 한도를 초과하지 않도록 해야 합니다. 현재는 문자 수만 표시하고 있어, 사용자가 제한을 초과한 후 제출 시 오류가 발생할 수 있습니다.제안하는 수정 사항
+private const val MAX_REASON_LENGTH = 500 // 서버 제한에 맞게 조정 + `@Composable` internal fun ReasonSection( reason: String, onReasonChange: (String) -> Unit, ) { Column(modifier = Modifier.imePadding(), verticalArrangement = Arrangement.spacedBy(8.dp)) { Text( text = stringResource(R.string.content_recommendation_reason_label), style = NDGLTheme.typography.bodyMdSemiBold, color = NDGLTheme.colors.black700, ) BasicTextField( value = reason, - onValueChange = onReasonChange, + onValueChange = { if (it.length <= MAX_REASON_LENGTH) onReasonChange(it) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ReasonSection.kt` around lines 32 - 51, The reason input lacks an enforced max-length, so update ReasonSection (the BasicTextField instance using value = reason and onValueChange = onReasonChange) to enforce the server-side length limit: define a constant MAX_REASON_LENGTH, clamp or ignore additional characters inside the onValueChange handler (only pass up to MAX_REASON_LENGTH), and update any character counter logic to reflect MAX_REASON_LENGTH so users cannot type beyond the allowed length and the displayed remaining/used count stays consistent with the enforced limit.feature/content-recommendation/src/main/res/values/strings.xml (1)
18-18: 💤 Low value규칙적 공백 사용으로 단순화 검토 필요
Line 18에서
\u0020을 사용하고 있으나, 이는 프로젝트 전체 strings.xml 파일에서 유일한 유니코드 이스케이프입니다. XML 공백 처리 방지를 위해 의도적으로 사용된 것이 아니라면, 일반 공백으로 단순화하는 것이 좋습니다.<string name="content_recommendation_no_link_prefix">링크가 없나요? </string>특별한 이유로
\u0020이 필요하다면 코드에 주석을 추가하여 명시하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/content-recommendation/src/main/res/values/strings.xml` at line 18, The string resource content_recommendation_no_link_prefix currently uses a Unicode escape (\u0020) for a trailing space; replace the escape with a normal ASCII space in the string value (i.e., change the value for string name content_recommendation_no_link_prefix to use a regular space) unless the escape is intentionally required—if so, add a clarifying comment near the string explaining why \u0020 must be used to prevent future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/di/TravelNetworkModule.kt`:
- Around line 181-183: provideYoutubeOembedRetrofit currently injects an
OkHttpClient qualified as `@ExchangeRateClient` which breaks the
qualifier-consistency pattern; change the parameter to use the
`@YoutubeOembedClient` qualifier (or create a dedicated provider that returns an
OkHttpClient annotated with `@YoutubeOembedClient`) and update its
provider/binding so YouTube oEmbed uses its own qualified client instead of
`@ExchangeRateClient`, keeping the rest of the method signature
(provideYoutubeOembedRetrofit) and existing Json parameter unchanged.
In `@feature/content-recommendation/build.gradle.kts`:
- Around line 10-12: local.properties 파일이 없을 경우 FileNotFoundException으로 빌드가 실패하니
load 호출 전에 파일 존재 여부를 검사하거나 예외를 잡아 무시하도록 변경하세요: locate the localProperties
initialization (variable localProperties) and replace the direct
load(rootProject.file("local.properties").bufferedReader()) with logic that
checks rootProject.file("local.properties").exists() (or wraps the load in
try/catch) and only calls load when present, otherwise leave localProperties
empty or log a warning so cloning 환경에서도 빌드가 실패하지 않도록 만드세요.
In
`@feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationViewModel.kt`:
- Around line 41-43: The metadata fetch races when multiple URLs are entered
because fetchMetadata(url) runs without cancellation or a response guard; modify
ContentRecommendationViewModel to track and cancel the previous metadata job
(e.g., a metadataJob Coroutine Job) before launching a new one in the same
handler that calls reduce { copy(contentUrl = url, metadataState =
MetadataState.Loading) }, or alternatively include a requestId/url guard inside
fetchMetadata's response handling so you only reduce state if the current
contentUrl still equals the requested url; ensure you reference and update the
same fetchMetadata invocation and the reduce call so stale responses no longer
overwrite newer state.
- Around line 84-85: The isYoutubeUrl method is too loose (uses String.contains)
and can false-positive on query values; replace its implementation to parse the
input as a URL/URI in ContentRecommendationViewModel.kt (isYoutubeUrl) and
validate the host component: normalize to lowercase and accept hosts that are
exactly "youtube.com" or "youtu.be" or end with ".youtube.com" (to allow
subdomains like "www.youtube.com"), and exact "youtu.be"; gracefully handle
invalid URLs (return false) and avoid matching when host is missing or part of a
query/path.
- Around line 93-98: The submit() function currently no-ops when isSubmitEnabled
is true; implement a temporary UI feedback flow before API integration by
toggling a loading flag on state (use state.value and update the view model
state), emit side-effects for progress and outcome (e.g., send a "ShowLoading"
side effect, then a "ShowSuccess" or "ShowError" side effect), and clear/reset
loading when finished; keep this in submit() as the placeholder for the eventual
API call so the UI receives immediate success/failure feedback even without
backend integration.
---
Nitpick comments:
In
`@feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ReasonSection.kt`:
- Around line 32-51: The reason input lacks an enforced max-length, so update
ReasonSection (the BasicTextField instance using value = reason and
onValueChange = onReasonChange) to enforce the server-side length limit: define
a constant MAX_REASON_LENGTH, clamp or ignore additional characters inside the
onValueChange handler (only pass up to MAX_REASON_LENGTH), and update any
character counter logic to reflect MAX_REASON_LENGTH so users cannot type beyond
the allowed length and the displayed remaining/used count stays consistent with
the enforced limit.
In `@feature/content-recommendation/src/main/res/values/strings.xml`:
- Line 18: The string resource content_recommendation_no_link_prefix currently
uses a Unicode escape (\u0020) for a trailing space; replace the escape with a
normal ASCII space in the string value (i.e., change the value for string name
content_recommendation_no_link_prefix to use a regular space) unless the escape
is intentionally required—if so, add a clarifying comment near the string
explaining why \u0020 must be used to prevent future confusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0672360-add7-4ae3-8f30-e01814988ac5
📒 Files selected for processing (32)
.gitignoreapp/build.gradle.ktsapp/src/main/java/com/yapp/ndgl/ui/NDGLApp.ktcore/ui/src/main/res/drawable/ic_24_alert.xmlcore/ui/src/main/res/drawable/ic_24_asterisk.xmlcore/ui/src/main/res/drawable/ic_24_youtube.xmldata/travel/src/main/java/com/yapp/ndgl/data/travel/api/YoutubeOembedApi.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/di/TravelNetworkModule.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/model/YoutubeOembedResponse.ktdata/travel/src/main/java/com/yapp/ndgl/data/travel/repository/ContentMetadataRepository.ktfeature/content-recommendation/.gitignorefeature/content-recommendation/build.gradle.ktsfeature/content-recommendation/consumer-rules.profeature/content-recommendation/proguard-rules.profeature/content-recommendation/src/main/AndroidManifest.xmlfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentLinkSection.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationContract.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationScreen.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationViewModel.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/HeaderSection.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ReasonSection.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/TravelThemeSection.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/model/TravelTheme.ktfeature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/navigation/ContentRecommendationEntry.ktfeature/content-recommendation/src/main/res/values/strings.xmlfeature/home/src/main/java/com/yapp/ndgl/feature/home/navigation/HomeEntry.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/settings/SettingsContract.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/settings/SettingsScreen.ktfeature/home/src/main/java/com/yapp/ndgl/feature/home/settings/SettingsViewModel.ktgradle/libs.versions.tomlnavigation/src/main/java/com/yapp/ndgl/navigation/Route.ktsettings.gradle.kts
Comment on lines
+41
to
+43
| reduce { copy(contentUrl = url, metadataState = MetadataState.Loading) } | ||
| fetchMetadata(url) | ||
| } |
There was a problem hiding this comment.
URL 연속 입력 시 이전 요청 응답이 최신 상태를 덮어쓰는 레이스가 발생합니다.
이전 fetchMetadata 작업 취소/가드가 없어서 느린 이전 응답이 나중 URL의 상태를 덮을 수 있습니다.
🔧 제안 수정안
+import kotlinx.coroutines.Job
...
class ContentRecommendationViewModel `@Inject` constructor(
@@
) {
+ private var metadataJob: Job? = null
@@
else -> {
+ metadataJob?.cancel()
reduce { copy(contentUrl = url, metadataState = MetadataState.Loading) }
- fetchMetadata(url)
+ metadataJob = fetchMetadata(url)
}
}
}
- private fun fetchMetadata(url: String) = viewModelScope.launch {
+ private fun fetchMetadata(url: String): Job = viewModelScope.launch {
suspendRunCatching {
contentMetadataRepository.getMetadata(url)
}
.onSuccess { response ->
+ if (state.value.contentUrl != url) return@onSuccess
reduce {
copy(
@@
.onFailure {
+ if (state.value.contentUrl != url) return@onFailure
reduce {
copy(Also applies to: 47-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationViewModel.kt`
around lines 41 - 43, The metadata fetch races when multiple URLs are entered
because fetchMetadata(url) runs without cancellation or a response guard; modify
ContentRecommendationViewModel to track and cancel the previous metadata job
(e.g., a metadataJob Coroutine Job) before launching a new one in the same
handler that calls reduce { copy(contentUrl = url, metadataState =
MetadataState.Loading) }, or alternatively include a requestId/url guard inside
fetchMetadata's response handling so you only reduce state if the current
contentUrl still equals the requested url; ensure you reference and update the
same fetchMetadata invocation and the reduce call so stale responses no longer
overwrite newer state.
Contributor
Author
There was a problem hiding this comment.
너무 엣지 케이스라 판단하여 해당 로직은 작성하지 않았습니다.
There was a problem hiding this comment.
@mj010504, 이해했습니다. 현재 UX 패턴(URL 직접 붙여넣기 위주)에서는 빈번히 발생할 가능성이 낮은 엣지 케이스로 판단하신 점 수용합니다. 향후 요청 빈도가 높아지거나 UX가 변경되는 시점에 재검토해 주시면 될 것 같습니다.
🧠 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.
Learnt from: mj010504
Repo: YAPP-Github/NDGL-android PR: 45
File: feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationViewModel.kt:93-98
Timestamp: 2026-05-13T15:20:09.098Z
Learning: When performing code reviews in this repo, do not add review comments on any line(s) or code blocks that contain placeholder markers `//TODO` or `//FIXME`. Treat these as intentional placeholders and avoid automated feedback on them (unless the reported issue is clearly unrelated to the TODO/FIXME content itself).
Comment on lines
+84
to
+85
| private fun isYoutubeUrl(url: String): Boolean = | ||
| url.contains("youtube.com", ignoreCase = true) || url.contains("youtu.be", ignoreCase = true) |
There was a problem hiding this comment.
YouTube URL 검증이 너무 느슨해서 오탐 URL을 허용합니다.
contains("youtube.com") 방식은 https://evil.com/?next=youtube.com 같은 값도 통과시킵니다. host 기반 검증으로 바꿔주세요.
🔧 제안 수정안
+import android.net.Uri
...
- private fun isYoutubeUrl(url: String): Boolean =
- url.contains("youtube.com", ignoreCase = true) || url.contains("youtu.be", ignoreCase = true)
+ private fun isYoutubeUrl(url: String): Boolean {
+ val host = runCatching { Uri.parse(url).host?.lowercase() }.getOrNull() ?: return false
+ return host == "youtu.be" || host == "youtube.com" || host.endsWith(".youtube.com")
+ }📝 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 isYoutubeUrl(url: String): Boolean = | |
| url.contains("youtube.com", ignoreCase = true) || url.contains("youtu.be", ignoreCase = true) | |
| private fun isYoutubeUrl(url: String): Boolean { | |
| val host = runCatching { Uri.parse(url).host?.lowercase() }.getOrNull() ?: return false | |
| return host == "youtu.be" || host == "youtube.com" || host.endsWith(".youtube.com") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/content-recommendation/src/main/java/com/yapp/ndgl/feature/contentrecommendation/ContentRecommendationViewModel.kt`
around lines 84 - 85, The isYoutubeUrl method is too loose (uses
String.contains) and can false-positive on query values; replace its
implementation to parse the input as a URL/URI in
ContentRecommendationViewModel.kt (isYoutubeUrl) and validate the host
component: normalize to lowercase and accept hosts that are exactly
"youtube.com" or "youtu.be" or end with ".youtube.com" (to allow subdomains like
"www.youtube.com"), and exact "youtu.be"; gracefully handle invalid URLs (return
false) and avoid matching when host is missing or part of a query/path.
Contributor
Author
There was a problem hiding this comment.
가장 단순한 방법으로 해당 방법 선택했습니다.
There was a problem hiding this comment.
@mj010504, 네, 이해했습니다! 단순함을 우선시하는 선택이시군요. 다만 참고로, contains 방식은 https://evil.com/?redirect=youtube.com 같은 URL도 YouTube URL로 인식할 수 있어서, 향후 보안이 중요해지는 시점에 호스트 기반 검증으로 개선을 고려해 보시면 좋을 것 같습니다. 현재 결정을 존중합니다! 🙂
🧠 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.
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.
개요
연관 문서
디자인
스크린샷
bandicam.2026-05-13.23-57-29-100.mp4
변경사항