fix: FetchFirstInstallUseCase nil 처리 로직 개선#801
Conversation
- Keychain에 값이 없으면 기존 사용자로 간주
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
14th-team5-iOS/Domain/Sources/UseCases/My/FetchFirstInstallUseCase.swift (1)
22-23: 온보딩 조회는 필요 시점으로 지연하면 흐름이 더 선명해집니다.
installed가nil이면 조기 반환하므로,fetchIsFirstOnboarding()는 그 이후에 읽어도 동일 동작입니다.제안 diff
public func execute() -> Bool { let hasInstalledBefore = myRepositroy.fetchFirstInstall() - let hasCompletedOnboarding = myRepositroy.fetchIsFirstOnboarding() guard let installed = hasInstalledBefore else { myRepositroy.updateFirstInstall(true) return false } if !installed { return true } + + let hasCompletedOnboarding = myRepositroy.fetchIsFirstOnboarding() if let onboarded = hasCompletedOnboarding, !onboarded { return true } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@14th-team5-iOS/Domain/Sources/UseCases/My/FetchFirstInstallUseCase.swift` around lines 22 - 23, The onboarding flag (myRepositroy.fetchIsFirstOnboarding()) should be fetched lazily after you confirm installed isn't nil to avoid unnecessary work and keep control flow clear: call myRepositroy.fetchFirstInstall() first, check the returned installed value and early-return if nil, and only then call myRepositroy.fetchIsFirstOnboarding() to determine onboarding status; update FetchFirstInstallUseCase (references: fetchFirstInstall, fetchIsFirstOnboarding, myRepositroy) accordingly.14th-team5-iOS/Bibbi/Sources/Presentation/Home/Reactor/MainViewReactor.swift (1)
209-219: 테스트 레이블이 있는.debug()호출을 제거하거나 DEBUG 빌드로 제한해 주세요.라인 209와 219의
.debug("true 값 테스트 입니다잉"),.debug("false 값 테스트 입니다잉")은 실제 운영 경로에서 불필요한 로그를 생성합니다.#if DEBUG가드로 감싸거나 공통 로거로 통일하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@14th-team5-iOS/Bibbi/Sources/Presentation/Home/Reactor/MainViewReactor.swift` around lines 209 - 219, Remove the inline `.debug("true 값 테스트 입니다잉")` and `.debug("false 값 테스트 입니다잉")` calls in the Observable chain inside MainViewReactor (the branches that return Observable.concat with `.just(.setInTime(true))`/`.just(.setInTime(false))` near the calls to `mutate(action: .fetchMainNightUseCase)` and `mutate(action: .setTimer(isInTime, time))`); either delete these `.debug(...)` calls entirely or wrap them behind a build-time guard (`#if DEBUG`) or replace them with calls to the shared logger used across the app so that they do not emit in production. Ensure the change keeps the surrounding Observable.concat and existing mutations intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@14th-team5-iOS/Bibbi/Sources/Presentation/Home/Reactor/MainViewReactor.swift`:
- Around line 209-219: Remove the inline `.debug("true 값 테스트 입니다잉")` and
`.debug("false 값 테스트 입니다잉")` calls in the Observable chain inside
MainViewReactor (the branches that return Observable.concat with
`.just(.setInTime(true))`/`.just(.setInTime(false))` near the calls to
`mutate(action: .fetchMainNightUseCase)` and `mutate(action: .setTimer(isInTime,
time))`); either delete these `.debug(...)` calls entirely or wrap them behind a
build-time guard (`#if DEBUG`) or replace them with calls to the shared logger
used across the app so that they do not emit in production. Ensure the change
keeps the surrounding Observable.concat and existing mutations intact.
In `@14th-team5-iOS/Domain/Sources/UseCases/My/FetchFirstInstallUseCase.swift`:
- Around line 22-23: The onboarding flag (myRepositroy.fetchIsFirstOnboarding())
should be fetched lazily after you confirm installed isn't nil to avoid
unnecessary work and keep control flow clear: call
myRepositroy.fetchFirstInstall() first, check the returned installed value and
early-return if nil, and only then call myRepositroy.fetchIsFirstOnboarding() to
determine onboarding status; update FetchFirstInstallUseCase (references:
fetchFirstInstall, fetchIsFirstOnboarding, myRepositroy) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d21d9628-d065-4712-9ecf-15339bf16674
📒 Files selected for processing (2)
14th-team5-iOS/Bibbi/Sources/Presentation/Home/Reactor/MainViewReactor.swift14th-team5-iOS/Domain/Sources/UseCases/My/FetchFirstInstallUseCase.swift
There was a problem hiding this comment.
Pull request overview
Keychain에 저장된 “첫 설치 여부” 플래그가 nil일 때의 동작을 변경해, 첫 설치/온보딩 판단 로직을 더 안전하게 처리하려는 PR입니다.
Changes:
FetchFirstInstallUseCase에서fetchFirstInstall()결과가nil인 경우를 별도로 처리하도록 로직 수정MainViewReactor에서 first install 분기 처리 시 Rxdebug로그를 추가
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| 14th-team5-iOS/Domain/Sources/UseCases/My/FetchFirstInstallUseCase.swift | 첫 설치/온보딩 여부 조회 시 nil 케이스를 guard로 분기 처리 |
| 14th-team5-iOS/Bibbi/Sources/Presentation/Home/Reactor/MainViewReactor.swift | first install 분기에서 Rx debug()로 값 확인 로그 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if !hasInstalledBefore { | ||
| guard let installed = hasInstalledBefore else { | ||
| myRepositroy.updateFirstInstall(true) |
There was a problem hiding this comment.
execute()가 hasInstalledBefore == nil일 때 updateFirstInstall(true)를 저장하면서 false(=기존 사용자로 판단)를 반환하고 있는데, 다른 경로에서는 UpdateExistingUserUseCase가 기존 사용자 처리로 updateFirstInstall(false)를 저장하고 있어 값의 의미가 코드 전반에서 상충합니다. fetchFirstInstall/updateFirstInstall에 저장되는 Bool의 의미(첫설치 여부 vs hasLaunchedBefore 여부)를 한 가지로 정리하고, nil 처리 시 저장하는 값도 그 의미에 맞게 통일해 주세요.
| myRepositroy.updateFirstInstall(true) | |
| myRepositroy.updateFirstInstall(false) |
| return Observable.concat([ | ||
| .just(.setInTime(true)), | ||
| .just(.setInTime(true)).debug("true 값 테스트 입니다잉"), | ||
| .just(.setEnabeldCamera(false)), | ||
| .just(.setCameraType(.midNight)), | ||
| self.mutate(action: .fetchMainNightUseCase), |
There was a problem hiding this comment.
Observable.debug가 그대로 남아 있고 로그 메시지도 테스트용/비격식 문구(“true 값 테스트 입니다잉”)입니다. 프로덕션 로그 오염 및 성능/운영 이슈가 될 수 있으니 제거하거나 #if DEBUG로 감싸서 디버그 빌드에서만 동작하도록 변경해 주세요.
| return Observable.concat([ | ||
| .just(.setInTime(false)), | ||
| .just(.setInTime(false)).debug("false 값 테스트 입니다잉"), | ||
| .just(.setGuideLineView(false)), | ||
| .just(.setCameraType(.midNight)), | ||
| self.mutate(action: .fetchMainNightUseCase), |
There was a problem hiding this comment.
아래 분기에서도 Observable.debug에 테스트용 메시지(“false 값 테스트 입니다잉”)가 포함되어 있습니다. 릴리즈 빌드에 남지 않도록 제거하거나 #if DEBUG 조건부 로깅으로 대체해 주세요.
- MainViewReactor 디버깅용 Observable 코드 제거
🔵PR을 올리기 전 아래 사항을 확인해주세요.
(위 내용은 지워주세요)
😽개요
🛠️작업 내용
(소제목)
(소제목)
🟡차후 계획 (Optional)
🗾이미지 (Optional)
✅테스트 케이스
🙏🏻아래와 같이 PR을 리뷰해주세요.
Summary by CodeRabbit
변경 사항