-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] 로그인 화면 의존성 주입 및 아키텍처 개선 #258
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로그인 화면 및 관련 의존성 주입, 뷰모델, 컨트롤러 코드가 리팩토링되었습니다. 여러 UseCase 및 리포지토리의 명시적 주입이 제거되고, 뷰모델 내부에서 직접 리포지토리와 토큰 스토리지를 사용하도록 변경되었습니다. FetchUserAuthUseCase가 완전히 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginViewController
participant LoginViewModel
participant LoginRepository
participant UserSessionRepository
participant TokenStorage
participant ProfileRepository
User->>LoginViewController: 로그인 버튼 탭
LoginViewController->>LoginViewModel: fetchUserAuth 호출
LoginViewModel->>LoginRepository: fetchUserAuth(platform)
LoginRepository-->>LoginViewModel: Account 반환
LoginViewModel->>TokenStorage: updateToken(accessToken, refreshToken)
LoginViewModel->>UserSessionRepository: updateUserSession(account)
LoginViewModel-->>LoginViewController: 로그인 성공/실패 알림
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 변경사항에는 linked issue의 범위를 벗어난 기능적 코드 변경이 발견되지 않았습니다.) 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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 (
|
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: 1
🔭 Outside diff range comments (1)
Wable-iOS/Presentation/Login/LoginViewModel.swift (1)
113-135: 비동기 프로필 업데이트 로직의 오류 처리 개선 고려
updateUserProfile메서드에서try await를 사용하지만 해당 오류가 catch되지 않아 Task가 실패할 수 있습니다.다음과 같이 수정하여 비동기 오류를 적절히 처리하는 것을 권장합니다:
func updateUserProfile(userID: Int, cancelBag: CancelBag) { Task { - let authorizedStatus = await UNUserNotificationCenter.current().notificationSettings().authorizationStatus - let isAuthorized = authorizedStatus == .authorized - let profile = try await profileRepository.fetchUserProfile(memberID: userID) + do { + let authorizedStatus = await UNUserNotificationCenter.current().notificationSettings().authorizationStatus + let isAuthorized = authorizedStatus == .authorized + let profile = try await profileRepository.fetchUserProfile(memberID: userID) - profileRepository.updateUserProfile( + profileRepository.updateUserProfile( profile: profile, isPushAlarmAllowed: isAuthorized, isAlarmAllowed: nil, image: nil, fcmToken: nil, defaultProfileType: nil ) .catch { error -> AnyPublisher<Void, Never> in self.loginErrorSubject.send(error) return .just(()) } .sink(receiveValue: {}) .store(in: cancelBag) + } catch { + let wableError = ErrorMapper.map(error) + self.loginErrorSubject.send(wableError) + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Wable-iOS.xcodeproj/project.pbxproj(4 hunks)Wable-iOS/App/AppDelegate+InjectDependency.swift(1 hunks)Wable-iOS/App/SceneDelegate.swift(1 hunks)Wable-iOS/Data/RepositoryImpl/ProfileRepositoryImpl.swift(1 hunks)Wable-iOS/Domain/UseCase/Login/FetchUserAuthUseCase.swift(0 hunks)Wable-iOS/Presentation/Login/LoginViewController.swift(4 hunks)Wable-iOS/Presentation/Login/LoginViewModel.swift(3 hunks)Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift(1 hunks)Wable-iOS/Presentation/Splash/SplashViewController.swift(2 hunks)
💤 Files with no reviewable changes (1)
- Wable-iOS/Domain/UseCase/Login/FetchUserAuthUseCase.swift
🧰 Additional context used
🧬 Code Graph Analysis (1)
Wable-iOS/App/AppDelegate+InjectDependency.swift (1)
Wable-iOS/Core/DI/DIContainer.swift (2)
register(35-37)register(39-41)
🔇 Additional comments (17)
Wable-iOS/Presentation/Splash/SplashViewController.swift (3)
21-21: 코드 간소화 잘 적용됨completion handler를 한 줄로 단순화하여 가독성이 개선되었습니다.
25-25: 주석 개선 잘 적용됨주석이 더 정확하고 구체적으로 개선되었습니다.
Also applies to: 35-35
43-43: 논리적 그룹핑 개선addSubview 호출을 setupConstraint() 메서드로 이동하여 관련 코드를 한곳에 모으는 것이 좋은 개선입니다.
Wable-iOS/App/AppDelegate+InjectDependency.swift (1)
15-26: 의존성 주입 등록이 올바르게 구현됨새로운 의존성들이 적절한 구현체와 함께 등록되었고, 기존 패턴과 일관성을 유지하고 있습니다. 로그인 관련 아키텍처 리팩토링을 잘 지원하는 변경입니다.
Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift (1)
102-102: 의존성 주입 단순화가 올바르게 적용됨LoginViewModel 생성이 크게 단순화되어 코드 복잡성이 감소했습니다. DI Container를 통한 의존성 주입 패턴으로 전환된 것이 PR 목적과 일치합니다.
Wable-iOS/App/SceneDelegate.swift (1)
63-63: 앱 시작 시 로그인 화면 설정 단순화 완료SceneDelegate에서도 LoginViewModel 생성이 일관되게 단순화되어 전체 앱에서 통일된 의존성 주입 패턴을 사용하고 있습니다.
Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift (1)
468-468: 로그아웃 플로우에서도 일관된 단순화 적용프로필 화면의 로그아웃 시에도 동일한 패턴으로 LoginViewController 생성이 단순화되어 전체 앱에서 일관성이 유지되고 있습니다.
Wable-iOS.xcodeproj/project.pbxproj (3)
146-146: UpdateFCMTokenUseCase 빌드 파일 추가가 적절합니다.
UpdateFCMTokenUseCase.swift가 프로젝트 빌드에 포함되도록 설정되었습니다. 이는 기존 Login 그룹에서 Onboarding 그룹으로 이동한 파일이 여전히 빌드 대상에 포함되어 있음을 의미합니다.
523-523: 파일 참조가 올바르게 유지되었습니다.
UpdateFCMTokenUseCase.swift파일의 참조가 적절하게 설정되어 있으며, 파일이 프로젝트에서 인식되도록 되어 있습니다.
1513-1513: UseCase 그룹 구조 개선이 완료되었습니다.
UpdateFCMTokenUseCase.swift가Onboarding그룹 하위로 이동되었습니다. 이는 PR 목표에서 언급한 "Login 그룹에서 Onboarding 그룹으로 이동"과 일치하며, 기능적으로 더 적절한 위치로 보입니다. FCM 토큰 업데이트는 로그인보다는 온보딩 과정의 일부로 분류하는 것이 합리적입니다.Wable-iOS/Data/RepositoryImpl/ProfileRepositoryImpl.swift (1)
91-98: 기존 메서드 오버로드로 하위 호환성 유지 확인됨새로운
updateUserProfile(profile:isPushAlarmAllowed:isAlarmAllowed:image:fcmToken:defaultProfileType:)메서드를 추가하면서
기존의updateUserProfile(nickname:fcmToken:)오버로드도 여전히 남아 있어
SceneDelegate, UpdateFCMTokenUseCase 등 기존 호출부가 모두 정상 작동합니다.
따라서 호출 지점을 일괄 수정할 필요 없이 API 변경에 따른 문제가 발생하지 않습니다.Likely an incorrect or invalid review comment.
Wable-iOS/Presentation/Login/LoginViewModel.swift (2)
20-23: 의존성 주입을 통한 깔끔한 아키텍처 개선
@Injected속성을 사용한 의존성 주입 방식으로 변경하고 UseCase 레이어를 제거한 것은 불필요한 추상화를 줄이고 코드를 단순화하는 좋은 개선입니다.
77-99: 토큰 및 사용자 세션 관리 로직 개선새로 추가된
updateToken과updateUserSession헬퍼 메서드는 관심사를 잘 분리하고 코드 가독성을 높였습니다. 토큰 저장 실패 시 오류 처리도 적절히 구현되어 있습니다.Wable-iOS/Presentation/Login/LoginViewController.swift (4)
34-39: UI 컴포넌트 속성 설정 순서 정리
titleLabel의 속성 설정 순서를 일관성 있게 정리한 것이 코드 가독성을 향상시켰습니다.
69-72: 초기화 메서드 개선
@available(*, unavailable)속성을 사용하여 지원하지 않는 초기화 메서드를 명시적으로 표시한 것이 더 명확한 API 설계입니다.
84-127: 설정 메서드 통합으로 코드 구조 개선
setupView()와setupConstraint()메서드를 하나로 통합하고 뷰 추가와 제약 조건 설정을 함께 처리하도록 변경한 것이 코드 구조를 단순화하고 유지보수성을 향상시켰습니다.
175-201: 헬퍼 메서드 구조 정리내비게이션 관련 헬퍼 메서드들을 별도 extension으로 분리하여 코드 구조를 더욱 명확하게 정리했습니다.
JinUng41
left a comment
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.
DI를 미리 등록해두니, 코드가 확실히 깔끔해진 것 같아요.
PR에서 체크리스트가 차지하는 부분이 과도하게 많은 것 같아요.
해당 부분은 토글 형태로 구현해도 좋을 것 같습니다.
아래와 같은 형태는 어떨까요?
✅ 체크리스트
잠깐 확인하고 갈까요?
-
들여쓰기를 5번 이하로 준수했는지, 코드 가독성이 적절한지 확인해주세요.
-
한 줄당 120자 제한을 준수했는지 확인해주세요.
-
MARK 주석이 정해진 순서와 형식에 맞게 작성되었는지 확인해주세요.
-
반복되는 상수 값이 있는지, 있다면 Constant enum으로 분리되어 있는지 확인해주세요.
-
삼항 연산자가 길어질 경우 적절히 개행되어 있는지 확인해주세요.
-
조건문에서 중괄호가 올바르게 사용되었는지 확인해주세요.
-
라이브러리 import가 퍼스트파티와 서드파티로 구분되고 알파벳순으로 정렬되었는지 확인해주세요.
-
용량이 큰 리소스나 호출되지 않을 가능성이 있는 프로퍼티에 lazy var가 적절히 사용되었는지 확인해주세요.
-
메모리 누수 방지를 위한 weak 참조가 필요한 곳에 적용되었는지 확인해주세요.
-
도메인 로직과 UI 로직이 적절히 분리되어 있는지 확인해주세요.
토글 형태로 체크리스트를 넣으면 나중에는 귀찮아서 보지 않는(ㅠㅠ) 이슈가 생길 것 같아 따로 토글로 넣지는 않았습니다. ✅ 이번 PR에서 이런 부분을 중점적으로 체크해주세요!
잠깐 확인하고 갈까요?
아래와 같이 사용 가능합니다. ✅ 이번 PR에서 이런 부분을 중점적으로 체크해주세요!
잠깐 확인하고 갈까요?
|
넵 확인했습니당 |
[Refactor] 로그인 화면 의존성 주입 및 아키텍처 개선
👻 PULL REQUEST
📄 작업 내용
DIContainer에 필요한 의존성을 등록하고LoginViewModel에 의존성을 주입함으로써 불필요한 생성자 호출을 방지했어요.Repository<-UseCase<-ViewModel순으로 호출되던 로직 중UseCase를 제거하고ViewModel에서 바로Repository를 선언해 사용할 수 있도록 코드를 수정했어요.UseCase가 단순히Repository를 선언하고 호출하는 역할에 그침에 따라 불필요한 로직을 없애 코드와 성능을 개선하고자 했어요.✅ 이번 PR에서 이런 부분을 중점적으로 체크해주세요!
잠깐 확인하고 갈까요?
들여쓰기를 5번 이하로 준수했는지, 코드 가독성이 적절한지 확인해주세요.
한 줄당 120자 제한을 준수했는지 확인해주세요.
MARK 주석이 정해진 순서와 형식에 맞게 작성되었는지 확인해주세요.
반복되는 상수 값이 있는지, 있다면 Constant enum으로 분리되어 있는지 확인해주세요.
삼항 연산자가 길어질 경우 적절히 개행되어 있는지 확인해주세요.
조건문에서 중괄호가 올바르게 사용되었는지 확인해주세요.
라이브러리 import가 퍼스트파티와 서드파티로 구분되고 알파벳순으로 정렬되었는지 확인해주세요.
용량이 큰 리소스나 호출되지 않을 가능성이 있는 프로퍼티에 lazy var가 적절히 사용되었는지 확인해주세요.
메모리 누수 방지를 위한 weak 참조가 필요한 곳에 적용되었는지 확인해주세요.
도메인 로직과 UI 로직이 적절히 분리되어 있는지 확인해주세요.
🔗 연결된 이슈
Summary by CodeRabbit
버그 수정
리팩터
스타일
기타