feat: 검색 > 도서 등록 상태별 Sheet 분기 처리 #228
Merged
Conversation
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
WalkthroughBookRegistrationStatus에 상태별 텍스트 반환 메서드 3개가 추가되고, SearchViewModel에 선택된 상태를 저장하는 프로퍼티가 도입되었으며, SearchViewController의 presentNoteSuggestion이 상태를 인자로 받아 시트의 제목·자막·버튼 구성을 상태 기반으로 결정하도록 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SearchViewController
participant SearchViewModel
participant BookRegistrationStatus
User->>SearchViewController: 도서 선택 / 업서트 트리거
SearchViewController->>SearchViewModel: upsertBook(isbn, status)
SearchViewModel->>SearchViewModel: state.selectedStatus = status
SearchViewModel-->>SearchViewController: emit (isbn, status)
SearchViewController->>BookRegistrationStatus: getSubTitle()/getCancelButtonTitle()/getNextButtonTitle()
BookRegistrationStatus-->>SearchViewController: 상태별 텍스트 반환
SearchViewController->>SearchViewController: presentNoteSuggestion(isbn, status) - 상태 기반 UI 구성
SearchViewController-->>User: 시트 표시 (동적 제목/자막/버튼)
User->>SearchViewController: 버튼 선택 (취소/다음)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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 (4)
src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift (2)
95-95: Optional에 대한 불필요한 nil 초기화 제거SwiftLint 경고처럼 Optional은 기본값 없이 선언해도 nil입니다. 불필요한 초기화를 제거해주세요.
- var selectedStatus: BookRegistrationStatus? = nil + var selectedStatus: BookRegistrationStatus?
274-281: 시트 표시 후 상태 정리: selectedStatus도 함께 초기화 권장업서트 성공 후 시트가 표시되면 isUpserted만 nil로 초기화되고 selectedStatus는 유지됩니다. 다음 흐름에서 오작동은 없겠지만(업서트 시 직전에 덮어쓰이긴 함), 상태 일관성을 위해 noteSuggestionShown에서 selectedStatus도 nil로 리셋하는 편이 안전합니다.
case .noteSuggestionShown: newState.isUpserted = nil + newState.selectedStatus = nilsrc/Projects/BKPresentation/Sources/MainFlow/Search/View/SearchViewController.swift (2)
170-186: 중복 제거 로직 단순화 가능(String, BookRegistrationStatus) 튜플은 Equatable이므로 커스텀 비교 클로저 없이 removeDuplicates()만으로 중복 제거가 가능합니다. 가독성을 위해 단순화를 제안합니다.
- .removeDuplicates { previous, current in - return previous.isbn == current.isbn && previous.status == current.status - } + .removeDuplicates()
284-330: 버튼 그룹 Optional 제거 및 문자열 로컬라이즈 권장
- buttonGroup는 모든 분기에서 초기화되므로 Optional일 필요가 없습니다. 불변 let + 삼항/스위치로 단순화하세요.
- 시트 타이틀/버튼 텍스트는 로컬라이즈 리소스로 관리하는 것이 바람직합니다.
- var buttonGroup: BKButtonGroup? - - if status == .before { - buttonGroup = .singleFullButton( - title: status.getCancelButtonTitle(), - action: cancelAction - ) - } else { - buttonGroup = .twoButtonGroup( - leftTitle: status.getCancelButtonTitle(), - rightTitle: status.getNextButtonTitle(), - leftAction: cancelAction, - rightAction: nextAction - ) - } + let buttonGroup: BKButtonGroup = { + switch status { + case .before: + return .singleFullButton( + title: status.getCancelButtonTitle(), + action: cancelAction + ) + case .inProgress, .after: + return .twoButtonGroup( + leftTitle: status.getCancelButtonTitle(), + rightTitle: status.getNextButtonTitle(), + leftAction: cancelAction, + rightAction: nextAction + ) + } + }()추가로, 아래 하드코딩 문자열의 로컬라이즈 적용을 고려해 주세요.
- sheetTitle: "도서가 등록되었어요!"
- cancel/next 버튼 타이틀(상태별 문자열도 리소스 키로 관리)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Projects/BKPresentation/Sources/MainFlow/Search/VO/BookRegistrationStatus.swift(1 hunks)src/Projects/BKPresentation/Sources/MainFlow/Search/View/SearchViewController.swift(2 hunks)src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Projects/BKPresentation/Sources/MainFlow/Search/View/SearchViewController.swift (5)
src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift (1)
hideLoading(77-79)src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift (1)
send(175-183)src/Projects/BKCore/Sources/Tracking/ScreenLoggable.swift (1)
logScreenView(14-32)src/Projects/BKDesign/Sources/Components/Button/BKButtonGroup.swift (2)
singleFullButton(189-202)twoButtonGroup(167-187)src/Projects/BKPresentation/Sources/MainFlow/Search/VO/BookRegistrationStatus.swift (3)
getCancelButtonTitle(43-50)getNextButtonTitle(52-61)getSubTitle(32-41)
🪛 SwiftLint (0.57.0)
src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift
[Warning] 95-95: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/Projects/BKPresentation/Sources/MainFlow/Search/VO/BookRegistrationStatus.swift (1)
32-61: 리뷰 코멘트의 getNextButtonTitle 기본값 지적은 부정확합니다검증 결과, SearchViewController.swift의 호출부(307줄)에서
if status == .before조건문으로 상태를 분기하고 있습니다:
.before케이스:.singleFullButton만 사용 (getNextButtonTitle 미호출)- 그 외(
.inProgress,.after):.twoButtonGroup에서 getNextButtonTitle 호출따라서
getNextButtonTitle()의default: return ""부분은 실행될 수 없습니다. 호출부가 이미 조건을 통해 안전하게 보호하고 있으므로, 기본값 반환이 호출 실수를 숨긴다는 우려는 해당되지 않습니다.남은 제안은 선택적 개선 사항입니다:
- get 접두사 제거 및 계산 프로퍼티화 (Swift 가이드라인)
- 하드코딩 문자열의 i18n 처리
Likely an incorrect or invalid review comment.
doyeonk429
force-pushed
the
BOOK-357-feature/#227
branch
from
d349d70 to
45a2234
Compare
doyeonk429
force-pushed
the
BOOK-357-feature/#227
branch
from
45a2234 to
f0a99f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift (2)
95-95: 불필요한 nil 초기화 제거 (SwiftLint 경고 해결)Optional은 기본값이 nil이므로 초기화가 중복입니다. 경고를 없애고 가독성을 높여주세요.
- var selectedStatus: BookRegistrationStatus? = nil + var selectedStatus: BookRegistrationStatus?
273-281: selectedStatus 보존/리셋 타이밍 점검업서트 시작 시 상태를 보존하고(.upsertBook) 시트 표시 직후(.noteSuggestionShown) nil로 리셋하는 흐름은 합리적입니다. 다만 재시도 끝에 실패하여 최종 에러를 노출한 뒤에도 selectedStatus가 남아 있을 수 있습니다. 재방문 시 잔존 상태가 오해를 유발할 수 있으니, “최종 실패 확정” 시점에서 정리하는 방안을 고려해주세요.
예시(최종 재시도 실패 알럿 이후 정리):
case .lastRetryTapped: newState.isLoading = false newState.isRetrying = false newState.isReRetrying = false + newState.selectedStatus = nil또는 에러를 실제로 노출하는 분기(두 번째 에러에서 newState.error 할당)에서 정리:
- } else { - newState.isRetrying = false - newState.error = error - } + } else { + newState.isRetrying = false + newState.error = error + newState.selectedStatus = nil + }의도적으로 “재시도 보존”을 원하신다면 현행 유지 + 위 정리 타이밍만 합의되면 충분합니다.
Also applies to: 287-304, 313-318
src/Projects/BKPresentation/Sources/MainFlow/Search/View/SearchViewController.swift (2)
171-185: 중복 표시 방지 로직 좋습니다. Equatable 기반으로 간결화 가능(isbn, status) 튜플을 기준으로 시트를 한 번만 띄우는 의도가 명확합니다. 튜플은 기본적으로 Equatable이므로 커스텀 비교 클로저 없이도 동작합니다.
- .removeDuplicates { previous, current in - return previous.isbn == current.isbn && previous.status == current.status - } + .removeDuplicates()
284-327: 상태별 버튼 구성 OK. 옵셔널 제거와 문자열 로컬라이즈 제안
- buttonGroup을 옵셔널로 둘 필요가 없어 보입니다. switch로 분기하여 let 상수로 정리하면 더 명확합니다.
- "도서가 등록되었어요!"는 Localizable로 이동하는 것을 권장합니다. 상태별 서브타이틀과 버튼 타이틀이 이미 Enum 메서드로 캡슐화되어 있어 일관성 있게 관리할 수 있습니다.
- var buttonGroup: BKButtonGroup? - - if status == .before { - buttonGroup = .singleFullButton( - title: status.getCancelButtonTitle(), - action: cancelAction - ) - } else { - buttonGroup = .twoButtonGroup( - leftTitle: status.getCancelButtonTitle(), - rightTitle: status.getNextButtonTitle(), - leftAction: cancelAction, - rightAction: nextAction - ) - } + let buttonGroup: BKButtonGroup = { + switch status { + case .before: + return .singleFullButton( + title: status.getCancelButtonTitle(), + action: cancelAction + ) + default: + return .twoButtonGroup( + leftTitle: status.getCancelButtonTitle(), + rightTitle: status.getNextButtonTitle(), + leftAction: cancelAction, + rightAction: nextAction + ) + } + }() let sheet = BKBottomSheetViewController( - title: sheetTitle, + title: sheetTitle, // TODO: Localizable로 이동 권장 subtitle: status.getSubTitle(), style: .centered, suppliedContentStyle: .upper(graphicView), buttonConfiguration: buttonGroup )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Projects/BKPresentation/Sources/MainFlow/Search/View/SearchViewController.swift(2 hunks)src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Projects/BKPresentation/Sources/MainFlow/Search/View/SearchViewController.swift (5)
src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift (1)
hideLoading(77-79)src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift (1)
send(175-183)src/Projects/BKCore/Sources/Tracking/ScreenLoggable.swift (1)
logScreenView(14-32)src/Projects/BKDesign/Sources/Components/Button/BKButtonGroup.swift (2)
singleFullButton(189-202)twoButtonGroup(167-187)src/Projects/BKPresentation/Sources/MainFlow/Search/VO/BookRegistrationStatus.swift (3)
getCancelButtonTitle(43-50)getNextButtonTitle(52-61)getSubTitle(32-41)
🪛 SwiftLint (0.57.0)
src/Projects/BKPresentation/Sources/MainFlow/Search/ViewModel/SearchViewModel.swift
[Warning] 95-95: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
clxxrlove
approved these changes
Oct 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
ScreenRecording_10-18-2025.16-00-32_1.1.mp4
✅ PR 체크리스트
Summary by CodeRabbit
New Features
개선 사항