[Fix] #867 - 콕찌르기 온보딩 노출 시 기존 유저를 체크하도록 변경#869
Open
kwonseokki wants to merge 4 commits into
Open
Conversation
|
ℹ️ 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 (1)
Summary by CodeRabbit
Walkthrough저장소에 Changes콕찌르기 온보딩 흐름
Sequence DiagramsequenceDiagram
actor User
participant ViewModel as PokeMainViewModel
participant UseCase as DefaultPokeMainUseCase
participant Repo as PokeMainRepository
participant Service as PokeService
participant UserDef as UserDefaults
User->>ViewModel: viewDidLoad
ViewModel->>UseCase: checkPokeOnboardingNeeded()
UseCase->>UserDef: isVisitedPokeMainView 확인
alt 이미 방문
UserDef-->>UseCase: true
UseCase->>UseCase: isNewUser에 false 발행
else 미방문
UserDef-->>UseCase: false
UseCase->>Repo: getIsNewUser()
Repo->>Service: isNewUser()
Service-->>Repo: isNew 응답
Repo-->>UseCase: Bool 퍼블리셔 반환
UseCase->>UseCase: 결과를 isNewUser로 발행
end
UseCase-->>ViewModel: isNewUser 이벤트
ViewModel->>User: onPokeOnboardingNeeded 콜백 실행
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 분 Possibly related PRs
Suggested reviewers
개요콕찌르기 온보딩 로직을 저장소-유스케이스-뷰모델 계층에 걸쳐 리팩토링하여, 사용자가 이전에 방문했는지 여부를 퍼블리셔 기반으로 관리합니다. 변경사항신규 사용자 조회 플로우
시퀀스 다이어그램sequenceDiagram
actor User
participant ViewModel as PokeMainViewModel
participant UseCase as DefaultPokeMainUseCase
participant Repo as PokeMainRepository
participant Service as PokeService
participant UserDef as UserDefaults
User->>ViewModel: viewDidLoad 발생
ViewModel->>UseCase: checkPokeOnboardingNeeded() 호출
UseCase->>UserDef: isVisitedPokeMainView 확인
alt 이미 방문
UserDef-->>UseCase: true
UseCase->>UseCase: false 즉시 발행
else 미방문
UserDef-->>UseCase: false
UseCase->>Repo: getIsNewUser() 호출
Repo->>Service: isNewUser() 호출
Service-->>Repo: isNew 반환
Repo-->>UseCase: Bool 퍼블리셔 반환
UseCase->>UseCase: 결과를 isNewUser로 발행
end
UseCase-->>ViewModel: isNewUser 퍼블리셔 업데이트
ViewModel->>User: onPokeOnboardingNeeded 콜백 실행
예상 코드 리뷰 난이도🎯 3 (Moderate) | ⏱️ ~20 분 관련 PR
제안 리뷰어
시 🐰
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (1)
SOPT-iOS/Projects/Domain/Sources/UseCase/PokeMainUseCase.swift (1)
97-97: 💤 Low value
#warning이슈 트래킹 필요이
#warning코멘트가 명시하듯,isVisitedPokeMainView기반 단락 로직은 임시 조치이며 추후 제거가 필요합니다. 별도 이슈로 추적하지 않으면 코드에 그대로 남을 가능성이 있습니다.해당 항목을 추적할 GitHub 이슈를 생성해 드릴까요?
🤖 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 `@SOPT-iOS/Projects/Domain/Sources/UseCase/PokeMainUseCase.swift` at line 97, The inline `#warning` in PokeMainUseCase.swift flags a temporary workaround around isVisitedPokeMainView vs getIsNewUser that must be tracked; create a GitHub issue describing the temporary on-boarding workaround and planned removal, then replace the `#warning` with a concise TODO comment that includes the created issue number (and optionally a short action: e.g., "remove isVisitedPokeMainView fallback and restore getIsNewUser logic"); reference the affected symbols isVisitedPokeMainView and getIsNewUser in the issue and TODO so reviewers can locate and remove the temporary logic later.
🤖 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.
Nitpick comments:
In `@SOPT-iOS/Projects/Domain/Sources/UseCase/PokeMainUseCase.swift`:
- Line 97: The inline `#warning` in PokeMainUseCase.swift flags a temporary
workaround around isVisitedPokeMainView vs getIsNewUser that must be tracked;
create a GitHub issue describing the temporary on-boarding workaround and
planned removal, then replace the `#warning` with a concise TODO comment that
includes the created issue number (and optionally a short action: e.g., "remove
isVisitedPokeMainView fallback and restore getIsNewUser logic"); reference the
affected symbols isVisitedPokeMainView and getIsNewUser in the issue and TODO so
reviewers can locate and remove the temporary logic later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53fb9c52-4b0d-47c7-a40e-90ba9211b87f
📒 Files selected for processing (4)
SOPT-iOS/Projects/Data/Sources/Repository/PokeMainRepository.swiftSOPT-iOS/Projects/Domain/Sources/RepositoryInterface/PokeMainRepositoryInterface.swiftSOPT-iOS/Projects/Domain/Sources/UseCase/PokeMainUseCase.swiftSOPT-iOS/Projects/Features/PokeFeature/Sources/PokeMainScene/ViewModel/PokeMainViewModel.swift
yungu0010
reviewed
May 6, 2026
Member
yungu0010
left a comment
yungu0010
left a comment
There was a problem hiding this comment.
고생하셨습니다👍👍👍👍 리뷰 한 번 확인해주세요~!
더불어 isVisitedPokeMainView = true 저장 위치를 Repository로 옮기는 것을 제안드립니다.
isVisitedPokeMainView는 "온보딩을 노출해야하는가"를 판단하는 플래그인데, getIsNewUser()를 호출하는 순간 온보딩 노출 여부가 결정되기 때문에 Repository 계층에서 유저디폴트 설정을 하는 것이 더 적합한 것 같습니다.
또한 현재 API 호출 에러가 발생하면 false를 반환하여 OnboardingVC가 생성되지 않아 true 설정이 누락될 위험성이 있어보여요. 한 번 고려해주세요.
추가로 Poke와 Stamp 모듈에서는 withUnretained가 잘 사용되지 않고 있는데 withUnretained이 컨벤션이기도 하고 코드도 간결해져서 사용하면 좋을 것 같습니다!
| func getFriend() -> AnyPublisher<[PokeUserModel], Error> | ||
| func getFriendRandomUser(randomType: String, size: Int) -> AnyPublisher<PokeFriendRandomUserModel, Error> | ||
| func getFriendRandomUser(randomType: String, size: Int) -> AnyPublisher<PokeFriendRandomUserModel, Error> | ||
| func getIsNewUser() -> AnyPublisher<Bool, any Error> |
Member
There was a problem hiding this comment.
기존 Error 타입도 any로 맞추면 좋을 것 같습니다! 추후 Swif6로 마이그레이션 할 때 같이 붙여보아요 . .
| }.store(in: self.cancelBag) | ||
| } | ||
|
|
||
| public func getIsNewUser() { |
Member
There was a problem hiding this comment.
해당 메소드의 퍼블리셔 반환값이 온보딩 노출 여부인만큼 checkPokeOnboardingNeeded라는 메소드명은 어떠신가요?
Comment on lines
+98
to
+110
| Just(UserDefaultKeyList.User.isVisitedPokeMainView ?? false) | ||
| .flatMap { [weak self] isVisited in | ||
| guard let self = self else { return Just(false).eraseToAnyPublisher() } | ||
|
|
||
| if isVisited { | ||
| return Just(false) | ||
| .eraseToAnyPublisher() | ||
| } else { | ||
| return repository.getIsNewUser() | ||
| .replaceError(with: false) | ||
| .eraseToAnyPublisher() | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| Just(UserDefaultKeyList.User.isVisitedPokeMainView ?? false) | |
| .flatMap { [weak self] isVisited in | |
| guard let self = self else { return Just(false).eraseToAnyPublisher() } | |
| if isVisited { | |
| return Just(false) | |
| .eraseToAnyPublisher() | |
| } else { | |
| return repository.getIsNewUser() | |
| .replaceError(with: false) | |
| .eraseToAnyPublisher() | |
| } | |
| } | |
| Just(UserDefaultKeyList.User.isVisitedPokeMainView ?? false) | |
| .withUnretained(self) | |
| .flatMap { owner, isVisited in | |
| if isVisited { | |
| return Just(false) | |
| .eraseToAnyPublisher() | |
| } else { | |
| return repository.getIsNewUser() | |
| .replaceError(with: false) | |
| .eraseToAnyPublisher() | |
| } |
withUnretained를 쓰면 프로젝트 컨벤션에도 맞고 코드도 훨씬 간결해질 것 같습니다!
| .eraseToAnyPublisher() | ||
| } | ||
| } | ||
| .sink { [weak self] isNewUser in |
Contributor
Author
getIsNewUser() 메서드에서 온보딩 노출 여부를 결정하는 게 아닌 단순히 노출 여부를 확인하는 용도이고 실제로는 사용자가 직접 온보딩뷰를 이탈하여 메인뷰에 진입했을때 플래그를 변경하는 것으로 이해를 했습니다. 그래서 OnboardingVC의 |
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.
🌴 PR 요약
메인뷰 진입 여부를 체크하고나서 결과값에 따라서 poke/isNew 를 호출해서 더블체크 하도록 수정했습니다.
🌱 작업한 브랜치
🌱 PR Point
DefaultPokeMainUseCase에서 isVisitedPokeMainView 값을 체크하고 결과값에 따라서 추가적으로 PokeMainRepository에 getIsNewUser 메서드를 추가적으로 호출하여 기존 사용자에게 온보딩 화면이 나타나지 않도록 수정했습니다.📌 참고 사항
📸 스크린샷
📮 관련 이슈