Conversation
Closed
1 task
📝 WalkthroughWalkthrough스포츠 선택 후 프로필 새로고침 로직이 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
SMASHING/Presentation/Home/HomeViewController.swift (1)
261-262: 수동 refresh에.viewWillAppear를 재사용하지 않는 편이 좋습니다.현재
HomeViewModel은.viewWillAppear마다 바로fetchHomeData()를 수행합니다. 비수명주기 코드에서 같은 input을 수동으로 보내면 실제viewWillAppear(_:)와 겹칠 때 동일 fetch가 중첩되고, 호출 의도도 흐려집니다.refreshHome같은 별도 input으로 분리해 두면 중복 요청 제어와 추적이 훨씬 쉬워집니다.Also applies to: 430-432
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SMASHING/Presentation/Home/HomeViewController.swift` around lines 261 - 262, The view lifecycle input `.viewWillAppear` is being reused for manual refreshes which causes overlapping/duplicate fetches; add a new input case (e.g., `.refreshHome`) to HomeViewModel and handle it by invoking `fetchHomeData()` (or the same fetching logic) separately from `.viewWillAppear`, then update HomeViewController to send `.refreshHome` instead of `.viewWillAppear` for manual refresh actions (replace occurrences where `self.input.send(.viewWillAppear)` is used for user-initiated refreshes, including the other instance referenced).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SMASHING/Presentation/Home/HomeViewController.swift`:
- Line 31: Replace the Boolean flag shouldRefreshAfterSportChange with an
optional pendingSportCode (e.g., String?) and change the logic around where you
set and consume that flag: when initiating a sport change set pendingSportCode =
newSportCode instead of true; in the myProfileFetched response handler compare
response.activeProfile.sportCode to pendingSportCode and only call
fetchHomeData() and clear pendingSportCode when they match; ensure other code
paths (e.g., viewWillAppear responses) do not blindly refresh when
pendingSportCode is non-nil and clear pendingSportCode only after a successful
matching refresh.
---
Nitpick comments:
In `@SMASHING/Presentation/Home/HomeViewController.swift`:
- Around line 261-262: The view lifecycle input `.viewWillAppear` is being
reused for manual refreshes which causes overlapping/duplicate fetches; add a
new input case (e.g., `.refreshHome`) to HomeViewModel and handle it by invoking
`fetchHomeData()` (or the same fetching logic) separately from
`.viewWillAppear`, then update HomeViewController to send `.refreshHome` instead
of `.viewWillAppear` for manual refresh actions (replace occurrences where
`self.input.send(.viewWillAppear)` is used for user-initiated refreshes,
including the other instance referenced).
🪄 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: 847a4fc9-2158-4588-9f24-677c25a9ae7c
📒 Files selected for processing (2)
SMASHING/Presentation/Home/HomeViewController.swiftSMASHING/Presentation/MatchResultCreate/MatchResultComponents/MatchResultCardView.swift
📜 Review details
🔇 Additional comments (1)
SMASHING/Presentation/MatchResultCreate/MatchResultComponents/MatchResultCardView.swift (1)
42-45: 점수 UI 제거 방향과 잘 맞습니다.
vs라벨로 역할을 단순화한 변경이 PR 목적과 일치하고, 제공된 상위 뷰 사용처 기준으로도 공개 API 축소가 안전해 보입니다. 중앙 정렬 제약도 현재 카드 레이아웃과 자연스럽게 맞습니다.Also applies to: 77-77, 105-108
|
|
||
| private var tooltipView: TooltipView? | ||
| private var tooltipDismissTap: UITapGestureRecognizer? | ||
| private var shouldRefreshAfterSportChange = false |
There was a problem hiding this comment.
스포츠 변경 완료를 Bool 플래그 하나로 추적하면 잘못된 프로필 응답에 반응할 수 있습니다.
myProfileFetched는 초기 로드/재진입/스포츠 변경 결과를 모두 타는 스트림인데, 지금 구조에서는 어떤 응답이 먼저 와도 shouldRefreshAfterSportChange == true면 홈을 새로고침합니다. 기존 .viewWillAppear 요청의 응답이 늦게 도착하면 변경 전 sport 기준으로 fetchHomeData()가 먼저 실행되고, 실제 sport 변경 완료 응답에서는 플래그가 이미 내려가 후속 갱신이 빠질 수 있습니다. Bool 대신 pending sport 값을 들고 있다가 response.activeProfile.sportCode와 일치할 때만 refresh 하도록 묶는 편이 안전합니다.
예시 방향
- private var shouldRefreshAfterSportChange = false
+ private var pendingSportRefresh: Sports?
...
- if self.shouldRefreshAfterSportChange {
- self.shouldRefreshAfterSportChange = false
+ if self.pendingSportRefresh == response.activeProfile.sportCode {
+ self.pendingSportRefresh = nil
self.input.send(.viewWillAppear)
}
...
- self.shouldRefreshAfterSportChange = true
+ self.pendingSportRefresh = sport
self.myProfileInput.send(.sportsCellTapped(sport))Also applies to: 259-262, 639-640
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SMASHING/Presentation/Home/HomeViewController.swift` at line 31, Replace the
Boolean flag shouldRefreshAfterSportChange with an optional pendingSportCode
(e.g., String?) and change the logic around where you set and consume that flag:
when initiating a sport change set pendingSportCode = newSportCode instead of
true; in the myProfileFetched response handler compare
response.activeProfile.sportCode to pendingSportCode and only call
fetchHomeData() and clear pendingSportCode when they match; ensure other code
paths (e.g., viewWillAppear responses) do not blindly refresh when
pendingSportCode is non-nil and clear pendingSportCode only after a successful
matching refresh.
Rudy-009
approved these changes
Apr 9, 2026
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
3 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.
✅ Check List
📌 Related Issue
📎 Work Description
📷 Screenshots
💬 To Reviewers
Summary by CodeRabbit
릴리스 노트
Bug Fixes
Refactor