[Refactor] 온보딩 및 프로필 편집 화면 코디네이터 패턴 적용, 코드 구조 및 의존성 개선 #282
Conversation
- AgreementViewController ViewModel 분리 및 Input/Output 패턴 적용 - ProfileRegisterViewController OnboardingProfileInfo 사용 - LCKTeamViewController OnboardingProfileInfo 사용 - OnboardingCoordinator 네비게이션 로직 개선 - LoginViewController 오타 수정
- ProfileEditViewModel 생성 (Input/Output 패턴) - ViewController 상태(lckTeam, sessionProfile, profileImageType) ViewModel로 이동 - ProfileEditView를 WableComponent로 이동 - 기존 ProfileEditView 삭제 - ProfileEditViewController에 userID 의존성 주입 방식으로 변경 - MyProfileViewModel userID를 internal로 변경 - MyProfileViewController에서 ProfileEditViewController에 userID 전달
Walkthrough온보딩을 코디네이터 + MVVM으로 재구성하고 프로필 편집에 MVVM 및 PhotoPickerHelper를 도입했으며, 여러 이미지 로딩 경로를 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant LoginVC as LoginViewController
participant LoginCoord as LoginCoordinator
participant Nav as UINavigationController
participant OnbCoord as OnboardingCoordinator
participant YearVC as LCKYearVC
participant TeamVC as LCKTeamVC
participant RegVC as ProfileRegisterVC
participant AgrVC as AgreementVC
participant Root as AppWindow
User->>LoginVC: 로그인 성공
LoginVC->>LoginCoord: navigateToOnboarding()
LoginCoord->>Nav: create full-screen UINavigationController
LoginCoord->>OnbCoord: init(nav) / start()
OnbCoord->>Nav: push LCKYearVC
User->>YearVC: 연도 선택 → next
YearVC-->>OnbCoord: navigateToLCKTeam(year)
OnbCoord->>Nav: push LCKTeamVC
User->>TeamVC: 팀 선택/skip
TeamVC-->>OnbCoord: navigateToProfileRegister(profileInfo)
OnbCoord->>Nav: push ProfileRegisterVC
User->>RegVC: 닉네임/이미지 설정 → next
RegVC-->>OnbCoord: navigateToAgreement(profileInfo)
OnbCoord->>Nav: push AgreementVC
User->>AgrVC: 약관 동의 → 완료
AgrVC-->>OnbCoord: navigateToHome()
OnbCoord->>Root: set root to TabBarController (cross-dissolve)
sequenceDiagram
autonumber
actor User
participant VC as AnyViewController
participant Helper as PhotoPickerHelper
participant PH as PHPicker
participant UI as UIApplication
User->>VC: 이미지 추가 버튼 탭
VC->>Helper: presentPhotoPicker(onImageSelected:)
Helper->>Helper: requestPhotoLibraryAccess() (async)
alt 권한 허용
Helper->>PH: present PHPicker
PH-->>Helper: didFinishPicking(results)
Helper->>Helper: loadImage(from: provider)
Helper-->>VC: onImageSelected(image)
else 권한 거부/제한
Helper->>VC: showSettingsAlert(...)
User->>UI: 설정 열기 선택
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Wable-iOS/Presentation/Login/LoginViewController.swift (1)
152-159: 기존 회원이 홈으로 이동하지 못합니다.
condition이 거짓인 경우에도navigateToOnboarding만 호출되어, 기존 회원이 홈 화면으로 갈 수 없습니다. 필히navigateToHome호출로 복구해야 합니다.다음과 같이 수정해주세요:
- condition ? owner.navigateToOnboarding?() : owner.navigateToOnboarding?() + condition ? owner.navigateToOnboarding?() : owner.navigateToHome?()Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift (1)
181-187: 이미지 로딩 방식이 일관되지 않습니다.
configureView에서는 Kingfisher를 직접 사용하지만(line 187),configureProfileView에서는 새로운setProfileImage확장 메서드를 사용합니다(line 206). 두 메서드 모두 동일한 확장 메서드를 사용하도록 통일해야 합니다.다음과 같이 수정하세요:
guard let profileImageURL = profileImageURL else { configureDefaultImage() return } - profileImageView.kf.setImage(with: profileImageURL) + profileImageView.setProfileImage(with: profileImageURL)
🧹 Nitpick comments (10)
Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift (1)
74-144: 스타일 갱신 로직을configureCellStyle로 일원화해주세요.두 메서드에서 테두리·텍스트 색상을 직접 건드리다 보니 helper와 로직이 벌어져 유지보수 부담이 큽니다. 동일 조건을 수정할 때 세 군데를 모두 바꿔야 하고, SwiftLint 경고(.indices 권장)도 함께 정리되지 않습니다.
configureCellStyle(_:isSelected:)을 재사용하면 중복 없이 한 지점에서만 스타일을 관리할 수 있습니다.- for (index, _) in randomTeamList.enumerated() { + for index in randomTeamList.indices { guard let cell = cellForItem(at: IndexPath(row: index, section: 0)) as? LCKTeamCollectionViewCell else { continue } - cell.layer.borderColor = UIColor.gray300.cgColor - cell.teamLabel.textColor = .gray700 + configureCellStyle(cell, isSelected: false) } @@ - guard let cell = cellForItem(at: selectedIndexPath) as? LCKTeamCollectionViewCell else { return } - cell.layer.borderColor = UIColor.purple50.cgColor - cell.teamLabel.textColor = .wableBlack + guard let cell = cellForItem(at: selectedIndexPath) as? LCKTeamCollectionViewCell else { return } + configureCellStyle(cell, isSelected: true)
updateAllCellsStyle도 동일하게 helper를 호출하도록 정리 부탁드립니다.Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
41-56: 헬퍼 메서드 구현이 적절합니다
random()메서드는 안전한 기본값을 제공하고,from(uppercased:)메서드는 서버 응답 매핑을 명확하게 처리합니다.
from(uppercased:)메서드를 더 간결하게 만들 수 있습니다:static func from(uppercased: String) -> DefaultProfileType? { - switch uppercased { - case "GREEN": - return .green - case "BLUE": - return .blue - case "PURPLE": - return .purple - default: - return nil - } + return allCases.first { $0.uppercased == uppercased } }이렇게 하면 새로운 케이스 추가 시
from(uppercased:)메서드를 수정할 필요가 없습니다.Wable-iOS/Presentation/Enum/NicknameValidationResult.swift (1)
10-14: 깔끔한 검증 결과 타입입니다.닉네임 검증 상태를 명확하게 표현하는 간결한 enum입니다.
향후 확장을 고려한다면 다음과 같은 케이스 추가를 검토할 수 있습니다:
tooShort/tooLong: 길이 제약 위반duplicate: 중복 닉네임containsInvalidCharacters: 특수문자 포함하지만 현재 요구사항에 충분하다면 필수는 아닙니다.
Wable-iOS/Presentation/WableComponent/View/PostUserInfoView.swift (1)
200-200: 중앙화된 이미지 로딩으로 잘 리팩토링되었습니다.복잡한 URL 처리 로직을 제거하고 단일 헬퍼 호출로 대체하여 코드를 단순화했습니다.
다른 셀(예:
NotificationCell)처럼 명시적인placeholder를 전달하는 것을 고려해볼 수 있습니다. 현재는 랜덤 기본 프로필이 사용되는데, 일관성을 위해 특정 placeholder를 지정하는 것도 방법입니다:profileImageView.setProfileImage(with: userProfileURL, placeholder: UIImage(resource: .imgProfileDefault))하지만 현재 동작이 의도된 것이라면 필수는 아닙니다.
Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift (3)
57-57: 버튼 이름이 덜 명확해졌습니다.
duplicationCheckButton을checkButton으로 변경했는데, 새 이름이 버튼의 목적(닉네임 중복 확인)을 명확히 드러내지 못합니다. 원래 이름이 더 명시적이었습니다.더 명확한 이름으로 되돌리는 것을 고려해보세요:
-let checkButton: UIButton = UIButton(configuration: .filled()).then { +let duplicationCheckButton: UIButton = UIButton(configuration: .filled()).then { $0.configuration?.attributedTitle = "중복확인".pretendardString(with: .body3)
177-188: 이미지 로딩 방식의 일관성이 필요합니다.
configureProfileView(line 206)에서는setProfileImage(with:)헬퍼를 사용하지만,configureView(line 187)에서는 직접kf.setImage를 사용합니다. 일관성을 위해 두 곳 모두 헬퍼 메서드를 사용하는 것이 좋습니다.
configureView도 헬퍼 메서드를 사용하도록 수정하세요:func configureView(profileImageURL: URL? = .none) { titleLabel.text = StringLiterals.ProfileSetting.registerTitle descriptionLabel.isHidden = false - guard let profileImageURL = profileImageURL else { - configureDefaultImage() - - return - } - - profileImageView.kf.setImage(with: profileImageURL) + profileImageView.setProfileImage(with: profileImageURL) }참고:
setProfileImage가 이미 nil을 처리하므로configureDefaultImage()호출도 불필요합니다.Also applies to: 206-206
177-188: 이미지 로딩 방식의 일관성이 필요합니다.
configureView메서드(187줄)에서는 여전히kf.setImage를 직접 사용하는 반면,configureProfileView메서드(206줄)에서는 새로운setProfileImage헬퍼를 사용합니다. 일관성을 위해 동일한 헬퍼를 사용하는 것이 좋습니다.187줄도 새로운 헬퍼를 사용하도록 통일하세요:
- profileImageView.kf.setImage(with: profileImageURL) + profileImageView.setProfileImage(with: profileImageURL)Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (2)
18-19: URL과 enum case를 구분하는 명확성이 필요합니다.
url.absoluteString을DefaultProfileType.from(uppercased:)에 전달하는 것은 URL이 실제로는 "GREEN", "BLUE" 같은 enum case 문자열이라고 가정합니다. 이는 URL 타입의 의미론적 사용과 맞지 않아 혼란을 줄 수 있습니다.
ProfileImageTypeenum을 사용하여 remote URL과 local default를 명확히 구분하는 것을 고려해보세요:enum ProfileImageSource { case remote(URL) case defaultType(DefaultProfileType) } extension UIImageView { func setProfileImage(source: ProfileImageSource?, placeholder: UIImage? = nil) { switch source { case .remote(let url): self.kf.setImage(with: url, placeholder: placeholder ?? DefaultProfileType.random().image) case .defaultType(let type): self.image = type.image case .none: self.image = placeholder ?? DefaultProfileType.random().image } } }
14-14: 랜덤 이미지 일관성을 고려하세요.
DefaultProfileType.random()이 여러 번 호출되면 매번 다른 이미지가 생성될 수 있습니다. nil URL과 placeholder nil일 때(line 14)와 Kingfisher placeholder로 사용될 때(line 23) 다른 이미지가 표시될 수 있습니다.일관성을 위해 한 번만 생성하는 것을 고려하세요:
func setProfileImage(with url: URL?, placeholder: UIImage? = nil) { let fallbackImage = placeholder ?? DefaultProfileType.random().image guard let url else { self.image = fallbackImage return } if let defaultType = DefaultProfileType.from(uppercased: url.absoluteString) { self.image = defaultType.image } else { self.kf.setImage(with: url, placeholder: fallbackImage) } }Also applies to: 23-23
Wable-iOS/Presentation/Onboarding/ViewController/LCKYearViewController.swift (1)
88-90: Amplitude 이벤트 추적 순서를 확인하세요.이벤트 추적(
trackEvent)이 네비게이션 콜백 호출 전에 위치합니다. 만약 콜백이 실패하거나 예외가 발생하면 실제 네비게이션이 일어나지 않았는데도 이벤트가 기록됩니다. 일반적으로 이벤트는 작업 성공 후 또는 콜백 내부에서 추적하는 것이 정확합니다.네비게이션 콜백 후 이벤트를 추적하거나, 콜백이 비동기가 아니라면 현재 순서를 유지하되 문서화하세요:
- AmplitudeManager.shared.trackEvent(tag: .clickNextYearSignup) navigateToLCKTeam?(selectedYear) + AmplitudeManager.shared.trackEvent(tag: .clickNextYearSignup)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
Wable-iOS.xcodeproj/project.pbxproj(22 hunks)Wable-iOS/App/AppDelegate+InjectDependency.swift(1 hunks)Wable-iOS/Core/Photo/PhotoPickerHelper.swift(1 hunks)Wable-iOS/Domain/UseCase/Onboarding/FetchNicknameDuplicationUseCase.swift(0 hunks)Wable-iOS/Domain/UseCase/Onboarding/UpdateFCMTokenUseCase.swift(0 hunks)Wable-iOS/Domain/UseCase/Onboarding/UserProfileUseCase.swift(0 hunks)Wable-iOS/Presentation/Enum/DefaultProfileType.swift(2 hunks)Wable-iOS/Presentation/Enum/NicknameValidationResult.swift(1 hunks)Wable-iOS/Presentation/Enum/ProfileImageType.swift(1 hunks)Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift(1 hunks)Wable-iOS/Presentation/Home/View/WritePostViewController.swift(2 hunks)Wable-iOS/Presentation/Login/LoginCoordinator.swift(2 hunks)Wable-iOS/Presentation/Login/LoginViewController.swift(2 hunks)Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift(1 hunks)Wable-iOS/Presentation/Onboarding/Model/OnboardingProfileInfo.swift(1 hunks)Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift(6 hunks)Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift(5 hunks)Wable-iOS/Presentation/Onboarding/ViewController/LCKTeamViewController.swift(1 hunks)Wable-iOS/Presentation/Onboarding/ViewController/LCKYearViewController.swift(3 hunks)Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift(1 hunks)Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift(1 hunks)Wable-iOS/Presentation/Onboarding/ViewModel/ProfileRegisterViewModel.swift(1 hunks)Wable-iOS/Presentation/Profile/Component/ProfileInfoCell.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift(6 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift(1 hunks)Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift(1 hunks)Wable-iOS/Presentation/Viewit/List/View/Cell/ViewitListCell.swift(1 hunks)Wable-iOS/Presentation/WableComponent/Cell/NotificationCell.swift(1 hunks)Wable-iOS/Presentation/WableComponent/View/PostUserInfoView.swift(1 hunks)Wable-iOS/Presentation/WableComponent/View/ProfileEditView.swift(4 hunks)Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift(4 hunks)Wable-iOS/Presentation/WableComponent/ViewController/PhotoDetailViewController.swift(2 hunks)
💤 Files with no reviewable changes (3)
- Wable-iOS/Domain/UseCase/Onboarding/FetchNicknameDuplicationUseCase.swift
- Wable-iOS/Domain/UseCase/Onboarding/UpdateFCMTokenUseCase.swift
- Wable-iOS/Domain/UseCase/Onboarding/UserProfileUseCase.swift
👮 Files not reviewed due to content moderation or server errors (5)
- Wable-iOS/Presentation/Login/LoginCoordinator.swift
- Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift
- Wable-iOS/Presentation/Onboarding/ViewController/LCKYearViewController.swift
- Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift
- Wable-iOS/Presentation/WableComponent/View/ProfileEditView.swift
🧰 Additional context used
🧬 Code graph analysis (20)
Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
Wable-iOS/Presentation/Enum/DefaultProfileType.swift (2)
random(41-43)from(45-56)
Wable-iOS/App/AppDelegate+InjectDependency.swift (1)
Wable-iOS/Core/DI/DIContainer.swift (2)
register(35-37)register(39-41)
Wable-iOS/Presentation/WableComponent/ViewController/PhotoDetailViewController.swift (1)
Wable-iOS/Core/Photo/PhotoPickerHelper.swift (2)
requestPhotoLibraryAccess(77-94)requestPhotoLibraryAccess(96-110)
Wable-iOS/Presentation/Home/View/WritePostViewController.swift (1)
Wable-iOS/Core/Photo/PhotoPickerHelper.swift (1)
presentPhotoPicker(26-42)
Wable-iOS/Presentation/Onboarding/ViewController/LCKYearViewController.swift (4)
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift (1)
setupConstraints(52-59)Wable-iOS/Presentation/Onboarding/ViewController/LCKTeamViewController.swift (1)
setupConstraints(75-111)Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift (1)
setupConstraints(61-68)Wable-iOS/Core/Amplitude/AmplitudeManager.swift (1)
trackEvent(21-23)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (4)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (1)
viewWillAppear(59-64)Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Core/Logger/WableLogger.swift (2)
error(45-52)log(14-25)Wable-iOS/Data/RepositoryImpl/ProfileRepositoryImpl.swift (1)
fetchFCMToken(44-50)
Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift (2)
Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
random(41-43)Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
setProfileImage(12-26)
Wable-iOS/Presentation/Login/LoginCoordinator.swift (1)
Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift (2)
start(18-20)showHome(63-79)
Wable-iOS/Presentation/Viewit/List/View/Cell/ViewitListCell.swift (1)
Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
setProfileImage(12-26)
Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift (4)
Wable-iOS/Presentation/Onboarding/ViewController/LCKTeamViewController.swift (1)
nextButtonDidTap(133-141)Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift (2)
setupView(103-115)configureDefaultImage(91-94)Wable-iOS/Presentation/Onboarding/ViewModel/ProfileRegisterViewModel.swift (2)
transform(41-65)getProfileInfo(71-77)Wable-iOS/Core/Photo/PhotoPickerHelper.swift (1)
presentPhotoPicker(26-42)
Wable-iOS/Presentation/Onboarding/ViewModel/ProfileRegisterViewModel.swift (1)
Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)
Wable-iOS/Presentation/WableComponent/View/ProfileEditView.swift (2)
Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
random(41-43)Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
setProfileImage(12-26)
Wable-iOS/Presentation/WableComponent/Cell/NotificationCell.swift (1)
Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
setProfileImage(12-26)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (4)
Wable-iOS/Presentation/WableComponent/View/ProfileEditView.swift (3)
setupView(114-127)configureView(194-198)configureDefaultImage(102-105)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (1)
transform(55-116)Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Core/Photo/PhotoPickerHelper.swift (1)
presentPhotoPicker(26-42)
Wable-iOS/Presentation/Profile/Component/ProfileInfoCell.swift (1)
Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
setProfileImage(12-26)
Wable-iOS/Presentation/WableComponent/View/PostUserInfoView.swift (1)
Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (1)
setProfileImage(12-26)
Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift (2)
Wable-iOS/Presentation/Login/LoginCoordinator.swift (2)
start(19-32)showHome(59-63)Wable-iOS/Core/Logger/WableLogger.swift (2)
log(14-25)debug(36-43)
Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (5)
Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Data/RepositoryImpl/UserSessionRepositoryImpl.swift (1)
fetchActiveUserSession(39-44)Wable-iOS/Data/RepositoryImpl/ProfileRepositoryImpl.swift (1)
fetchFCMToken(44-50)Wable-iOS/Core/Logger/WableLogger.swift (3)
error(45-52)log(14-25)debug(36-43)Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
random(41-43)
Wable-iOS/Presentation/Onboarding/ViewController/LCKTeamViewController.swift (6)
Wable-iOS/Presentation/WableComponent/Button/WableButton.swift (1)
updateStyle(51-53)Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift (1)
nextButtonDidTap(135-141)Wable-iOS/Presentation/Onboarding/ViewController/LCKYearViewController.swift (1)
nextButtonDidTap(84-90)Wable-iOS/Presentation/Helper/Extension/UIView+.swift (1)
addSubviews(32-36)Wable-iOS/Presentation/Helper/Extension/ConstraintMaker+.swift (1)
adjustedHeightEqualTo(45-48)Wable-iOS/Core/Amplitude/AmplitudeManager.swift (1)
trackEvent(21-23)
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift (2)
Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (1)
getWelcomeMessage(60-62)
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift
[Warning] 74-74: When the item is not used, .indices should be used instead of .enumerated()
(unused_enumerated)
Wable-iOS/Presentation/Login/LoginViewController.swift
[Warning] 158-158: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🔇 Additional comments (47)
Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift (1)
22-22: 변경 사항 승인: userID 프로퍼티 노출이 필요합니다.
MyProfileViewController(203)에서 viewModel.userID를 참조하며 ProfileEditViewModel(32)로 전달되고 있어private→internal 변경이 적절합니다.Wable-iOS/App/AppDelegate+InjectDependency.swift (1)
22-25: LGTM!AccountRepository 등록이 기존 DI 패턴과 일관되게 추가되었습니다. UserSession과 Login 사이에 배치된 것도 논리적입니다.
Wable-iOS/Presentation/WableComponent/ViewController/PhotoDetailViewController.swift (1)
113-128: LGTM!사진 라이브러리 접근 권한 체크 로직을
PhotoPickerHelper로 중앙화한 것은 좋은 리팩토링입니다. async/await 패턴이 적절히 사용되었고, 에러 처리도 유지되고 있습니다.Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
26-35: LGTM!기본 프로필 이미지 매핑을 enum에 중앙화한 것은 좋은 접근입니다. 각 케이스에 대한 이미지 매핑이 명확합니다.
Wable-iOS/Core/Photo/PhotoPickerHelper.swift (5)
26-42: LGTM!사진 선택 흐름이 명확하고 논리적입니다. 권한 체크 후 적절히 피커를 표시하거나 설정 알림을 보여줍니다.
47-71: LGTM!PHPicker 설정과 이미지 로딩 로직이 적절합니다. 메인 스레드에서 콜백을 실행하는 것도 올바릅니다.
112-134: LGTM!설정 페이지로 이동하는 로직과 알림이 적절하게 구현되었습니다.
139-146: LGTM!PHPickerViewController 델리게이트 구현이 표준 패턴을 따르고 있습니다.
12-24: onImageSelected 클로저 안전성 확인됨모든
presentPhotoPicker호출부에서[weak self]를 사용해 순환 참조를 방지하고 있으므로, 추가 작업 없이 현 상태로 유지해도 됩니다.Wable-iOS/Presentation/Home/View/WritePostViewController.swift (2)
20-20: lazy var 사용이 적절합니다
photoPickerHelper를 lazy var로 선언한 것은 좋은 선택입니다. 사진 첨부는 선택적 기능이므로 사용되지 않을 경우 초기화를 지연시키는 것이 효율적입니다.PR 체크리스트에서 요청한 대로, lazy var 사용이 적절한 경우입니다.
227-239: LGTM!
PhotoPickerHelper를 사용하도록 리팩토링되었고, 콜백에서[weak self]를 사용하여 메모리 누수를 방지하고 있습니다. UI 업데이트 로직도 명확합니다.PR 체크리스트에서 요청한 약한 참조 사용이 올바르게 적용되었습니다.
Wable-iOS/Presentation/Onboarding/Model/OnboardingProfileInfo.swift (1)
10-15: 구조체 추가 잘 확인했어요.온보딩 단계별 상태를 한 번에 넘길 수 있어서 이후 흐름 관리가 깔끔해졌습니다.
Wable-iOS/Presentation/Enum/ProfileImageType.swift (1)
10-13: 프로필 이미지 타입 추상화가 적절합니다.커스텀 이미지와 기본 이미지를 명확히 구분해 이후 분기 처리가 쉬워졌어요.
Wable-iOS/Presentation/Profile/Component/ProfileInfoCell.swift (1)
142-143: 이미지 세팅 로직 일원화 👍공용
setProfileImage확장으로 분기 로직이 정리되어 유지보수가 쉬워졌습니다.Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift (1)
203-207: 명시적 데이터 전달로 개선되었습니다.
userID를 명시적으로 전달하는 패턴은 MVVM 리팩토링 목표와 일치하며 타입 안전성을 향상시킵니다.다만,
userID가nil인 경우 조용히 반환하면 내비게이션 실패를 사용자가 알 수 없습니다. 다음을 확인해주세요:
- 정상적인 흐름에서
userID가nil일 수 있는지 검증nil인 경우 로깅이나 사용자 알림 추가 고려guard let userID = self?.viewModel.userID else { WableLogger.log("userID is nil, cannot navigate to profile edit", for: .debug) return }Wable-iOS/Presentation/WableComponent/Cell/NotificationCell.swift (1)
78-78: LGTM! 중앙화된 이미지 로딩 헬퍼를 올바르게 활용했습니다.명시적인 placeholder를 전달하여 일관된 폴백 동작을 보장하고, 이전의 로컬 헬퍼 메서드(
configureDefaultProfileImage)를 제거하여 코드 중복을 제거했습니다.Wable-iOS/Presentation/Helper/Extension/UIImageView+.swift (4)
12-26: 이미지 로딩 로직이 잘 중앙화되었습니다.URL 기반 프로필 이미지 로딩을 단일 확장 메서드로 통합하여 코드 중복을 효과적으로 제거했습니다. Kingfisher와의 통합도 적절합니다.
18-19: 스크립트 실행 후 결과를 알려주세요.
12-26: 프로필 이미지 로딩 로직을 잘 중앙화했습니다.여러 곳에서 중복되던 이미지 로딩 코드를 단일 확장 메서드로 통합한 것은 좋은 리팩토링입니다.
12-26: 직접 Kingfisher 호출 제거 및 중앙 이미지 헬퍼 적용 검증
Onboarding/ProfileRegisterView.swift 등에서profileImageView.kf.setImage(…)호출을setProfileImage(…)로 대체하고,DefaultProfileType.from(uppercased:)가 전체 URL과 “GREEN”, “BLUE” 같은 키워드 모두 올바르게 매핑하는지 확인하세요.Wable-iOS/Presentation/Onboarding/ViewController/LCKYearViewController.swift (7)
19-19: 코디네이터 패턴을 위한 네비게이션 콜백이 적절히 추가되었습니다.
navigateToLCKTeam콜백을 통해 화면 간 네비게이션 책임을 코디네이터로 위임하는 구조가 올바르게 구현되었습니다.
40-40: 메서드명 일관성 개선이 좋습니다.
setupConstraint를setupConstraints로 변경하여 다른 뷰 컨트롤러들과 네이밍 일관성을 맞춘 점이 좋습니다.
19-19: 코디네이터 패턴이 적절히 적용되었습니다.
navigateToLCKTeam콜백을 통해 뷰 컨트롤러와 네비게이션 로직을 분리한 것은 좋은 구현입니다.
30-30: 메서드 이름 일관성이 개선되었습니다.
setupConstraint()를setupConstraints()로 변경하여 다른 온보딩 뷰 컨트롤러들과 이름 규칙을 통일한 것이 좋습니다.Also applies to: 40-40
84-90: AmplitudeManager import 불필요
AmplitudeManager는 동일 모듈 내internal클래스로 선언되어 있어 별도import없이 사용 가능합니다.Likely an incorrect or invalid review comment.
19-19: 코디네이터 콜백 프로퍼티가 올바르게 추가되었습니다.코디네이터 패턴을 적용하여 직접 네비게이션 대신 콜백을 사용하도록 개선되었습니다. 뷰 컨트롤러가 다음 화면에 대한 지식을 갖지 않아 결합도가 낮아졌습니다.
30-30: LGTM!메서드명 오타 수정(
setupConstraint→setupConstraints)이 코드 일관성을 개선합니다.Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift (7)
91-94: 기본 프로필 이미지 로직이 잘 구현되었습니다.
DefaultProfileType.random()과.image프로퍼티를 사용하여 기본 이미지 처리를 명확하고 간결하게 구현했습니다.
17-17: 기본 이미지 관리가 개선되었습니다.
defaultImageList배열 대신currentDefaultImage단일 프로퍼티를 사용하는 것이 더 명확하고 의도를 잘 드러냅니다.private(set)을 통한 접근 제어도 적절합니다.
92-94: 랜덤 이미지 생성 로직이 명확해졌습니다.
DefaultProfileType.random()을 사용하여 직접 이미지를 설정하는 방식이 이전 shuffle 로직보다 훨씬 명확합니다.
57-57: 리뷰 코멘트 무시: ProfileRegisterView 외부에 duplicationCheckButton 참조 없음Likely an incorrect or invalid review comment.
17-17: LGTM!
private(set)을 사용하여 프로퍼티 캡슐화가 개선되었습니다. 외부에서는 읽기만 가능하고 내부에서만 수정할 수 있어 안전합니다.
57-57: LGTM!버튼 이름이
duplicationCheckButton에서checkButton으로 간소화되어 가독성이 향상되었습니다.
92-94: LGTM!
currentDefaultImage프로퍼티를 활용하여 로직이 명확해지고, 이전의 배열 기반 접근 방식보다 단순해졌습니다.Wable-iOS/Presentation/WableComponent/View/ProfileEditView.swift (12)
17-17: 현재 기본 이미지 상태 관리가 적절합니다.
currentDefaultImage프로퍼티를private(set)으로 선언하여 외부에서는 읽기만 가능하고 내부에서만 수정할 수 있도록 적절히 캡슐화했습니다.
65-65: 콜백 매개변수 이름이 개선되었습니다.
cellDidTapped를didTapped로 변경하여 더 간결하면서도 의미가 명확한 이름이 되었습니다.
102-105: 기본 이미지 설정 로직이 ProfileRegisterView와 일관성 있게 구현되었습니다.
DefaultProfileType.random()과.image를 사용하여 ProfileRegisterView와 동일한 패턴으로 기본 이미지를 처리하고 있어 코드베이스 전반의 일관성이 유지됩니다.
194-198: 중앙화된 이미지 로딩 확장 메서드를 올바르게 사용했습니다.
setProfileImage(with:)확장 메서드를 사용하여 이미지 로딩 로직을 통합하고 코드 중복을 제거했습니다.
17-17: 기본 이미지 관리 개선이 일관되게 적용되었습니다.
ProfileRegisterView와 동일한 패턴으로currentDefaultImage프로퍼티를 도입한 것이 좋습니다. 코드베이스 전반의 일관성을 유지합니다.
103-105: 랜덤 이미지 생성 로직이 명확해졌습니다.이전
defaultImageList기반 로직보다DefaultProfileType.random()을 직접 사용하는 것이 훨씬 명확하고 간단합니다.
197-197: 새로운 이미지 로딩 헬퍼를 잘 활용했습니다.
setProfileImage(with:)확장 메서드를 사용하여 이미지 로딩 로직을 중앙화한 것이 좋습니다.
65-65: TeamCollectionView API 변경 확인됨: init 파라미터명이didTapped로 변경되었으며 정의와 모든 사용처가 일치합니다.
17-17: LGTM!
private(set)을 사용한 프로퍼티 캡슐화가 적절합니다.
65-65: LGTM!
TeamCollectionView초기화 인자 이름이cellDidTapped에서didTapped로 간소화되어 명명 일관성이 개선되었습니다.
103-105: LGTM!
currentDefaultImage프로퍼티를 활용하여 기본 이미지 설정 로직이 명확하고 간결해졌습니다.
194-198: LGTM!새로운
setProfileImage헬퍼를 사용하여 이미지 로딩 로직이 중앙 집중화되었습니다. 코드 중복이 줄어들고 유지보수성이 향상되었습니다.Wable-iOS/Presentation/Login/LoginCoordinator.swift (1)
45-47: UINavigationController 설정이 적절합니다.네비게이션 바를 숨기고 fullScreen 모달로 표시하는 것은 온보딩 플로우에 적합한 구현입니다.
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift
Show resolved
Hide resolved
Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift
Show resolved
Hide resolved
Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift
Outdated
Show resolved
Hide resolved
Wable-iOS/Presentation/Onboarding/ViewModel/ProfileRegisterViewModel.swift
Show resolved
Hide resolved
- viewWillAppear에서 텍스트필드를 nil로 비우는 로직 제거 - profileLoaded 바인딩에서 기존 닉네임을 텍스트필드에 주입 - 프로필 편집 화면 진입 시 현재 닉네임이 정상적으로 표시되도록 개선
- PHPickerViewController는 사진 선택 시 권한 불필요 - presentPhotoPicker에서 requestPhotoLibraryAccess 호출 제거 - 사진 저장 기능(PhotoDetailViewController)은 기존 권한 요청 유지
- Coordinator 프로토콜에 onFinish 콜백 추가 - childDidFinish 메서드 추가하여 자식 Coordinator 정리 - OnboardingCoordinator 완료 시 childCoordinators에서 제거 - LoginCoordinator에 onFinish 프로퍼티 추가 - 일관된 Coordinator 생명주기 관리 패턴 적용
- 회원가입 완료 시 업데이트된 닉네임과 푸시 알림 설정이 세션에 반영되도록 수정 - saveUserSession에서 기존 userSession 대신 업데이트된 값 전달 - nickname: profileInfo.nickname 사용 - isPushAlarmAllowed: isMarketingAgreed 사용 - notificationBadgeCount 옵셔널 처리
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Wable-iOS/Core/Coordinator/Coordinator.swift (1)
19-27:childDidFinish에서 nil 가드와firstIndex를 활용해 경고를 없애주세요.옵셔널 인자를 바로
guard let으로 걸러주고firstIndex(where:)를 쓰면 SwiftLint의for_where경고도 사라지고, 루프 중복 제거가 명확해집니다.- func childDidFinish(_ child: Coordinator?) { - for (index, coordinator) in childCoordinators.enumerated() { - if coordinator === child { - childCoordinators.remove(at: index) - break - } - } - } + func childDidFinish(_ child: Coordinator?) { + guard let child else { return } + if let index = childCoordinators.firstIndex(where: { $0 === child }) { + childCoordinators.remove(at: index) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Wable-iOS/Core/Coordinator/Coordinator.swift(1 hunks)Wable-iOS/Core/Photo/PhotoPickerHelper.swift(1 hunks)Wable-iOS/Presentation/Login/LoginCoordinator.swift(2 hunks)Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift(1 hunks)Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift(5 hunks)Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift
- Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift
🧰 Additional context used
🧬 Code graph analysis (4)
Wable-iOS/Core/Coordinator/Coordinator.swift (2)
Wable-iOS/Presentation/Login/LoginCoordinator.swift (1)
start(20-33)Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift (1)
start(19-21)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (3)
Wable-iOS/Presentation/WableComponent/View/ProfileEditView.swift (3)
setupView(114-127)configureView(194-198)configureDefaultImage(102-105)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (1)
transform(55-116)Wable-iOS/Core/Photo/PhotoPickerHelper.swift (1)
presentPhotoPicker(26-29)
Wable-iOS/Presentation/Login/LoginCoordinator.swift (2)
Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift (2)
start(19-21)showHome(64-82)Wable-iOS/Core/Coordinator/Coordinator.swift (1)
childDidFinish(20-27)
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift (3)
Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift (1)
nextButtonDidTap(135-141)Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (1)
getWelcomeMessage(60-62)
🪛 SwiftLint (0.57.0)
Wable-iOS/Core/Coordinator/Coordinator.swift
[Warning] 22-22: where clauses are preferred over a single if inside a for
(for_where)
🔇 Additional comments (8)
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift (8)
29-33: LGTM!의존성 주입 패턴이 잘 적용되었으며,
OnboardingProfileInfo를 통해 필요한 데이터를 받아 ViewModel을 초기화하는 구조가 깔끔합니다.
40-46: LGTM!
viewDidLoad에서 제약, 액션, 바인딩 설정을 순서대로 수행하는 구조가 명확하며, MVVM 패턴에 부합합니다.
52-59: LGTM!과거 리뷰에서 지적된 제약 설정 순서 문제가 수정되었습니다.
addSubview가makeConstraints이전에 호출되어 크래시가 방지됩니다.
61-87: LGTM!액션 바인딩이 명확하게 구성되어 있으며, 멀티라인 포맷팅이 일관성 있게 적용되었습니다.
120-129: LGTM!URL 처리 로직이 삼항 조건식으로 간결하게 개선되었으며, guard를 통한 안전한 언래핑이 적용되었습니다.
146-149: LGTM!UI 이벤트와 비즈니스 로직이 Relay를 통해 명확히 분리되었으며, MVVM 패턴이 올바르게 적용되었습니다.
172-190: LGTM! 웰컴 시트 표시 로직 확인 권장웰컴 메시지를 ViewModel에서 가져오는 것은 도메인 로직 분리 측면에서 좋습니다.
rootViewController에서 직접 present하는 방식은 홈 화면 위에 표시하기 위한 의도로 보입니다.다만, 네비게이션 흐름과 시트 표시 타이밍이 의도대로 동작하는지 확인해주세요.
navigateToHome이 먼저 실행되고 0.6초 후 시트가 표시되는데, 홈 화면 전환 애니메이션과 시트 표시가 자연스럽게 연결되는지 검증이 필요합니다.
17-17: 메모리 누수 우려 없음
navigateToHome클로저 할당 시 이미[weak self]를 사용하고 있어 순환 참조가 발생하지 않습니다.
| coordinator.onFinish = { [weak self] in | ||
| self?.childDidFinish(coordinator) | ||
| } |
There was a problem hiding this comment.
coordinator를 약한 참조로 캡처하지 않아 retain cycle 이 발생합니다.
OnboardingCoordinator가 자신의 onFinish 클로저를 강하게 보유하는데, 클로저 안에서 다시 coordinator를 강하게 캡처하고 있어 흐름 종료 후에도 해제가 되지 않습니다. 약한 캡처로 전환해 누수를 막아주세요.
- coordinator.onFinish = { [weak self] in
- self?.childDidFinish(coordinator)
- }
+ coordinator.onFinish = { [weak self, weak coordinator] in
+ guard let self, let coordinator else { return }
+ self.childDidFinish(coordinator)
+ }📝 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.
| coordinator.onFinish = { [weak self] in | |
| self?.childDidFinish(coordinator) | |
| } | |
| // Before starting the child coordinator, capture both self and coordinator weakly | |
| coordinator.onFinish = { [weak self, weak coordinator] in | |
| guard let self, let coordinator else { return } | |
| self.childDidFinish(coordinator) | |
| } |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Login/LoginCoordinator.swift around lines 51–53, the
closure assigned to coordinator.onFinish captures coordinator strongly causing a
retain cycle; change the capture list to capture coordinator weakly (e.g., [weak
self, weak coordinator]) and safely unwrap coordinator inside the closure
(guard/if let) before calling self?.childDidFinish(coordinator) so the
coordinator can be released when finished.
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift
Show resolved
Hide resolved
| output.profileLoaded | ||
| .receive(on: DispatchQueue.main) | ||
| .withUnretained(self) | ||
| .sink { owner, profile in | ||
| owner.rootView.configureView( | ||
| profileImageURL: profile.user.profileURL, | ||
| team: profile.user.fanTeam | ||
| ) | ||
| owner.rootView.nickNameTextField.text = profile.user.nickname | ||
| } |
There was a problem hiding this comment.
초기 닉네임 상태 동기화 누락
profileLoaded에서 텍스트 필드만 세팅하고 nicknameTextChangedRelay로 전달하지 않아 뷰모델이 여전히 빈 문자열 상태를 유지합니다. 그 결과 닉네임 검증/중복 검사 UI가 갱신되지 않아 버튼이 비활성화된 채로 남을 수 있습니다. 불필요한 편집 없이도 서버 닉네임을 기준으로 초기 상태가 맞춰지도록 이벤트를 함께 전파해 주세요.
owner.rootView.configureView(
profileImageURL: profile.user.profileURL,
team: profile.user.fanTeam
)
owner.rootView.nickNameTextField.text = profile.user.nickname
+ owner.nicknameTextChangedRelay.send(profile.user.nickname)📝 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.
| output.profileLoaded | |
| .receive(on: DispatchQueue.main) | |
| .withUnretained(self) | |
| .sink { owner, profile in | |
| owner.rootView.configureView( | |
| profileImageURL: profile.user.profileURL, | |
| team: profile.user.fanTeam | |
| ) | |
| owner.rootView.nickNameTextField.text = profile.user.nickname | |
| } | |
| output.profileLoaded | |
| .receive(on: DispatchQueue.main) | |
| .withUnretained(self) | |
| .sink { owner, profile in | |
| owner.rootView.configureView( | |
| profileImageURL: profile.user.profileURL, | |
| team: profile.user.fanTeam | |
| ) | |
| owner.rootView.nickNameTextField.text = profile.user.nickname | |
| owner.nicknameTextChangedRelay.send(profile.user.nickname) | |
| } |
JinUng41
left a comment
JinUng41
left a comment
There was a problem hiding this comment.
고생하셨습니다.
코디네이터의 부모-자식 간 종료 시 처리를 위한 로직을 구현하셨네요.
고민하고 있던 부분을 대신 구현해 주셔서 감사합니다.
PR 단위가 길어 다소 힘들었지만, 객체 분리를 시도한 점과 기존 로직을 개선하려는 노력에 칭찬을 드리고 싶습니다~
| func childDidFinish(_ child: Coordinator?) { | ||
| for (index, coordinator) in childCoordinators.enumerated() { | ||
| if coordinator === child { | ||
| childCoordinators.remove(at: index) | ||
| break | ||
| } |
There was a problem hiding this comment.
where 절을 사용해 보는 것도 좋을 것 같아요.
There was a problem hiding this comment.
반영했습니다! 불필요한 if 절을 줄일 수 있어서 더 효율적인 것 같아요.
| picker.dismiss(animated: true) | ||
|
|
||
| guard let result = results.first else { return } | ||
| loadImage(from: result.itemProvider) |
There was a problem hiding this comment.
현재 코드는 순차적으로 처리하셨는데 특별한 이유가 있는지 궁금합니다.
저는 dismiss의 컴플리션 핸들러를 활용해볼 생각이 들었거든요.
There was a problem hiding this comment.
completionHandler의 경우 코드 뎁스가 깊어져서 정말 필요한 경우가 아니면 사용하지 않았습니다!
| extension DefaultProfileType { | ||
| static func random() -> DefaultProfileType { | ||
| return allCases.randomElement() ?? .blue | ||
| } | ||
|
|
||
| static func from(uppercased: String) -> DefaultProfileType? { | ||
| switch uppercased { | ||
| case "GREEN": | ||
| return .green | ||
| case "BLUE": | ||
| return .blue | ||
| case "PURPLE": | ||
| return .purple | ||
| default: | ||
| return nil | ||
| } | ||
| } |
| static func requestPhotoLibraryAccess(completion: @escaping (Bool) -> Void) { | ||
| let status = PHPhotoLibrary.authorizationStatus(for: .addOnly) | ||
|
|
||
| switch status { | ||
| case .authorized, .limited: | ||
| completion(true) | ||
| case .denied, .restricted: | ||
| completion(false) | ||
| case .notDetermined: | ||
| PHPhotoLibrary.requestAuthorization(for: .addOnly) { newStatus in | ||
| DispatchQueue.main.async { | ||
| completion(newStatus == .authorized || newStatus == .limited) | ||
| } | ||
| } | ||
| @unknown default: | ||
| completion(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
프로젝트에서 Combine을 적극적으로 사용하기 때문에, Combine API로 제공하는 것이 학습의 연장으로도 이어질 것 같은데 어떠세요?
There was a problem hiding this comment.
일단 리팩은 해보았는데 ... 로직이 꽤 복잡해져서 기존 코드가 나은 것 같기도 합니닷. 그치만 학습의 연장선이라면 해볼 만 한 것 같기도 하네요 ㅎㅎ
| // MARK: - Helper Method | ||
|
|
||
| extension PhotoPickerHelper { |
There was a problem hiding this comment.
헬퍼 메서드를 타입 메서드로 구현하신 이유를 들어보고 싶어요.
There was a problem hiding this comment.
현재 타입 메서드로 구현된 기능들의 경우 권한 요청 및 사진 저장과 관련된 기능으로, 따로 상태 저장을 할 필요가 없어 인스턴스를 생성할 필요성을 느끼지 못해 타입 메서드로 구현했습니다!
| .flatMap { [weak self] _ -> AnyPublisher<Void, Never> in | ||
| guard let self else { return Empty().eraseToAnyPublisher() } | ||
|
|
||
| userSessionRepository.updateUserSession( | ||
| userID: userSession.id, | ||
| nickname: self.profileInfo.nickname ?? userSession.nickname, | ||
| profileURL: userSession.profileURL, | ||
| isPushAlarmAllowed: isMarketingAgreed, | ||
| isAdmin: userSession.isAdmin, | ||
| isAutoLoginEnabled: true, | ||
| notificationBadgeCount: userSession.notificationBadgeCount ?? 0 | ||
| ) | ||
|
|
||
| WableLogger.log("세션 저장 완료", for: .debug) | ||
| return .just(()) | ||
| } |
There was a problem hiding this comment.
flatMap은 업스트림에서 방출된 값을 이용하여 새로운 Publisher를 만들어 반환하고 이를 평탄화하는 것이 주된 목적인데요.
그런 면에서 userSessionRepository.updateUserSession()은 새로운 스트림을 만들기 보다는 사이드 이펙트를 처리하는 목적이 강해 보입니다.
이 부분을 handleEvents로 바꿔보는 것은 어떨까요? 스트림에 영향을 주지 않고, 특정 이벤트에 따라 사이드 이펙트를 처리할 수 있는데요. 좀 더 코드의 의도가 명확화 될 것 같습니다.
There was a problem hiding this comment.
헉 그 부분은 생각해보지 않았는데요, flatMap이 방출된 값을 평탄화하는 정도의 역할만 수행하고 있다고 생각해서 저렇게 구현했던 것 같습니다. 컴바인 공부가 더 필요하겠네요 ㅜ.ㅜ 감사합니다 반영했습니다!!
| private extension ProfileEditViewModel { | ||
| enum Constant { | ||
| static let nicknamePattern = "^[ㄱ-ㅎ가-힣a-zA-Z0-9]+$" | ||
| } | ||
| } |
There was a problem hiding this comment.
StringLiterals로 수정도 고려해봐주세요.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
Wable-iOS/Core/Coordinator/Coordinator.swift (1)
19-26: LGTM! 문서화 추가를 고려해보세요.기본 구현이 올바르게 작성되었습니다:
- Identity 비교(
===)로 올바른 자식을 찾습니다where절로 코드가 간결합니다break로 첫 매칭 후 중단하여 효율적입니다선택사항: 사용 패턴을 명확히 하기 위해 문서 주석 추가를 고려해보세요.
extension Coordinator { /// 완료된 자식 코디네이터를 제거합니다. /// - Parameter child: 제거할 자식 코디네이터. nil인 경우 무시됩니다. /// - Note: 자식 코디네이터의 onFinish 클로저에서 호출되어야 합니다. func childDidFinish(_ child: Coordinator?) { for (index, coordinator) in childCoordinators.enumerated() where coordinator === child { childCoordinators.remove(at: index) break } } }Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (1)
17-17: 불필요한 인스턴스 생성을 방지하기 위해lazy var사용을 고려하세요.
registrationCompletedSubject는transform메서드가 호출되기 전까지는 사용되지 않습니다. PR 설명에서 언급한 대로 큰 객체나 사용되지 않을 수 있는 프로퍼티는lazy var로 선언하는 것이 좋습니다.다음 diff를 적용하세요:
- private let registrationCompletedSubject = PassthroughSubject<Void, Never>() + private lazy var registrationCompletedSubject = PassthroughSubject<Void, Never>()Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (1)
122-129: 프로필 로드 실패 시 사용자 피드백을 고려하세요.현재 프로필 로드가 실패하면
Empty()를 반환하여 조용히 실패합니다. 네트워크 오류나 권한 문제 등으로 프로필을 불러올 수 없을 때 사용자가 이를 인지할 수 있도록 에러 스트림을 Output에 추가하거나, 최소한 토스트 메시지를 표시하는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Wable-iOS/Core/Coordinator/Coordinator.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Profile.swift(0 hunks)Wable-iOS/Core/Literals/String/StringLiterals+ProfileSetting.swift(1 hunks)Wable-iOS/Core/Photo/PhotoPickerHelper.swift(1 hunks)Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift(1 hunks)Wable-iOS/Presentation/Viewit/Coordinator/ViewitCoordinator.swift(1 hunks)Wable-iOS/Presentation/WableComponent/ViewController/PhotoDetailViewController.swift(4 hunks)
💤 Files with no reviewable changes (1)
- Wable-iOS/Core/Literals/String/StringLiterals+Profile.swift
🧰 Additional context used
🧬 Code graph analysis (5)
Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (6)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (2)
updateProfile(148-181)extractImageData(183-192)Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Data/RepositoryImpl/UserSessionRepositoryImpl.swift (1)
fetchActiveUserSession(39-44)Wable-iOS/Data/RepositoryImpl/ProfileRepositoryImpl.swift (1)
fetchFCMToken(44-50)Wable-iOS/Core/Logger/WableLogger.swift (3)
log(14-25)debug(36-43)error(45-52)Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
random(41-43)
Wable-iOS/Core/Coordinator/Coordinator.swift (3)
Wable-iOS/Presentation/Viewit/Coordinator/ViewitCoordinator.swift (1)
start(19-58)Wable-iOS/Presentation/Onboarding/Coordinator/OnboardingCoordinator.swift (1)
start(19-21)Wable-iOS/Presentation/Login/LoginCoordinator.swift (1)
start(20-33)
Wable-iOS/Presentation/WableComponent/ViewController/PhotoDetailViewController.swift (3)
Wable-iOS/Core/Photo/PhotoPickerHelper.swift (1)
saveImage(125-148)Wable-iOS/Presentation/WableComponent/View/ToastView.swift (1)
show(144-160)Wable-iOS/Core/Logger/WableLogger.swift (2)
error(45-52)log(14-25)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (5)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (1)
viewWillAppear(59-63)Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (1)
extractImageData(142-151)Wable-iOS/Core/Combine+/Publisher+.swift (1)
withUnretained(34-40)Wable-iOS/Core/Logger/WableLogger.swift (1)
log(14-25)Wable-iOS/Data/RepositoryImpl/ProfileRepositoryImpl.swift (1)
fetchFCMToken(44-50)
Wable-iOS/Core/Photo/PhotoPickerHelper.swift (1)
Wable-iOS/Presentation/WableComponent/ViewController/PhotoDetailViewController.swift (1)
saveImage(116-128)
🪛 SwiftLint (0.57.0)
Wable-iOS/Core/Photo/PhotoPickerHelper.swift
[Warning] 114-114: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (12)
Wable-iOS/Core/Coordinator/Coordinator.swift (2)
13-13: LGTM!코디네이터 생명주기 관리를 위한 완료 콜백 추가가 적절합니다. 부모 코디네이터가 자식의 종료를 감지하고 정리할 수 있도록 합니다.
16-16: LGTM!자식 코디네이터 정리를 위한 메서드 추가가 적절합니다. 메모리 누수 방지를 위한 명시적인 정리 메커니즘을 제공합니다.
Wable-iOS/Presentation/Onboarding/ViewModel/AgreementViewModel.swift (6)
40-54: LGTM!
withUnretained와weak self사용으로 메모리 누수를 적절히 방지하고 있으며, Input을 Output으로 변환하는 로직이 명확합니다.
60-62: LGTM!환영 메시지 생성 로직이 간단하고 명확하며, nil 처리도 적절합니다.
68-77: LGTM!세션을 가져와서 프로필 업데이트로 연결하는 Publisher 체인이 정확하며,
withUnretained로 메모리 누수도 방지하고 있습니다.
79-126: 이전 리뷰 지적사항이 잘 반영되었습니다.
handleEvents를 사용하여 세션 업데이트 사이드 이펙트를 처리하도록 수정되었습니다 (lines 106-120).profileInfo.nickname과isMarketingAgreed를 사용하여 세션이 올바르게 업데이트되고 있습니다 (lines 111, 113).- 에러 핸들링과 weak self 사용이 적절합니다.
Based on past review comments
128-140: LGTM!FCM 토큰 업데이트 로직이 명확하며, nil 처리 및 에러 핸들링이 적절합니다.
142-151: LGTM!이미지 데이터 추출 로직이
ProfileEditViewModel의 구현과 일관성 있게 작성되었으며, 각 케이스가 적절히 처리되고 있습니다.Wable-iOS/Core/Literals/String/StringLiterals+ProfileSetting.swift (1)
23-23: 정규식 패턴 상수 추가를 승인합니다.닉네임 검증 로직을 중앙화하기 위해 정규식 패턴을 상수로 분리한 것은 좋은 접근입니다. 이는 코드 중복을 줄이고 유지보수성을 향상시킵니다.
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift (3)
12-34: LGTM! 깔끔한 MVVM ViewModel 구조입니다.프로퍼티가 적절히 private으로 선언되었고, 의존성 주입도 올바르게 구현되었습니다. PassthroughSubject를 사용한 반응형 스트림 설계도 적절합니다.
55-116: 메모리 관리가 올바르게 구현되었습니다.모든 클로저에서
withUnretained(self)또는[weak self]를 사용하여 retain cycle을 방지했습니다. 반응형 바인딩 로직도 명확하고 깔끔합니다.
158-158: fanTeam 파라미터는 Optional이므로 LCKTeam(rawValue:)가 nil을 반환해도 런타임 오류가 발생하지 않습니다 fanTeam 타입이LCKTeam?로 선언되어 있어nil할당이 허용됩니다.Likely an incorrect or invalid review comment.
| static let checkInvaildError = "닉네임에 사용할 수 없는 문자가 포함되어 있어요." | ||
| static let checkDuplicateError = "이미 사용 중인 닉네임입니다." | ||
| static let checkVaildMessage = "사용 가능한 닉네임입니다." | ||
| static let nicknamePattern = "^[ㄱ-ㅎ가-힣a-zA-Z0-9]+$" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
정규식 패턴에서 한글 자음 및 모음 처리를 확인해 주세요.
현재 정규식 패턴 ^[ㄱ-ㅎ가-힣a-zA-Z0-9]+$은 한글 자음(ㄱ-ㅎ)은 포함하지만 모음(ㅏ-ㅣ)은 제외하고 있습니다. 다음 사항들을 확인해 주세요:
- 자음만으로 이루어진 닉네임 허용 여부: "ㄱㄴㄷ"과 같은 닉네임이 의도적으로 허용되는 것인지 확인이 필요합니다.
- 모음 제외 의도 확인: 모음(ㅏ-ㅣ)이 의도적으로 제외된 것인지 확인해 주세요. 불완전한 문자 입력을 방지하기 위한 것이라면 합당하지만, "ㅏㅑㅓ"와 같은 입력도 막게 됩니다.
- 길이 검증 처리 확인: 정규식에는 길이 제한이 없으므로, line 19의 "10자리 이내" 제약은
ProfileRegisterViewModel에서 별도로 검증되는지 확인이 필요합니다.
다음 스크립트로 ProfileRegisterViewModel의 길이 검증 로직을 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# Description: ProfileRegisterViewModel에서 닉네임 길이 검증 로직 확인
# ProfileRegisterViewModel에서 닉네임 길이 체크 로직 검색
rg -n -A 5 -B 5 'count|length|characters\.count' --type swift Wable-iOS/Presentation/Onboarding/ViewModel/ProfileRegisterViewModel.swiftLength of output: 1310
정규식 범위 및 길이 검증 보완 필요
- 패턴
^[ㄱ-ㅎ가-힣a-zA-Z0-9]+$가 standalone 자음(ㄱ-ㅎ)은 허용하나 모음(ㅏ-ㅣ)은 제외합니다. 자음·모음 단독 입력 허용 여부를 명확히 해주세요. validateNickname(_:)에 10자 이하 길이 제한 검증 로직을 추가해야 합니다.
🤖 Prompt for AI Agents
In Wable-iOS/Core/Literals/String/StringLiterals+ProfileSetting.swift around
line 23, update the nickname regex and length validation: if you intend to allow
standalone Hangul vowels as well as consonants, change the pattern to include
the ㅏ-ㅣ range (e.g. include ㄱ-ㅎㅏ-ㅣ가-힣 in the character class) or if you want to
disallow standalone Jamo remove ㄱ-ㅎ; then in validateNickname(_:) add an
explicit check that the nickname length is <= 10 (return false or fail
validation when length > 10) so both character range and max-length are
enforced.
| .flatMap { isAuthorized -> AnyPublisher<Void, Error> in | ||
| guard isAuthorized else { | ||
| return Empty().eraseToAnyPublisher() | ||
| } | ||
| return saveImageToPhotoLibrary(image) | ||
| } | ||
| .receive(on: DispatchQueue.main) | ||
| .sink( |
There was a problem hiding this comment.
권한 거부 시 사용자 피드백이 누락됩니다
requestPhotoLibraryAccess()가 false를 반환하면 현재 Empty()를 반환해 스트림이 조용히 종료됩니다. 이 경우 PhotoDetailViewController.saveImage()에서도 성공/실패 콜백이 모두 호출되지 않아 사용자에게 아무런 안내가 전달되지 않습니다. 권한이 없을 때는 명시적으로 실패를 방출해 상위에서 에러 토스트나 설정 이동 안내를 보여줄 수 있도록 해야 합니다.
- guard isAuthorized else {
- return Empty().eraseToAnyPublisher()
- }
+ guard isAuthorized else {
+ return Fail(
+ error: NSError(
+ domain: "PhotoPickerHelper",
+ code: -1,
+ userInfo: [NSLocalizedDescriptionKey: "Photo library access denied"]
+ )
+ )
+ .eraseToAnyPublisher()
+ }📝 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.
| .flatMap { isAuthorized -> AnyPublisher<Void, Error> in | |
| guard isAuthorized else { | |
| return Empty().eraseToAnyPublisher() | |
| } | |
| return saveImageToPhotoLibrary(image) | |
| } | |
| .receive(on: DispatchQueue.main) | |
| .sink( | |
| .flatMap { isAuthorized -> AnyPublisher<Void, Error> in | |
| guard isAuthorized else { | |
| return Fail( | |
| error: NSError( | |
| domain: "PhotoPickerHelper", | |
| code: -1, | |
| userInfo: [NSLocalizedDescriptionKey: "Photo library access denied"] | |
| ) | |
| ) | |
| .eraseToAnyPublisher() | |
| } | |
| return saveImageToPhotoLibrary(image) | |
| } | |
| .receive(on: DispatchQueue.main) | |
| .sink( |
| func extractImageData(from profileImageType: ProfileImageType?) -> (image: UIImage?, defaultProfileType: String?) { | ||
| switch profileImageType { | ||
| case .custom(let image): | ||
| return (image, nil) | ||
| case .default(let type): | ||
| return (nil, type.uppercased) | ||
| case .none: | ||
| return (nil, nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
코드 중복을 제거하세요.
extractImageData 메서드가 AgreementViewModel.swift의 동일한 메서드와 거의 중복됩니다 (차이점은 .none 케이스 처리뿐). DRY 원칙을 준수하기 위해 이 로직을 공통 유틸리티나 ProfileImageType extension으로 추출하는 것을 권장합니다.
다음과 같이 ProfileImageType에 extension을 추가할 수 있습니다:
extension ProfileImageType {
func extractImageData(useRandomDefault: Bool = false) -> (image: UIImage?, defaultProfileType: String?) {
switch self {
case .custom(let image):
return (image, nil)
case .default(let type):
return (nil, type.uppercased)
case .none:
return (nil, useRandomDefault ? DefaultProfileType.random().uppercased : nil)
}
}
}그런 다음 이 파일에서는:
-let (image, defaultProfileType) = extractImageData(from: currentProfileImageType)
+let (image, defaultProfileType) = currentProfileImageType?.extractImageData() ?? (nil, nil)AgreementViewModel에서는:
-let (image, defaultProfileType) = extractImageData(from: profileImageType)
+let (image, defaultProfileType) = profileImageType?.extractImageData(useRandomDefault: true) ?? (nil, nil)🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditViewModel.swift around lines
183-192, extractImageData duplicates logic from AgreementViewModel; move this
logic into a single ProfileImageType extension (e.g. func
extractImageData(useRandomDefault: Bool = false) -> (UIImage?, String?)) that
handles .custom, .default (use type.uppercased()) and .none (return nil or a
random default when useRandomDefault is true), then replace the existing method
in this file to call that extension; ensure callers pass useRandomDefault
appropriately and remove the duplicated implementation.
| final class ViewitCoordinator: Coordinator { | ||
| var childCoordinators: [Coordinator] = [] | ||
| var navigationController: UINavigationController | ||
| var onFinish: (() -> Void)? |
There was a problem hiding this comment.
🧩 Analysis chain
onFinish 클로저가 호출되지 않아 메모리 누수 방지 로직이 불완전합니다.
PR 목표에 따르면 onFinish는 코디네이터가 완료될 때 부모 코디네이터가 childDidFinish를 호출하여 childCoordinators 배열에서 제거할 수 있도록 하기 위한 것입니다. 그러나 현재 코드에서는 onFinish?()를 호출하는 지점이 없어, 의도한 메모리 누수 방지 메커니즘이 작동하지 않습니다.
ViewitCoordinator의 플로우가 종료되는 시점(예: 네비게이션이 pop되거나 dismiss될 때)에 onFinish?()를 호출해야 합니다.
다음 스크립트를 실행하여 Coordinator 프로토콜 정의와 다른 코디네이터에서 onFinish를 어떻게 사용하는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Coordinator 프로토콜 정의 및 onFinish 사용 패턴 확인
echo "=== Coordinator 프로토콜 정의 검색 ==="
rg -A 10 "protocol Coordinator"
echo -e "\n=== onFinish 호출 패턴 검색 ==="
rg -n "onFinish\?\(\)" --type swift
echo -e "\n=== childDidFinish 메서드 검색 ==="
rg -n "childDidFinish" --type swiftLength of output: 1328
ViewitCoordinator에 onFinish 호출 로직 추가 필요
ViewitCoordinator의 플로우 종료 시점(예: 네비게이션 pop/dismiss)에서 self.onFinish?()를 호출해 부모 코디네이터의 childDidFinish가 실행되어야 합니다.
OnboardingCoordinator.swift(79행)에서의 패턴을 참고해 동일한 호출을 추가하세요.
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Viewit/Coordinator/ViewitCoordinator.swift around line
13, the coordinator defines var onFinish: (() -> Void)? but never calls it when
the flow ends; add a call to self.onFinish?() at the point(s) where the Viewit
flow is finished (for example inside the method that handles navigation pop or
dismiss or a dedicated finish() method), mirroring the pattern used in
OnboardingCoordinator.swift at line 79 so the parent coordinator's
childDidFinish is invoked; ensure this call is made after any cleanup and UI
dismissal to notify the parent.
[Refactor] 온보딩 및 프로필 편집 화면 코디네이터 패턴 적용, 코드 구조 및 의존성 개선
📄 작업 내용
💻 주요 코드 설명
Enum을 이용한 공통 타입 분리
온보딩 MVVM 패턴 적용
AgreementViewController,ProfileEditViewController에만 적용했습니다!불필요한 UseCase 레이어 제거, Repository 직접 호출 방식으로 로직 개선
FetchNicknameDuplicationUseCase,UpdateFCMTokenUseCase,UserProfileUseCase를 제거함으로써 총 127줄을 감소시켰습니다.Coordinator 메모리 누수 방지를 위한 childCoordinators 제거 로직 구현
개선 전
개선 후
권한 요청 및 사진 기능 관리를 위한
PhotoPickerHelper구현PHPickerViewController는 사진을 저장하지 않는다면 따로 권한 요청을 필요로 하지 않습니다.개선 전
개선 후
👀 리뷰어에게 전달할 사항
✅ 이번 PR에서 이런 부분을 중점적으로 체크해주세요!
잠깐 확인하고 갈까요?
들여쓰기를 5번 이하로 준수했는지, 코드 가독성이 적절한지 확인해주세요.
한 줄당 120자 제한을 준수했는지 확인해주세요.
MARK 주석이 정해진 순서와 형식에 맞게 작성되었는지 확인해주세요.
반복되는 상수 값이 있는지, 있다면 Constant enum으로 분리되어 있는지 확인해주세요.
삼항 연산자가 길어질 경우 적절히 개행되어 있는지 확인해주세요.
조건문에서 중괄호가 올바르게 사용되었는지 확인해주세요.
라이브러리 import가 퍼스트파티와 서드파티로 구분되고 알파벳순으로 정렬되었는지 확인해주세요.
🔗 연결된 이슈
Summary by CodeRabbit
신기능
개선