Conversation
[REFAC] 리팩토링 및 토큰 로직 수정
Walkthrough인증 로그아웃 흐름을 개선하고, 네트워크 오류 처리를 강화하며, UI 접근 제어를 확대하고, 사용자 데이터 매핑을 안전한 enum 기반 파싱으로 변경했습니다. 또한 문자열 리소스를 업데이트하고 대시보드 레이아웃을 조정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RootView
participant AuthVM as AuthViewModel
participant Interceptor as AuthRequestInterceptor
participant Network as NetworkManager
rect rgb(200, 220, 255)
note right of User: Token Expired Scenario
User->>Network: API Request
Network->>Interceptor: Check/Adapt Request
Interceptor->>Network: Forward Request
Network->>Network: Failure (401)
Network->>Interceptor: Retry with Refresh
Interceptor->>Network: POST /auth/refresh
Network->>Network: Refresh Success
Interceptor->>AuthVM: Posts didRequestLogout (if retry count exceeded)
end
rect rgb(220, 250, 220)
note right of AuthVM: Logout Reactions
AuthVM->>RootView: isLoggedIn = false
RootView->>RootView: Reset LoginUiState
RootView->>RootView: Hide SignUp View
end
rect rgb(250, 240, 200)
note right of AuthVM: Navigation to Login
AuthVM->>RootView: shouldNavigateToLogin = true
RootView->>RootView: Hide SignUp
RootView->>AuthVM: Call resetNavigationState()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
33Auto-Bot
requested review from
CHOOSLA,
Lee-Jong-Jin,
taemin3,
vivivim and
yangjiseonn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
SampoomManagement/Features/Order/UI/OrderDetailContent.swift (1)
159-162: 색상 참조 패턴 불일치 및 불필요한 뷰 래핑이 코드에는 두 가지 개선 가능한 부분이 있습니다:
색상 참조 불일치: 라인 160에서
.textSecondary를 직접 사용하고 있지만, 파일의 다른 부분(라인 68, 79, 100, 143)에서는Color("TextSecondary")패턴을 일관되게 사용하고 있습니다. 일관성을 위해 동일한 패턴을 사용하는 것이 좋습니다.불필요한 VStack 래퍼: 단일 Divider를 VStack으로 감쌀 필요가 없습니다. 이는 불필요한 뷰 계층을 추가합니다.
다음 diff를 적용하여 코드를 간소화하고 일관성을 개선할 수 있습니다:
- VStack { - Divider().background(.textSecondary) - } - .padding(.horizontal, 16) + Divider() + .background(Color("TextSecondary")) + .padding(.horizontal, 16)SampoomManagement/Features/Dashboard/UI/DashboardView.swift (1)
197-204: 권한 로직을 중앙화하는 것을 권장합니다권한 체크 로직이 뷰 레이어에 구현되어 있습니다. 이러한 비즈니스 로직은 재사용성과 유지보수성을 위해 중앙화하는 것이 좋습니다.
다음 위치 중 하나로 이동을 고려해보세요:
UserPositionenum에 computed property로 추가 (예:var isManagerOrAbove: Bool)- 별도의 도메인 서비스나 유틸리티로 분리
- ViewModel로 이동
예시: UserPosition enum에 추가하는 방법
// UserPosition enum에 추가 extension UserPosition { var isManagerOrAbove: Bool { switch self { case .manager, .deputyGeneralManager, .generalManager, .director, .vicePresident, .president, .chairman: return true default: return false } } }그런 다음 뷰에서는:
if let user = viewModel.user, user.position.isManagerOrAbove { // ... }SampoomManagement/Core/Network/AuthRequestInterceptor.swift (2)
46-51: URL 체크 방식을 더 견고하게 개선할 수 있습니다.현재
absoluteString.contains("/auth/refresh")를 사용한 문자열 기반 체크는 대부분의 경우 잘 작동하지만, 쿼리 파라미터나 프래그먼트를 포함한 전체 URL을 대상으로 하므로 의도하지 않은 매칭이 발생할 수 있습니다.더 명확한 체크를 원하신다면 다음과 같이 개선할 수 있습니다:
- if let urlString = adaptedRequest.url?.absoluteString, - urlString.contains("/auth/refresh") { + if let url = adaptedRequest.url, + url.path.hasSuffix("/auth/refresh") { adaptedRequest.setValue(nil, forHTTPHeaderField: "Authorization") completion(.success(adaptedRequest)) return }
77-85: 재시도 제한 로직이 정확하지만 주석으로 의도를 명확히 하면 좋겠습니다.토큰 갱신 성공 후
retryCount >= 1을 체크하는 것은 무한 재시도 루프를 방지하는 서킷 브레이커 패턴입니다. 로직은 올바르게 동작하지만, 향후 유지보수를 위해 다음과 같은 주석을 추가하면 의도가 더 명확해집니다._ = try await refreshCoordinator.refresh(using: tokenRefreshService) + // 서킷 브레이커: 토큰 갱신 성공 후에도 401이 재발하면 권한 문제로 간주하고 로그아웃 + // (예: 새 토큰도 무효, 권한 변경 등) if request.retryCount >= 1 { await authPreferences.clear() DispatchQueue.main.async { NotificationCenter.default.post(name: .didRequestLogout, object: nil) } completion(.doNotRetry) return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
SampoomManagement/App/RootView.swift(1 hunks)SampoomManagement/Core/Network/AuthRequestInterceptor.swift(3 hunks)SampoomManagement/Core/Network/NetworkManager.swift(4 hunks)SampoomManagement/Core/Resources/StringResources.swift(1 hunks)SampoomManagement/Features/Auth/UI/AuthViewModel.swift(1 hunks)SampoomManagement/Features/Auth/UI/LoginView.swift(1 hunks)SampoomManagement/Features/Auth/UI/LoginViewModel.swift(1 hunks)SampoomManagement/Features/Dashboard/UI/DashboardView.swift(5 hunks)SampoomManagement/Features/Order/UI/OrderDetailContent.swift(1 hunks)SampoomManagement/Features/User/Data/Mappers/UserMappers.swift(1 hunks)SampoomManagement/Features/User/Data/Remote/DTO/EmployeeDTO.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
SampoomManagement/App/RootView.swift (1)
SampoomManagement/Features/Auth/UI/AuthViewModel.swift (2)
resetNavigationState(66-68)handleTokenExpired(53-64)
SampoomManagement/Features/Auth/UI/LoginViewModel.swift (1)
SampoomManagement/Features/Auth/UI/LoginUiState.swift (1)
copy(47-65)
SampoomManagement/Features/Auth/UI/AuthViewModel.swift (1)
SampoomManagement/Features/Auth/Domain/UseCase/CheckLoginStateUseCase.swift (1)
execute(17-19)
SampoomManagement/Core/Network/AuthRequestInterceptor.swift (2)
SampoomManagement/Core/Network/NetworkManager.swift (2)
request(42-112)request(114-190)SampoomManagement/Features/Auth/Data/Local/Preferences/AuthPreferences.swift (1)
clear(145-164)
SampoomManagement/Features/Auth/UI/LoginView.swift (1)
SampoomManagement/Features/Auth/UI/LoginUiState.swift (1)
copy(47-65)
🔇 Additional comments (12)
SampoomManagement/Features/User/Data/Remote/DTO/EmployeeDTO.swift (1)
18-19: DTO 타입 변경이 적절하며 구현이 올바릅니다.검증 결과, enum에서 String으로의 변경이 정확하게 구현되어 있습니다:
- EmployeeDTO의
position: String,status: String?은 API 응답을 유연하게 수용합니다- 매퍼는
UserPosition(rawValue: self.position) ?? .staff,EmployeeStatus(rawValue: (self.status ?? "").uppercased()) ?? .active로 안전한 변환을 수행합니다- 도메인 모델의
position: UserPosition,status: EmployeeStatus기대 타입과 일치합니다DTO와 도메인 모델의 적절한 분리로 API 유연성과 타입 안전성이 동시에 확보되었습니다.
SampoomManagement/Features/User/Data/Mappers/UserMappers.swift (1)
82-83: Since I've confirmed that Swift enum rawValue matching is case-sensitive, let me search for the actual enum definitions to understand the design intent.Based on my verification of the codebase and Swift language behavior:
검증된 일관성 문제 확인 - 기본값 지정 및 조용한 대체에 대한 재검토 필요
귀사의 코드에서 발견된 불일치는 검증되었습니다:
- Position:
UserPosition(rawValue: self.position) ?? .staff- 대문자 변환 없음- Status:
EmployeeStatus(rawValue: (self.status ?? "").uppercased()) ?? .active- 항상 대문자 변환Swift의 enum rawValue 초기화는 대소문자를 구분하므로, status를 먼저 대문자로 변환하는 것은 API 응답의 대소문자 변동을 처리하기 위한 의도된 방어 코드로 보입니다. 그러나 이 패턴은 다음 사항에 대해 재검토가 필요합니다:
- 일관성 확인: position도 같은 대문자 변환 처리가 필요한지 API 응답 형식 확인 필요
- 기본값 검증: 파싱 실패 시
.staff와.active모두가 안전한 기본값인지 비즈니스 로직 확인 필요- 데이터 품질 추적: 조용한 대체로 인한 API 데이터 품질 문제 감지 불가능 - 로깅 고려
SampoomManagement/Core/Resources/StringResources.swift (1)
23-25: 문자열 리소스 추가 확인대시보드에 사용될 새로운 문자열 상수들이 적절하게 추가되었습니다.
SampoomManagement/Features/Dashboard/UI/DashboardView.swift (4)
91-92: 문자열 리소스 업데이트 확인새로운 문자열 리소스 키들이 올바르게 적용되었습니다. StringResources.swift의 변경사항과 일치합니다.
Also applies to: 95-95
46-46: 레이아웃 간격 조정 확인섹션 간 간격이 적절하게 추가되었습니다.
113-113: 폰트 스타일링 추가 확인버튼 카드의 값 텍스트에 굵은 글꼴이 적절하게 적용되었습니다.
28-28: 직원 관리 기능 접근 권한 확대 확인됨 — 백엔드 권한 검증 필요코드 검증 결과, 직원 관리 기능에 대한 접근 권한이 다음과 같이 변경되었음이 확인되었습니다:
- 변경 내용: 관리자(admin) 전용 → 매니저 이상 7개 직급 허용 (
isManagerOrAbove함수 사용)- 적용 위치: DashboardView.swift 라인 28, 85 (UI 레벨 조건부 렌더링)
보안 관련 주의사항: 현재 권한 검증이 UI 레이 에서만 수행되고 있습니다.
EditEmployeeUseCase와UpdateEmployeeStatusUseCase에서는 권한 확인 로직이 없으며, 비즈니스 로직 계층에서 권한 검증이 이루어지지 않고 있습니다.백엔드 API에서 해당 역할들에 대한 권한 검증이 제대로 이루어지고 있는지 확인이 필수입니다. 클라이언트 단에서만 UI를 숨기는 것으로는 충분하지 않습니다.
- 백엔드 API가 매니저 이상의 역할에 대해 직원 관리 엔드포인트(
editEmployee,updateEmployeeStatus등)에 대한 접근을 허용하는지 확인- 권한 검증이 비즈니스 로직 계층(UseCase/Repository)에서도 이루어져야 하는지 검토
SampoomManagement/Features/Auth/UI/LoginViewModel.swift (1)
53-55: 의도 유지 확인
들여쓰기만 정리된 변경으로 동작 변화가 없는지 검토했는데, 기존 흐름이 그대로 유지됩니다.SampoomManagement/Core/Network/NetworkManager.swift (1)
77-106: 에러 매핑 강화 👍
새롭게 추가된 단계별 파싱으로 서버 응답에서 상태 코드와 메시지를 최대한 복구해 주는 점이 좋습니다. 디버깅 시 원인을 훨씬 빠르게 파악할 수 있겠어요.SampoomManagement/Features/Auth/UI/LoginView.swift (1)
114-120: 성공 후 상태 리셋 흐름 확인
짧은 딜레이 뒤에 onSuccess를 호출하고 success 플래그를 초기화해 다음 로그인에서도 onChange가 정상 동작하도록 한 점 좋습니다.SampoomManagement/Core/Network/AuthRequestInterceptor.swift (2)
11-13: 로그아웃 알림을 위한 깔끔한 구현입니다.Notification.Name 확장을 통해 로그아웃 이벤트를 전역적으로 전달하는 방식이 적절합니다. UI와 인증 로직의 결합도를 낮추는 좋은 접근입니다.
88-95: 토큰 갱신 실패 시 로그아웃 처리가 적절합니다.에러 경로에서 인증 상태를 클리어하고 메인 큐에서 알림을 게시하는 것이 올바른 구현입니다. 재시도 제한 초과 경로(lines 78-84)와 일관성 있게 동일한 알림을 사용하고 있습니다.
2 participants
📝 Summary
리팩토링 및 토큰 로직 수정
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항