-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/#144 : 운영 Url 불러오기 실패 처리 #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
""" Walkthrough이 변경사항은 운영 URL(문의, 이용약관, 개인정보처리방침 등) 불러오기 실패 시 글로벌 토스트로 실패를 알리는 처리를 도입합니다. 이를 위해 URL을 가져오는 기존의 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ViewModel
participant Repository
participant API
User->>ViewModel: URL 열기 요청 (예: 약관 클릭)
alt URL 캐시 있음
ViewModel->>User: 브라우저 오픈 side effect
else URL 캐시 없음
ViewModel->>Repository: suspend fun getUrl()
Repository->>API: 원격 URL 요청
API-->>Repository: URL 반환
Repository-->>ViewModel: URL 반환
alt URL 정상 반환
ViewModel->>User: 브라우저 오픈 side effect
else URL 실패(null)
ViewModel->>User: ShowUrlLoadFailToast side effect
end
end
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (3)feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt (2)
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt (2)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt (2)
🔇 Additional comments (17)
✨ Finishing Touches
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt (1)
84-99
: 재시도 로직-UX 동일 이슈
SettingViewModel
과 동일하게 재시도 직후 즉시 실패 토스트를 표시하여 성공/실패 여부가 확정되기 전 사용자에게 혼란을 줄 수 있습니다.
앞서 제시한 개선안을 함께 적용해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
core/data-api/src/main/java/com/yapp/dataapi/OperationsRepository.kt
(1 hunks)core/data/src/main/java/com/yapp/core/data/data/repository/OperationsRepositoryImpl.kt
(2 hunks)core/ui/build.gradle.kts
(1 hunks)core/ui/src/main/java/com/yapp/core/ui/extension/Context.kt
(0 hunks)core/ui/src/main/java/com/yapp/core/ui/extension/UriHandler.kt
(1 hunks)core/ui/src/main/res/values/strings.xml
(1 hunks)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
(5 hunks)feature/login/src/main/java/com/yapp/feature/login/LoginContract.kt
(1 hunks)feature/login/src/main/java/com/yapp/feature/login/LoginScreen.kt
(3 hunks)feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt
(4 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpContract.kt
(1 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpScreen.kt
(5 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt
(5 hunks)
💤 Files with no reviewable changes (1)
- core/ui/src/main/java/com/yapp/core/ui/extension/Context.kt
🧰 Additional context used
🧬 Code Graph Analysis (3)
feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt (2)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt (1)
updateUrl
(121-147)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt (1)
updateUrl
(235-247)
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt (2)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt (1)
updateUrl
(121-147)feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt (1)
updateUrl
(155-173)
feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt (2)
feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt (1)
updateUrl
(155-173)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt (1)
updateUrl
(235-247)
🪛 detekt (1.23.7)
core/ui/src/main/java/com/yapp/core/ui/extension/UriHandler.kt
[warning] 16-16: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
🔇 Additional comments (26)
core/ui/build.gradle.kts (1)
15-15
: 필요한 종속성 추가 확인됨안전한 URI 처리와 로깅을 위한 core.commonAndroid와 timber 종속성이 적절하게 추가되었습니다. 이는 URL 불러오기 실패 처리를 위한 핵심 기능을 지원합니다.
Also applies to: 19-19
feature/login/src/main/java/com/yapp/feature/login/LoginContract.kt (1)
38-38
: URL 로드 실패 처리를 위한 사이드 이펙트 추가 확인URL 불러오기 실패 시 토스트 메시지를 표시하는 사이드 이펙트가 적절하게 추가되었습니다. 이는 다른 기능 모듈과 일관되게 URL 오류 처리 방식을 개선합니다.
core/ui/src/main/res/values/strings.xml (1)
9-9
: URL 로드 실패 메시지 추가 확인URL 불러오기 실패 시 사용자에게 표시할 메시지가 적절하게 추가되었습니다. 메시지 내용이 명확하고 사용자 친화적입니다.
feature/home/src/main/java/com/yapp/feature/home/setting/SettingContract.kt (1)
29-29
: URL 로드 실패 처리를 위한 사이드 이펙트 추가 확인설정 화면에서 URL 불러오기 실패 시 토스트 메시지를 표시하는 사이드 이펙트가 적절하게 추가되었습니다. 이는 로그인 및 회원가입 모듈과 일관된 방식으로 구현되어 앱 전체적으로 URL 오류 처리 패턴이 통일되었습니다.
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpContract.kt (1)
55-55
: URL 로드 실패를 표시하는 Side Effect 추가 👍
ShowUrlLoadFailToast
Side Effect를 추가하여 URL 로드 실패 시 사용자에게 알림을 제공하는 방식이 좋습니다. 이는 다른 기능(Login, Setting)과 일관되게 구현되어 사용자 경험을 향상시킵니다.feature/home/src/main/java/com/yapp/feature/home/setting/SettingScreen.kt (4)
3-3
: 필요한 import 문 추가 확인URL 처리 로직 개선을 위한 import 추가가 적절합니다. Toast, LocalUriHandler, safeOpenUri 확장 함수 및 문자열 리소스를 위한 import가 포함되어 있습니다.
Also applies to: 16-16, 31-31, 33-33
46-46
: UriHandler 인스턴스 추가 확인
LocalUriHandler.current
를 사용하는 방식은 Jetpack Compose의 권장 방식입니다.
53-53
: 더 안전한 URL 열기 방식으로 변경
context.openUrl()
에서uriHandler.safeOpenUri()
로 변경하여 URL 처리가 더 안전해졌습니다. 이 방식은 URL 스키마 검증 및 오류 로깅을 포함하여 더 견고한 구현을 제공합니다.
60-62
: URL 로드 실패 처리 추가URL 로드 실패 시 토스트 메시지를 표시하는 로직이 추가되었습니다. 사용자에게 명확한 피드백을 제공하여 사용자 경험이 향상되었습니다.
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpScreen.kt (5)
3-3
: 필요한 import 문 추가 확인URL 처리 로직 개선을 위한 import 추가가 적절합니다. Toast, LocalUriHandler, safeOpenUri 확장 함수 및 문자열 리소스를 위한 import가 포함되어 있습니다.
Also applies to: 24-24, 37-37, 40-40
60-60
: UriHandler 인스턴스 추가 확인
LocalUriHandler.current
를 사용하는 방식은 Jetpack Compose의 권장 방식입니다.
67-67
: 더 안전한 URL 열기 방식으로 변경
safeOpenUri()
확장 함수를 사용하여 URL 열기 방식이 개선되었습니다. 이는 URL 유효성 검사와 오류 처리를 포함하여 더 안전한 구현을 제공합니다.
68-74
: URL 로드 실패 처리 추가URL 로드 실패 시 토스트 메시지를 표시하는 로직이 추가되었습니다. 이는 사용자에게 명확한 피드백을 제공하고 다른 기능(Login, Setting)과 일관된 사용자 경험을 제공합니다.
201-201
: 코드 포맷팅 수정
if
조건문 뒤에 공백을 추가하여 코드 가독성이 향상되었습니다.feature/login/src/main/java/com/yapp/feature/login/LoginScreen.kt (4)
15-15
: 필요한 import 문 추가 확인URL 처리 로직 개선을 위한 import 추가가 적절합니다. LocalUriHandler, safeOpenUri 확장 함수 및 문자열 리소스를 위한 import가 포함되어 있습니다.
Also applies to: 18-18, 24-24
43-43
: UriHandler 인스턴스 추가 확인
LocalUriHandler.current
를 사용하는 방식은 Jetpack Compose의 권장 방식입니다.
53-53
: 더 안전한 URL 열기 방식으로 변경
context.openUrl()
에서uriHandler.safeOpenUri()
로 변경하여 URL 처리가 더 안전해졌습니다. 이 방식은 URL 스키마 검증 및 오류 로깅을 포함하여 더 견고한 구현을 제공합니다.
63-69
: URL 로드 실패 처리 추가URL 로드 실패 시 토스트 메시지를 표시하는 로직이 추가되었습니다. 사용자에게 명확한 피드백을 제공하여 사용자 경험이 향상되었습니다.
core/data-api/src/main/java/com/yapp/dataapi/OperationsRepository.kt (1)
8-12
: Flow에서 suspend 함수로 URL 반환 타입 변경URL을 가져오는 메서드의 반환 타입을
Flow<String>
에서suspend fun
으로 변경한 것은 비동기 처리를 단순화하고 코드 가독성을 향상시키는 좋은 결정입니다.feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt (3)
33-33
: inquiryLink 속성을 nullable로 변경
inquiryLink
속성을 null로 초기화하여 URL이 로드되지 않은 상태를 명확하게 표현할 수 있게 되었습니다.
68-68
: 화면 진입 시 URL 로드 로직 개선화면 진입 시 즉시 URL을 업데이트하는 방식으로 변경한 것은 좋은 접근입니다.
187-194
: URL 로드 실패 시 사용자 피드백 추가URL이 null일 경우 사용자에게 토스트 메시지를 표시하는 처리가 잘 추가되었습니다. 이는 사용자 경험을 향상시키는 좋은 개선점입니다.
core/data/src/main/java/com/yapp/core/data/data/repository/OperationsRepositoryImpl.kt (2)
25-27
: URL 캐싱을 위한 속성 추가URL을 캐싱하기 위한 속성들을 추가한 것은 네트워크 호출을 최소화하고 성능을 향상시키는 좋은 접근입니다.
50-66
: URL 반환 메소드의 효율적인 구현URL을 가져오는 메소드들이 캐싱 로직을 포함하여 효율적으로 구현되었습니다. 기존 값이 있으면 반환하고, 없을 경우에만 API를 호출하는 방식은 네트워크 리소스를 절약하는 좋은 패턴입니다.
또한
also
블록을 사용하여 API 응답을 캐싱하는 방식도 간결하고 가독성이 좋습니다.feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt (2)
23-26
: 동시 접근 시 가시성 문제 가능성
privacyPolicyLink
등 세 변수는var
로 선언되었습니다. 동일 ViewModel 내부에서만 접근하더라도 다중 코루틴이 병렬로updateUrl()
을 실행하면 최근 값이 덮어써지는 경쟁 조건이 발생할 수 있습니다.
Mutex
혹은atomic
변수를 이용해 한 번만 갱신- 또는
private val _links = mutableStateOf<Links?>(null)
등 불변 객체로 묶어 변경 지점을 단일화검토 부탁드립니다.
121-147
:⚠️ Potential issue제너릭 타입 불일치로 인한 컴파일 오류 가능성 –
Result<String>
↔Result<String?>
if
/else
두 분기에서 반환 타입이 달라async{}
블럭의 공통 타입을 맞추지 못해 컴파일이 실패할 수 있습니다.
runCatching { … }
→Result<String>
Result.success(privacyPolicyLink)
→Result<String?>
Result
는 불공변(invariant)이므로Result<String>
을Result<String?>
로 승격할 수 없습니다. 이후await().getOrNull()
결과를String?
으로 받을 때도 타입 불일치가 발생합니다.아래와 같이 Result 대신 최종 값(String?)만 반환하도록 수정하면 타입 문제가 사라지고 코드도 간결해집니다.
-private fun updateUrl() = viewModelScope.launch { - val privacyPolicyDeferred = async { - if (privacyPolicyLink == null) { - runCatching { operationsRepository.getPrivacyPolicyLink() } - } else { - Result.success(privacyPolicyLink) - } - } +private fun updateUrl() = viewModelScope.launch { + val privacyPolicyDeferred = async { + privacyPolicyLink ?: runCatching { operationsRepository.getPrivacyPolicyLink() }.getOrNull() + } - val termsDeferred = async { - if (termsLink == null) { - runCatching { operationsRepository.getTermsOfServiceLink() } - } else { - Result.success(termsLink) - } - } + val termsDeferred = async { + termsLink ?: runCatching { operationsRepository.getTermsOfServiceLink() }.getOrNull() + } - val inquiryDeferred = async { - if (inquiryLink == null) { - runCatching { operationsRepository.getUsageInquiryLink() } - } else { - Result.success(inquiryLink) - } - } + val inquiryDeferred = async { + inquiryLink ?: runCatching { operationsRepository.getUsageInquiryLink() }.getOrNull() + } - - privacyPolicyLink = privacyPolicyDeferred.await().getOrNull() - termsLink = termsDeferred.await().getOrNull() - inquiryLink = inquiryDeferred.await().getOrNull() + privacyPolicyLink = privacyPolicyDeferred.await() + termsLink = termsDeferred.await() + inquiryLink = inquiryDeferred.await() }Likely an incorrect or invalid review comment.
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt
Outdated
Show resolved
Hide resolved
feature/home/src/main/java/com/yapp/feature/home/setting/SettingViewModel.kt
Outdated
Show resolved
Hide resolved
feature/login/src/main/java/com/yapp/feature/login/LoginViewModel.kt
Outdated
Show resolved
Hide resolved
url 업데이트 로직의 coroutine scope를 viewModelScope에서 coroutineScope로 변경합니다.
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드래빗 야무지네요....
고생하셨습니다!
💡 Issue
Summary by CodeRabbit
Summary by CodeRabbit
버그 수정
신규 기능
리팩터링
문서 및 리소스