[WEAV-100] 회원가입 - SMS 발송 / 인증 API 연결 #28
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough이 풀 리퀘스트는 여러 파일에서의 변경 사항을 포함하고 있습니다. Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (21)
Projects/Core/Model/Sources/Network/SMSVerificationResponse.swift (1)
Line range hint
1-1: 파일 이름 변경 제안현재 파일 이름
SMSVerificationResponse.swift가 새로운 구조체 이름ExistingUserVerificationResponse와 일치하지 않습니다. 코드의 일관성과 가독성을 위해 파일 이름을ExistingUserVerificationResponse.swift로 변경하는 것이 좋겠습니다.파일 이름을
ExistingUserVerificationResponse.swift로 변경해주세요.Projects/Core/Model/Sources/Network/SMSSendResponse.swift (2)
11-14: 열거형 정의를 간소화하세요.
UserType열거형의 정의를 다음과 같이 간소화할 수 있습니다:public enum UserType: String { case NEW case EXISTING }이렇게 하면 코드가 더 간결해지고 Swift의 암시적 원시 값 할당을 활용할 수 있습니다.
🧰 Tools
🪛 SwiftLint
[Warning] 12-12: String enum values can be omitted when they are equal to the enumcase name
(redundant_string_enum_value)
[Warning] 13-13: String enum values can be omitted when they are equal to the enumcase name
(redundant_string_enum_value)
16-30: 구조체 구현이 잘 되었습니다. 문서화를 추가하는 것이 좋겠습니다.
SMSSendResponse구조체의 구현이 적절합니다. 불변성을 보장하고 public 접근 제어자를 사용하여 외부에서 접근 가능하도록 한 점이 좋습니다.다만, 각 프로퍼티와 이니셜라이저에 대한 문서 주석을 추가하면 더 좋을 것 같습니다. 예를 들어:
public struct SMSSendResponse { /// 사용자 유형 (신규 또는 기존) public let userType: UserType /// SMS 인증 코드의 고유 식별자 public let authCodeId: String /// SMS가 전송된 전화번호 public let phoneNumber: String /// SMSSendResponse 인스턴스를 생성합니다. /// - Parameters: /// - userType: 사용자 유형 /// - authCodeId: SMS 인증 코드의 고유 식별자 /// - phoneNumber: SMS가 전송된 전화번호 public init( userType: UserType, authCodeId: String, phoneNumber: String ) { self.userType = userType self.authCodeId = authCodeId self.phoneNumber = phoneNumber } }이렇게 문서화를 추가하면 API 사용자가 각 필드의 의미를 더 쉽게 이해할 수 있습니다.
XcodeTemplate/SwiftUI MVI.xctemplate/___FILEBASENAME___Intent.swift (1)
10-10: 변경 사항이 일관성 있게 적용되었습니다.
externalData에서input으로의 이름 변경이 클래스 전체에 걸쳐 일관되게 적용되었습니다. 이는 코드의 가독성을 향상시킵니다.추가적인 개선사항으로,
input프로퍼티에 대한 간단한 문서 주석을 추가하는 것이 좋겠습니다. 예를 들어:/// 이 Intent에 대한 입력 데이터 모델 private let input: DataModel이렇게 하면 이 프로퍼티의 목적을 더 명확히 할 수 있습니다.
Also applies to: 15-15, 17-17
Projects/App/Sources/Navigation/NavigationStack.swift (1)
26-27: SMS 응답 데이터 전달 구현이 적절합니다.SMS 인증 기능 구현을 위해
authPhoneVerify케이스에smsResponse매개변수를 추가한 것은 적절한 변경입니다. 이를 통해 SMS 발송 단계에서 인증 단계로 필요한 데이터를 전달할 수 있게 되었습니다.가독성 향상을 위해 다음과 같이 매개변수에 레이블을 추가하는 것을 고려해보세요:
case .authPhoneVerify(let smsResponse): AuthPhoneVerifyView(smsResponse: smsResponse)이렇게 하면 코드의 의도가 더 명확해질 것입니다.
.gitignore (1)
70-71: 변경 사항이 적절해 보입니다.Tuist 빌드 아티팩트와 Secret.swift 파일을 무시하는 것은 좋은 관행입니다. 특히 Secret.swift를 무시하는 것은 민감한 정보를 보호하는 데 도움이 됩니다.
보안을 더욱 강화하기 위해 다음과 같이 모든 Secret*.swift 파일을 무시하는 것을 고려해 보세요:
-Projects/Core/CoreKit/Sources/Secret.swift +Projects/Core/CoreKit/Sources/Secret*.swift이렇게 하면 향후 추가될 수 있는 유사한 비밀 파일도 자동으로 무시됩니다.
Projects/DesignSystem/DesignCore/Sources/NavigationBar.swift (1)
47-55: 새로운setPopNavigation메서드가 잘 구현되었습니다.이 메서드는 백 버튼이 있는 네비게이션 바를 설정하는 편리한 방법을 제공합니다. 기존의
NavigationBarViewModifier를 재사용하여 코드 일관성을 유지하고 있습니다.일관성을 위해
setNavigation메서드와 유사한 형태로 파라미터를 구성하는 것을 고려해 보세요. 예를 들어:func setPopNavigation( handler: @escaping () -> Void ) -> some View { // ... 현재 구현 ... }이렇게 하면 향후 파라미터가 추가될 경우 더 쉽게 확장할 수 있습니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementView.swift (1)
60-62: 네비게이션 처리 방식의 개선을 확인했습니다.이 변경은 네비게이션 로직을 개선하고 있습니다.
AppCoordinator.shared.pop()을 사용하여 명시적으로 pop 동작을 정의하는 것은 좋은 접근 방식입니다. 이는 앱 전체의 네비게이션 일관성을 향상시킬 수 있습니다.다만, 더 나은 재사용성과 테스트 용이성을 위해 다음과 같은 개선을 고려해 보시는 것은 어떨까요?
.setPopNavigation(coordinator: AppCoordinator.shared)이렇게 하면
AppCoordinator에 대한 의존성을 주입할 수 있어, 향후 테스트나 기능 확장 시 유연성이 높아질 것 같습니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyModel.swift (1)
75-80: 새로운 메서드들이 올바르게 구현되었습니다.
resetText()와setInitialPhoneNumber(_:)메서드의 구현이 간단하고 명확합니다. 다만,setInitialPhoneNumber(_:)메서드에서 전화번호 형식 검증이나 포맷팅을 추가하는 것을 고려해 보시는 것이 좋을 것 같습니다.예를 들어, 다음과 같이 전화번호 형식을 검증하고 포맷팅하는 로직을 추가할 수 있습니다:
func setInitialPhoneNumber(_ phone: String) { // 전화번호 형식 검증 guard isValidPhoneNumber(phone) else { // 유효하지 않은 전화번호 처리 return } // 전화번호 포맷팅 (예: 010-1234-5678) sentPhoneNumber = formatPhoneNumber(phone) } private func isValidPhoneNumber(_ phone: String) -> Bool { // 정규식을 사용한 전화번호 형식 검증 로직 // ... } private func formatPhoneNumber(_ phone: String) -> String { // 전화번호 포맷팅 로직 // ... }이렇게 하면 잘못된 형식의 전화번호가 저장되는 것을 방지하고, 일관된 형식의 전화번호를 유지할 수 있습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputView.swift (1)
84-86: 네비게이션 처리 방식의 개선을 승인합니다.새로운 구현 방식은 더 유연하고 중앙 집중식 네비게이션 관리를 가능하게 합니다.
AppCoordinator.shared.pop()을 직접 호출하는 것은 네비게이션 로직을 일관되게 유지하는 데 도움이 됩니다.일관성을 위해
AppCoordinator.shared를 뷰의 초기화 시점에 주입받는 것을 고려해 보세요. 이렇게 하면 테스트 가능성이 향상되고 의존성 관리가 더 쉬워질 수 있습니다.public init(coordinator: AppCoordinator = .shared) { // ... 기존 초기화 코드 ... self.coordinator = coordinator } // 그리고 나서 사용할 때: .setPopNavigation { self.coordinator.pop() }Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift (1)
97-99: 네비게이션 처리 방식의 개선을 확인했습니다.변경된 코드는 네비게이션 pop 동작을 더 명시적으로 처리하고 있어 좋습니다. 하지만 몇 가지 고려해야 할 점이 있습니다:
AppCoordinator.shared를 직접 사용하는 것은 뷰와 네비게이션 로직 간의 결합도를 높일 수 있습니다.- 싱글톤 패턴의 사용은 테스트 가능성을 저하시킬 수 있습니다.
다음과 같은 개선을 고려해 보시는 것은 어떨까요?
- 의존성 주입을 통해
AppCoordinator를 주입받도록 변경:struct AuthNameInputView: View { let coordinator: AppCoordinator // ... var body: some View { // ... .setPopNavigation { coordinator.pop() } // ... } }이렇게 하면 테스트 가능성이 향상되고, 뷰와 네비게이션 로직 간의 결합도를 낮출 수 있습니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (2)
56-56: 전화번호 입력값 전달 방식 개선이 좋습니다.
onTapNextButton메서드에state.phoneInputText를 직접 전달하는 방식으로 변경한 것은 좋은 개선입니다. 이는 상태 일관성을 유지하는 데 도움이 됩니다.다만, 더 나은 가독성을 위해 다음과 같이 변수를 사용하는 것을 고려해 보시는 건 어떨까요?
let phoneNumber = state.phoneInputText intent.onTapNextButton(with: phoneNumber)이렇게 하면 코드의 의도가 더 명확해질 수 있습니다.
68-70: 네비게이션 처리 방식 변경에 대한 검토가 필요합니다.네비게이션 처리를 클로저 기반 접근 방식으로 변경한 것은 좋은 시도입니다. 하지만 몇 가지 고려해야 할 점이 있습니다:
AppCoordinator.shared를 직접 사용하는 것은 뷰의 재사용성과 테스트 가능성을 저하시킬 수 있습니다.- 싱글톤 패턴(
shared)의 사용은 의존성 주입을 어렵게 만들 수 있습니다.다음과 같은 개선을 고려해 보시는 것은 어떨까요?
- 네비게이션 로직을 상위 레벨(예: 코디네이터 또는 뷰 모델)로 이동시키고, 뷰에는 콜백만 전달하는 방식.
- 의존성 주입을 통해
AppCoordinator를 주입받는 방식.예시:
struct AuthPhoneInputView: View { var onPop: () -> Void var body: some View { // ... .setPopNavigation(onPop) // ... } }이렇게 하면 뷰의 재사용성과 테스트 가능성이 향상될 수 있습니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift (2)
15-15: MARK 주석 형식 수정 필요라인 15에서 MARK 주석은
//MARK: - Intent로 작성되어 있습니다. 코드 스타일 가이드에 따라 슬래시 뒤에 공백을 추가하여// MARK: - Intent로 수정하는 것이 좋습니다.적용할 수정 사항:
-//MARK: - Intent +// MARK: - Intent🧰 Tools
🪛 SwiftLint
[Warning] 15-15: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 15-15: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
48-48: MARK 주석 형식 수정 필요라인 48에서 MARK 주석은
//MARK: - Intentable로 작성되어 있습니다. 슬래시 뒤에 공백을 추가하여// MARK: - Intentable로 수정하세요.적용할 수정 사항:
-//MARK: - Intentable +// MARK: - Intentable🧰 Tools
🪛 SwiftLint
[Warning] 48-48: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 48-48: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
OpenApiGenerator/Sources/OpenapiGenerated/Client.swift (3)
Line range hint
279-398: 유사한 로직의 재사용성 향상 제안
newUserVerifyCode메소드에서도existingUserVerifyCode와 유사한 직렬화 및 역직렬화 로직이 사용되고 있습니다. 공통 로직을 분리하여 함수화하면 코드 중복을 줄이고 유지보수성을 높일 수 있습니다.
Line range hint
604-676: 에러 응답 처리의 일관성 개선 필요
refreshToken메소드에서 400, 401 상태 코드에 대한 에러 응답 처리가 유사하게 이루어지고 있습니다. 에러의 종류에 따라 보다 구체적인 에러 메시지를 제공하여 클라이언트 측에서 정확한 대응이 가능하도록 개선하면 좋을 것 같습니다.
Line range hint
900-998: 에러 상태 코드 처리 누락 확인 필요
getCompanyDetails메소드에서 500 상태 코드에 대한 에러 처리가 되어 있지 않은 것 같습니다. 서버 오류에 대비하여 500 에러에 대한 응답 처리를 추가하는 것을 추천드립니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift (3)
34-34: 매개변수 이름을 소문자로 수정하세요.Swift 코딩 컨벤션에 따르면 매개변수 이름은 소문자로 시작해야 합니다.
Phone을phone으로 변경하여 일관성을 유지하세요.적용할 변경사항:
-func onTapNextButton(with Phone: String) +func onTapNextButton(with phone: String)
53-58: 매개변수 이름을 소문자로 수정하세요.
onTapNextButton(with Phone: String)메서드의 매개변수 이름을 소문자로 변경하고, 내부에서도 동일하게 수정하세요.적용할 변경사항:
-func onTapNextButton(with Phone: String) { +func onTapNextButton(with phone: String) { model?.setLoading(status: true) Task { - await requestSendSMS(phone: Phone) + await requestSendSMS(phone: phone) } }
91-91: 디버그용
print(#function)은 디버깅을 위한 코드로 보입니다. 프로덕션 코드에서는 불필요한 출력일 수 있으므로 제거하거나 필요한 경우 로깅 프레임워크를 사용하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (26)
- .gitignore (1 hunks)
- OpenApiGenerator/Sources/OpenapiGenerated/Client.swift (15 hunks)
- OpenApiGenerator/Sources/openapi-generator-cli/3days-oas (1 hunks)
- Projects/App/Project.swift (1 hunks)
- Projects/App/Sources/Navigation/NavigationStack.swift (1 hunks)
- Projects/Core/CommonKit/Sources/AppCoordinator.swift (1 hunks)
- Projects/Core/CommonKit/Sources/Navigation+Ext.swift (0 hunks)
- Projects/Core/CommonKit/Sources/Path/PathTypes.swift (2 hunks)
- Projects/Core/Model/Sources/Network/SMSSendResponse.swift (1 hunks)
- Projects/Core/Model/Sources/Network/SMSVerificationResponse.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/AuthService/AuthEndpoint.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/RetryingMiddleware.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/ServerType.swift (2 hunks)
- Projects/DesignSystem/DesignCore/Sources/NavigationBar.swift (1 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementView.swift (1 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (3 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift (3 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift (3 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyModel.swift (4 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (3 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputView.swift (1 hunks)
- XcodeTemplate/SwiftUI MVI.xctemplate/___FILEBASENAME___Intent.swift (1 hunks)
- XcodeTemplate/SwiftUI MVI.xctemplate/___FILEBASENAME___View.swift (2 hunks)
💤 Files with no reviewable changes (2)
- Projects/Core/CommonKit/Sources/Navigation+Ext.swift
- Projects/Core/NetworkKit/Sources/AuthService/AuthEndpoint.swift
✅ Files skipped from review due to trivial changes (1)
- OpenApiGenerator/Sources/openapi-generator-cli/3days-oas
🧰 Additional context used
🪛 SwiftLint
Projects/Core/Model/Sources/Network/SMSSendResponse.swift
[Warning] 12-12: String enum values can be omitted when they are equal to the enumcase name
(redundant_string_enum_value)
[Warning] 13-13: String enum values can be omitted when they are equal to the enumcase name
(redundant_string_enum_value)
Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift
[Warning] 59-59: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 59-59: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
[Warning] 69-69: TODOs should be resolved (Error 타입 정의)
(todo)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift
[Warning] 15-15: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 15-15: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 48-48: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 48-48: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
🔇 Additional comments (20)
Projects/Core/Model/Sources/Network/SMSVerificationResponse.swift (1)
15-18: public 초기화 메서드 추가에 대한 검토
ExistingUserVerificationResponse구조체에 public 초기화 메서드가 추가되었습니다. 이를 통해 모듈 외부에서도 이 구조체의 인스턴스를 생성할 수 있게 되었습니다.이 변경은 구조체의 사용성을 향상시키고, 외부 모듈에서의 유연한 활용을 가능하게 합니다.
Projects/Core/NetworkKit/Sources/NetworkCore/ServerType.swift (2)
19-21: baseURL 속성의 개선된 구현 승인하드코딩된 URL 문자열 대신 Secret 구조체를 사용하도록 변경한 것은 좋은 개선입니다. 이는 보안성과 유지보수성을 향상시킵니다.
Secret 구조체의 구현을 확인하기 위해 다음 스크립트를 실행해 주세요:
#!/bin/bash # Secret 구조체 구현 확인 echo "Secret 구조체 정의:" rg --type swift "struct Secret" ./Projects echo "\ndevBaseUrl 및 prodBaseUrl 정의:" rg --type swift "static let (dev|prod)BaseUrl" ./Projects이 결과를 바탕으로 Secret 구조체가 적절히 구현되어 있는지, 그리고 URL 값들이 안전하게 관리되고 있는지 확인해 주세요.
10-10: CoreKit 모듈 의존성 추가 확인 필요CoreKit 모듈을 import하는 것이 추가되었습니다. 이는 NetworkKit 모듈의 의존성에 영향을 줄 수 있습니다.
다음 스크립트를 실행하여 CoreKit의 사용 범위를 확인해 주세요:
이 결과를 바탕으로 CoreKit 의존성 추가가 필요한지, 그리고 다른 파일에서도 일관되게 사용되고 있는지 확인해 주세요.
✅ Verification successful
CoreKit 및 Secret 구조체 사용 현황 재확인 필요
이전 스크립트 실행 중
rg명령어에 오타가 발생하여 CoreKit의 사용 현황을 정확히 파악하지 못했습니다. 아래 스크립트를 실행하여 CoreKit과 Secret 구조체의 사용 범위를 다시 확인해 주세요:이 스크립트가 올바르게 실행되어 CoreKit의 import 및 Secret 구조체의 사용 여부를 확인할 수 있도록 해주세요.
CoreKit 모듈 의존성 확인 완료
NetworkKit모듈 내 여러 파일에서CoreKit을 정상적으로 import하고 있으며,Secret구조체를 사용하고 있습니다. 따라서CoreKit모듈의 의존성 추가는 필요하며, 코드베이스에 일관되게 반영되었습니다.
ServerType.swift:
import CoreKitSecret.devBaseUrl,Secret.prodBaseUrl사용AuthService.swift,AuthorizationMiddleware.swift,Interceptor.swift에서도import CoreKit사용🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # CoreKit 사용 범위 확인 echo "CoreKit 사용 현황:" rg --type swift "import CoreKit" ./Projects/Core/NetworkKit echo "\nSecret 구조체 사용 현황:" rg --type swift "Secret\." ./Projects/Core/NetworkKitLength of output: 295
Script:
#!/bin/bash # CoreKit 사용 범위 재확인 echo "CoreKit 사용 현황:" rg --type swift "import CoreKit" ./Projects/Core/NetworkKit echo "\nSecret 구조체 사용 현황:" rg --type swift "Secret\." ./Projects/Core/NetworkKitLength of output: 750
Projects/Core/CommonKit/Sources/AppCoordinator.swift (1)
22-26:⚠️ Potential issue구현이 단순화되었지만 사용자 경험에 영향을 줄 수 있습니다.
새로운 구현은 다음과 같은 특징이 있습니다:
- 코드가 간단해져 이해하기 쉬워졌습니다.
- 비동기 작업을 제거하여 잠재적인 타이밍 문제를 해결했습니다.
그러나 다음 사항을 고려해야 합니다:
- 이전 구현의 애니메이션 효과가 사용자 경험에 중요했다면, 새 구현으로 인해 UX가 저하될 수 있습니다.
- 주석 처리된 코드가 남아있어 변경에 대한 불확실성을 나타낼 수 있습니다.
다음 사항을 확인해 주세요:
- 이 변경이 의도적이며 애니메이션 효과가 더 이상 필요하지 않은지 확인해주세요.
- 주석 처리된 코드를 제거하거나, 유지해야 할 이유가 있다면 그 이유를 설명하는 주석을 추가해주세요.
사용자 경험에 미치는 영향을 확인하기 위해 다음 스크립트를 실행해 주세요:
XcodeTemplate/SwiftUI MVI.xctemplate/___FILEBASENAME___View.swift (2)
41-43: 네비게이션 처리 방식의 개선이 보입니다.
.setNavigationWithPop()에서.setPopNavigation { AppCoordinator.shared.pop() }로의 변경은 네비게이션 처리를 더 명시적으로 만들어 줍니다. 이는 네비게이션 동작에 대한 더 나은 제어와 유연성을 제공할 수 있습니다.그러나
AppCoordinator.shared가 모든 필요한 곳에서 적절히 초기화되고 접근 가능한지 확인이 필요합니다. 다음 스크립트를 실행하여AppCoordinator의 사용을 확인해 주세요:#!/bin/bash # Description: AppCoordinator의 초기화와 사용을 확인 rg --type swift 'class AppCoordinator' rg --type swift 'AppCoordinator\.shared'
19-19: 매개변수 이름 변경이 적절해 보입니다.
externalData에서input으로의 매개변수 이름 변경은 의도를 더 명확하게 전달할 수 있어 보입니다. 이는 코드의 가독성을 향상시킬 수 있는 좋은 변경사항입니다.다만, 이 변경이 다른 부분에 영향을 미치지 않는지 확인이 필요합니다. 다음 스크립트를 실행하여
externalData가 다른 곳에서 사용되고 있는지 확인해 주세요:✅ Verification successful
매개변수 이름 변경 검증 완료
externalData가 코드베이스의 다른 부분에서 사용되지 않음을 확인하여input으로의 변경을 승인합니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 'externalData'가 다른 파일에서 사용되고 있는지 확인 rg --type swift 'externalData'Length of output: 2091
Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementView.swift (1)
Line range hint
1-77: 전반적인 변경 사항에 대한 의견이 PR에서의 변경 사항은 네비게이션 처리 방식을 개선하는 데 중점을 두고 있습니다. 이는 앱의 전반적인 구조와 일관성을 향상시키는 긍정적인 변화로 보입니다. 다만, 앞서 제안한 대로 의존성 주입을 고려하여 코드의 유연성과 테스트 용이성을 더욱 높일 수 있을 것 같습니다.
또한, PR 설명에서 언급된 에러 처리 개선에 대해서는 이 파일에서 직접적인 변경이 보이지 않습니다. 에러 처리 로직을 추가하거나 개선하는 것을 고려해보시면 좋을 것 같습니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyModel.swift (3)
22-22: 프로토콜에 새 속성이 적절히 추가되었습니다.
Stateful프로토콜에sentPhoneNumber속성을 추가한 것은 SMS 기능 구현을 위한 좋은 접근입니다. 이 속성은 전송된 전화번호를 추적하는 데 사용될 것으로 보입니다.
38-38: 클래스에 새 속성이 올바르게 구현되었습니다.
AuthPhoneVerifyModel클래스에@Published var sentPhoneNumber: String = ""를 추가한 것은 적절합니다.@Published래퍼를 사용하여 SwiftUI 뷰에서 이 값의 변경을 쉽게 관찰할 수 있게 했습니다.
55-56: 프로토콜에 새로운 메서드들이 적절히 추가되었습니다.
AuthPhoneVerifyModelActionable프로토콜에setInitialPhoneNumber(_:)와resetText()메서드를 추가한 것은 좋은 결정입니다. 이 메서드들은 전화번호 설정과 인증 코드 초기화를 위한 명확한 인터페이스를 제공합니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (2)
25-25:externalData초기화 변경에 대한 설명이 필요합니다.
AuthPhoneInputIntent의externalData초기화가 변경되었습니다. 이 변경의 목적과 영향에 대해 설명해 주시겠습니까? 이전에는 임시 문자열을 사용했을 것으로 추정되는데, 이제는 빈 초기화를 하고 있습니다. 이 변경이 인텐트의 초기 상태나 동작에 어떤 영향을 미치는지 명확히 해주시면 좋겠습니다.
Line range hint
1-116: 전반적인 변경사항에 대한 요약이번 변경사항들은 전반적으로 코드 구조와 데이터 흐름을 개선하고 있습니다. 주요 변경 사항은 다음과 같습니다:
AuthPhoneInputIntent초기화 방식 변경onTapNextButton메서드에 전화번호 직접 전달- 네비게이션 처리 방식의 변경
이러한 변경사항들은 대체로 긍정적이지만, 몇 가지 개선의 여지가 있습니다:
externalData초기화 변경의 목적과 영향에 대한 명확한 설명이 필요합니다.- 변수 사용을 통한 코드 가독성 향상을 고려해 볼 수 있습니다.
- 네비게이션 로직의 의존성 주입 및 테스트 가능성 개선이 필요합니다.
이러한 제안사항들을 고려하여 코드를 더욱 개선할 수 있을 것 같습니다. 전반적으로 좋은 변경이었습니다만, 제안된 개선사항들을 검토해 주시기 바랍니다.
Projects/App/Project.swift (1)
15-18:⚠️ Potential issue보안 설정 변경에 대한 주의 필요
NSAppTransportSecurity에NSAllowsArbitraryLoads를true로 설정하는 것은 앱의 보안에 중요한 영향을 미칠 수 있습니다.
- 이 설정은 모든 서버에 대해 안전하지 않은 HTTP 연결을 허용합니다.
- 개발 중이나 HTTPS를 지원하지 않는 서버와 통신할 때 편리할 수 있지만, 프로덕션 앱에서는 권장되지 않습니다.
- 대신 특정 도메인에 대해서만 예외를 허용하는 것이 더 안전합니다.
다음 대안을 고려해 보세요:
- 특정 도메인에 대해서만 예외를 허용하기
- 가능한 모든 서버에서 HTTPS를 사용하도록 하기
보안 설정의 필요성을 확인하기 위해 다음 스크립트를 실행해 보세요:
이 스크립트의 결과를 바탕으로
NSAllowsArbitraryLoads설정의 필요성을 재검토해 주세요.✅ Verification successful
NSAllowsArbitraryLoads 설정 확인 완료
NSAllowsArbitraryLoads는Projects/App/Project.swift파일의infoPlist에서만 설정되어 있으며, 다른 곳에서는 사용되지 않습니다. 이 설정은 모든 서버에 대해 안전하지 않은 HTTP 연결을 허용하므로, 가능하다면 특정 도메인에만 예외를 허용하는 것이 보안상 더 안전합니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 프로젝트에서 HTTP URL 사용 여부 확인 echo "HTTP URL 사용 현황:" rg --type swift 'http://' -n echo "\n안전하지 않은 네트워크 요청 사용 현황:" rg --type swift '\.allowsInsecureHTTPLoads' -nLength of output: 177
Script:
#!/bin/bash # Description: 프로젝트에서 NSAllowsArbitraryLoads 설정의 전체 사용 현황 확인 echo "NSAllowsArbitraryLoads 설정이 사용되는 모든 위치:" ast-grep --lang swift --pattern $'$_ = NSAllowsArbitraryLoads($_)' . echo "\nNSAllowsArbitraryLoads와 관련된 모든 네트워크 설정 확인:" rg --type swift 'NSAppTransportSecurity' -A 5Length of output: 561
Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (1)
23-23:⚠️ Potential issue기본 로깅 정책 변경에 대한 고려사항
기본
bodyLoggingConfiguration이.never에서.upTo(maxBytes: 2048)로 변경되었습니다. 이 변경으로 인해 다음 사항들을 고려해야 합니다:
- 보안: 기본적으로 요청 및 응답 본문의 일부가 로그에 포함될 수 있으므로, 민감한 정보가 노출되지 않도록 주의해야 합니다.
- 성능: 로깅이 증가하여 약간의 성능 저하가 있을 수 있습니다.
- 저장 공간: 로그 파일의 크기가 증가할 수 있으므로, 로그 순환 정책을 검토해야 할 수 있습니다.
다음 사항들을 확인해 주세요:
- 이 변경이 보안 정책에 부합하는지 검토했나요?
- 로그에 민감한 정보가 포함되지 않도록 하는 추가적인 필터링 메커니즘이 있나요?
- 이 변경이 성능에 미치는 영향을 측정했나요?
- 로그 저장 및 관리 정책을 업데이트할 필요가 있는지 고려했나요?
아래 스크립트를 실행하여 이 미들웨어의 사용 현황을 확인할 수 있습니다:
이 결과를 바탕으로 변경의 영향을 받는 부분을 식별하고, 필요한 경우 추가적인 보안 조치를 취하세요.
Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/RetryingMiddleware.swift (1)
132-132: 인증 서비스 리팩토링 확인 필요
AuthEndpoint.refreshAccessToken()에서AuthService.refreshAccessToken()로의 변경은 적절해 보입니다. 이는 인증 관련 코드의 구조를 개선하기 위한 리팩토링의 일부로 보입니다.다음 스크립트를 실행하여 전체 코드베이스에서 이 리팩토링이 일관되게 적용되었는지 확인하세요:
이 스크립트의 결과를 검토하여 모든 관련 파일에서 변경이 일관되게 이루어졌는지 확인하세요. 불일치가 발견되면 추가적인 업데이트가 필요할 수 있습니다.
✅ Verification successful
인증 서비스 리팩토링 확인 완료
전체 코드베이스에서
AuthEndpoint.refreshAccessToken()의 사용이 제거되고AuthService.refreshAccessToken()이 올바르게 적용된 것을 확인했습니다. 리팩토링이 일관되게 이루어졌으며 추가적인 문제가 발견되지 않았습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 설명: AuthEndpoint에서 AuthService로의 변경이 일관되게 적용되었는지 확인합니다. # 테스트: AuthEndpoint.refreshAccessToken() 사용 여부 확인 echo "AuthEndpoint.refreshAccessToken() 사용 검색 결과:" rg "AuthEndpoint\.refreshAccessToken\(\)" --type swift # 테스트: AuthService.refreshAccessToken() 사용 여부 확인 echo "\nAuthService.refreshAccessToken() 사용 검색 결과:" rg "AuthService\.refreshAccessToken\(\)" --type swiftLength of output: 472
Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift (1)
27-27: 옵션 값 처리에 대한 확인 필요
response.userStatus가nil인 경우 기본값으로"NEW"를 사용하고 있습니다. 이는 예상하지 못한 사용자 상태를 초래할 수 있으므로, 기본값 설정이 비즈니스 로직에 부합하는지 확인이 필요합니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (3)
13-13: 'Model' 모듈 임포트 확인되었습니다.'SMSSendResponse' 타입을 사용하기 위해 'Model' 모듈을 임포트한 것으로 보입니다.
27-27: 'externalData' 초기화가 적절합니다.'AuthPhoneVerifyIntent'에 'smsResponse'를 전달하여 필요한 외부 데이터를 주입하는 것이 확인되었습니다.
91-93: 네비게이션 동작 확인 요청
.setPopNavigation을 사용하여AppCoordinator.shared.pop()을 호출하고 있습니다. 이 변경 사항이 의도한 대로 네비게이션 동작을 수행하는지 확인 부탁드립니다.OpenApiGenerator/Sources/OpenapiGenerated/Client.swift (1)
46-46: 엔드포인트 변경에 따른 영향 검토 필요
requestVerification메소드의 엔드포인트가/users/verifications에서/auth/codes로 변경되었습니다. 이 변경이 코드베이스 전반에 걸쳐 일관되게 적용되었는지 확인해주세요.다음 스크립트를 사용하여 이전 엔드포인트를 참조하는 부분이 있는지 확인할 수 있습니다:
| public struct ExistingUserVerificationResponse { | ||
| public let refreshToken: String | ||
| public let accessToken: String |
There was a problem hiding this comment.
💡 Codebase verification
파일 이름 불일치 문제 수정 권장
파일 이름이 ExistingUserVerificationResponse 구조체 이름과 일치하지 않아 혼동을 줄 수 있습니다. 파일 이름을 ExistingUserVerificationResponse.swift로 변경하는 것을 권장합니다.
🔗 Analysis chain
구조체 이름 변경 및 속성 접근 수준 변경에 대한 검토
구조체 이름이 SMSVerificationResponse에서 ExistingUserVerificationResponse로 변경되었습니다. 이는 구조체의 목적이나 범위가 변경되었음을 시사합니다.
또한, refreshToken과 accessToken 속성의 접근 수준이 public으로 변경되었습니다. 이는 모듈 외부에서 이 속성들에 직접 접근할 수 있게 되었음을 의미합니다.
다음 스크립트를 실행하여 이 변경이 다른 파일에 미치는 영향을 확인해주세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 구조체 이름 변경과 관련된 참조 확인
# Test 1: 이전 구조체 이름 사용 확인
echo "Checking for uses of old struct name 'SMSVerificationResponse':"
rg --type swift "SMSVerificationResponse"
# Test 2: 새 구조체 이름 사용 확인
echo "Checking for uses of new struct name 'ExistingUserVerificationResponse':"
rg --type swift "ExistingUserVerificationResponse"
# Test 3: refreshToken과 accessToken 속성 사용 확인
echo "Checking for uses of 'refreshToken' and 'accessToken':"
rg --type swift "\.refreshToken|\.accessToken"
Length of output: 7292
| public static func == (lhs: SignUpSubViewType, rhs: SignUpSubViewType) -> Bool { | ||
| return lhs.hashValue == rhs.hashValue | ||
| } |
There was a problem hiding this comment.
[수정 필요] Equatable 구현 시 '==' 연산자에서 'hashValue' 비교 대신 실제 값 비교 필요
Equatable 프로토콜을 구현할 때 == 연산자에서 hashValue를 비교하는 것은 권장되지 않습니다. hashValue는 유일성을 보장하지 않으며, 동일하지 않은 객체가 동일한 해시 값을 가질 수 있습니다. 대신 각 enum 케이스와 그 연관된 값들을 직접 비교하여 정확한 동등성 검사를 구현해야 합니다.
| try await client.send( | ||
| input: input, | ||
| forOperation: Operations.searchCompanies.id, | ||
| serializer: { input in | ||
| let path = try converter.renderedPath( | ||
| template: "/companies", | ||
| parameters: [] | ||
| ) | ||
| var request: HTTPTypes.HTTPRequest = .init( | ||
| soar_path: path, | ||
| method: .get | ||
| ) | ||
| suppressMutabilityWarning(&request) | ||
| try converter.setQueryItemAsURI( | ||
| in: &request, | ||
| style: .form, | ||
| explode: true, | ||
| name: "name", | ||
| value: input.query.name | ||
| ) | ||
| try converter.setQueryItemAsURI( | ||
| in: &request, | ||
| style: .form, | ||
| explode: true, | ||
| name: "next", | ||
| value: input.query.next | ||
| ) | ||
| try converter.setQueryItemAsURI( | ||
| in: &request, | ||
| style: .form, | ||
| explode: true, | ||
| name: "limit", | ||
| value: input.query.limit | ||
| ) | ||
| converter.setAcceptHeader( | ||
| in: &request.headerFields, | ||
| contentTypes: input.headers.accept | ||
| ) | ||
| return (request, nil) | ||
| }, | ||
| deserializer: { response, responseBody in | ||
| switch response.status.code { | ||
| case 200: | ||
| let contentType = converter.extractContentTypeIfPresent(in: response.headerFields) | ||
| let body: Operations.searchCompanies.Output.Ok.Body | ||
| let chosenContentType = try converter.bestContentType( | ||
| received: contentType, | ||
| options: [ | ||
| "application/json" | ||
| ] | ||
| ) | ||
| switch chosenContentType { | ||
| case "application/json": | ||
| body = try await converter.getResponseBodyAsJSON( | ||
| Components.Schemas.SearchCompaniesResponse.self, | ||
| from: responseBody, | ||
| transforming: { value in | ||
| .json(value) | ||
| } | ||
| ) | ||
| default: | ||
| preconditionFailure("bestContentType chose an invalid content type.") | ||
| } | ||
| return .ok(.init(body: body)) | ||
| case 500: | ||
| let contentType = converter.extractContentTypeIfPresent(in: response.headerFields) | ||
| let body: Components.Responses.InternalServerError.Body | ||
| let chosenContentType = try converter.bestContentType( | ||
| received: contentType, | ||
| options: [ | ||
| "application/json" | ||
| ] | ||
| ) | ||
| switch chosenContentType { | ||
| case "application/json": | ||
| body = try await converter.getResponseBodyAsJSON( | ||
| Components.Schemas.ErrorResponse.self, | ||
| from: responseBody, | ||
| transforming: { value in | ||
| .json(value) | ||
| } | ||
| ) | ||
| default: | ||
| preconditionFailure("bestContentType chose an invalid content type.") | ||
| } | ||
| return .internalServerError(.init(body: body)) | ||
| default: | ||
| return .undocumented( | ||
| statusCode: response.status.code, | ||
| .init() | ||
| ) | ||
| } | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복 코드를 줄이기 위한 리팩토링 제안
searchCompanies 메소드에서도 searchLocations와 유사한 쿼리 파라미터 처리 및 응답 처리가 이루어지고 있습니다. 공통된 로직을 추출하여 재사용함으로써 코드의 중복을 줄이고 유지보수성을 향상시킬 수 있습니다.
| let response = try await AuthService.requestSendSMS(phone: editedPhone) | ||
| await pushNextView(smsResponse: response) | ||
| } catch { | ||
| // TODO: Error 타입 정의 |
There was a problem hiding this comment.
TODO 주석을 처리하세요.
// TODO: Error 타입 정의 주석이 있습니다. 오류 처리를 위해 Error 타입을 정의하고 구체적인 예외 처리를 구현하는 것이 좋습니다.
Error 타입 정의와 오류 처리 개선을 도와드릴까요?
🧰 Tools
🪛 SwiftLint
[Warning] 69-69: TODOs should be resolved (Error 타입 정의)
(todo)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (25)
Tuist/ProjectDescriptionHelpers/TestableTarget+extensions.swift (1)
1-13: 구현이 잘 되었습니다. 몇 가지 개선 사항을 고려해 보세요.이 확장은
TestableTarget을 생성하는 편리한 방법을 제공합니다. 구현이 명확하고 기존 메서드를 잘 활용하고 있습니다.개선을 위해 다음 사항을 고려해 보세요:
TargetName매개변수의 유효성을 검사하는 로직을 추가하는 것이 좋습니다.- 메서드에 대한 문서 주석(documentation comment)을 추가하여 사용 방법과 목적을 명확히 설명하는 것이 좋습니다.
- 오류 처리를 개선하여 잘못된 입력에 대해 더 명확한 피드백을 제공할 수 있습니다.
이러한 개선 사항에 대한 구현 예시가 필요하시면 말씀해 주세요.
Projects/Core/Model/Sources/Network/ExistingUserVerificationResponse.swift (1)
11-13: 구조체 선언과 프로퍼티가 적절합니다. 보안 개선 제안.
ExistingUserVerificationResponse구조체와 그 프로퍼티들이 사용자 인증 응답을 나타내기에 적절합니다.보안을 강화하기 위해
String대신 더 안전한 타입을 사용하는 것을 고려해 보세요. 예를 들어:import Foundation public struct ExistingUserVerificationResponse { public let refreshToken: Data public let accessToken: Data public init(refreshToken: Data, accessToken: Data) { self.refreshToken = refreshToken self.accessToken = accessToken } }이렇게 하면 토큰이 메모리에서 더 안전하게 처리될 수 있습니다.
Projects/Core/NetworkKit/Sources/NetworkCore/ServiceClient.swift (1)
Line range hint
15-27:ServiceClient구현이 적절해 보입니다.
ServiceClient구조체의 구현이 전반적으로 잘 되어 있습니다. 싱글톤 패턴을 사용하여 애플리케이션 전체에서 일관된 클라이언트 인스턴스를 사용할 수 있도록 했고, 인증, 재시도, 로깅 등의 미들웨어를 포함하여 적절히 구성되어 있습니다.다만, 향후 개선을 위해 다음 사항을 고려해 보시기 바랍니다:
- 테스트 용이성: 싱글톤 패턴은 단위 테스트를 어렵게 만들 수 있습니다. 의존성 주입을 고려해 보세요.
- 구성 유연성: 서버 URL이나 미들웨어 구성을 외부에서 주입할 수 있도록 하면 더 유연한 구조가 될 수 있습니다.
- 에러 처리: 현재 구현에서 에러 처리 방식이 명확하지 않습니다. 구체적인 에러 처리 전략을 고려해 보세요.
이러한 개선 사항들은 코드의 유지보수성과 확장성을 높일 수 있습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputIntent.swift (1)
16-16: 속성 이름 변경에 대한 고려사항
externalData에서input으로 속성 이름을 변경했습니다. 이는 코드의 간결성을 높일 수 있지만,input이라는 이름이externalData보다 덜 구체적일 수 있습니다.속성의 목적과 내용을 더 명확하게 표현할 수 있는 이름을 고려해 보는 것은 어떨까요? 예를 들어,
inputData또는userInputData와 같은 이름이 데이터의 성격을 더 잘 나타낼 수 있습니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementIntent.swift (2)
16-16: 속성 이름 변경이 적절합니다.
externalData에서input으로의 이름 변경은 의미를 더 명확하게 만들어 코드의 가독성을 향상시킵니다. 이는 좋은 변경사항입니다.더 나아가, 이 속성의 목적을 더 구체적으로 설명하는 이름을 고려해 볼 수 있습니다. 예를 들어,
agreementInput과 같이 말입니다. 이렇게 하면 이 데이터가 동의 프로세스와 관련된 것임을 더 명확히 할 수 있습니다.
23-23: 매개변수 사용이 올바르게 업데이트되었습니다.초기화 로직에서
input매개변수의 사용이 정확하게 반영되었습니다. 이는 이전 변경사항들과 일관성을 유지합니다.향후 개선사항으로, 입력 데이터의 유효성을 검사하는 로직을 추가하는 것을 고려해 보세요. 예를 들어:
guard input.isValid else { throw InvalidInputError.invalidAgreementData } self.input = input이렇게 하면 잘못된 데이터가 객체에 할당되는 것을 방지할 수 있습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputIntent.swift (1)
16-16: 변수명 변경이 적절합니다.
externalData에서input으로의 변경은 더 명확하고 일반적인 용어를 사용하여 코드의 가독성을 향상시킵니다.추가적인 개선을 위해
DataModel의 용도를 더 명확히 하는 이름을 고려해 보는 것은 어떨까요? 예를 들어,inputData또는profileInputData와 같이 말입니다.Projects/Features/SignUp/UnitTest/AuthPhoneInputTests.swift (3)
14-32: 클래스 구조와 설정/해제 메서드가 잘 구현되었습니다.테스트 클래스 구조와
setUp()/tearDown()메서드가 XCTest 규칙을 잘 따르고 있습니다. 각 테스트 전후로 상태를 초기화하고 정리하는 것이 좋습니다.개선 제안:
tearDown()메서드에서super.tearDown()을 먼저 호출하는 것이 좋습니다. 이렇게 하면 부모 클래스의 정리 작업이 마지막에 실행되어 더 안전합니다.override func tearDown() { + super.tearDown() model = nil intent = nil - super.tearDown() }
34-44:testOnChangePhoneInput()메서드가 잘 구현되었습니다.이 테스트 메서드는 유효한 번호와 유효하지 않은 번호 모두를 검사하여 긍정적인 케이스와 부정적인 케이스를 모두 다루고 있어 좋습니다.
가독성 개선을 위해 각 테스트 케이스를 분리하고 설명 주석을 추가하는 것이 좋겠습니다. 예를 들어:
func testOnChangePhoneInput() { // 유효한 전화번호 테스트 let validPhone = "01012345678" intent.onChangePhoneInput(phone: validPhone) XCTAssertTrue(model.isPhoneValidated, "유효한 번호는 검증을 통과해야 합니다") // 유효하지 않은 전화번호 테스트 let invalidPhone = "010" intent.onChangePhoneInput(phone: invalidPhone) XCTAssertFalse(model.isPhoneValidated, "유효하지 않은 번호는 검증을 통과하지 말아야 합니다") }
46-52:testOnTapNextButton()메서드의 기본 기능은 좋지만, 비동기 테스트를 개선할 수 있습니다.이 메서드는 다음 버튼을 탭했을 때의 기본 동작을 잘 테스트하고 있습니다. 하지만
async키워드를 사용하고 있음에도 실제로 비동기 동작을 테스트하지 않고 있습니다.비동기 동작을 더 잘 테스트하기 위해 다음과 같은 개선을 제안합니다:
AuthServiceMock에 비동기 동작을 시뮬레이션하는 기능을 추가합니다.onTapNextButton메서드가 완료될 때까지 기다리도록await키워드를 사용합니다.- 로딩 상태의 변화를 확인합니다.
예시 코드:
func testOnTapNextButton() async throws { let validPhone = "01012345678" intent.onChangePhoneInput(phone: validPhone) XCTAssertFalse(model.isLoading, "버튼 탭 전에는 로딩 상태가 아니어야 합니다") let expectation = XCTestExpectation(description: "다음 버튼 탭 완료") Task { await intent.onTapNextButton(with: validPhone) expectation.fulfill() } XCTAssertTrue(model.isLoading, "버튼 탭 직후에는 로딩 상태여야 합니다") await fulfillment(of: [expectation], timeout: 5.0) XCTAssertFalse(model.isLoading, "작업 완료 후에는 로딩 상태가 해제되어야 합니다") }이렇게 하면 비동기 동작의 전체 생명주기를 더 정확하게 테스트할 수 있습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputIntent.swift (1)
51-51:onAppear메서드 내 변수 사용이 일관되게 변경되었습니다.
onAppear메서드에서externalData를input으로 변경한 것은 앞선 변경사항들과 일치하여 적절합니다. 기능은 그대로 유지되면서 변수명만 업데이트되었습니다.코드 스타일 개선을 위해 다음과 같이 수정하는 것을 고려해보세요:
- model?.setTargetGender(input.targetGender) + model?.setTargetGender(input.targetGender)이렇게 하면 코드의 일관성과 가독성이 더욱 향상될 것입니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthGreeting/AuthGreetingView.swift (1)
25-25: 매개변수 이름 변경이 적절합니다.
externalData에서input으로의 매개변수 이름 변경은 코드의 가독성을 향상시키는 좋은 변화입니다. 이는 아마도 다른 부분의 코드와의 일관성을 위해 수행된 것으로 보입니다.일관성을 더욱 높이기 위해,
AuthGreetingIntent의 다른 부분에서도 유사한 매개변수 이름을 사용하고 있는지 확인해보시는 것이 좋겠습니다. 예를 들어:let intent = AuthGreetingIntent( model: model, input: .init() )이런 식으로
model매개변수도input과 유사한 스타일로 변경할 수 있을지 고려해보세요. 예:modelInput또는stateInput.Projects/Features/SignUp/UnitTest/AuthPhoneVerifyTest.swift (5)
1-17: 파일 헤더와 임포트가 적절합니다.파일 헤더와 임포트 문이 잘 구성되어 있습니다. 단위 테스트에 필요한 모듈들이 적절히 임포트되어 있습니다.
MockingErrorenum의 정의도 테스트 목적에 부합합니다.다만,
MockingError에 대한 간단한 주석을 추가하면 코드의 가독성이 더욱 향상될 것 같습니다. 예를 들어:/// 테스트를 위한 모의 오류 enum MockingError: Error { case mockError }
19-43: 테스트 클래스 설정이 잘 되어 있습니다.
setUp과tearDown메서드가 적절히 구현되어 있어 각 테스트 전후로 객체들이 올바르게 초기화되고 정리됩니다. 모의 데이터를 사용하여 테스트하는 것도 좋은 방식입니다.테스트의 견고성을 높이기 위해,
tearDown메서드에서 모든 프로퍼티를 명시적으로 nil로 설정하는 것이 좋습니다. 예를 들어:override func tearDown() { model = nil intent = nil // 다른 프로퍼티들도 nil로 설정 super.tearDown() }이렇게 하면 메모리 누수를 방지하고 각 테스트 간의 독립성을 보장할 수 있습니다.
45-50: 검증 코드 변경 테스트가 구현되어 있습니다.
testOnChangeVerifyCode메서드가 유효한 6자리 코드 입력 시 모델의 로딩 상태를 올바르게 확인하고 있습니다.테스트의 완성도를 높이기 위해 다음과 같은 추가 테스트 케이스를 고려해보세요:
- 6자리 미만의 코드 입력 시 로딩 상태가 변경되지 않는지 확인
- 숫자가 아닌 문자가 포함된 코드 입력 시의 동작 확인
- 공백이나 null 입력 시의 동작 확인
이러한 추가 테스트를 통해 다양한 시나리오에 대한 코드의 견고성을 검증할 수 있습니다.
52-61: 포커스 상태 변경 및 화면 등장 테스트가 잘 구현되어 있습니다.
testOnChangeFocusState와testOnAppear메서드가 각각 텍스트 필드의 포커스 상태 변경과 화면 등장 시 포커스 설정을 올바르게 검증하고 있습니다. UI 상호작용의 중요한 시나리오들을 잘 커버하고 있습니다.테스트의 가독성을 높이기 위해 각 테스트 메서드에 간단한 주석을 추가하는 것이 좋겠습니다. 예를 들어:
/// 포커스 상태 변경이 올바르게 적용되는지 테스트 func testOnChangeFocusState() { // ... 현재 코드 ... } /// 화면 등장 시 텍스트 필드에 포커스가 설정되는지 테스트 func testOnAppear() { // ... 현재 코드 ... }이렇게 하면 각 테스트의 목적을 더 명확히 이해할 수 있습니다.
63-69: 검증 결과에 따른 경로 설정 테스트가 구현되어 있습니다.
testVerificationResult메서드가 기존 사용자와 신규 사용자에 대해 올바른 경로를 반환하는지 잘 검증하고 있습니다. 이는 사용자 인증 후 흐름에 대한 중요한 비즈니스 로직을 커버하고 있습니다.테스트의 완성도를 높이기 위해 다음과 같은 개선사항을 고려해보세요:
- 모든 가능한
userType값에 대한 테스트 케이스 추가- 잘못된
userType값이 주어졌을 때의 동작 검증getNextPath메서드의 반환 타입이PathType임을 명시적으로 확인예를 들어:
func testVerificationResult() { XCTAssertEqual(intent.getNextPath(userType: .EXISTING), PathType.main) XCTAssertEqual(intent.getNextPath(userType: .NEW), PathType.signUp(.authAgreement)) // 추가 테스트 케이스 XCTAssertEqual(intent.getNextPath(userType: .DELETED), PathType.signUp(.authAgreement)) // 가정 // 잘못된 userType에 대한 테스트 (예: enum case 추가 시 대비) let unknownUserType = UserType(rawValue: 999) // 존재하지 않는 rawValue XCTAssertEqual(intent.getNextPath(userType: unknownUserType!), PathType.error) // 가정된 에러 경로 }이러한 추가 테스트를 통해
getNextPath메서드의 동작을 더욱 철저히 검증할 수 있습니다.Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift (3)
18-27: AuthServiceProtocol이 잘 정의되었습니다. 주석 형식 수정 필요.AuthServiceProtocol이 SMS 인증 흐름에 필요한 메서드들을 잘 정의하고 있습니다. 하지만 18번 줄의 주석 형식이 SwiftLint 가이드라인을 따르지 않고 있습니다.
다음과 같이 주석 형식을 수정해주세요:
-//MARK: - Service Protocol +// MARK: - Service Protocol🧰 Tools
🪛 SwiftLint
[Warning] 18-18: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 18-18: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
29-33: AuthService 클래스가 적절히 정의되었습니다. 주석 형식 수정 필요.AuthService 클래스가 싱글톤 패턴을 사용하여 잘 정의되었습니다. 이는 서비스 클래스에 적합한 패턴입니다. 하지만 29번 줄의 주석 형식이 SwiftLint 가이드라인을 따르지 않고 있습니다.
다음과 같이 주석 형식을 수정해주세요:
-//MARK: - Service +// MARK: - Service🧰 Tools
🪛 SwiftLint
[Warning] 29-29: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 29-29: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
1-97: 전반적으로 잘 구현된 AuthService, 몇 가지 개선 사항 제안AuthService의 전체적인 구조와 구현이 적절하고 잘 조직되어 있습니다. SMS 인증 및 토큰 갱신 기능이 잘 구현되어 있으며, async/await를 사용한 비동기 처리 방식은 현대적이고 효율적입니다.
개선을 위해 다음 사항들을 고려해 보시기 바랍니다:
- 주석 형식: SwiftLint 가이드라인에 맞게 모든 "MARK" 주석을 수정해 주세요.
- 에러 처리: 현재 에러를 상위로 전파하는 방식은 View 계층에서 처리한다는 전략과 일치하지만, 필요하다면 더 구체적인 에러 타입을 정의하거나 로깅을 추가하는 것을 고려해 보세요.
- 토큰 관리: refreshAccessToken 메서드에서 토큰 갱신 실패 시의 처리를 더 구체화하는 것을 고려해 보세요.
이러한 개선 사항들을 적용하면 코드의 가독성과 유지보수성이 더욱 향상될 것입니다.
🧰 Tools
🪛 SwiftLint
[Warning] 18-18: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 29-29: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 76-76: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 18-18: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 29-29: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 76-76: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (2)
23-27: 이니셜라이저 변경 승인 및 개선 제안SMS 기능 통합을 위한 이니셜라이저 변경이 적절히 이루어졌습니다. 'SMSSendResponse' 파라미터 추가는 PR 목표에 부합합니다.
하지만 이전 리뷰 의견을 반영하여, 파라미터 레이블을 명시적으로 추가하는 것이 좋겠습니다:
public init(smsResponse: SMSSendResponse) { // ... 현재 구현 ... }이렇게 하면 함수 호출 시 가독성과 명확성이 향상됩니다.
91-93: 네비게이션 처리 변경 승인 및 고려사항'AppCoordinator.shared.pop()'을 사용한 네비게이션 처리 변경은 적절합니다. 이는 네비게이션 로직을 중앙화하여 유지보수성을 향상시킵니다.
다만, 다음 사항을 고려해 보시기 바랍니다:
- 'AppCoordinator'가 전역 상태를 관리하므로, 테스트 용이성에 영향을 줄 수 있습니다.
- 뷰의 재사용성이 'AppCoordinator'에 의존하게 됩니다.
필요하다면, 의존성 주입을 통해 이러한 잠재적 문제를 완화할 수 있습니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift (2)
81-104: requestVerification 메서드 승인 및 개선 제안새 사용자와 기존 사용자에 대한 검증 처리가 잘 구조화되어 있습니다.
TokenManager업데이트도 적절합니다.다음과 같은 개선을 고려해 보세요:
- 각 케이스에 대한 로깅 추가
- 토큰 저장 실패에 대한 오류 처리
예시:
func requestVerification(code: String) async throws { switch input.smsResponse.userType { case .NEW: let registerToken = try await authService.requestNewUserVerifyCode( .init( verificationId: input.smsResponse.authCodeId, verificationCode: code ) ) Logger.log("New user verified successfully") do { try TokenManager.saveRegisterToken(registerToken) } catch { Logger.error("Failed to save register token: \(error)") throw error } case .EXISTING: // Similar changes for EXISTING case } }
15-15: 코드 스타일 개선 제안정적 분석 도구에서 제안한 몇 가지 minor 이슈가 있습니다. 기능에는 영향을 미치지 않지만, 코드의 일관성과 가독성을 향상시킬 수 있습니다.
다음 사항을 수정해 주세요:
- 주석 슬래시 뒤에 공백을 추가하세요. 예:
// MARK: - ...- MARK 주석 형식을 수정하세요. 예:
// MARK: - Intent또는// MARK: Intent이러한 변경은 SwiftLint 경고를 해결하고 코드 스타일의 일관성을 유지하는 데 도움이 됩니다.
Also applies to: 51-51
🧰 Tools
🪛 SwiftLint
[Warning] 15-15: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 15-15: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift (1)
98-98: 디버그용프로덕션 코드에서는 디버그를 위한
적용 가능한 수정 사항:
- print(#function)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (35)
- Projects/Core/Model/Sources/Network/ExistingUserVerificationResponse.swift (1 hunks)
- Projects/Core/Model/Sources/temp.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/AuthService/AuthServiceMock.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/ExampleService/ExampleEndpoint.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/EndpointType.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Interceptor.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Logger.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/RetryingMiddleware.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Provider.swift (0 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/ServiceClient.swift (1 hunks)
- Projects/Core/Project.swift (0 hunks)
- Projects/Features/Project.swift (1 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementIntent.swift (1 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementView.swift (2 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (3 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift (2 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift (3 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyModel.swift (4 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (4 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthGreeting/AuthGreetingIntent.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthGreeting/AuthGreetingView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputIntent.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift (2 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputIntent.swift (2 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputIntent.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputView.swift (2 hunks)
- Projects/Features/SignUp/UnitTest/AuthPhoneInputTests.swift (1 hunks)
- Projects/Features/SignUp/UnitTest/AuthPhoneVerifyTest.swift (1 hunks)
- Tuist/Package.resolved (0 hunks)
- Tuist/Package.swift (0 hunks)
- Tuist/ProjectDescriptionHelpers/Dependency+extensions.swift (0 hunks)
- Tuist/ProjectDescriptionHelpers/TestableTarget+extensions.swift (1 hunks)
- Workspace.swift (1 hunks)
💤 Files with no reviewable changes (10)
- Projects/Core/Model/Sources/temp.swift
- Projects/Core/NetworkKit/Sources/ExampleService/ExampleEndpoint.swift
- Projects/Core/NetworkKit/Sources/NetworkCore/EndpointType.swift
- Projects/Core/NetworkKit/Sources/NetworkCore/Interceptor.swift
- Projects/Core/NetworkKit/Sources/NetworkCore/Logger.swift
- Projects/Core/NetworkKit/Sources/NetworkCore/Provider.swift
- Projects/Core/Project.swift
- Tuist/Package.resolved
- Tuist/Package.swift
- Tuist/ProjectDescriptionHelpers/Dependency+extensions.swift
🚧 Files skipped from review as they are similar to previous changes (5)
- Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/RetryingMiddleware.swift
- Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementView.swift
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyModel.swift
- Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputView.swift
🧰 Additional context used
📓 Learnings (1)
Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift (1)
Learnt from: jisu15-kim PR: Student-Center/3days-ios#28 File: Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift:19-32 Timestamp: 2024-10-09T02:56:24.121Z Learning: AuthService의 `requestSendSMS` 메서드 등에서 API 호출에 대한 에러 처리와 로깅은 View 계층에서 수행됩니다.
🪛 SwiftLint
Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift
[Warning] 18-18: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 29-29: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 76-76: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 18-18: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 29-29: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 76-76: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
[Warning] 15-15: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 33-33: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 15-15: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 33-33: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 72-72: TODOs should be resolved (Error 타입 정의)
(todo)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift
[Warning] 15-15: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 15-15: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 51-51: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 51-51: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
🔇 Additional comments (34)
Projects/Core/Model/Sources/Network/ExistingUserVerificationResponse.swift (3)
1-8: 파일 헤더가 적절합니다.파일 헤더에는 필요한 정보가 모두 포함되어 있으며, 표준 관행을 따르고 있습니다.
9-9: Foundation 임포트가 적절합니다.Foundation 프레임워크의 임포트는 적절하며, 많은 Swift 프로젝트에서 필요한 표준 라이브러리입니다.
15-18: 이니셜라이저가 적절합니다.공개 이니셜라이저가 올바르게 구현되어 있으며, 매개변수를 해당 프로퍼티에 정확히 할당하고 있습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputIntent.swift (2)
21-23: 초기화 메서드 매개변수 이름 변경 승인초기화 메서드의 매개변수 이름을
externalData에서input으로 변경한 것은 앞서 변경된 속성 이름과 일치하여 일관성을 유지하고 있습니다. 이는 좋은 변경사항입니다.이 변경으로 코드의 일관성이 향상되었습니다. 속성 이름과 초기화 매개변수 이름을 일치시키는 것은 좋은 관행입니다.
16-23: 전체적인 변경 사항 요약이 파일에서의 주요 변경 사항은
externalData를input으로 이름을 변경한 것입니다. 이는 클래스 속성과 초기화 메서드 매개변수 모두에 적용되었습니다.이러한 변경으로 코드의 일관성이 개선되었습니다. 그러나
input이라는 이름이 다소 일반적일 수 있으므로, 필요에 따라 더 구체적인 이름을 고려해 볼 수 있습니다. 전반적으로 클래스의 기능과 구조는 그대로 유지되었으며, 이 변경으로 인한 부정적인 영향은 없어 보입니다.Projects/Features/Project.swift (1)
22-27: 단위 테스트 대상 추가에 대한 승인
signUp기능에 대한 단위 테스트 대상을 추가한 것은 매우 긍정적인 변경사항입니다. 이는 다음과 같은 이점을 제공합니다:
signUp기능에 대한 전용 테스트를 가능하게 합니다.- 코드 품질과 신뢰성을 향상시킵니다.
- 향후 기능 변경 시 회귀 테스트를 용이하게 합니다.
또한,
.project(target: .signUp)에 대한 종속성 설정이 올바르게 되어 있어 테스트 코드가 테스트 대상 코드에 접근할 수 있습니다.Projects/Features/SignUp/Sources/ProfileInput/AuthGreeting/AuthGreetingIntent.swift (1)
16-16: 변수명 변경이 일관성 있게 적용되었습니다.
externalData에서input으로의 변수명 변경이 클래스 속성, 초기화 메서드 매개변수, 그리고 초기화 로직에서 일관되게 적용되었습니다. 이는 코드의 가독성과 유지보수성을 향상시킵니다.변경된 이름인
input은 더 간결하고 일반적인 용어로, 데이터의 용도를 잘 나타내고 있습니다. 이는 좋은 변경으로 보입니다.Also applies to: 21-21, 23-23
Projects/Core/NetworkKit/Sources/AuthService/AuthServiceMock.swift (2)
1-11: LGTM: 파일 헤더와 임포트가 적절합니다.파일 헤더는 표준 정보를 포함하고 있으며, 임포트된 모듈들은 모의 서비스 구현에 적합해 보입니다.
13-14: LGTM: 클래스 선언과 초기화 메서드가 적절합니다.
AuthServiceMock클래스가public으로 선언되어 있고AuthServiceProtocol을 준수하고 있습니다. 매개변수가 없는public초기화 메서드는 테스트에서 쉽게 인스턴스화할 수 있게 해줍니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthAgreement/AuthAgreementIntent.swift (1)
21-21: 초기화 매개변수 이름 변경이 일관성 있게 적용되었습니다.
externalData에서input으로의 매개변수 이름 변경은 앞서 변경된 속성 이름과 일치하며, 코드의 일관성을 유지합니다. 이는 좋은 리팩토링 사례입니다.Projects/Features/SignUp/Sources/ProfileInput/AuthProfileGenderInput/AuthProfileGenderInputIntent.swift (2)
21-21: 초기화 매개변수 변경이 일관성 있게 적용되었습니다.
externalData에서input으로의 매개변수 이름 변경이 속성 이름 변경과 일치하여 적절히 적용되었습니다. 이는 코드의 일관성을 유지하는 데 도움이 됩니다.
23-23: 속성 할당이 올바르게 업데이트되었습니다.초기화 메서드에서
input속성에 대한 할당이 정확하게 변경되었습니다. 이는 앞서 변경된 매개변수 이름과 일치합니다.다만,
DataModel이 옵셔널이 아닌 것으로 보입니다. 잠재적인 null 값이나 유효하지 않은 입력에 대한 처리가 필요할 수 있습니다. 이에 대한 오류 처리 로직이 다른 곳에 구현되어 있는지 확인해 보시기 바랍니다.Projects/Features/SignUp/UnitTest/AuthPhoneInputTests.swift (1)
1-12: LGTM: 파일 헤더와 임포트가 적절합니다.파일 헤더와 임포트 문이 잘 구성되어 있습니다.
@testable import SignUp을 사용하여 내부 컴포넌트를 테스트할 수 있도록 한 점이 좋습니다.Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputIntent.swift (2)
16-16: 변수명 변경이 적절합니다.
externalData에서input으로의 변수명 변경은 코드의 가독성을 향상시킵니다.input이라는 이름이 더 일반적이고 목적을 잘 나타내며, 다른 개발자들이 코드를 이해하기 쉽게 만듭니다.
21-21: 초기화 메서드의 매개변수명 변경이 일관성 있게 적용되었습니다.초기화 메서드의 매개변수명을
externalData에서input으로 변경한 것은 클래스 프로퍼티의 이름 변경과 일치하여 좋습니다. 이러한 일관성은 코드의 유지보수성을 향상시키고 오류 가능성을 줄입니다.Workspace.swift (1)
46-48: 테스트 대상 선언이 개선되었습니다.테스트 대상 선언을 단순화한 것은 좋은 변경사항입니다. 코드의 가독성과 유지보수성이 향상되었습니다.
다만, 몇 가지 확인이 필요합니다:
.testableTarget에서.target으로 변경한 것이 각 모듈의 테스트 가능성에 영향을 주지 않는지 확인해주세요.- 새로 추가된
.signUp타겟의 목적과 필요성에 대해 설명해주실 수 있나요?다음 스크립트를 실행하여 변경된 타겟들이 여전히 테스트 가능한지 확인해주세요:
✅ Verification successful
[확인 중입니다.]
[확인 중입니다.]
테스트 가능성이 유지되고 있습니다.
변경된 타겟들이 여전히 테스트 가능한 것으로 확인되었습니다. 테스트 파일들이 적절한 위치에 존재하며,
XCTest임포트도 정상적으로 발견되었습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the changed targets are still testable # Test: Check if the targets have test-related files echo "Checking for test files in coreKit:" fd -e swift -p 'Tests?$' -d 3 coreKit echo "Checking for test files in designCore:" fd -e swift -p 'Tests?$' -d 3 designCore echo "Checking for test files in signUp:" fd -e swift -p 'Tests?$' -d 3 signUp # Test: Check if the targets have XCTest imports echo "Checking for XCTest imports:" rg 'import\s+XCTest' -t swiftLength of output: 1104
Script:
#!/bin/bash # Description: Verify that the changed targets are still testable by searching recursively # Test: Find directories named 'coreKit', 'designCore', and 'signUp' anywhere in the repository echo "Searching for 'coreKit' directories:" fd -td d coreKit echo "Searching for 'designCore' directories:" fd -td d designCore echo "Searching for 'signUp' directories:" fd -td d signUp # Test: Check if the found directories contain test-related files echo "Checking for test files in 'coreKit' directories:" fd -e swift -p 'Tests?$' -d 3 coreKit echo "Checking for test files in 'designCore' directories:" fd -e swift -p 'Tests?$' -d 3 designCore echo "Checking for test files in 'signUp' directories:" fd -e swift -p 'Tests?$' -d 3 signUp # Test: Confirm XCTest imports in the identified test files echo "Checking for XCTest imports in test files:" rg 'import\s+XCTest' -t swiftLength of output: 1851
Script:
#!/bin/bash # Description: Verify that the changed targets are still testable with case-insensitive search # Test: Find directories named 'CoreKit', 'DesignCore', and 'SignUp' anywhere in the repository (case-insensitive) echo "Searching for 'CoreKit' directories (case-insensitive):" fd -td d -i corekit echo "Searching for 'DesignCore' directories (case-insensitive):" fd -td d -i designcore echo "Searching for 'SignUp' directories (case-insensitive):" fd -td d -i signup # Test: Check if the found directories contain test-related files echo "Checking for test files in 'CoreKit' directories:" fd -e swift -i -p 'Tests?$' -d 3 corekit echo "Checking for test files in 'DesignCore' directories:" fd -e swift -i -p 'Tests?$' -d 3 designcore echo "Checking for test files in 'SignUp' directories:" fd -e swift -i -p 'Tests?$' -d 3 signup # Test: Confirm XCTest imports in the identified test files echo "Checking for XCTest imports in test files:" rg 'import\s+XCTest' -t swiftLength of output: 1983
Projects/Core/NetworkKit/Sources/AuthService/AuthService.swift (4)
1-12: 파일 헤더와 임포트가 적절합니다.파일 헤더와 임포트 문이 인증 서비스에 적합하게 구성되어 있습니다. Foundation, Model, CoreKit의 임포트는 이 서비스의 기능을 구현하는 데 필요한 것으로 보입니다.
13-16: AuthEndpointError 열거형이 잘 정의되었습니다.인증 관련 오류를 처리하기 위한 AuthEndpointError 열거형이 적절하게 정의되었습니다. emptyToken과 tokenResponseNotValid 케이스는 인증 과정에서 발생할 수 있는 주요 오류 상황을 잘 표현하고 있습니다.
35-74: AuthService 프로토콜 구현이 적절합니다.AuthServiceProtocol에 정의된 세 가지 메서드가 올바르게 구현되었습니다. 각 메서드는 API 호출을 수행하고 응답을 적절히 처리하고 있습니다. 비동기 작업에 async/await를 사용한 것은 현대적이고 효율적인 접근 방식입니다. 에러 처리는 View 계층에서 수행된다는 이전 학습 내용과 일치하게 에러를 상위로 전파하고 있습니다.
76-97: AccessToken Refresh 구현이 적절합니다. 주석 형식 수정 및 에러 처리 확인 필요.AccessToken Refresh 기능이 잘 구현되어 있습니다. 저장된 refresh 토큰을 사용하여 access 토큰을 갱신하고, 성공 시 TokenManager를 업데이트하는 로직이 적절합니다. 하지만 다음 사항들을 고려해 주세요:
- 76번 줄의 주석 형식이 SwiftLint 가이드라인을 따르지 않고 있습니다. 다음과 같이 수정해 주세요:
-//MARK: - AccessToken Refresh +// MARK: - AccessToken Refresh
- 네트워크 오류나 유효하지 않은 응답에 대한 구체적인 처리가 없습니다. 이는 의도적인 설계일 수 있지만, 필요하다면 추가적인 에러 처리를 고려해 보시기 바랍니다.
네트워크 오류나 유효하지 않은 응답에 대한 추가적인 에러 처리가 필요한지 확인해 주시겠습니까?
🧰 Tools
🪛 SwiftLint
[Warning] 76-76: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 76-76: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (3)
13-13: LGTM: Model 모듈 import 추가'Model' 모듈의 import 추가는 'SMSSendResponse' 타입 사용을 위한 적절한 변경으로 보입니다.
59-59: 버튼 레이블 개선 승인버튼 레이블에 'state.sentPhoneNumber'를 사용한 변경은 적절합니다. 이는 사용자에게 더 구체적인 정보를 제공하여 사용자 경험을 향상시킵니다.
100-106: 프리뷰 코드 업데이트 승인'AuthPhoneVerifyView' 초기화에 'SMSSendResponse'를 포함하도록 프리뷰 코드를 업데이트한 것은 적절합니다. 이 변경으로 프리뷰가 현재 구현을 정확히 반영하게 되었으며, 이전 리뷰에서 제기된 문제도 해결되었습니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (3)
25-25: 초기화 매개변수 변경 확인 필요
AuthPhoneInputIntent의input매개변수가 빈 인스턴스로 초기화되도록 변경되었습니다. 이는 코드를 개선한 것으로 보입니다.이 변경이
AuthPhoneInputIntent의 동작에 영향을 미치지 않는지 확인해 주세요. 특히 이전에 "temp" 문자열이 사용되었던 부분에서 문제가 없는지 검증이 필요합니다.
56-56: 메서드 호출 개선
onTapNextButton메서드 호출 시state.phoneInputText를 인자로 전달하도록 변경된 것은 좋은 개선입니다. 이는 데이터 흐름을 더 명확하게 만듭니다.
AuthPhoneInputIntent의onTapNextButton메서드 문서를 업데이트하여 새로운 매개변수를 반영하는 것이 좋겠습니다.
68-70: 네비게이션 처리 방식 변경네비게이션 팝 처리를 클로저 기반 접근 방식으로 변경한 것은 좋은 개선입니다. 이는 네비게이션 처리의 유연성을 높입니다.
이 접근 방식이 앱 전체에서 일관되게 사용되고 있는지 확인해 주세요. 또한,
AppCoordinator.shared.pop()이 모든 상황에서 적절하게 동작하는지 검증이 필요합니다.Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyIntent.swift (7)
12-13: 가져오기 및 클래스 정의 변경 승인새로운 가져오기와 클래스 속성 변경이 적절하게 이루어졌습니다.
externalData를input으로 변경한 것은 이전 리뷰 의견을 반영한 것으로 보입니다. 이는 코드의 일관성을 향상시킵니다.Also applies to: 18-19
24-25: 초기화 메서드 수정 승인초기화 메서드의 매개변수 이름 변경과 새로운
service매개변수 추가는 클래스 속성 변경과 일치하며, 초기화의 유연성을 향상시킵니다. 기본값을 제공하는 것은 좋은 접근 방식입니다.Also applies to: 27-27, 29-29
46-48: DataModel 구조체 개선 승인
DataModel구조체에smsResponse속성을 추가한 것은 좋은 변경입니다. 이는 데이터 모델에 더 많은 컨텍스트와 구조를 제공하여 타입 안전성과 코드 명확성을 향상시킵니다.
56-58: onAppear 메서드 개선 승인
onAppear메서드에setInitialPhoneNumber를 호출하여 SMS 응답의 전화번호로 초기화하는 것은 사용자 경험을 향상시키는 좋은 방법입니다. 이는 사용자가 번호를 다시 입력할 필요가 없게 만듭니다.
106-116: processError 메서드 승인오류 처리를 중앙화하고 UI 상태를 재설정하며 사용자에게 오류 알림을 표시하는 방식이 잘 구현되어 있습니다. 이는 일관된 오류 처리와 사용자 경험 향상에 도움이 됩니다.
119-124: getNextPath 메서드 승인사용자 유형에 따라 다음 경로를 결정하는 방식이 명확하고 간결합니다. 이는 코드의 가독성과 유지보수성을 향상시킵니다.
127-128: pushNextView 메서드 개선 승인
PathType매개변수를 받도록 변경된 것은 메서드의 유연성을 높이고 새로운 내비게이션 접근 방식과 일관성을 유지합니다.AppCoordinator를 사용하여 내비게이션을 처리하는 것은 좋은 방식입니다.Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputView.swift (1)
27-27: 매개변수 이름 변경이 적절해 보입니다.매개변수 이름을
externalData에서input으로 변경한 것이 확인되었습니다. 이 변경은 코드의 가독성을 향상시킬 수 있습니다.다만, 이 변경이
AuthProfileAgeInputIntent의 정의와 일치하는지 확인해 주시기 바랍니다. 다음 스크립트를 실행하여AuthProfileAgeInputIntent의 정의를 확인할 수 있습니다:
| func testErrorReceivced() { | ||
| intent.processError(error: MockingError.mockError) | ||
| XCTAssertEqual(model.verifyCode, "") | ||
| XCTAssertEqual(model.isLoading, false) | ||
| XCTAssertEqual(model.verifyTextFieldFocused, true) | ||
| XCTAssertNotNil(model.showErrorAlert) | ||
|
|
||
| let inputCode = "123" | ||
| intent.onChangeVerifyCode(code: inputCode) | ||
| XCTAssertNil(model.showErrorAlert) | ||
| } |
There was a problem hiding this comment.
오류 처리 메커니즘에 대한 테스트가 구현되어 있습니다.
testErrorReceivced 메서드가 오류 발생 시 모델의 여러 상태를 적절히 검증하고 있습니다. 오류 후 검증 코드 초기화, 로딩 상태 변경, 텍스트 필드 포커스, 오류 알림 표시 등 중요한 측면들을 커버하고 있습니다.
메서드 이름에 오타가 있습니다. "Receivced"를 "Received"로 수정해주세요.
- func testErrorReceivced() {
+ func testErrorReceived() {오류 처리의 견고성을 높이기 위해 다음과 같은 추가 테스트 시나리오를 고려해보세요:
- 다양한 유형의 오류(네트워크 오류, 서버 오류 등)에 대한 처리 검증
- 연속적인 오류 발생 시의 동작 확인
- 오류 발생 후 정상 동작으로의 복구 과정 검증
예를 들어:
func testMultipleErrorScenarios() {
// 네트워크 오류 시나리오
intent.processError(error: NSError(domain: NSURLErrorDomain, code: NSURLErrorNotConnectedToInternet, userInfo: nil))
XCTAssertNotNil(model.showErrorAlert)
XCTAssertEqual(model.errorMessage, "인터넷 연결을 확인해주세요.") // 가정된 에러 메시지
// 서버 오류 시나리오
intent.processError(error: NSError(domain: "ServerErrorDomain", code: 500, userInfo: nil))
XCTAssertNotNil(model.showErrorAlert)
XCTAssertEqual(model.errorMessage, "서버 오류가 발생했습니다. 잠시 후 다시 시도해주세요.") // 가정된 에러 메시지
// 연속 오류 후 정상 복구 시나리오
intent.onChangeVerifyCode(code: "123456")
XCTAssertNil(model.showErrorAlert)
XCTAssertTrue(model.isLoading)
}이러한 추가 테스트를 통해 다양한 오류 상황에서의 애플리케이션 동작을 더욱 철저히 검증할 수 있습니다.
| model?.onChangedUserInput(value: code) | ||
| if code.count == 6 { | ||
| model?.setLoading(status: true) | ||
| Task { | ||
| await pushNextView() | ||
| do { | ||
| try await requestVerification(code: code) | ||
| let nextPath = getNextPath(userType: input.smsResponse.userType) | ||
| await pushNextView(to: nextPath) | ||
| } catch { | ||
| processError(error: error) | ||
| } |
There was a problem hiding this comment.
onChangeVerifyCode 메서드 개선 승인 및 제안
검증 프로세스 처리를 위한 비동기 작업 추가와 오류 처리는 좋은 개선입니다. 그러나 성공 시에도 로딩 상태를 해제해야 합니다.
다음과 같이 수정하는 것이 좋습니다:
func onChangeVerifyCode(code: String) {
model?.onChangedUserInput(value: code)
if code.count == 6 {
model?.setLoading(status: true)
Task {
do {
defer {
model?.setLoading(status: false)
}
try await requestVerification(code: code)
let nextPath = getNextPath(userType: input.smsResponse.userType)
await pushNextView(to: nextPath)
} catch {
processError(error: error)
}
}
}
}이렇게 하면 성공이나 실패와 관계없이 로딩 상태가 항상 해제됩니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
Show resolved
Hide resolved
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| //MARK: - Intentable | ||
| extension AuthPhoneInputIntent { | ||
| protocol Intentable { | ||
| // content | ||
| func onTapNextButton() | ||
| func onTapNextButton(with Phone: String) |
There was a problem hiding this comment.
매개변수 이름을 소문자로 수정하세요.
Swift의 명명 규칙에 따르면, 매개변수 이름은 lowerCamelCase를 사용해야 합니다. 따라서 with Phone: String을 with phone: String으로 수정하는 것이 좋습니다.
적용 가능한 수정 사항:
- func onTapNextButton(with Phone: String)
+ func onTapNextButton(with phone: String)📝 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.
| func onTapNextButton(with Phone: String) | |
| func onTapNextButton(with phone: String) |
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
Show resolved
Hide resolved
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneIntent.swift
Show resolved
Hide resolved
…hPhoneIntent.swift Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
구현사항
고민(선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
문서화