Feat/#470 강제 업데이트 구현#471
Open
y-eonee wants to merge 8 commits into
Hidden character warning
The head ref may contain hidden characters: "feat/#470-\uac15\uc81c-\uc5c5\ub370\uc774\ud2b8-\uad6c\ud604"
Open
Conversation
1 task
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughFirebase Remote Config 기반 강제 업데이트 판정 로직이 추가되었고, Splash 화면은 업데이트 필요 여부에 따라 모달 표시 또는 자동 로그인으로 분기하도록 변경되었습니다. 관련 DI 등록, 유스케이스, 모달 UI, 앱스토어 상수도 함께 추가되었습니다. Changes강제 업데이트 기능
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SplashViewController
participant SplashViewModel
participant CheckForceUpdateUseCase
participant DefaultForceUpdateRepository
participant RemoteConfig
participant ForceUpdateModalView
SplashViewController->>SplashViewModel: action(.viewDidLoad)
SplashViewModel->>CheckForceUpdateUseCase: execute()
CheckForceUpdateUseCase->>DefaultForceUpdateRepository: checkForUpdate()
DefaultForceUpdateRepository->>RemoteConfig: fetch()
DefaultForceUpdateRepository->>RemoteConfig: activate()
DefaultForceUpdateRepository-->>CheckForceUpdateUseCase: Bool
CheckForceUpdateUseCase-->>SplashViewModel: Bool
SplashViewModel-->>SplashViewController: forceUpdatePublisher(Bool)
alt 업데이트 필요
SplashViewController->>ForceUpdateModalView: present
else 업데이트 불필요
SplashViewController->>SplashViewModel: action(.tryAutoLogin)
end
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win타입명 표기 불일치:
Usecase→UseCase.
DefaultCheckForceUpdateUsecase만 소문자case로 되어 있어, 코드베이스 전반의Default*UseCase명명 규칙과 어긋납니다.♻️ 제안 수정
-struct DefaultCheckForceUpdateUsecase: CheckForceUpdateUseCase { +struct DefaultCheckForceUpdateUseCase: CheckForceUpdateUseCase {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift` at line 14, The type name in DefaultCheckForceUpdateUsecase is inconsistent with the project’s Default*UseCase naming convention. Rename the struct to DefaultCheckForceUpdateUseCase and update any matching references so the CheckForceUpdateUseCase implementation follows the same capitalization pattern as the rest of the codebase.ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift (3)
17-21: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win프로덕션에서
minimumFetchInterval = 0은 스로틀링 위험이 있습니다.디버그/개발용이 아닌 앱 시작 시마다(매 실행) 무조건 fetch interval을 0으로 설정하고 있습니다. Firebase 공식 문서에서는 이 설정을 개발 전용으로 명시하며, 트래픽이 많은 앱에서는 시간당 서버 측 쿼터에 걸려 스로틀링될 수 있다고 경고합니다. 스로틀링이 발생하면
fetch()가 실패하고checkForUpdate()는 항상false를 반환하게 되어, 강제 업데이트가 필요한 사용자도 감지하지 못할 수 있습니다.DEBUG 빌드에서만 0으로 설정하고, 프로덕션에서는 합리적인 간격(예: 몇 시간 단위)을 사용하는 것을 권장합니다.
♻️ 제안 수정
init() { let settings = RemoteConfigSettings() - settings.minimumFetchInterval = 0 + `#if` DEBUG + settings.minimumFetchInterval = 0 + `#else` + settings.minimumFetchInterval = 3600 + `#endif` RemoteConfig.remoteConfig().configSettings = settings }Firebase 문서 확인 결과: "During development, it's recommended to set a relatively low minimum fetch interval... Keep in mind that this setting should be used for development only, not for an app running in production."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift` around lines 17 - 21, ForceUpdateManager.init() is setting RemoteConfigSettings.minimumFetchInterval to 0 unconditionally, which should be limited to development use only. Update the initialization logic in ForceUpdateManager so that RemoteConfig.remoteConfig().configSettings uses a zero interval only for DEBUG builds, and uses a reasonable production interval otherwise; keep the change localized to the init() setup and preserve the existing RemoteConfig configuration flow.
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial파일 헤더 주석이 실제 파일명과 다릅니다.
주석에는
RemoteConfigeManager.swift로 되어 있지만 실제 파일명은ForceUpdateManager.swift입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift` around lines 1 - 6, The file header comment is using the wrong filename, so update the top-of-file header in ForceUpdateManager.swift to match the actual file name instead of RemoteConfigeManager.swift. Keep the rest of the header unchanged and ensure any generated template or metadata in this file reflects ForceUpdateManager.
23-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
throws선언이 무의미하고, 오류가 조용히 삼켜집니다.
checkForUpdate()는 내부에서 모든 에러를catch해 항상false를 반환하므로 실제로는 절대 throw하지 않습니다. 그런데도 프로토콜과 함수 시그니처는throws를 유지하고 있어, 호출부(CheckForceUpdateUseCase.execute(),SplashViewModel.checkForceUpdate())에서 불필요한try/do-catch가 이중으로 존재하게 됩니다. 또한 fetch/activate 실패 시 에러 로그가 전혀 남지 않아 실패 원인 파악이 어렵습니다.
throws를 제거해 계약을 명확히 하거나, 최소한 catch 블록에서 에러를 로깅하는 것을 제안합니다.♻️ 제안 수정 (에러 로깅 추가)
func checkForUpdate() async throws -> Bool { do { try await remoteConfig.fetch() try await remoteConfig.activate() let minimumVersion = remoteConfig["min_version_code"].stringValue return isUpdateRequired(minimumVersion: minimumVersion) } catch { + ByeBooLogger.debug("강제 업데이트 확인 실패: \(error)") return false } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift` around lines 23 - 34, `ForceUpdateManager.checkForUpdate()` currently swallows every error and always returns false, so the `throws` on the method and its protocol contract are misleading. Either remove `throws` from `checkForUpdate()` and the related call sites that reference it, or keep the non-throwing behavior but add error logging inside the `catch` so fetch/activate failures are visible. Update the usages in `CheckForceUpdateUseCase.execute()` and `SplashViewModel.checkForceUpdate()` accordingly so they no longer wrap this call in unnecessary try/do-catch logic.ByeBoo-iOS/ByeBoo-iOS/Domain/DomainDependencyAssembler.swift (1)
258-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win다른 리포지토리와 달리
DefaultForceUpdateManager()가 DIContainer를 거치지 않고 직접 인스턴스화됩니다.같은 파일의 다른 모든 등록은 파일 상단
guard let ... = DIContainer.shared.resolve(type: ...Interface.self)로 먼저 리포지토리를 해석한 뒤 UseCase 생성 시 주입하는 패턴을 따릅니다. 반면CheckForceUpdateUseCase등록에서는DefaultForceUpdateManager()를 인라인으로 직접 생성해 주입합니다. 이는 기존 DI 컨벤션과 다르며, 테스트 시 목(mock) 구현체로 교체하기 어렵게 만듭니다.
ForceUpdateManager도 DIContainer에 등록한 뒤 resolve하여 사용하는 방식으로 통일하는 것을 제안합니다.♻️ 제안 수정
+ DIContainer.shared.register(type: ForceUpdateManager.self) { _ in + return DefaultForceUpdateManager() + } + + guard let forceUpdateManager = DIContainer.shared.resolve(type: ForceUpdateManager.self) else { + ByeBooLogger.error(ByeBooError.DIFailedError) + return + } + DIContainer.shared.register(type: CheckForceUpdateUseCase.self) { _ in - return DefaultCheckForceUpdateUsecase(repository: DefaultForceUpdateManager()) + return DefaultCheckForceUpdateUseCase(repository: forceUpdateManager) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Domain/DomainDependencyAssembler.swift` around lines 258 - 261, The CheckForceUpdateUseCase registration is bypassing the existing DI pattern by creating DefaultForceUpdateManager() directly instead of resolving it through DIContainer.shared. Update the DomainDependencyAssembler registration so ForceUpdateManager is registered and then injected via resolve, matching the other use case bindings in this file and keeping the dependency swappable for tests. Use the CheckForceUpdateUseCase and DefaultForceUpdateManager symbols to locate the affected registration and align it with the surrounding repository injection style.ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewModel/SplashViewModel.swift (1)
60-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
catch블록에서 에러 로깅 누락
autoLogin()은 실패 시ByeBooLogger.error(error)로 에러를 남기는 반면,checkForceUpdate()의catch는 에러를 그대로 버리고false만 전송합니다. Remote Config fetch 실패가 강제 업데이트라는 핵심 안전장치의 동작을 좌우하는 만큼, 실패 원인을 남겨두는 것이 운영/디버깅에 유리합니다.🪵 제안 수정
} catch { + ByeBooLogger.error(error) forceUpdateSubject.send(false) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewModel/SplashViewModel.swift` around lines 60 - 74, `SplashViewModel.checkForceUpdate()`의 `catch` 블록에서 에러가 버려지고 있어 실패 원인을 추적할 수 없습니다. `autoLogin()`처럼 `ByeBooLogger.error(error)`를 추가해 Remote Config fetch 실패를 로그로 남기고, 그 뒤 기존처럼 `forceUpdateSubject.send(false)`를 유지하도록 수정하세요. `checkForceUpdateUseCase.execute()`를 감싸는 `Task` 내부의 예외 처리 흐름을 찾아 반영하면 됩니다.ByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/ForceUpdateModalView.swift (1)
18-19: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win타이틀/설명 라벨에 줄바꿈·가로 제약 추가 권장
titleLabel,descriptionLabel은centerX제약만 있고numberOfLines나 leading/trailing 제약이 없습니다. 기본값(numberOfLines = 1)이라 큰 접근성 폰트 크기나 더 긴 문구로 바뀔 경우 텍스트가 잘리거나 모달 경계를 벗어날 수 있습니다.✏️ 제안 수정
private let titleLabel = UILabel() private let descriptionLabel = UILabel()override func setStyle() { backgroundColor = .grayscale900 layer.cornerRadius = 12 + titleLabel.numberOfLines = 0 + descriptionLabel.numberOfLines = 0 + titleLabel.applyByeBooFont(titleLabel.snp.makeConstraints { $0.top.equalToSuperview().inset(24.adjustedH) - $0.centerX.equalToSuperview() + $0.horizontalEdges.equalToSuperview().inset(24.adjustedW) + $0.centerX.equalToSuperview() } descriptionLabel.snp.makeConstraints { $0.top.equalTo(titleLabel.snp.bottom).offset(16.adjustedH) - $0.centerX.equalToSuperview() + $0.horizontalEdges.equalToSuperview().inset(24.adjustedW) + $0.centerX.equalToSuperview() }Also applies to: 29-39, 43-50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/ForceUpdateModalView.swift` around lines 18 - 19, `ForceUpdateModalView`의 `titleLabel`과 `descriptionLabel`은 현재 가로 제약이 부족하고 기본 단일 줄 설정이라 긴 문구나 큰 접근성 폰트에서 잘릴 수 있습니다. `setupUI`/레이블 설정 로직에서 두 라벨의 `numberOfLines`를 여러 줄로 허용하고, `centerX`만 쓰는 대신 leading/trailing 제약을 추가해 모달 폭 안에서 줄바꿈되도록 수정하세요. `titleLabel`과 `descriptionLabel`을 참조하는 레이아웃 구성 전체를 함께 점검해 안전한 여백을 유지하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift`:
- Around line 14-20: Move the CheckForceUpdateUseCase dependency off the
Data-layer ForceUpdateManager and into a Domain interface so the use case only
depends on Domain abstractions. Update DefaultCheckForceUpdateUsecase to
reference the Domain protocol, add or relocate the ForceUpdateManager protocol
under the Domain/Interface pattern used by the other use cases, and make
DefaultForceUpdateManager conform to that protocol.
In
`@ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewController/SplashViewController.swift`:
- Around line 105-119: `presentForceUpdateModal()` currently presents
`ForceUpdateModalView` through `ModalBuilder`, but the underlying
`CustomModalController` still dismisses on background taps. Update the modal
presentation path so the force-update flow cannot be closed by tapping outside
the modal, keeping only the update action from `openAppstore()` available; use
the `ModalBuilder`/`CustomModalController` configuration or presentation flags
associated with `ForceUpdateModalView` to disable outside-tap dismiss.
- Around line 32-37: In SplashViewController, update the sink inside
bindCheckForceUpdate() to capture self weakly so the Combine subscription stored
in cancellables does not retain the view controller. Keep bindAutoLogin()
unchanged, and make sure the appDidBecomeActive flow still triggers
viewModel.action(.viewDidLoad) only when the VC is alive without creating a
retain cycle.
---
Nitpick comments:
In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift`:
- Around line 17-21: ForceUpdateManager.init() is setting
RemoteConfigSettings.minimumFetchInterval to 0 unconditionally, which should be
limited to development use only. Update the initialization logic in
ForceUpdateManager so that RemoteConfig.remoteConfig().configSettings uses a
zero interval only for DEBUG builds, and uses a reasonable production interval
otherwise; keep the change localized to the init() setup and preserve the
existing RemoteConfig configuration flow.
- Around line 1-6: The file header comment is using the wrong filename, so
update the top-of-file header in ForceUpdateManager.swift to match the actual
file name instead of RemoteConfigeManager.swift. Keep the rest of the header
unchanged and ensure any generated template or metadata in this file reflects
ForceUpdateManager.
- Around line 23-34: `ForceUpdateManager.checkForUpdate()` currently swallows
every error and always returns false, so the `throws` on the method and its
protocol contract are misleading. Either remove `throws` from `checkForUpdate()`
and the related call sites that reference it, or keep the non-throwing behavior
but add error logging inside the `catch` so fetch/activate failures are visible.
Update the usages in `CheckForceUpdateUseCase.execute()` and
`SplashViewModel.checkForceUpdate()` accordingly so they no longer wrap this
call in unnecessary try/do-catch logic.
In `@ByeBoo-iOS/ByeBoo-iOS/Domain/DomainDependencyAssembler.swift`:
- Around line 258-261: The CheckForceUpdateUseCase registration is bypassing the
existing DI pattern by creating DefaultForceUpdateManager() directly instead of
resolving it through DIContainer.shared. Update the DomainDependencyAssembler
registration so ForceUpdateManager is registered and then injected via resolve,
matching the other use case bindings in this file and keeping the dependency
swappable for tests. Use the CheckForceUpdateUseCase and
DefaultForceUpdateManager symbols to locate the affected registration and align
it with the surrounding repository injection style.
In `@ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift`:
- Line 14: The type name in DefaultCheckForceUpdateUsecase is inconsistent with
the project’s Default*UseCase naming convention. Rename the struct to
DefaultCheckForceUpdateUseCase and update any matching references so the
CheckForceUpdateUseCase implementation follows the same capitalization pattern
as the rest of the codebase.
In `@ByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/ForceUpdateModalView.swift`:
- Around line 18-19: `ForceUpdateModalView`의 `titleLabel`과 `descriptionLabel`은
현재 가로 제약이 부족하고 기본 단일 줄 설정이라 긴 문구나 큰 접근성 폰트에서 잘릴 수 있습니다. `setupUI`/레이블 설정 로직에서 두
라벨의 `numberOfLines`를 여러 줄로 허용하고, `centerX`만 쓰는 대신 leading/trailing 제약을 추가해 모달 폭
안에서 줄바꿈되도록 수정하세요. `titleLabel`과 `descriptionLabel`을 참조하는 레이아웃 구성 전체를 함께 점검해 안전한
여백을 유지하세요.
In
`@ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewModel/SplashViewModel.swift`:
- Around line 60-74: `SplashViewModel.checkForceUpdate()`의 `catch` 블록에서 에러가 버려지고
있어 실패 원인을 추적할 수 없습니다. `autoLogin()`처럼 `ByeBooLogger.error(error)`를 추가해 Remote
Config fetch 실패를 로그로 남기고, 그 뒤 기존처럼 `forceUpdateSubject.send(false)`를 유지하도록
수정하세요. `checkForceUpdateUseCase.execute()`를 감싸는 `Task` 내부의 예외 처리 흐름을 찾아 반영하면
됩니다.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8728c5f-3963-49ca-925b-9e62506a522e
📒 Files selected for processing (9)
ByeBoo-iOS/ByeBoo-iOS.xcodeproj/project.pbxprojByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swiftByeBoo-iOS/ByeBoo-iOS/Domain/DomainDependencyAssembler.swiftByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/ForceUpdateModalView.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/Enum/AppConstants.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewController/SplashViewController.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewModel/SplashViewModel.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/PresentationDependencyAssembler.swift
Comment on lines
+105
to
+119
|
|
||
| private func presentForceUpdateModal() { | ||
| let modal = ModalBuilder( | ||
| modalView: ForceUpdateModalView(), | ||
| action: openAppstore, | ||
| rootViewController: self | ||
| ) | ||
| modal.present() | ||
| } | ||
|
|
||
| private func openAppstore() { | ||
| guard let url = URL(string: AppConstants.appStoreURL) else { return } | ||
| UIApplication.shared.open(url) | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ModalBuilder/ModalProtocol의 배경 탭/스와이프 dismiss 처리 여부 확인
rg -n -B3 -A20 'class ModalBuilder' ByeBoo-iOS/ByeBoo-iOS --type=swift
rg -n 'dismissButton|modalType|tapGesture|backgroundView' ByeBoo-iOS/ByeBoo-iOS --type=swift -g '*Modal*'Repository: 36-APPJAM-HEARTZ/BYEBOO-iOS
Length of output: 4217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ModalProtocol ==\n'
sed -n '1,120p' ByeBoo-iOS/ByeBoo-iOS/Presentation/Protocol/ModalProtocol.swift
printf '\n== CustomModalController ==\n'
sed -n '1,180p' ByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/CustomModalController.swift
printf '\n== ForceUpdateModalView ==\n'
sed -n '1,120p' ByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/ForceUpdateModalView.swift
printf '\n== ModalBuilder ==\n'
sed -n '1,160p' ByeBoo-iOS/ByeBoo-iOS/Presentation/Common/Modal/ModalBuilder.swiftRepository: 36-APPJAM-HEARTZ/BYEBOO-iOS
Length of output: 5627
배경 탭으로 닫히지 않게 막아야 합니다
ForceUpdateModalView는 닫기 버튼이 없더라도, CustomModalController가 모달 바깥 탭에서 dismiss(animated: false)를 호출합니다. 강제 업데이트는 업데이트 버튼 외에는 종료되지 않도록 배경 탭 dismiss를 비활성화해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewController/SplashViewController.swift`
around lines 105 - 119, `presentForceUpdateModal()` currently presents
`ForceUpdateModalView` through `ModalBuilder`, but the underlying
`CustomModalController` still dismisses on background taps. Update the modal
presentation path so the force-update flow cannot be closed by tapping outside
the modal, keeping only the update action from `openAppstore()` available; use
the `ModalBuilder`/`CustomModalController` configuration or presentation flags
associated with `ForceUpdateModalView` to disable outside-tap dismiss.
dev-domo
approved these changes
Jul 5, 2026
dev-domo
left a comment
dev-domo
left a comment
Collaborator
There was a problem hiding this comment.
좋네요 감사합니다! 코멘트 확인 부탁드릴게요🙏
| private func bindCheckForceUpdate() { | ||
| viewModel.output.forceUpdatePublisher | ||
| .receive(on: DispatchQueue.main) | ||
| .sink { result in |
Collaborator
There was a problem hiding this comment.
[weak self] 사용해주면 좋을 것 같아요!
| private func checkForceUpdate() { | ||
| Task { | ||
| do { | ||
| if try await checkForceUpdateUseCase.execute() { |
Collaborator
There was a problem hiding this comment.
execute 메서드가 에러를 던지는 함수가 아니라서 try await -> await으로 호출해도 될 것 같은데 맞을까요?
|
|
||
| struct DefaultCheckForceUpdateUsecase: CheckForceUpdateUseCase { | ||
|
|
||
| private let repository: ForceUpdateManager |
Collaborator
There was a problem hiding this comment.
도메인 영역에 별도의 Interface를 만드는 것이 아니라 데이터 영역에 선언한 프로토콜을 주입하게 된 이유가 있나요?
현재 구현에서는 도메인 영역에서 데이터 영역에 선언된 ForceUpdateManager를 알고 있는 구조가 되는 것 같아서요!
Member
Author
There was a problem hiding this comment.
아 맞네여 😇😇😇 프로토콜을 도메인 레이어로 옮겼습니다 !!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift (2)
10-17: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
DefaultForceUpdateManager참조를DefaultForceUpdateRepository로 맞춰야 합니다.Domain/DomainDependencyAssembler.swift:260에서 존재하지 않는DefaultForceUpdateManager()를 생성하고 있어 컴파일이 깨집니다. 실제 정의된 타입명에 맞추거나, DI 등록과 클래스명을 함께 정리해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift` around lines 10 - 17, The DI registration is referencing a non-existent DefaultForceUpdateManager while the actual type defined here is DefaultForceUpdateRepository. Update DomainDependencyAssembler to instantiate the real class name, or rename the repository and its registration consistently so the symbol names match across the assembler and ForceUpdateManager.swift.
13-30: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
minimumFetchInterval = 0은 프로덕션에서 쓰지 마세요.지금처럼 항상 0으로 두면 Remote Config fetch throttling이 걸릴 수 있고,
checkForUpdate()가 모든 예외를false로 삼켜서 강제 업데이트가 조용히 우회됩니다.
DEBUG/RELEASE로 분기하거나 프로덕션에서는 기본값 수준의 간격을 쓰고,catch에서는 최소한 에러를 남겨 원인을 추적할 수 있게 해주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift` around lines 13 - 30, The ForceUpdateManager init() currently hardcodes RemoteConfigSettings.minimumFetchInterval to 0, which should be limited to DEBUG only; change it so production uses a normal fetch interval and only debug builds use zero. In checkForUpdate(), avoid silently swallowing all errors by logging the failure in the catch block before returning false, so RemoteConfig throttling or fetch/activate issues can be traced.ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift (1)
16-19: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
ForceUpdateManager를ForceUpdateInterface로 바꿔야 합니다.
현재 프로젝트에ForceUpdateManager타입은 없어서, 이 생성자와 주입 코드가 컴파일되지 않습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift` around lines 16 - 19, The CheckForceUpdateUseCase initializer is typed against a non-existent ForceUpdateManager, so update the dependency to use ForceUpdateInterface instead. Adjust the repository property and init(repository:) signature in CheckForceUpdateUseCase to accept the interface, and make sure any matching dependency injection or call sites pass a ForceUpdateInterface-conforming instance.
🧹 Nitpick comments (1)
ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift (1)
36-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win세그먼트 수가 다른 버전 문자열은 직접 비교하지 마세요
String.compare(_:options: .numeric)는 숫자 구간만 자연 정렬할 뿐,1.3와1.3.0처럼 세그먼트 개수가 다른 버전을 같은 값으로 보지 않습니다.CFBundleShortVersionString과min_version_code형식이 조금만 달라져도 불필요한 업데이트가 트리거될 수 있으니, 세그먼트를 정수로 파싱해 길이를 맞춘 뒤 비교하는 쪽이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift` around lines 36 - 39, The version check in isUpdateRequired(minimumVersion:) is relying on String.compare(_:options: .numeric), which can mis-handle version strings with different segment counts like 1.3 and 1.3.0. Update the comparison logic in ForceUpdateManager to parse currentAppVersion and minimumVersion into numeric version segments, normalize missing segments by padding as needed, and compare the resulting components directly instead of using string comparison. Keep the existing logging in isUpdateRequired(minimumVersion:) but ensure the decision is based on the parsed version arrays.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift`:
- Around line 10-17: The DI registration is referencing a non-existent
DefaultForceUpdateManager while the actual type defined here is
DefaultForceUpdateRepository. Update DomainDependencyAssembler to instantiate
the real class name, or rename the repository and its registration consistently
so the symbol names match across the assembler and ForceUpdateManager.swift.
- Around line 13-30: The ForceUpdateManager init() currently hardcodes
RemoteConfigSettings.minimumFetchInterval to 0, which should be limited to DEBUG
only; change it so production uses a normal fetch interval and only debug builds
use zero. In checkForUpdate(), avoid silently swallowing all errors by logging
the failure in the catch block before returning false, so RemoteConfig
throttling or fetch/activate issues can be traced.
In `@ByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swift`:
- Around line 16-19: The CheckForceUpdateUseCase initializer is typed against a
non-existent ForceUpdateManager, so update the dependency to use
ForceUpdateInterface instead. Adjust the repository property and
init(repository:) signature in CheckForceUpdateUseCase to accept the interface,
and make sure any matching dependency injection or call sites pass a
ForceUpdateInterface-conforming instance.
---
Nitpick comments:
In `@ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swift`:
- Around line 36-39: The version check in isUpdateRequired(minimumVersion:) is
relying on String.compare(_:options: .numeric), which can mis-handle version
strings with different segment counts like 1.3 and 1.3.0. Update the comparison
logic in ForceUpdateManager to parse currentAppVersion and minimumVersion into
numeric version segments, normalize missing segments by padding as needed, and
compare the resulting components directly instead of using string comparison.
Keep the existing logging in isUpdateRequired(minimumVersion:) but ensure the
decision is based on the parsed version arrays.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 844f9e58-820f-46ea-8102-d6452d112175
📒 Files selected for processing (5)
ByeBoo-iOS/ByeBoo-iOS/Data/Persistence/Service/ForceUpdateManager.swiftByeBoo-iOS/ByeBoo-iOS/Domain/Interface/ForeceUpdateInterface.swiftByeBoo-iOS/ByeBoo-iOS/Domain/UseCase/CheckForceUpdateUseCase.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewController/SplashViewController.swiftByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewModel/SplashViewModel.swift
✅ Files skipped from review due to trivial changes (1)
- ByeBoo-iOS/ByeBoo-iOS/Domain/Interface/ForeceUpdateInterface.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewController/SplashViewController.swift
- ByeBoo-iOS/ByeBoo-iOS/Presentation/Feature/Login/ViewModel/SplashViewModel.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 연결된 이슈
📄 작업 내용
💻 주요 코드 설명
Firebase Remote Config에 min_version_code 매개변수 추가
현재 최소버전은 다음 업데이트때 반영될 1.3.0으로 걸어둔 상태입니다!
전체 플로우
ForceUpdateManager
의존성 주입 부분에서 고민을 많이 했는데 혹시 더 좋은 방법이 있다면 말씀해주세요 !!!!!
Summary by CodeRabbit
요약