[Feature/#184] 테이스팅 노트 작성 플로우 > 빈티지 선택 화면 추가 #185
Conversation
📝 WalkthroughSummary by CodeRabbit
Walkthrough빈티지 선택 공용 프로토콜을 추가하고, 보유와인/테이스팅노트 매니저가 이를 채택하도록 확장했다. 빈티지 선택 화면을 MVVM 주입형(ReusableVintageSelectionViewController)으로 교체하고, onComplete 콜백 기반 네비게이션으로 전환했다. 뷰의 상단 섹션 API가 이름+설명 설정 방식으로 변경되었다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AddNewWineVC
participant MyOwnedWineManager as ViewModel(MyOwnedWineManager)
participant VintageVC as ReusableVintageSelectionVC
participant BuyDateVC
User->>AddNewWineVC: 와인 선택
AddNewWineVC->>ViewModel(MyOwnedWineManager): setWineId / setWineName
AddNewWineVC->>VintageVC: init(viewModel)
Note over VintageVC: 상단 섹션 = screenTitle/screenDescription
User->>VintageVC: 연도 선택 후 다음
VintageVC->>ViewModel(MyOwnedWineManager): save(vintage)
VintageVC-->>AddNewWineVC: onComplete(selectedYear)
AddNewWineVC->>BuyDateVC: push
sequenceDiagram
autonumber
actor User
participant SearchWineVC
participant TNWineDataManager as ViewModel(TNWineDataManager)
participant VintageVC as ReusableVintageSelectionVC
participant TastedDateVC
User->>SearchWineVC: 와인 선택
SearchWineVC->>TNWineDataManager: updateWineData(id/name/...)
SearchWineVC->>VintageVC: init(viewModel)
User->>VintageVC: 연도 선택 후 다음
VintageVC->>TNWineDataManager: save(vintage)
VintageVC-->>SearchWineVC: onComplete(selectedYear)
SearchWineVC->>TastedDateVC: push
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewersPre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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: 0
🧹 Nitpick comments (10)
DE/DE/Sources/Core/CommonUI/Protocols/VintageSelectionViewModel.swift (1)
5-17: VC에서 약한 참조를 허용하도록 AnyObject 제약 추가 제안VC가 viewModel을 weak로 잡을 수 있도록 프로토콜에 class(AnyObject) 제약을 거는 것을 권장합니다. 순환 참조 여지를 줄입니다.
-public protocol VintageSelectionViewModel { +public protocol VintageSelectionViewModel: AnyObject {추가로, save(vintage:)가 비동기 저장을 포함할 수 있다면 async/throws 시그니처를 고려해 주세요(현재 요구가 동기면 유지 OK).
DE/DE/Sources/Features/TastingNote/Models/TNWineDataManager.swift (1)
75-93: 뒤로가기 시 resetData() 전체 초기화는 UX/데이터 유실 위험 — MyOwnedWineManager와 동작 불일치MyOwnedWineManager는 handleBackButton에서 vintage만 리셋하는 반면, 여기서는 모든 필드를 초기화합니다. 사용자가 ‘검색 → 빈티지 선택’에서 뒤로가기를 누를 때 와인 선택까지 사라지는 것은 의도와 다를 수 있습니다.
빈티지만 초기화하는 쪽으로 정렬을 제안합니다:
- func handleBackButton() { - resetData() - } + func handleBackButton() { + vintage = 0 // 앱에서 0을 '미선택'으로 취급한다는 전제 + }참고: 0을 '미선택'으로 쓰는지 전역 규칙을 확인해 주세요. 만약 Optional이 더 자연스럽다면 모델을 Int?로 리팩터링하는 방안도 있습니다(범위 외 제안).
DE/DE/Sources/Features/Setting/Models/MyOwnedWine.swift (1)
146-162: 타이틀에는 '와인 이름'만 노출 권장 — 선택 화면에서 빈티지 중복 방지TastingNote 쪽 TNWineDataManager는 screenTitle에 순수 이름을 반환하지만, 여기서는 getWineName()으로 기존 빈티지가 포함될 수 있습니다. 빈티지 선택 화면에서는 이름만 노출하는 편이 일관되고 중복이 없습니다.
- public var screenTitle: String { - return getWineName() - } + public var screenTitle: String { + return getWine().wineName + }또한 "빈티지를 선택해 주세요" 문자열은 공용 상수/로컬라이즈로 통일하면 좋습니다.
DE/DE/Sources/Features/Setting/ViewControllers/MyWine/AddNewWineViewController.swift (1)
193-205: 미사용 클로저 파라미터 경고 제거SwiftLint 경고(unused_closure_parameter) 해결을 위해 파라미터를 _로 교체하세요.
- vc.onComplete = { [weak self] selectYear in + vc.onComplete = { [weak self] _ in let nextVC = BuyNewWineDateViewController() self?.navigationController?.pushViewController(nextVC, animated: true) }onComplete가 백그라운드에서 불릴 가능성이 없다면 현재 구현으로 충분합니다. 혹시 VC 내부 구현에 따라 비동기 콜백이 될 수 있다면 main 보장도 고려해 주세요:
vc.onComplete = { [weak self] _ in DispatchQueue.main.async { let nextVC = BuyNewWineDateViewController() self?.navigationController?.pushViewController(nextVC, animated: true) } }DE/DE/Sources/Features/Vintage/ReusableVintageSelectionViewController.swift (4)
22-24: Storyboard 경로 차단을 명시적으로 표시Interface Builder 경로를 쓰지 않는다면 unavailable 속성을 붙여 컴파일 타임에 막아 주세요.
- required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") - } + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + }
60-66: nextButton 활성화 방식 일관화
isEnabled(isEnabled:)메서드 호출과isEnabled프로퍼티 직접 대입이 혼재합니다. 하나로 통일해 가독성을 높여주세요.- self?.vintageView.nextButton.isEnabled(isEnabled: true) + self?.vintageView.nextButton.isEnabled = trueAlso applies to: 75-79
109-122: 다중 탭 방지 및 onComplete 미주입 대비 가드 추가 제안빠른 연타 시 중복 push 가능성이 있습니다. 탭 시 버튼 비활성화로 방지하고, onComplete 미주입 상황을 로깅/토스트로 방어해 주세요.
@objc func nextButtonTapped() { - // logButtonClick(screenName: screenName, buttonName: Tracking.ButtonEvent.nextBtnTapped, fileName: #file) - guard let selectedYear = vintageView.yearPicker.selectedYear else { showToastMessage(message: "연도가 선택되지 않았습니다.", yPosition: view.frame.height * 0.5) return } - - // 5. ViewModel에 선택된 빈티지 저장 + // 중복 탭 방지 + vintageView.nextButton.isEnabled = false + + // 5. ViewModel에 선택된 빈티지 저장 viewModel.save(vintage: selectedYear) - - // 6. 외부에서 주입받은 onComplete 클로저 실행 - onComplete?(selectedYear) + + // 6. 외부에서 주입받은 onComplete 클로저 실행 + if let onComplete { + onComplete(selectedYear) + } else { + // 미주입 방어: 필요한 경우 토스트/로그 + showToastMessage(message: "다음 단계로 이동할 수 없습니다.", yPosition: view.frame.height * 0.5) + vintageView.nextButton.isEnabled = true + } }
49-58: 내부 전용 메서드 접근 제어자 명시외부에서 호출되지 않는
setupUI,setupActions,setupNavigationBar는private로 좁혀 주세요.- func setupUI() { + private func setupUI() { ... - func setupActions() { + private func setupActions() { ... - private func setupNavigationBar() { + private func setupNavigationBar() {Also applies to: 60-94, 96-103
DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/SearchWineViewController.swift (2)
174-181: unused closure parameter 경고 해결
selectedYear를 사용하지 않으므로 언더스코어로 치환해 SwiftLint 경고를 없애세요.- selectVintageVC.onComplete = { [weak self] selectedYear in + selectVintageVC.onComplete = { [weak self] _ in let vc = TastedDateViewController() self?.navigationController?.pushViewController(vc, animated: true) }
181-182: 셀 선택 해제로 UX 개선푸시 후 선택 상태를 해제해 주세요.
- navigationController?.pushViewController(selectVintageVC, animated: true) + navigationController?.pushViewController(selectVintageVC, animated: true) + tableView.deselectRow(at: indexPath, animated: true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
DE/DE/Sources/Core/CommonUI/Protocols/VintageSelectionViewModel.swift(1 hunks)DE/DE/Sources/Features/Setting/Models/MyOwnedWine.swift(2 hunks)DE/DE/Sources/Features/Setting/ViewControllers/MyWine/AddNewWineViewController.swift(2 hunks)DE/DE/Sources/Features/TastingNote/Models/TNWineDataManager.swift(2 hunks)DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/SearchWineViewController.swift(2 hunks)DE/DE/Sources/Features/Vintage/ReusableVintageSelectionViewController.swift(4 hunks)DE/DE/Sources/Features/Vintage/VintageSelectionView.swift(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: doyeonk429
PR: Drink-Easy/DE_iOS#180
File: DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/RecordGraphViewController.swift:44-46
Timestamp: 2025-09-02T12:53:54.129Z
Learning: 사용자 doyeonk429는 와인 상세에서 테이스팅노트로 이동하는 플로우에서 빈티지가 무조건 설정되어 넘어간다고 설명했습니다. WineDetailViewController에서 goToTastingNote 메서드에서 vintage 가드 조건이 있어 빈티지 선택이 필수입니다.
Learnt from: doyeonk429
PR: Drink-Easy/DE_iOS#0
File: :0-0
Timestamp: 2025-08-30T13:14:55.618Z
Learning: 사용자 doyeonk429는 한국어로 코드 리뷰를 받기를 선호합니다.
📚 Learning: 2025-09-02T12:53:54.129Z
Learnt from: doyeonk429
PR: Drink-Easy/DE_iOS#180
File: DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/RecordGraphViewController.swift:44-46
Timestamp: 2025-09-02T12:53:54.129Z
Learning: 사용자 doyeonk429는 와인 상세에서 테이스팅노트로 이동하는 플로우에서 빈티지가 무조건 설정되어 넘어간다고 설명했습니다. WineDetailViewController에서 goToTastingNote 메서드에서 vintage 가드 조건이 있어 빈티지 선택이 필수입니다.
Applied to files:
DE/DE/Sources/Features/Vintage/VintageSelectionView.swiftDE/DE/Sources/Core/CommonUI/Protocols/VintageSelectionViewModel.swiftDE/DE/Sources/Features/TastingNote/Models/TNWineDataManager.swiftDE/DE/Sources/Features/Setting/Models/MyOwnedWine.swiftDE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/SearchWineViewController.swiftDE/DE/Sources/Features/Vintage/ReusableVintageSelectionViewController.swiftDE/DE/Sources/Features/Setting/ViewControllers/MyWine/AddNewWineViewController.swift
🧬 Code graph analysis (4)
DE/DE/Sources/Features/TastingNote/Models/TNWineDataManager.swift (1)
DE/DE/Sources/Features/TastingNote/Models/NewTastingNoteManager.swift (1)
resetData(117-130)
DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/SearchWineViewController.swift (1)
DE/DE/Sources/Features/TastingNote/Models/TNWineDataManager.swift (1)
updateWineData(41-59)
DE/DE/Sources/Features/Vintage/ReusableVintageSelectionViewController.swift (1)
DE/DE/Sources/Features/Vintage/VintageSelectionView.swift (1)
setTopSection(28-35)
DE/DE/Sources/Features/Setting/ViewControllers/MyWine/AddNewWineViewController.swift (3)
DE/DE/Sources/Features/Setting/Models/MyOwnedWine.swift (3)
resetWine(137-139)setWineId(77-79)setWineName(82-84)DE/DE/Sources/Features/Setting/ViewControllers/MyWine/BuyNewWineDateViewController.swift (1)
nextVC(88-104)DE/DE/Sources/Features/Setting/ViewControllers/MyWine/PriceNewWineViewController.swift (1)
nextVC(70-98)
🪛 SwiftLint (0.57.0)
DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/SearchWineViewController.swift
[Warning] 176-176: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
DE/DE/Sources/Features/Setting/ViewControllers/MyWine/AddNewWineViewController.swift
[Warning] 202-202: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (12)
DE/DE/Sources/Features/Vintage/VintageSelectionView.swift (2)
6-6: SnapKit import 적절합니다이 파일에서 snp 제약을 사용하므로 의존성 추가 OK입니다.
28-35: setTopSection: descText를 Optional로 만들고 기본 문구는 중앙화하세요DE/DE/Sources/Features/Vintage/VintageSelectionView.swift (약 28–35행) — setTopSection이 매 호출마다 descText를 요구하지 않도록 Optional 기본값을 주고, 기본 설명 문구는 중앙집중화하세요. despText라는 오타 프로퍼티가 보이면 rename 권장.
- func setTopSection(name: String, descText: String) { + func setTopSection(name: String, descText: String? = nil) { topView.setTitleLabel(title: name, titleStyle: AppTextStyle.KR.subtitle1, titleColor: AppColor.purple100, - description: descText, + description: descText ?? UIString.vintageSelectPrompt, descriptionStyle: AppTextStyle.KR.head, descriptionColor: AppColor.black) }
- 변경 요점: descText를 Optional로 만들어 호출부를 단순화하고, 기본값은 중앙화된 상수(Localizable.strings 또는 UIString.vintageSelectPrompt)에 두세요.
- 오타 점검: 코드에 despText가 존재하면 descText로 변경하여 혼동 제거.
- 중복 문자열 정리: "빈티지를 선택해 주세요"는 Localizable.strings나 공용 상수로 이동.
리포지토리에서 구 API/오타 흔적(예: setWineName, SelectVintageViewController, despText 등)을 직접 확인해 주세요. 다음 명령을 실행한 결과를 붙여주시면 추가로 검증하겠습니다:
rg -nP --hidden -uu '\bsetWineName\s*\(' -S || true rg -nP --hidden -uu '\bSelectVintageViewController\b' -S || true rg -nP --hidden -uu '빈티지를 선택해 주세요' -S || true rg -nP --hidden -uu '\bdespText\b|\bdescText\b' -S || trueDE/DE/Sources/Features/TastingNote/Models/TNWineDataManager.swift (1)
4-4: CoreModule import 적절합니다DE/DE/Sources/Features/Setting/Models/MyOwnedWine.swift (1)
3-3: CoreModule import 적절합니다DE/DE/Sources/Features/Setting/ViewControllers/MyWine/AddNewWineViewController.swift (1)
18-18: 싱글톤 뷰모델 주입 OK해당 VC 수명 동안 동일 인스턴스를 사용하므로 일관성 측면에서 문제없습니다. 필요 시 DI로 대체 가능하지만 현 단계에선 과하지 않습니다.
DE/DE/Sources/Features/Vintage/ReusableVintageSelectionViewController.swift (5)
13-21: MVVM 주입과 onComplete 공개, 방향성 좋습니다뷰모델 주입과 외부 콜백 기반 흐름으로 분리한 점 아주 👍
26-30: 상단 섹션을 ViewModel로 구성한 점 적절합니다문자열 하드코딩 제거되고 테스트/재사용 용이해졌습니다.
32-35: nav bar hidden 토글, 후속 화면에 영향 확인 필요viewWillDisappear에서 네비게이션바를 숨기면 다음 화면(TastedDate 등)이 잠깐 숨김 상태로 표시될 수 있습니다. 다음 화면에서 명시적으로
isNavigationBarHidden = false를 보장하는지 확인 부탁드립니다.
96-101: 백 버튼 처리 OK네비게이션 매니저를 통한 일관된 백 동작과 ViewModel 훅 호출 모두 적절합니다.
127-131: 모달 dismiss 시 피커 상태 동기화 OK프레젠테이션 델리게이트 활용 적절합니다.
DE/DE/Sources/Features/TastingNote/ViewControllers/CreateVCs/SearchWineViewController.swift (2)
23-23: 네비게이션바 표시 위치(viewWillAppear)로 이동, 적절합니다화면 복귀 시점에 맞춘 노출 제어로 일관성 좋아졌습니다.
172-173: 데이터 매니저 업데이트 순서 적절결과 선택 → 데이터 업데이트 → 빈티지 선택 화면 진입 플로우 타이밍 괜찮습니다.
🚀 PR 개요
💡 PR 유형
✏️ 변경 사항 요약
🔗 관련 이슈
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-09-12.at.17.05.10.mp4
✅ PR 체크리스트
develop입니다💬 추가 설명 or 리뷰 포인트 (선택)