-
Notifications
You must be signed in to change notification settings - Fork 1
[Fix-T3-206] 탈퇴하기 기능 QA 이슈 수정 #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughPrimaryButton 초기화에서 전달된 상태에 따라 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as "사용자"
participant VC as "WithdrawViewController"
participant Sys as "iOS Keyboard\n(System)"
participant Dispatcher as "입력 Dispatcher"
rect rgba(100,149,237,0.5)
User->>VC: 텍스트뷰 탭 (편집 시작)
VC->>Sys: becomeFirstResponder()
Sys-->>VC: keyboardWillAppear(notification)
VC->>VC: keyboardWillAppear 처리\n(뷰 위치 보정, 버튼 가시성 보장)
end
rect rgba(60,179,113,0.5)
User->>Sys: 키보드 입력
Sys->>VC: textViewDidBeginEditing / 입력 이벤트
VC->>Dispatcher: inputWithdrawReason (기존 텍스트가 있으면 즉시 디스패치)
end
rect rgba(220,20,60,0.5)
User->>VC: 배경 탭 (키보드 닫기)
VC->>VC: dismissKeyboard()
VC->>Sys: resignFirstResponder()
Sys-->>VC: keyboardWillDisappear(notification)
VC->>VC: keyboardWillDisappear 처리\n(뷰 원래 위치 복구)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Projects/Presentation/Sources/Withdraw/View/WithdrawViewController.swift`:
- Around line 66-69: viewWillDisappear에서만 removeKeyboardNotification()을 호출하고
configureKeyboardNotification()는 configureAttribute()에서 한 번만 호출되어 화면이 다시 나타날 때
키보드 알림이 등록되지 않을 수 있으니, 키보드 알림의 등록/해제를 일관된 생명주기로 옮기세요: configureAttribute()에서
configureKeyboardNotification() 호출을 제거하고 viewWillAppear(_: )에서
configureKeyboardNotification()를 등록하며 기존 viewWillDisappear(_:)의
removeKeyboardNotification()은 유지하여 알림 등록/해제 패턴을 일치시킵니다; 대상 함수/메서드:
configureKeyboardNotification(), removeKeyboardNotification(),
configureAttribute(), viewWillAppear(_:), viewWillDisappear(_:).
🧹 Nitpick comments (1)
Projects/Presentation/Sources/Withdraw/View/WithdrawViewController.swift (1)
341-358: 매직 넘버를 상수로 추출하는 것을 권장합니다.Line 353의
+ 50은 버튼과 키보드 사이의 여백을 위한 값으로 보입니다. 코드 가독성과 유지보수를 위해Layoutenum에 상수로 정의하는 것을 권장합니다.제안하는 수정
private enum Layout { // ... existing constants ... + static let keyboardButtonPadding: CGFloat = 50 }- let offset = buttonBottom - visibleHeight + 50 + let offset = buttonBottom - visibleHeight + Layout.keyboardButtonPadding
| override func viewWillDisappear(_ animated: Bool) { | ||
| super.viewWillDisappear(animated) | ||
| removeKeyboardNotification() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
키보드 알림 등록/해제 시점 불일치 가능성
configureKeyboardNotification()은 configureAttribute()에서 한 번만 호출되지만, removeKeyboardNotification()은 viewWillDisappear에서 매번 호출됩니다. 만약 이 화면이 navigation stack에서 다시 나타나는 경우, 키보드 알림이 재등록되지 않아 키보드 처리가 동작하지 않을 수 있습니다.
탈퇴 플로우 특성상 화면을 벗어났다가 다시 돌아오는 경우가 드물지만, 일관성을 위해 viewWillAppear에서 등록하고 viewWillDisappear에서 해제하는 패턴을 권장합니다.
제안하는 수정 방법
+ override func viewWillAppear(_ animated: Bool) {
+ super.viewWillAppear(animated)
+ configureKeyboardNotification()
+ }
+
override func viewWillDisappear(_ animated: Bool) {
super.viewWillDisappear(animated)
removeKeyboardNotification()
}그리고 configureAttribute()에서 configureKeyboardNotification() 호출을 제거합니다.
Also applies to: 160-160, 311-323
🤖 Prompt for AI Agents
In `@Projects/Presentation/Sources/Withdraw/View/WithdrawViewController.swift`
around lines 66 - 69, viewWillDisappear에서만 removeKeyboardNotification()을 호출하고
configureKeyboardNotification()는 configureAttribute()에서 한 번만 호출되어 화면이 다시 나타날 때
키보드 알림이 등록되지 않을 수 있으니, 키보드 알림의 등록/해제를 일관된 생명주기로 옮기세요: configureAttribute()에서
configureKeyboardNotification() 호출을 제거하고 viewWillAppear(_: )에서
configureKeyboardNotification()를 등록하며 기존 viewWillDisappear(_:)의
removeKeyboardNotification()은 유지하여 알림 등록/해제 패턴을 일치시킵니다; 대상 함수/메서드:
configureKeyboardNotification(), removeKeyboardNotification(),
configureAttribute(), viewWillAppear(_:), viewWillDisappear(_:).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build_test.yml:
- Around line 33-34: The CI run calls "tuist install" then "tuist generate
--no-open" while the repo contains Tuist/Package.resolved, so make "tuist
install" deterministic by adding the --force-resolved-versions flag; update the
workflow to run "tuist install --force-resolved-versions" followed by "tuist
generate --no-open" to enforce the exact versions in Tuist/Package.resolved
(including transitive deps).
- Around line 19-22: The "Install Tuist" workflow step currently runs an
unpinned curl|bash installer which is a supply-chain risk; change it to install
a fixed Tuist version by either (A) using the mise action (jdx/mise-action@v2)
and commit a .tool-versions entry like "tuist x.y.z" so the workflow uses the
pinned version, or (B) install a specific formula via Homebrew (e.g., brew
install --formula tuist@4) in that step; optionally add a post-install
verification using the official verify.sh script to validate the installer.
| tuist install | ||
| tuist generate --no-open |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, examine the workflow file
cat -n .github/workflows/build_test.yml | head -50Repository: YAPP-Github/Bitnagil-iOS
Length of output: 1476
🏁 Script executed:
# Search for Tuist-related configuration files
fd -e lock -e lockfile -e "tuist" 2>/dev/null | head -20Repository: YAPP-Github/Bitnagil-iOS
Length of output: 50
🏁 Script executed:
# Check for any Tuist configuration files
find . -name ".tuist*" -o -name "Tuist.json" -o -name "tuist.lock" 2>/dev/nullRepository: YAPP-Github/Bitnagil-iOS
Length of output: 50
🌐 Web query:
Tuist lockfile deterministic dependency resolution CI reproducibility
💡 Result:
Tuist’s dependency “lockfile” story is essentially SwiftPM’s Package.resolved:
- What locks versions (incl. transitives): Tuist resolves dependencies via Swift Package Manager and relies on
Package.resolvedto pin the exact versions (including transitive dependencies). EnsurePackage.resolvedis committed (not gitignored), otherwise CI/dev machines may pick up newer transitive releases and break builds. [1] - How to make CI deterministic: On CI, run:
tuist install --force-resolved-versions
This forces resolution to the exact versions inPackage.resolvedto avoid non-determinism in resolution. [2]
- When versions change:
tuist installeffectively runsswift package resolve(generally respectingPackage.resolved), whiletuist install --updaterunsswift package updateand can move versions forward within the ranges inPackage.swift. [3] - CI reproducibility + caching: If you cache dependencies, Tuist recommends caching the SwiftPM clone/build directory (often
.build) and keying it off thePackage.resolvedpath used by Xcode/SwiftPM; you can also pass-clonedSourcePackagesDirPath .buildvia Tuist config /xcodebuildto keep clones in a predictable location for caching. [4]
Practical checklist for reproducible CI
- Commit
Package.resolved. - Use
tuist install --force-resolved-versionsin CI. - Only run updates intentionally (e.g.,
tuist install --updatein a dedicated “deps update” workflow/PR). - Cache
.build(or your configured clone dir) keyed byPackage.resolved. [1][2][4]
Sources: Tuist best practices (CI flag) [2], Tuist community threads on transitive locking & install/update behavior [1][3], Tuist registry CI/caching guide [4].
🏁 Script executed:
# Check for Package.resolved file
fd "Package.resolved" 2>/dev/nullRepository: YAPP-Github/Bitnagil-iOS
Length of output: 90
🏁 Script executed:
# Check for Package.swift to understand dependency management
fd "Package.swift" 2>/dev/nullRepository: YAPP-Github/Bitnagil-iOS
Length of output: 87
tuist install 시 --force-resolved-versions 플래그 추가 필요
리포지토리에 Tuist/Package.resolved 파일이 있으므로 의존성 버전이 잠금되어 있습니다. 다만, 현재 CI 워크플로우의 tuist install 명령어는 플래그 없이 실행되어 매번 해상도 과정을 거칠 수 있습니다.
CI 재현성을 보장하려면 다음과 같이 수정하세요:
tuist install --force-resolved-versions
tuist generate --no-open
이 플래그는 Package.resolved의 정확한 버전(이행 의존성 포함)을 강제하여 CI의 결정적 동작을 보장합니다.
🤖 Prompt for AI Agents
In @.github/workflows/build_test.yml around lines 33 - 34, The CI run calls
"tuist install" then "tuist generate --no-open" while the repo contains
Tuist/Package.resolved, so make "tuist install" deterministic by adding the
--force-resolved-versions flag; update the workflow to run "tuist install
--force-resolved-versions" followed by "tuist generate --no-open" to enforce the
exact versions in Tuist/Package.resolved (including transitive deps).
taipaise
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!! 레츠 기릿~~!!!
🌁 Background
QA에서 찾은 '탈퇴하기' 기능 관련 이슈들을 처리했어요 ~~~
📱 Screenshot
1. 유의사항 확인 전, 탈퇴하기 버튼 비활성화
After에서 탈퇴하기 진짜 눌렀어유 근데 안되는것임 !!!! ㅎㅎ. ...
2. 탈퇴 사유 작성 시, 키보드에 가려지는 화면 offset 조정
👩💻 Contents
📝 Review Note
댓쏘이지한 큐에이부터 ... 찬찬히 해치워나갈게유 ㅋ.ㅋ
아자아자 파이팅 ~~
📣 Related Issue
Summary by CodeRabbit
버그 수정
잡무(Chores)
✏️ Tip: You can customize this high-level summary in your review settings.