Feature/#161 : 마이페이지 이용문의 카카오톡 눌렀을 시 이동하도록 처리 #163
Conversation
|
Important Review skippedAuto 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 Walkthrough이번 변경에서는 설정(Setting) 및 프로필(Profile) 화면에서 외부 웹 링크 이동 및 실패 시 토스트 메시지 표시 등 사이드 이펙트 처리가 명확하게 분리 및 개선되었습니다. SettingIntent와 SettingSideEffect가 각각 앱 버전 클릭 및 마켓 URL 오픈으로 변경되었고, ProfileSideEffect는 웹 브라우저 오픈과 실패 토스트로 확장되었습니다. ViewModel 로직도 이에 맞게 수정되어, 각각의 클릭 이벤트에 따라 적절한 사이드 이펙트가 발생하도록 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingScreen
participant SettingViewModel
participant AndroidSystem
User->>SettingScreen: 앱 버전 클릭
SettingScreen->>SettingViewModel: onIntent(ClickAppVersion)
SettingViewModel->>SettingScreen: postSideEffect(OpenMarket(url))
SettingScreen->>AndroidSystem: Intent로 마켓 URL 오픈
sequenceDiagram
participant User
participant ProfileScreen
participant ProfileViewModel
participant OperationsRepository
participant AndroidSystem
User->>ProfileScreen: 이용문의 클릭
ProfileScreen->>ProfileViewModel: onIntent(ClickUsage)
ProfileViewModel->>OperationsRepository: getUsageInquiryLink()
OperationsRepository-->>ProfileViewModel: 링크 반환
ProfileViewModel->>ProfileScreen: postSideEffect(OpenWebBrowser(link))
ProfileScreen->>AndroidSystem: URI Handler로 웹 브라우저 오픈
alt 링크 실패 시
ProfileViewModel->>ProfileScreen: postSideEffect(ShowUrlLoadFailToast)
ProfileScreen->>User: 토스트 메시지 표시
end
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ 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: 4
🔭 Outside diff range comments (1)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt (1)
22-111: 🧹 Nitpick (assertive)사용하지 않는 inquiryLink 변수가 있습니다.
ClickInquiryItemIntent 처리가 제거되었지만,inquiryLink변수와 관련 로직은 여전히 남아있습니다. 이 변수와 관련 로직이 더 이상 사용되지 않는다면 제거하는 것을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingContract.kt(1 hunks)feature/home/src/main/java/com/yapp/feature/home/setting/SettingScreen.kt(4 hunks)feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt(1 hunks)feature/profile/src/main/java/com/yapp/feature/profile/ProfileContract.kt(1 hunks)feature/profile/src/main/java/com/yapp/feature/profile/ProfileScreen.kt(5 hunks)feature/profile/src/main/java/com/yapp/feature/profile/ProfileViewModel.kt(3 hunks)
🔇 Additional comments (10)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingContract.kt (2)
14-14: 새로운 Intent 추가가 잘 되었습니다.앱 버전 클릭에 대한 새로운 Intent를 추가하여 이전 코드와의 일관성을 유지하면서 기능을 확장했습니다.
20-20: Market URL을 열기 위한 SideEffect 추가가 적절합니다.마켓 URL을 열기 위한 전용 SideEffect를 추가하여 의도가 명확해졌습니다. 웹 브라우저와 마켓 앱을 구분하여 처리하는 것은 좋은 접근 방식입니다.
feature/profile/src/main/java/com/yapp/feature/profile/ProfileContract.kt (1)
34-35: URL 처리를 위한 SideEffect 추가가 적절합니다.웹 브라우저 열기와 URL 로드 실패 시 토스트 메시지 표시에 대한 SideEffect를 추가하여 사용자 경험이 향상되었습니다. 이는 이전의 단순 네비게이션보다 더 명확하고 구체적인 처리 방식입니다.
feature/home/src/main/java/com/yapp/feature/home/setting/SettingScreen.kt (2)
3-5: 필요한 임포트가 잘 추가되었습니다.Intent, Uri 및 toUri 확장 함수에 대한 임포트가 적절히 추가되었습니다.
Also applies to: 22-22
64-67: 마켓 URL을 여는 기능이 잘 구현되었습니다.마켓 URL을 URI로 변환하고 Intent.ACTION_VIEW를 사용하여 외부 앱을 여는 로직이 적절히 구현되었습니다.
feature/profile/src/main/java/com/yapp/feature/profile/ProfileScreen.kt (3)
3-3: 필요한 임포트가 올바르게 추가되었습니다.외부 웹 브라우저 열기와 토스트 메시지 표시에 필요한 임포트들이 잘 추가되었습니다.
Also applies to: 15-16, 28-28
43-44: 로컬 컨텍스트와 URI 핸들러가 적절하게 초기화되었습니다.외부 URL 열기와 토스트 메시지 표시를 위해 필요한 Android 컨텍스트와 URI 핸들러를 Compose의 Local 컴포저블로부터 가져오는 것은 좋은 방법입니다.
64-73: 카카오톡 URL 열기 및 오류 처리가 잘 구현되었습니다.사이드 이펙트를 통해 웹 브라우저로 카카오톡 링크를 열고, URL 로딩 실패 시 사용자에게 토스트 메시지로 피드백을 제공하는 방식이 잘 구현되었습니다.
safeOpenUri확장 함수를 사용하여 안전하게 URI를 여는 것도 좋은 접근법입니다.feature/profile/src/main/java/com/yapp/feature/profile/ProfileViewModel.kt (2)
9-10: 필요한 의존성과 임포트가 올바르게 추가되었습니다.URL 처리를 위한
OperationsRepository의존성과 관련 코루틴 기능들이 잘 추가되었습니다.Also applies to: 16-17, 19-20, 28-29
31-32: URL 캐싱을 위한 변수가 적절하게 추가되었습니다.
inquiryLink변수를 통해 URL을 캐싱하는 방식은 동일한 URL을 반복해서 가져오는 것을 방지하여 효율성을 높입니다.
| SettingIntent.ClickAppVersion -> { | ||
| postSideEffect(SettingSideEffect.OpenMarket("market://details?id=com.yapp.app.official")) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
앱 버전 클릭 시 마켓 페이지 이동 로직이 잘 구현되었습니다.
하드코딩된 마켓 URL을 사용하는 것은 현재 구현에서는 문제가 없어 보입니다. 다만, 향후 유지보수를 위해 상수로 분리하거나 설정 파일에서 관리하는 방안을 고려해볼 수 있습니다.
+ // 상수로 정의하여 재사용성과 유지보수성 향상
+ private companion object {
+ private const val MARKET_URL = "market://details?id=com.yapp.app.official"
+ }
SettingIntent.ClickAppVersion -> {
- postSideEffect(SettingSideEffect.OpenMarket("market://details?id=com.yapp.app.official"))
+ postSideEffect(SettingSideEffect.OpenMarket(MARKET_URL))
}📝 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.
| SettingIntent.ClickAppVersion -> { | |
| postSideEffect(SettingSideEffect.OpenMarket("market://details?id=com.yapp.app.official")) | |
| } | |
| // 상수로 정의하여 재사용성과 유지보수성 향상 | |
| private companion object { | |
| private const val MARKET_URL = "market://details?id=com.yapp.app.official" | |
| } | |
| SettingIntent.ClickAppVersion -> { | |
| postSideEffect(SettingSideEffect.OpenMarket(MARKET_URL)) | |
| } |
| @@ -159,7 +167,7 @@ fun SettingScreen( | |||
| ), | |||
| text = stringResource(id = R.string.setting_screen_item_app_version), | |||
| onClick = { | |||
| onIntent(SettingIntent.ClickInquiryItem) | |||
| // onIntent(SettingIntent.ClickAppVersion) | |||
There was a problem hiding this comment.
앱 버전 클릭 기능이 비활성화되어 있습니다.
앱 버전 클릭 시 마켓으로 이동하는 기능이 PR 목표에 포함되어 있지만, 클릭 핸들러가 주석 처리되어 있어 기능이 작동하지 않을 것입니다. 주석을 제거하여 앱 버전 클릭 시 마켓으로 이동하도록 해야 합니다.
onClick = {
- //onIntent(SettingIntent.ClickAppVersion)
+ onIntent(SettingIntent.ClickAppVersion)
},📝 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.
| // onIntent(SettingIntent.ClickAppVersion) | |
| onClick = { | |
| onIntent(SettingIntent.ClickAppVersion) | |
| }, |
| private fun updateUrl() { | ||
| viewModelScope.launch { | ||
| val inquiryDeferred = async { | ||
| if (inquiryLink == null) { | ||
| runCatching { operationsRepository.getUsageInquiryLink() } | ||
| } else { | ||
| Result.success(inquiryLink) | ||
| } | ||
| } | ||
|
|
||
| inquiryLink = inquiryDeferred.await().getOrNull() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
URL 업데이트 로직의 개선이 필요합니다.
updateUrl() 메서드의 구현이 약간 복잡합니다. 현재는 코루틴 내에서 또 다른 코루틴(launch 내부에 async)을 시작하고 있는데, 이는 불필요한 중첩된 코루틴을 생성합니다. 또한 단일 비동기 작업에 대해 async/await 패턴을 사용하는 것은 오버헤드가 될 수 있습니다.
다음과 같이 간소화할 수 있습니다:
- private fun updateUrl() {
- viewModelScope.launch {
- val inquiryDeferred = async {
- if (inquiryLink == null) {
- runCatching { operationsRepository.getUsageInquiryLink() }
- } else {
- Result.success(inquiryLink)
- }
- }
-
- inquiryLink = inquiryDeferred.await().getOrNull()
- }
- }
+ private suspend fun updateUrl() {
+ if (inquiryLink == null) {
+ inquiryLink = runCatching {
+ operationsRepository.getUsageInquiryLink()
+ }.getOrNull()
+ }
+ }📝 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.
| private fun updateUrl() { | |
| viewModelScope.launch { | |
| val inquiryDeferred = async { | |
| if (inquiryLink == null) { | |
| runCatching { operationsRepository.getUsageInquiryLink() } | |
| } else { | |
| Result.success(inquiryLink) | |
| } | |
| } | |
| inquiryLink = inquiryDeferred.await().getOrNull() | |
| } | |
| } | |
| private suspend fun updateUrl() { | |
| if (inquiryLink == null) { | |
| inquiryLink = runCatching { | |
| operationsRepository.getUsageInquiryLink() | |
| }.getOrNull() | |
| } | |
| } |
There was a problem hiding this comment.
R:
위에 코멘트 반영하시면은 해당 함수는 지워주세요
feature/profile/src/main/java/com/yapp/feature/profile/ProfileViewModel.kt
Outdated
Show resolved
Hide resolved
|
테스트 제가 해볼게요! |
feature/profile/src/main/java/com/yapp/feature/profile/ProfileViewModel.kt
Outdated
Show resolved
Hide resolved
| viewModelScope.launch { | ||
| updateUrl() | ||
| inquiryLink?.let { | ||
| sideEffect(ProfileSideEffect.OpenWebBrowser(it)) | ||
| } ?: run { | ||
| sideEffect(ProfileSideEffect.ShowUrlLoadFailToast) | ||
| } | ||
| } |
There was a problem hiding this comment.
R:
ProfileIntent.ClickUsage -> {
viewModelScope.launch {
runCatchingIgnoreCancelled {
operationsRepository.getUsageInquiryLink()
}.onSuccess { url ->
sideEffect(ProfileSideEffect.OpenWebBrowser(url))
}.onFailure { e ->
sideEffect(ProfileSideEffect.ShowUrlLoadFailToast)
e.record()
}
}
}이미 OperationsRepositoryImpl에서 캐싱처리를 해주고 있기 때문에 따로 뷰모델에 캐싱할 필요가 없을 것 같습니다
private fun updateUrl() {
viewModelScope.launch {
val inquiryDeferred = async {
if (inquiryLink == null) {
runCatching { operationsRepository.getUsageInquiryLink() }
} else {
Result.success(inquiryLink)
}
}
inquiryLink = inquiryDeferred.await().getOrNull()
}
}또한 updateUrl()이 내부적으로 viewModelScope.launch로 별도 코루틴에서 실행되므로 inquiryLink가 업데이트되기 전에 바로 inquiryLink?.let { ... } 로직이 실행되어
최초 한번은 확정적으로 ShowUrlLoadFailToast 가 실행되고 있습니다.
There was a problem hiding this comment.
다음과 같이 수정했는데 실행 후 확인 한번만 부탁드리겠습니다 🙏
| private fun updateUrl() { | ||
| viewModelScope.launch { | ||
| val inquiryDeferred = async { | ||
| if (inquiryLink == null) { | ||
| runCatching { operationsRepository.getUsageInquiryLink() } | ||
| } else { | ||
| Result.success(inquiryLink) | ||
| } | ||
| } | ||
|
|
||
| inquiryLink = inquiryDeferred.await().getOrNull() | ||
| } | ||
| } |
There was a problem hiding this comment.
R:
위에 코멘트 반영하시면은 해당 함수는 지워주세요
|
@TaeseongYun 이용 문의 클릭 시, 카카오톡 오픈 채팅방 열리는 것 확인했습니다 
피그마 찾아보니까 앱 버전은 클릭해도 아무 동작 안하는 것 같은데 맞을까요? |
엇 네 아무동작하지 않는게 맞긴한데 사용자마다 앱 버전이 다를거라 앱 버전 누르면 마켓으로 이동하게 처리하는거에 대해 이거 한번 물어보도록 하겠습니다 |
TaeseongYun
force-pushed
the
feature/#161-kakao-use-inquiry
branch
from
1038108 to
67bbc30
Compare
💡 Issue
🌱 Key changes
📸 스크린샷
현재 앱 실행자체가 되지 않아 테스트는 못했습니다 ,,
Summary by CodeRabbit
신규 기능
버그 수정
기타