Merged
Conversation
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new CallVan feature: domain models, use cases (with mocks), extensive UI (list, post, data, chat, notifications, report), picker and bottom-sheet components, assets, and Home integration; plus small core extensions and project file updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as rgba(50,120,200,0.5)
participant ListVC as rgba(100,200,150,0.5)
participant ListVM as rgba(200,150,100,0.5)
participant FetchUseCase as rgba(180,100,200,0.5)
participant DataVC as rgba(100,150,250,0.5)
participant ChatVC as rgba(250,150,100,0.5)
User->>ListVC: open CallVan list
ListVC->>ListVM: viewDidLoad / request posts
ListVM->>FetchUseCase: execute(request)
FetchUseCase-->>ListVM: publish CallVanList
ListVM-->>ListVC: publish posts
User->>ListVC: tap post
ListVC->>DataVC: present CallVanData(postId)
DataVC->>FetchUseCase: fetch post data (postId)
DataVC-->>User: show participants, actions
User->>DataVC: open chat
DataVC->>ChatVC: present Chat(postId)
ChatVC->>ChatVC: use CallVanChatViewModel for messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (20)
Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift-73-80 (1)
73-80:⚠️ Potential issue | 🟠 MajorAvoid
fatalError()in production navigation code.If
CallVanDataViewControlleris not in the navigation stack, the app will crash. Use a safer fallback such as popping to root or dismissing.🐛 Proposed fix
`@objc` private func reportButtonTapped() { if let viewController = navigationController?.viewControllers.first(where: { $0 is CallVanDataViewController }) { navigationController?.popToViewController(viewController, animated: true) showToast(message: "사용자가 신고되었습니다.") } else { - fatalError() + navigationController?.popToRootViewController(animated: true) + showToast(message: "사용자가 신고되었습니다.") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift` around lines 73 - 80, The reportButtonTapped method in CallVanReportEvidenceViewController currently calls fatalError() if a CallVanDataViewController instance isn't found on the navigation stack; replace this crash path with a safe fallback: if navigationController contains a CallVanDataViewController pop to it and show the toast as before, otherwise attempt a non-crashing fallback such as navigationController?.popToRootViewController(animated: true) or dismiss(animated: true) (and still show the toast or an error message). Update the logic inside reportButtonTapped to handle a nil navigationController and to avoid fatalError(), referencing CallVanReportEvidenceViewController.reportButtonTapped and CallVanDataViewController to locate the code.Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swift-53-71 (1)
53-71:⚠️ Potential issue | 🟠 MajorKeep the initial selection typed instead of reconstructing it from display text.
place ?? CallVanPlace.frontGate.descriptionturns “no current selection” into a real choice, and matching against button titles loses whether the original state was.custom. That means an untouched sheet can submitfrontGate, and a custom value equal to a preset label will reopen as the preset.Suggested direction
- init(title: Title, place: String?, onApplyButtonTapped: `@escaping` (CallVanPlace, String?)->Void) { + init( + title: Title, + selectedPlace: CallVanPlace?, + customPlace: String?, + onApplyButtonTapped: `@escaping` (CallVanPlace, String?)->Void + ) { self.onApplyButtonTapped = onApplyButtonTapped super.init(frame: .zero) configureView() setAddTargets() setDelegate() titleLabel.text = title.rawValue - let place = place ?? CallVanPlace.frontGate.description (buttons1 + buttons2).forEach { - $0.isSelected = $0.title == place.description + $0.isSelected = $0.title == selectedPlace?.description } - if let _ = (buttons1 + buttons2).first(where: { $0.isSelected }) { + if selectedPlace == .custom { + customButton.isSelected = true + customPlaceTextField.text = customPlace + // expand custom field + } else { customButton.isSelected = false - } else { - customButton.isSelected = true - customPlaceTextField.text = place - ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swift` around lines 53 - 71, The initializer currently accepts place: String? and reconstructs selection from display text which loses the original CallVanPlace kind; change the init signature to init(title: Title, place: CallVanPlace?, onApplyButtonTapped: `@escaping` (CallVanPlace, String?)->Void) (or if signature change isn't allowed, convert the incoming String? to a CallVanPlace? once upstream) and then use that typed CallVanPlace? directly: when place is nil leave all preset buttons unselected and customButton false, when place is a preset find the matching button in (buttons1 + buttons2) and set its isSelected, and when place == .custom mark customButton.isSelected = true and set customPlaceTextField.text = place.description; stop defaulting to CallVanPlace.frontGate.description and avoid matching by button title to preserve the original .custom state.Koin/Core/View/BottomSheetViewControllerB.swift-110-136 (1)
110-136:⚠️ Potential issue | 🟠 MajorFix the keyboard notification handler signatures.
The
keyboardWillShow()andkeyboardWillHide()methods must accept aNotificationparameter. When registering withNotificationCenter.addObserver(_:selector:name:object:), the target selector must point to a method that takes exactly one argument—the posted notification object. The current zero-argument methods are incorrect and will fail at runtime.Update the handlers to:
`@objc` private func keyboardWillShow(_ notification: Notification) { // ... } `@objc` private func keyboardWillHide(_ notification: Notification) { // ... }This also enables access to keyboard animation metadata for synchronized transitions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Core/View/BottomSheetViewControllerB.swift` around lines 110 - 136, The keyboard notification handlers registered in addObserver currently point to zero-argument selectors and will crash; update the methods keyboardWillShow and keyboardWillHide to accept a Notification parameter (e.g., change signatures to keyboardWillShow(_ notification: Notification) and keyboardWillHide(_ notification: Notification)), update any internal calls to use the notification if needed (e.g., keyboard animation info), and keep the NotificationCenter.addObserver calls as-is so the selectors match the single-argument methods.Koin/Presentation/Home/ServiceSelect/ServiceSelectViewController.swift-155-164 (1)
155-164:⚠️ Potential issue | 🟠 MajorDon't wire the main menu entry to
MockFetchCallVanListUseCase.This route is now reachable from the service selector, so it will always show fixture data and never exercise the real loading/error path. Please swap in the production fetch use case here, or gate the menu entry until the real integration is ready.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/Home/ServiceSelect/ServiceSelectViewController.swift` around lines 155 - 164, The pushCallVanList method is wired to MockFetchCallVanListUseCase causing fixture-only behavior; replace MockFetchCallVanListUseCase with the production fetch use case (e.g. DefaultFetchCallVanListUseCase or the real FetchCallVanListUseCase implementation) when constructing CallVanListViewModel, or alternatively disable/hide the menu entry until the real integration exists; update the instantiation in pushCallVanList where DefaultUserRepository, DefaultUserService, DefaultCheckLoginUseCase, and CallVanListViewModel are created so the viewModel receives the production fetch use case instead of the mock.Koin/Presentation/Home/Home/HomeViewController.swift-645-652 (1)
645-652:⚠️ Potential issue | 🟠 MajorReplace the mock use case before merging.
MockFetchCallVanListUseCase()is wired into the real home entry point, so this flow will always show fixture data in normal builds. Please inject the actual fetch use case here, or gate the mock behind a debug-only path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/Home/Home/HomeViewController.swift` around lines 645 - 652, The navigateToCallVanList() function is instantiating MockFetchCallVanListUseCase and wiring fixture data into the real flow; replace MockFetchCallVanListUseCase with the production implementation (e.g., DefaultFetchCallVanListUseCase or your concrete FetchCallVanListUseCase) when creating the CallVanListViewModel, or alternatively wrap the mock instantiation behind a debug-only flag (e.g., `#if` DEBUG) so CallVanListViewModel receives the real fetch use case in release builds; update the instantiation in navigateToCallVanList() accordingly.Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanButton.swift-38-43 (1)
38-43:⚠️ Potential issue | 🟠 MajorReset the border when the next state has no border.
If this button is reused, configuring a bordered state first and then a borderless state leaves the old 1pt border in place because the
nilbranch never clearslayer.borderWidth/layer.borderColor.🐛 Suggested fix
- if let _ = state.borderColor { + if let borderColor = state.borderColor { layer.borderWidth = 1 - layer.borderColor = state.borderColor?.cgColor + layer.borderColor = borderColor.cgColor + } else { + layer.borderWidth = 0 + layer.borderColor = nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanButton.swift` around lines 38 - 43, The current configuration leaves a previous border when state.borderColor is nil; update the view's configuration logic (where you set backgroundColor = state.backgroundColor and layer.cornerRadius = 4) to explicitly clear the border when state.borderColor is nil by setting layer.borderWidth = 0 and layer.borderColor = nil in the else branch that checks state.borderColor, ensuring any prior 1pt border is removed (refer to state.borderColor, layer.borderWidth, and layer.borderColor in the CallVanButton configuration method).Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift-43-45 (1)
43-45:⚠️ Potential issue | 🟠 MajorDon't ship the chat screen hardwired to
CallVanChatDto.dummy().These lines ignore the injected
viewModeland always render the same room/messages. As written, real chat data can never reach this controller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift` around lines 43 - 45, The controller is using CallVanChatDto.dummy().toDomain() directly which ignores the injected viewModel; replace this hardcoded data by pulling the real chat from the viewModel (e.g. use a viewModel property/publisher such as callVanChat / chatPublisher or a load/fetch method) and call callVanChatTableView.configure(callVanChat: ...) and set title from that real model when the viewModel emits or after fetching; remove the dummy usage (CallVanChatDto.dummy(), toDomain()) and wire the UI update to the viewModel's data flow instead.Koin/Presentation/CallVan/CallVanList/Subviews/CallVanFilterButton.swift-14-19 (1)
14-19:⚠️ Potential issue | 🟠 MajorApply the default style during initialization.
The title and border color are only configured inside
isSelected.didSet. A freshly created button starts in the defaultfalsestate, so it can render without text/border until selection changes, and its initial intrinsic width is off as well.Suggested fix
init(title: String) { self.title = title super.init(frame: .zero) layer.cornerRadius = 17 layer.borderWidth = 1 + updateAppearance() } @@ override var isSelected: Bool { didSet { - let foregroundColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral500) - let borderColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral300) - - setAttributedTitle(NSAttributedString(string: title, attributes: [ - .font : UIFont.appFont(.pretendardBold, size: 14), - .foregroundColor : foregroundColor - ]), for: .normal) - - layer.borderColor = borderColor.cgColor + updateAppearance() } } + + private func updateAppearance() { + let foregroundColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral500) + let borderColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral300) + + setAttributedTitle( + NSAttributedString( + string: title, + attributes: [ + .font: UIFont.appFont(.pretendardBold, size: 14), + .foregroundColor: foregroundColor + ] + ), + for: .normal + ) + layer.borderColor = borderColor.cgColor + }Also applies to: 28-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanFilterButton.swift` around lines 14 - 19, The CallVanFilterButton's init currently sets title only in isSelected.didSet, causing new instances to render without text/border; update the initializer(s) (init(title:), and required init?(coder:)) to apply the default (isSelected == false) style immediately by calling the same styling logic used in isSelected.didSet: set the button title (title property) for .normal, configure layer.borderColor and any titleColor/background for the unselected state so the button has correct text, border and intrinsic size on creation; ensure you reference the isSelected.didSet styling code (or extract it into a private applyStyle() helper used from both isSelected.didSet and initializers) and use the existing title property to set the displayed text.Koin/Domain/UseCase/CallVan/FetchCallVanListUseCase.swift-38-43 (1)
38-43:⚠️ Potential issue | 🟠 MajorKeep the mock pagination metadata consistent with the sample posts.
This publisher returns 9 posts, but
totalCount,currentPage, andtotalPageare all zero. Any empty-state or paging UI driven by those fields will render incorrectly against the mock.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Domain/UseCase/CallVan/FetchCallVanListUseCase.swift` around lines 38 - 43, The mock CallVanListDto is returning 9 posts but leaves pagination fields at zero; update the CallVanListDto initialization in FetchCallVanListUseCase (the call that constructs CallVanListDto(posts: postsDto, totalCount: 0, currentPage: 0, totalPage: 0)) to provide consistent pagination metadata (e.g., set totalCount to postsDto.count or a realistic total, set currentPage to 1 if this is the first page, and compute totalPage accordingly) so the returned callVanList produced by callVanListDto.toDomain() reflects correct paging for UI tests.Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift-72-112 (1)
72-112:⚠️ Potential issue | 🟠 MajorThe custom navigation title view never gets populated.
titleLabelandparitipantsLabelare local views with no text set before they are embedded intonavigationItem.titleView. Because a custom title view is installed on Line 112, the latertitle = ...assignment will not show either. These labels need to be stored properties and updated when the chat data is bound.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift` around lines 72 - 112, The custom title labels (titleLabel and paritipantsLabel) are created as local variables inside configureNavigationBar so they never get updated later; make them stored properties on the CallVanChatViewController (e.g., private let titleLabel = UILabel(), private let paritipantsLabel = UILabel()) and update configureNavigationBar to use those properties when building the titleView; then set their text when binding chat data (where the ViewModel is applied, e.g., in your bindViewModel or after viewModel is assigned) using viewModel.callVanPost.title / departureTime and viewModel.callVanPost.currentParticipants / maxParticipants so the navigationItem.titleView actually displays populated values.Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swift-42-48 (1)
42-48:⚠️ Potential issue | 🟠 MajorZero-pad the minute column to match the formatter output.
outputFormatteremitsmmvalues like03, butgetItems()currently creates0...9as"0"..."9". For any time in the first 10 minutes of the hour,selectedItemwill not exist in the third column, soresetcan land on the wrong minute.Suggested fix
- let minutes = Array(00...59).map { "\($0)" } + let minutes = Array(0...59).map { String(format: "%02d", $0) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swift` around lines 42 - 48, getItems() currently builds minutes as Array(00...59).map { "\($0)" } which yields single-digit strings for 0-9 causing mismatches with outputFormatter's "mm" (e.g. "03"); update the minutes mapping to zero-pad to two digits (use String(format: "%02d", value)) so the third column values match outputFormatter, keeping the function name getItems() and return structure ([amPm, hours, minutes]) unchanged.Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swift-8-20 (1)
8-20:⚠️ Potential issue | 🟠 MajorAdd missing
import Thenstatement.This file uses
.thenon lines 13 and 17 to initializeDateFormatterinstances, but does not import theThenmodule. This will cause a compilation error. Addimport Thenafter theimport Foundationline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swift` around lines 8 - 20, The file KoinPickerDropDownViewTimeDelegate uses the Then syntactic sugar when creating inputFormatter and outputFormatter (DateFormatter().then { ... }) but misses the Then import; add the statement to import Then immediately after the existing import Foundation so the .then extension is available to the DateFormatter initializations in KoinPickerDropDownViewTimeDelegate.Koin/Presentation/Home/Home/SubViews/CallVanView.swift-66-94 (1)
66-94:⚠️ Potential issue | 🟠 MajorAdd explicit accessibility labels to the custom buttons.
These
UIButtons do not usesetTitle; their visible text lives in child labels. VoiceOver will typically announce them as unlabeled buttons, which blocks this entry point for screen-reader users. Set button-levelaccessibilityLabel/accessibilityHintvalues and mark the nested labels/images as non-accessibility elements.Suggested fix
[findButton, postButton].forEach { $0.backgroundColor = UIColor.appColor(.neutral50) $0.layer.cornerRadius = 8 } + findButton.isAccessibilityElement = true + findButton.accessibilityLabel = "콜밴팟 찾기" + findButton.accessibilityHint = "콜밴팟 목록 화면으로 이동" + postButton.isAccessibilityElement = true + postButton.accessibilityLabel = "콜밴팟 모집" + postButton.accessibilityHint = "콜밴팟 글 작성 화면으로 이동" @@ [findRightArrowImageView, postRightArrowImageView].forEach { $0.image = UIImage.appImage(asset: .chevronRightHome)?.withRenderingMode(.alwaysTemplate) $0.tintColor = UIColor.appColor(.neutral800) } + [ + findTitleLabel, findDescriptionLabel, findRightArrowImageView, + postTitleLabel, postDescriptionLabel, postRightArrowImageView + ].forEach { + $0.isAccessibilityElement = false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/Home/Home/SubViews/CallVanView.swift` around lines 66 - 94, In setUpStyles, add accessibility on the custom buttons: set findButton.accessibilityLabel and postButton.accessibilityLabel (e.g., match the visible text like findTitleLabel.text / postTitleLabel.text), add descriptive accessibilityHint for each, and ensure both findButton and postButton have accessibilityTraits = .button; also mark the nested views (findTitleLabel, postTitleLabel, findDescriptionLabel, postDescriptionLabel, findRightArrowImageView, postRightArrowImageView) as isAccessibilityElement = false so VoiceOver announces the button labels/hints only.Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift-207-214 (1)
207-214:⚠️ Potential issue | 🟠 Major
chatButtonandcallButtonhave identical constraints — they will overlap.Both buttons are constrained to
trailing.equalToSuperview().offset(-24)andtop.equalToSuperview().offset(16.5). When bothshowChatButtonandshowCallButtonare true (lines 104-105), the buttons will render on top of each other.🐛 Proposed fix: Stack or offset buttons horizontally
chatButton.snp.makeConstraints { $0.top.equalToSuperview().offset(16.5) - $0.trailing.equalToSuperview().offset(-24) + $0.trailing.equalTo(callButton.snp.leading).offset(-8) } callButton.snp.makeConstraints { $0.top.equalToSuperview().offset(16.5) $0.trailing.equalToSuperview().offset(-24) }Alternatively, use a horizontal stack view for these action buttons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift` around lines 207 - 214, The constraints for chatButton and callButton in CallVanListCollectionViewCell are identical, causing overlap when both showChatButton and showCallButton are true; update the layout so the two controls are horizontally arranged (e.g., place them in a UIStackView or adjust constraints to set callButton.leading equalTo chatButton.trailing with spacing) and ensure constraints are applied only when the corresponding showChatButton/showCallButton flags are used to show/hide the buttons so they don't reserve conflicting layout space.Koin/Presentation/CallVan/CallVanNotification/CallVanNotificationViewController.swift-68-101 (1)
68-101:⚠️ Potential issue | 🟠 MajorThese notification actions are still no-ops.
onReadButtonTappedand the confirm closure are empty, so the dropdown exposes read/delete-all flows that never update the list. At minimum, hide or disable these actions until the corresponding view model inputs exist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanNotification/CallVanNotificationViewController.swift` around lines 68 - 101, The rightBarButtonTapped currently provides empty handlers (onReadButtonTapped, onDeleteButtonTapped, and the onMainButtonTapped confirm closure) which leave the Read/Delete-All flows as no-ops; either wire these closures to the view model inputs (e.g., call viewModel.markAllAsRead(), viewModel.deleteAllNotifications(), and the confirmation flow in CallVanBottomSheetViewController) and refresh the list (reload table/collection or call viewModel.fetchNotifications()), or disable/hide the corresponding actions in CallVanNotificationDropdownViewController and the confirmation button in CallVanBottomSheetViewController until those view model methods exist; update references in rightBarButtonTapped, CallVanNotificationDropdownViewController initializer, and CallVanBottomSheetViewController usage accordingly.Koin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportCustomReasonButton.swift-14-18 (1)
14-18:⚠️ Potential issue | 🟠 MajorExpose this custom option to accessibility.
Because this button uses a nested label instead of a
UIButtontitle, VoiceOver won't announce the reason text or selected state unless you set accessibility metadata yourself.Suggested fix
final class CallVanReportCustomReasonButton: UIButton { override var isSelected: Bool { didSet { radioButtonImageView.image = isSelected ? UIImage.appImage(asset: .callVanRadioButtonSelected) : UIImage.appImage(asset: .callVanRadioButtonDeselected) + updateAccessibility() } } ... init() { super.init(frame: .zero) configureView() isSelected = false + isAccessibilityElement = true + accessibilityLabel = reasonLabel.text + updateAccessibility() } required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } } extension CallVanReportCustomReasonButton { + private func updateAccessibility() { + accessibilityTraits = isSelected ? [.button, .selected] : [.button] + } + private func configureView() { setUpStyles() setUpLayouts() setUpConstraints() }Also applies to: 25-29, 43-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportCustomReasonButton.swift` around lines 14 - 18, The custom button CallVanReportCustomReasonButton uses a nested label so update accessibility by making the control an accessibility element and exposing the label text and selected state: set isAccessibilityElement = true (e.g., in init), set accessibilityLabel = reasonLabel.text (or the nested label's text), and update accessibilityTraits to include .selected when override var isSelected changes (update inside the isSelected didSet alongside radioButtonImageView.image) so VoiceOver announces both the reason text and selection state; also update accessibilityValue or accessibilityHint if present to match any additional metadata.Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift-98-103 (1)
98-103:⚠️ Potential issue | 🟠 MajorSend sub-button taps through
subButtonTappedPublisher.The sub-button branch currently emits through
mainButtonTappedPublisher, so consumers cannot distinguish the two actions.Suggested fix
cell.subButtonTappedPublisher.sink { [weak self] postId in guard let self else { return } if let subState = posts.first(where: { $0.postId == postId })?.subState { - mainButtonTappedPublisher.send((postId, subState)) + subButtonTappedPublisher.send((postId, subState)) print(subState.rawValue) } }.store(in: &cell.subscriptions)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift` around lines 98 - 103, The sub-button handler is currently emitting events to mainButtonTappedPublisher; change it to emit to subButtonTappedPublisher so consumers can distinguish actions: inside the sink on cell.subButtonTappedPublisher where you look up subState from posts (using posts.first(where: { $0.postId == postId })?.subState), call subButtonTappedPublisher.send((postId, subState)) instead of mainButtonTappedPublisher.send(...); also remove or keep the debug print as desired.Koin/Presentation/CallVan/CallVanData/CallVanDataViewModel.swift-50-56 (1)
50-56:⚠️ Potential issue | 🟠 MajorDon't ignore the failure branch.
receiveCompletionswallows.failure, so the view controller never learns thatexecute(postId:)failed.Suggested fix
enum Output { case update(CallVanData) + case showError(ErrorResponse) } ... private func fetchData() { fetchCallVanDataUseCase.execute(postId: postId).sink( - receiveCompletion: { _ in }, + receiveCompletion: { [weak self] completion in + guard case let .failure(error) = completion else { return } + self?.outputSubject.send(.showError(error)) + }, receiveValue: { [weak self] callVanData in self?.outputSubject.send(.update(callVanData)) } ).store(in: &subscriptions) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanData/CallVanDataViewModel.swift` around lines 50 - 56, fetchData currently ignores failures from fetchCallVanDataUseCase.execute by leaving receiveCompletion empty; update fetchData to handle the completion parameter, detect .failure(let error) in receiveCompletion and forward an appropriate error/update to the view via outputSubject (e.g., send a .failure/.error case or a dedicated .loadingComplete(error:) state) so the view controller can react, while still handling .finished normally; keep the existing receiveValue behavior and preserve storing the returned AnyCancellable in subscriptions.Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swift-54-61 (1)
54-61:⚠️ Potential issue | 🟠 MajorPublish user-selected dates from this sink.
This only updates
dateLabel;dateChangedPublisherandselectedItemPublisherstay stale after picker interaction, so the parent keeps the initial value fromupdate(_:).Suggested fix
private func bind() { dateDropDownView.selectedItemPublisher.sink { [weak self] selectedItems in - self?.dateLabel.text = "\(selectedItems[0]) \(selectedItems[1]) \(selectedItems[2])" + guard let self, selectedItems.count == 3 else { return } + + let displayText = "\(selectedItems[0]) \(selectedItems[1]) \(selectedItems[2])" + self.dateLabel.text = displayText + self.selectedItemPublisher.send(selectedItems) + + self.formatter.dateFormat = "yyyy년 M월 d일" + guard let date = self.formatter.date(from: displayText) else { return } + self.formatter.dateFormat = "yyyy-MM-dd" + self.dateChangedPublisher.send(self.formatter.string(from: date)) }.store(in: &subscriptions) dateDropDownView.applyButtonTappedPublisher.sink { [weak self] in self?.dismissDateDropDownView() }.store(in: &subscriptions)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swift` around lines 54 - 61, The sink in bind() currently only updates dateLabel, so the outside state never sees the new selection; after updating dateLabel in the dateDropDownView.selectedItemPublisher.sink closure, also publish the new selection to the view's outward-facing publisher by calling dateChangedPublisher.send(selectedItems) (or the equivalent send/emit on that subject) and ensure the drop-down's internal selected state is updated (e.g., call a setter like dateDropDownView.setSelectedItems(selectedItems) or send into the dropDown's backing subject) so both dateChangedPublisher and dateDropDownView.selectedItemPublisher reflect the user-picked values; keep the dismissDateDropDownView() behavior unchanged.Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift-60-76 (1)
60-76:⚠️ Potential issue | 🟠 MajorMissing time change emission when user selects time via dropdown.
When the user changes the time through the dropdown picker and taps apply, only the UI labels (
amPmLabel,timeLabel) are updated. ThetimeChangedPublisheris never emitted with the new 24-hour formatted time. The parent view/controller won't receive the user's selection.The
update(_ date: Date)method emitstimeChangedPublisher, but it's only called for external initialization—not when the user interacts with the dropdown.🐛 Proposed fix: emit timeChangedPublisher when apply is tapped
timeDropDownView.applyButtonTappedPublisher.sink { [weak self] in + guard let self else { return } + // Convert displayed time to 24-hour format and emit + let amPm = amPmLabel.text ?? "오전" + let time = timeLabel.text ?? "00:00" + let components = time.split(separator: ":") + if components.count == 2, var hour = Int(components[0]), let minute = Int(components[1]) { + if amPm == "오후" && hour != 12 { + hour += 12 + } else if amPm == "오전" && hour == 12 { + hour = 0 + } + let formatted = String(format: "%02d:%02d", hour, minute) + timeChangedPublisher.send(formatted) + } self?.dismissTimeDropDownView() }.store(in: &subscriptions)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift` around lines 60 - 76, The apply button handler currently only dismisses the dropdown and updates UI labels; modify the applyButtonTappedPublisher sink (in the same bind() where timeDropDownView.selectedItemPublisher and applyButtonTappedPublisher are used) to also emit the chosen time: retrieve the latest selected items from timeDropDownView (AM/PM, hour, minute), convert hour to 24-hour form (add 12 when PM and hour != "12", set to "00" when AM and hour == "12"), create a Date from those components (or call the existing update(_ date: Date) helper) and then emit via timeChangedPublisher so the parent receives the user's selection before/after calling dismissTimeDropDownView().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97059353-b5af-4f48-b409-38a8682f8e59
⛔ Files ignored due to path filters (26)
Koin/Resources/Assets.xcassets/CallVan/Bell.imageset/Bell.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/BellNotification.imageset/BellWithDot.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/CallVanCar.imageset/CallVanCar.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/CallVanListPeople.imageset/f7_person-2.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/CallVanRoute.imageset/ill.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanCall.imageset/callVanCall.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanChat.imageset/callVanChat.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanMinus.imageset/callVanMinus.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanPlus.imageset/callVanPlus.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile0.imageset/callVanProfile0.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile1.imageset/callVanProfile1.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile2.imageset/callVanProfile2.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile3.imageset/callVanProfile3.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile4.imageset/callVanProfile4.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile5.imageset/callVanProfile5.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile6.imageset/callVanProfile6.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfile7.imageset/callVanProfile7.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanProfileMine.imageset/callVanProfileMine.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanRadioButtonDeselected.imageset/callVanRadioButtonDeselected.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanRadioButtonSelected.imageset/callVanRadioButtonSelected.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanReportImage.imageset/callVanReportImage.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanSendImage.imageset/callVanSendImage.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanSendMessage.imageset/callVanSendMessage.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanSwapCircle.imageset/callVanSwapCircle.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/callVanThreeCircle.imageset/callVanThreeCircle.svgis excluded by!**/*.svgKoin/Resources/Assets.xcassets/CallVan/newCancel.imageset/newCancel.svgis excluded by!**/*.svg
📒 Files selected for processing (99)
Koin/Core/Extensions/Date+.swiftKoin/Core/Extensions/UIColor+.swiftKoin/Core/Extensions/UIImage+.swiftKoin/Core/View/BottomSheetViewControllerB.swiftKoin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swiftKoin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swiftKoin/Core/View/KoinPickerDropDownView/KoinPickerDropDownView.swiftKoin/Core/View/KoinPickerDropDownView/Subviews/KoinPickerViewB.swiftKoin/Data/DTOs/Decodable/CallVan/CallVanChatDto.swiftKoin/Data/DTOs/Decodable/CallVan/CallVanDataDto.swiftKoin/Data/DTOs/Decodable/CallVan/CallVanListDto.swiftKoin/Data/DTOs/Decodable/CallVan/CallVanNotificationDto.swiftKoin/Data/DTOs/Encodable/CallVan/CallVanListRequest.swiftKoin/Data/DTOs/Encodable/CallVan/CallVanPlace.swiftKoin/Data/DTOs/Encodable/CallVan/CallVanPostRequest.swiftKoin/Data/DTOs/Encodable/CallVan/CallVanReportRequest.swiftKoin/Domain/Model/CallVan/CallVanChat.swiftKoin/Domain/Model/CallVan/CallVanData.swiftKoin/Domain/Model/CallVan/CallVanList.swiftKoin/Domain/Model/CallVan/CallVanNotification.swiftKoin/Domain/Model/CallVan/CallVanState.swiftKoin/Domain/UseCase/CallVan/FetchCallVanDataUseCase.swiftKoin/Domain/UseCase/CallVan/FetchCallVanListUseCase.swiftKoin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swiftKoin/Presentation/CallVan/CallVanBottomSheetViewController.swiftKoin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swiftKoin/Presentation/CallVan/CallVanChat/CallVanChatViewModel.swiftKoin/Presentation/CallVan/CallVanChat/Subviews/CallVanChatTableView/CallVanChatDateHeaderView.swiftKoin/Presentation/CallVan/CallVanChat/Subviews/CallVanChatTableView/CallVanChatLeftCell.swiftKoin/Presentation/CallVan/CallVanChat/Subviews/CallVanChatTableView/CallVanChatRightCell.swiftKoin/Presentation/CallVan/CallVanChat/Subviews/CallVanChatTableView/CallVanChatTableView.swiftKoin/Presentation/CallVan/CallVanData/CallVanDataViewController.swiftKoin/Presentation/CallVan/CallVanData/CallVanDataViewModel.swiftKoin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableView.swiftKoin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableViewCell.swiftKoin/Presentation/CallVan/CallVanData/Subviews/CallVanDataView.swiftKoin/Presentation/CallVan/CallVanList/CallVanListViewController.swiftKoin/Presentation/CallVan/CallVanList/CallVanListViewModel.swiftKoin/Presentation/CallVan/CallVanList/CallVanPartyListTableView.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanFilterButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanFilterButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListFilterViewController.swiftKoin/Presentation/CallVan/CallVanNotification/CallVanNotificationViewController.swiftKoin/Presentation/CallVan/CallVanNotification/CallVanNotificationViewModel.swiftKoin/Presentation/CallVan/CallVanNotification/Subviews/CallVanNotificationDropdownViewController.swiftKoin/Presentation/CallVan/CallVanNotification/Subviews/CallVanNotificationEmptyView.swiftKoin/Presentation/CallVan/CallVanNotification/Subviews/CallVanNotificationTableView/CallVanNotificationTableView.swiftKoin/Presentation/CallVan/CallVanNotification/Subviews/CallVanNotificationTableView/CallVanNotificationTableViewCell.swiftKoin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swiftKoin/Presentation/CallVan/CallVanPost/CallVanPostViewModel.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostParticipantsView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swiftKoin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swiftKoin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swiftKoin/Presentation/CallVan/CallVanReport/CallVanReportViewModel.swiftKoin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportCustomReasonButton.swiftKoin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportEvidenceImagesCollectionView/CallVanReportImagesCollectionView.swiftKoin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportEvidenceImagesCollectionView/CallVanReportImagesCollectionViewCell.swiftKoin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportReasonButton.swiftKoin/Presentation/Home/Home/HomeViewController.swiftKoin/Presentation/Home/Home/HomeViewModel.swiftKoin/Presentation/Home/Home/SubViews/CallVanView.swiftKoin/Presentation/Home/ServiceSelect/ServiceSelectViewController.swiftKoin/Presentation/Home/ServiceSelect/Subviews/ServiceSelectTableView.swiftKoin/Resources/Assets.xcassets/CallVan/Bell.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/BellNotification.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/CallVanCar.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/CallVanListPeople.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/CallVanRoute.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanCall.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanChat.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanMinus.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanPlus.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile0.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile1.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile2.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile3.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile4.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile5.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile6.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfile7.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanProfileMine.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanRadioButtonDeselected.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanRadioButtonSelected.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanReportImage.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanSendImage.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanSendMessage.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanSwapCircle.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/callVanThreeCircle.imageset/Contents.jsonKoin/Resources/Assets.xcassets/CallVan/newCancel.imageset/Contents.jsonkoin.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- Koin/Presentation/Home/Home/HomeViewModel.swift
Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift
Show resolved
Hide resolved
Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift
Show resolved
Hide resolved
Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift
Show resolved
Hide resolved
Comment on lines
+51
to
+110
| private func bind() { | ||
| viewModel.transform(with: inputSubject.eraseToAnyPublisher()) | ||
| .receive(on: DispatchQueue.main) | ||
| .sink { [weak self] output in | ||
| guard let self else { return } | ||
| switch output { | ||
| case let .enablePostButton(isEnabled): | ||
| postButton.isEnabled = isEnabled | ||
| postButton.backgroundColor = isEnabled ? UIColor.appColor(.new500) : UIColor.appColor(.neutral400) | ||
| case let .updateDeparture(placeType, customPlace): | ||
| placeView.updateDeparture(placeType: placeType, customPlace: customPlace) | ||
| case let .updateArrival(placeType, customPlace): | ||
| placeView.updateArrival(placeType: placeType, customPlace: customPlace) | ||
| } | ||
| }.store(in: &subscriptions) | ||
|
|
||
| placeView.departureButtonTappedPublisher.sink { [weak self] in | ||
| self?.presentDeparturePlaceBottomSheet() | ||
| }.store(in: &subscriptions) | ||
|
|
||
| placeView.arrivalButtonTappedPublisher.sink { [weak self] in | ||
| self?.presentArrivalPlaceBottomSheet() | ||
| }.store(in: &subscriptions) | ||
|
|
||
| placeView.swapButtonTappedPublisher.sink { [weak self] in | ||
| self?.inputSubject.send(.swapButtonTapped) | ||
| }.store(in: &subscriptions) | ||
|
|
||
| placeView.departureChangedPublisher.sink { [weak self] (departureType, customPlace) in | ||
| self?.inputSubject.send(.updateDeparture(departureType, customPlace)) | ||
| }.store(in: &subscriptions) | ||
|
|
||
| placeView.arrivalChangedPublisher.sink { [weak self] (departureType, customPlace) in | ||
| self?.inputSubject.send(.updateArrival(departureType, customPlace)) | ||
| }.store(in: &subscriptions) | ||
|
|
||
| dateView.dateButtonTappedPublisher | ||
| .receive(on: DispatchQueue.main) | ||
| .sink { [weak self] in | ||
| self?.timeView.dismissTimeDropDownView() | ||
| }.store(in: &subscriptions) | ||
|
|
||
| dateView.dateChangedPublisher.sink { [weak self] date in | ||
| self?.inputSubject.send(.updateDepartureDate(date)) | ||
| }.store(in: &subscriptions) | ||
|
|
||
| timeView.timeButtonTappedPublisher | ||
| .receive(on: DispatchQueue.main) | ||
| .sink { [weak self] in | ||
| self?.dateView.dismissDateDropDownView() | ||
| }.store(in: &subscriptions) | ||
|
|
||
| timeView.timeChangedPublisher.sink { [weak self] time in | ||
| self?.inputSubject.send(.updateDepartureTime(time)) | ||
| }.store(in: &subscriptions) | ||
|
|
||
| participantsView.participantsChangedPublisher.sink { [weak self] participants in | ||
| self?.inputSubject.send(.updateMaxParticipants(participants)) | ||
| }.store(in: &subscriptions) | ||
| } |
There was a problem hiding this comment.
Wire the submit button to the view model.
postButton is enabled/disabled and styled, but nothing listens for .touchUpInside or forwards a submit input. As written, tapping "작성 완료" cannot complete the flow.
Also applies to: 174-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift` around
lines 51 - 110, The post button is never wired to the view model so taps don't
send a submit input; inside bind() (next to where postButton.isEnabled is set)
add a UIControl action or Combine publisher on postButton .touchUpInside that
calls inputSubject.send(.submit) (or the project's submit enum case) so the
viewModel receives the submit event; do the same wiring where postButton is
configured elsewhere (the other block that configures postButton) to ensure both
places forward the submit input to inputSubject.
...t/Subviews/CallVanReportEvidenceImagesCollectionView/CallVanReportImagesCollectionView.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (1)
Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift (1)
113-119:⚠️ Potential issue | 🔴 CriticalImplement the submit path before exposing this screen.
postButtonis wired now, butpostButtonTapped()is still empty, and the providedCallVanPostViewModel.Inputsurface does not show any submit event. Even after validation passes, tapping "작성 완료" still dead-ends the flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift` around lines 113 - 119, postButtonTapped is empty so tapping "작성 완료" never sends a submit event; add a submit path by extending CallVanPostViewModel.Input with a submit publisher/subject (e.g., submit: AnyPublisher<Void, Never> or submit: PassthroughSubject<Void, Never>) and wire postButtonTapped to emit a value on that subject (or call the input submit publisher) so the viewModel receives the submit event; update setAddTargets/postButton target remains the same, implement postButtonTapped in CallVanPostViewController to send the submit event to CallVanPostViewModel.Input and ensure transform/subscribe logic handles it.
🧹 Nitpick comments (2)
Koin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swift (1)
16-36: API inconsistency with existingCallVanButton.configure(state:).The existing
CallVanButtonatCallVanListCollectionView/CallVanButton.swiftderives the inset internally based on state (8 for.재모집/.이용완료, 12 for others). This implementation requires callers to pass inset explicitly, which:
- Creates inconsistent APIs for similar components
- Requires callers to know the correct inset values per state
Additionally, line 33 unconditionally sets
layer.borderWidth = 1, whereas the existing implementation conditionally sets it. While this works (clear borders are invisible), it's an inconsistency worth noting.If these buttons serve different purposes, consider unifying the API or documenting when to use each. If they serve the same purpose, consolidate them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swift` around lines 16 - 36, The new CallVanButton.configure(state:inset:) API diverges from the existing CallVanButton.configure(state:) by forcing callers to pass inset and by always setting layer.borderWidth = 1; update this implementation to match the existing behavior: make inset derived from CallVanState internally (use the same rule: inset = 8 for .재모집/.이용완료, 12 otherwise) and set layer.borderWidth conditionally the same way the other CallVanButton does; modify the configure method on CallVanButton (and keep CallVanState usage) so callers call configure(state:) with no inset parameter and the border handling mirrors the original implementation.Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift (1)
90-103: Remove the leftover tap debug prints.These will log on every button tap in production UI code. If you still need this during development, keep it behind a debug-only logger.
Suggested cleanup
cell.mainButtonTappedPublisher.sink { [weak self] postId in guard let self else { return } if let mainState = posts.first(where: { $0.postId == postId })?.mainState { mainButtonTappedPublisher.send((postId, mainState)) - print(mainState.rawValue) } }.store(in: &cell.subscriptions) cell.subButtonTappedPublisher.sink { [weak self] postId in guard let self else { return } if let subState = posts.first(where: { $0.postId == postId })?.subState { subButtonTappedPublisher.send((postId, subState)) - print(subState.rawValue) } }.store(in: &cell.subscriptions)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift` around lines 90 - 103, Remove the leftover debug prints in CallVanListCollectionView.swift that log on every button tap: delete the print(mainState.rawValue) in the cell.mainButtonTappedPublisher sink and the print(subState.rawValue) in the cell.subButtonTappedPublisher sink, or replace them with calls to a debug-only logger (e.g., wrap with a `#if` DEBUG or use a debugLog helper) so only development builds emit these messages while preserving the existing mainButtonTappedPublisher and subButtonTappedPublisher send behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift`:
- Around line 8-21: The file declares KoinPickerDropDownViewDateDelegate and
uses the `then` extension on DateFormatter for `inputFormatter` and
`outputFormatter` but only imports Foundation; add `import Then` at the top of
the file so the `then` extension is available to
KoinPickerDropDownViewDateDelegate and its properties `inputFormatter` and
`outputFormatter`.
In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swift`:
- Around line 10-14: The class CallVanButton in this file conflicts with an
existing CallVanButton declaration; rename this class (for example to
CallVanActionButton or CallVanStatusButton) and update all usages, initializers,
and any Interface Builder/outlet references to the new symbol, or
remove/consolidate this implementation into the existing CallVanButton if they
serve the same purpose; ensure init() and any overridden methods match the new
class name and that all references across the module are updated to avoid
"Invalid redeclaration".
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift`:
- Around line 8-10: The file uses the `.then` extension on UI objects in
CallVanListCollectionView (e.g., the `.then` call around line 26) but doesn't
import the Then library; add `import Then` at the top of the file alongside
UIKit and Combine so the `.then` extension is available and the file compiles.
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift`:
- Around line 214-221: chatButton and callButton are currently constrained to
the exact same top/trailing anchors which causes them to overlap when both
showChatButton and showCallButton are true; update CallVanListCollectionViewCell
to avoid duplicate anchors by either placing the two buttons into a horizontal
UIStackView and constrain that stack (preferred) or keep one button anchored to
trailing and constrain the other to the first button’s leading anchor with a
spacing constant, ensuring both buttons maintain the same top offset; locate the
layout block where chatButton.snp.makeConstraints and
callButton.snp.makeConstraints are set and replace the duplicate trailing
constraint with a stack or a relative constraint so the buttons no longer render
on top of each other.
- Around line 70-72: The destination label string in
CallVanListCollectionViewCell uses "도착 :" (space before the colon) which causes
inconsistent rendering vs "출발: "; update the assignment in destinationLabel
(where it currently sets $0.text = "도착 :" + post.arrival) to use a consistent
format such as "도착: " + post.arrival or string interpolation "도착:
\(post.arrival)" so the colon has no preceding space and matches the "출발: "
formatting.
- Around line 158-163: The chatButton and callButton are image-only and lack
accessibility labels; update the CallVanListCollectionViewCell setup where
chatButton and callButton are configured (the block that sets their images) to
assign descriptive accessibilityLabel values (e.g., "Chat with driver" and "Call
driver") and mark them as accessibilityTraits .button (or ensure
isAccessibilityElement = true) so VoiceOver announces a meaningful control name;
use the symbols chatButton, callButton, and CallVanListCollectionViewCell to
locate and modify the configuration code.
In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swift`:
- Around line 31-50: The view currently uses a PassthroughSubject so the initial
send in update(_:) is lost; change dateChangedPublisher to a
CurrentValueSubject<String, Never> (or other replaying subject) and set its
initial value inside update(_:) after formatting, and also call
dateChangedPublisher.send(...) from the dateDropDownView selection/apply handler
(where the dropdown selection is applied) so both the initial date and
subsequent dropdown selections are emitted to subscribers (refer to
dateChangedPublisher, update(_:), dateDropDownView reset/selection/apply
callback, bind()).
In
`@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swift`:
- Around line 188-191: In CallVanPostPlaceBottomSheetView update the
custom-place branch to trim whitespace/newlines before validating and passing
the value: replace usage of customPlace from customPlaceTextField.text with a
trimmed string using customPlace.trimmingCharacters(in:
.whitespacesAndNewlines), ensure the non-empty check uses the trimmed value, and
call onApplyButtonTapped(.custom, trimmedCustomPlace) so pure-whitespace inputs
are rejected.
In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift`:
- Around line 33-56: The init currently calls update(Date()) before the parent
subscribes and bind() only updates UI labels, so emit the selected time whenever
the user picks a time and not just on init: in bind() subscribe to the time
selection event from timeDropDownView (or add the selection target used in
setAddTargets()) and when handling a new Date call the existing update(_:) or
explicitly call timeChangedPublisher.send(formattedString) after updating
amPmLabel/timeLabel; ensure timeChangedPublisher.send(...) is invoked both from
update(_:) (already present) and from the selection handler in bind()/selection
action so the parent receives the chosen time on user interaction as well as
initial load.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift`:
- Around line 129-150: The picked image is converted to JPEG in
handleSelectedImage but not saved or used, so update handleSelectedImage to
persist the image data into the view controller's attachment state, enforce
maxImagesCount, refresh the UI, and emit the upload event: append imageData to
whatever backing array/list used for evidence (or create one if missing), cap
additions when evidence.count >= maxImagesCount, call
evidenceImagesCollectionView.reloadData() (on main thread) and update
evidenceCountLabel to reflect "\(evidence.count)/\(maxImagesCount)", and finally
send the upload event via inputSubject.send(.uploadFile([imageData])) as
originally commented; keep these updates inside handleSelectedImage and ensure
UI updates occur on the main queue.
- Around line 73-80: The reportButtonTapped handler currently ignores the
viewModel and unconditionally pops; change it to call the view model's
report-submission API (e.g., viewModel.submitReport or similar method on the
viewModel used by CallVanReportEvidenceViewController), wait for its
completion/result on the main thread, and only pop to the
CallVanDataViewController and call showToast("사용자가 신고되었습니다.") when the
submission succeeds; on failure, present an error toast/alert and do not pop.
Ensure you reference the existing viewModel property in
CallVanReportEvidenceViewController, perform UI updates on the main queue, and
gate navigation to navigationController?.popToViewController(viewController,
animated: true) on success.
- Around line 166-187: The count label is updated but input isn't constrained:
enforce maxContextTextCount by truncating input when exceeded; in
textViewDidChange(_:) check contextTextView.textColor != UIColor.appColor(.gray)
then if contextTextView.text.count > maxContextTextCount set
contextTextView.text = String(contextTextView.text.prefix(maxContextTextCount));
call updateEvidenceCount() afterward. Optionally implement
textView(_:shouldChangeTextIn:replacementText:) to prevent inserts/pastes that
would exceed maxContextTextCount using contextTextView, maxContextTextCount and
updateEvidenceCount() to keep UI consistent.
---
Duplicate comments:
In `@Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift`:
- Around line 113-119: postButtonTapped is empty so tapping "작성 완료" never sends
a submit event; add a submit path by extending CallVanPostViewModel.Input with a
submit publisher/subject (e.g., submit: AnyPublisher<Void, Never> or submit:
PassthroughSubject<Void, Never>) and wire postButtonTapped to emit a value on
that subject (or call the input submit publisher) so the viewModel receives the
submit event; update setAddTargets/postButton target remains the same, implement
postButtonTapped in CallVanPostViewController to send the submit event to
CallVanPostViewModel.Input and ensure transform/subscribe logic handles it.
---
Nitpick comments:
In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swift`:
- Around line 16-36: The new CallVanButton.configure(state:inset:) API diverges
from the existing CallVanButton.configure(state:) by forcing callers to pass
inset and by always setting layer.borderWidth = 1; update this implementation to
match the existing behavior: make inset derived from CallVanState internally
(use the same rule: inset = 8 for .재모집/.이용완료, 12 otherwise) and set
layer.borderWidth conditionally the same way the other CallVanButton does;
modify the configure method on CallVanButton (and keep CallVanState usage) so
callers call configure(state:) with no inset parameter and the border handling
mirrors the original implementation.
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift`:
- Around line 90-103: Remove the leftover debug prints in
CallVanListCollectionView.swift that log on every button tap: delete the
print(mainState.rawValue) in the cell.mainButtonTappedPublisher sink and the
print(subState.rawValue) in the cell.subButtonTappedPublisher sink, or replace
them with calls to a debug-only logger (e.g., wrap with a `#if` DEBUG or use a
debugLog helper) so only development builds emit these messages while preserving
the existing mainButtonTappedPublisher and subButtonTappedPublisher send
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 823abb61-d7d9-4f70-b430-b540132530f5
📒 Files selected for processing (17)
Koin/Core/View/BottomSheetViewControllerB.swiftKoin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swiftKoin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swiftKoin/Data/DTOs/Decodable/CallVan/CallVanDataDto.swiftKoin/Domain/Model/CallVan/CallVanState.swiftKoin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swiftKoin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swiftKoin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostParticipantsView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swiftKoin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swiftKoin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportEvidenceImagesCollectionView/CallVanReportImagesCollectionView.swift
🚧 Files skipped from review as they are similar to previous changes (8)
- Koin/Data/DTOs/Decodable/CallVan/CallVanDataDto.swift
- Koin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swift
- Koin/Presentation/CallVan/CallVanReport/Subviews/CallVanReportEvidenceImagesCollectionView/CallVanReportImagesCollectionView.swift
- Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewTimeDelegate.swift
- Koin/Domain/Model/CallVan/CallVanState.swift
- Koin/Core/View/BottomSheetViewControllerB.swift
- Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostParticipantsView.swift
- Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift
Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift
Show resolved
Hide resolved
Koin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swift
Outdated
Show resolved
Hide resolved
...ation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift
Show resolved
Hide resolved
...n/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift
Show resolved
Hide resolved
Comment on lines
+158
to
+163
| chatButton.do { | ||
| $0.setImage(UIImage.appImage(asset: .callVanChat), for: .normal) | ||
| } | ||
| callButton.do { | ||
| $0.setImage(UIImage.appImage(asset: .callVanCall), for: .normal) | ||
| } |
There was a problem hiding this comment.
Add accessibility labels to the icon-only buttons.
chatButton and callButton only expose images right now, so VoiceOver users won't get a meaningful control name.
Suggested fix
chatButton.do {
$0.setImage(UIImage.appImage(asset: .callVanChat), for: .normal)
+ $0.accessibilityLabel = "채팅"
+ $0.accessibilityHint = "콜밴팟 채팅으로 이동"
}
callButton.do {
$0.setImage(UIImage.appImage(asset: .callVanCall), for: .normal)
+ $0.accessibilityLabel = "전화"
+ $0.accessibilityHint = "작성자에게 전화"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| chatButton.do { | |
| $0.setImage(UIImage.appImage(asset: .callVanChat), for: .normal) | |
| } | |
| callButton.do { | |
| $0.setImage(UIImage.appImage(asset: .callVanCall), for: .normal) | |
| } | |
| chatButton.do { | |
| $0.setImage(UIImage.appImage(asset: .callVanChat), for: .normal) | |
| $0.accessibilityLabel = "채팅" | |
| $0.accessibilityHint = "콜밴팟 채팅으로 이동" | |
| } | |
| callButton.do { | |
| $0.setImage(UIImage.appImage(asset: .callVanCall), for: .normal) | |
| $0.accessibilityLabel = "전화" | |
| $0.accessibilityHint = "작성자에게 전화" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift`
around lines 158 - 163, The chatButton and callButton are image-only and lack
accessibility labels; update the CallVanListCollectionViewCell setup where
chatButton and callButton are configured (the block that sets their images) to
assign descriptive accessibilityLabel values (e.g., "Chat with driver" and "Call
driver") and mark them as accessibilityTraits .button (or ensure
isAccessibilityElement = true) so VoiceOver announces a meaningful control name;
use the symbols chatButton, callButton, and CallVanListCollectionViewCell to
locate and modify the configuration code.
Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swift
Show resolved
Hide resolved
Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift
Show resolved
Hide resolved
Comment on lines
+73
to
+80
| @objc private func reportButtonTapped() { | ||
| guard let viewController = navigationController?.viewControllers.first(where: { $0 is CallVanDataViewController }) else { | ||
| assert(false) | ||
| return | ||
| } | ||
| navigationController?.popToViewController(viewController, animated: true) | ||
| showToast(message: "사용자가 신고되었습니다.") | ||
| } |
There was a problem hiding this comment.
Submit through the view model before leaving this screen.
This handler never uses viewModel, so tapping 신고하기 always pops and shows a success message even though no report request was triggered here. Given CallVanDataViewController only pushes this flow and does not observe a completion event, this is also the last point in the flow that can reliably gate navigation on a real success result.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift`
around lines 73 - 80, The reportButtonTapped handler currently ignores the
viewModel and unconditionally pops; change it to call the view model's
report-submission API (e.g., viewModel.submitReport or similar method on the
viewModel used by CallVanReportEvidenceViewController), wait for its
completion/result on the main thread, and only pop to the
CallVanDataViewController and call showToast("사용자가 신고되었습니다.") when the
submission succeeds; on failure, present an error toast/alert and do not pop.
Ensure you reference the existing viewModel property in
CallVanReportEvidenceViewController, perform UI updates on the main queue, and
gate navigation to navigationController?.popToViewController(viewController,
animated: true) on success.
Comment on lines
+129
to
+150
| func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { | ||
| picker.dismiss(animated: true, completion: nil) | ||
|
|
||
| guard let provider = results.first?.itemProvider else { return } | ||
|
|
||
| if provider.canLoadObject(ofClass: UIImage.self) { | ||
| provider.loadObject(ofClass: UIImage.self) { [weak self] image, error in | ||
| DispatchQueue.main.async { | ||
| if let selectedImage = image as? UIImage { | ||
| self?.handleSelectedImage(image: selectedImage) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func handleSelectedImage(image: UIImage) { | ||
| guard let imageData = image.jpegData(compressionQuality: 0.5) else { | ||
| return | ||
| } | ||
| // inputSubject.send(.uploadFile([imageData])) | ||
| } |
There was a problem hiding this comment.
Persist the picked image and refresh the evidence UI.
handleSelectedImage creates imageData and then discards it, so the selected image never appears in evidenceImagesCollectionView, evidenceCountLabel stays at 0/10, and maxImagesCount is never enforced. Right now the picker can return an image, but the attachment state is lost immediately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift`
around lines 129 - 150, The picked image is converted to JPEG in
handleSelectedImage but not saved or used, so update handleSelectedImage to
persist the image data into the view controller's attachment state, enforce
maxImagesCount, refresh the UI, and emit the upload event: append imageData to
whatever backing array/list used for evidence (or create one if missing), cap
additions when evidence.count >= maxImagesCount, call
evidenceImagesCollectionView.reloadData() (on main thread) and update
evidenceCountLabel to reflect "\(evidence.count)/\(maxImagesCount)", and finally
send the upload event via inputSubject.send(.uploadFile([imageData])) as
originally commented; keep these updates inside handleSelectedImage and ensure
UI updates occur on the main queue.
Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift (2)
145-150:⚠️ Potential issue | 🟠 MajorKeep the picked image in state and refresh the evidence UI.
imageDatais created and then discarded, so attachments never appear inevidenceImagesCollectionView,evidenceCountLabelstays stale, andmaxImagesCountis never enforced. Persist the selected image data first, then reload/update the evidence UI from that state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift` around lines 145 - 150, handleSelectedImage currently creates imageData then discards it; persist the picked image bytes into the view controller's state (e.g., an array property like selectedImages or attachmentData), enforce maxImagesCount before inserting, then trigger UI updates by reloading evidenceImagesCollectionView and updating evidenceCountLabel, and finally forward the new data via inputSubject.send(.uploadFile([...])) as before; update references in handleSelectedImage and any related helpers so the collection view datasource reads from that new state.
73-80:⚠️ Potential issue | 🟠 MajorTrigger the actual report request before showing success.
reportButtonTapped()still never usesviewModel, so this screen always pops and shows a success toast even if no report was submitted. Submit through the view model first, then gate the pop/toast on the success result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift` around lines 73 - 80, reportButtonTapped currently just pops the VC and shows a success toast without calling the view model; change it to call the viewModel's reporting method (e.g., viewModel.submitReport or viewModel.reportEvidence) and wait for its completion/result, then only call navigationController?.popToViewController(...) and showToast on success; on failure present an error (e.g., showToast with failure message) and do not pop. Ensure the call is performed on the correct thread (use the view model's completion handler or async/await and dispatch UI updates to the main thread) and reference reportButtonTapped and the viewModel method when implementing.Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swift (1)
54-62:⚠️ Potential issue | 🟠 MajorDropdown selection not emitted to
dateChangedPublisher.When the user selects a date from the dropdown and taps apply, only the label is updated. The
dateChangedPublisheris never sent, so the view model won't receive the user-selected date and validation will use stale data.Emit the formatted date string when the apply button is tapped.
🐛 Proposed fix
private func bind() { dateDropDownView.selectedItemPublisher.sink { [weak self] selectedItems in self?.dateLabel.text = "\(selectedItems[0]) \(selectedItems[1]) \(selectedItems[2])" }.store(in: &subscriptions) - dateDropDownView.applyButtonTappedPublisher.sink { [weak self] in + dateDropDownView.applyButtonTappedPublisher.sink { [weak self] selectedItems in + guard let self else { return } + // Parse selected items: ["2026년", "3월", "12일"] + let year = selectedItems[0].filter { $0.isNumber } + let month = selectedItems[1].filter { $0.isNumber } + let day = selectedItems[2].filter { $0.isNumber } + let dateString = "\(year)-\(month.count == 1 ? "0" + month : month)-\(day.count == 1 ? "0" + day : day)" + dateChangedPublisher.send(dateString) self?.dismissDateDropDownView() }.store(in: &subscriptions) }Note: This fix assumes
applyButtonTappedPublishercan be changed to emit the selected items. Alternatively, store the last selected items in a property and emit on apply.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swift` around lines 54 - 62, In bind(), currently selectedItemPublisher only updates dateLabel and applyButtonTappedPublisher only calls dismissDateDropDownView(), so dateChangedPublisher is never emitted; fix by capturing the last selectedItems (e.g., a private lastSelectedItems property updated in the selectedItemPublisher sink) and when applyButtonTappedPublisher fires, format those lastSelectedItems into the same string used for dateLabel and send it on dateChangedPublisher before calling dismissDateDropDownView(); alternatively, if you can change dateDropDownView.applyButtonTappedPublisher to emit selectedItems directly, have its sink format and send to dateChangedPublisher and then call dismissDateDropDownView().Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift (1)
60-76:⚠️ Potential issue | 🟠 MajorDropdown selection not emitted to
timeChangedPublisher.Same issue as the date view: when the user selects a time from the dropdown and taps apply, only the labels are updated. The
timeChangedPublisheris never sent with the new selection, so the view model won't receive user-selected times.Convert the selected AM/PM and time to 24-hour format and emit on apply.
🐛 Proposed fix
private func bind() { timeDropDownView.selectedItemPublisher.sink { [weak self] selectedItems in self?.amPmLabel.text = selectedItems[0] == "AM" ? "오전" : "오후" var hour = selectedItems[1] if hour.count == 1 { hour = "0" + hour } let minute = selectedItems[2] self?.timeLabel.text = "\(hour):\(minute)" }.store(in: &subscriptions) - timeDropDownView.applyButtonTappedPublisher.sink { [weak self] in + timeDropDownView.applyButtonTappedPublisher.sink { [weak self] selectedItems in + guard let self else { return } + // Convert to 24-hour format + let isPM = selectedItems[0] == "PM" + var hourInt = Int(selectedItems[1]) ?? 0 + if isPM && hourInt != 12 { hourInt += 12 } + if !isPM && hourInt == 12 { hourInt = 0 } + let minute = selectedItems[2] + let timeString = String(format: "%02d:%@", hourInt, minute) + timeChangedPublisher.send(timeString) self?.dismissTimeDropDownView() }.store(in: &subscriptions) }Note: This assumes
applyButtonTappedPublishercan be modified to emit selected items. Alternatively, cache the last selected items in a property.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift` around lines 60 - 76, The bind() currently updates amPmLabel and timeLabel when timeDropDownView.selectedItemPublisher emits but never sends the selected value to timeChangedPublisher on apply; fix by storing the last selectedItems in a private property (e.g., lastSelectedTimeItems) inside the closure subscribed to timeDropDownView.selectedItemPublisher, and then in the timeDropDownView.applyButtonTappedPublisher sink convert that cached AM/PM + hour+minute into 24-hour format (use AM/PM from selectedItems[0] and hour from selectedItems[1]) and call timeChangedPublisher.send(formattedTime) before calling dismissTimeDropDownView(); update references in bind(), timeDropDownView.selectedItemPublisher, timeDropDownView.applyButtonTappedPublisher, timeChangedPublisher, and dismissTimeDropDownView() accordingly.
🧹 Nitpick comments (8)
Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift (2)
14-14: DuplicatedcolumnWidthsdefinition.The
columnWidthsproperty is defined on line 14 but the same literal[53, 31, 31]is hardcoded again on line 36. Use the property instead to avoid inconsistency if widths change.♻️ Proposed fix
- koinPicker.reset(items: items, selectedItem: selectedItem, columnWidths: [53, 31, 31]) + koinPicker.reset(items: items, selectedItem: selectedItem, columnWidths: columnWidths)Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift` at line 14, The duplicate numeric literal [53, 31, 31] should be replaced with the single source-of-truth property columnWidths to avoid inconsistency; locate the hardcoded array usage (the second occurrence) and change it to reference the existing columnWidths constant (same-scope or pass it into the method using it) so all width logic uses columnWidths rather than a repeated literal.
72-72: Typo in variable name:heighestYearInt→highestYearInt.✏️ Fix
- let heighestYearInt = thisYearInt + 1 - return Range(lowestYearInt...heighestYearInt).map { String($0)+"년" } + let highestYearInt = thisYearInt + 1 + return Range(lowestYearInt...highestYearInt).map { String($0)+"년" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift` at line 72, Rename the misspelled variable heighestYearInt to highestYearInt inside the KoinPickerDropDownViewDateDelegate (and/or any enclosing function where it's declared), and update every reference to that identifier so the code compiles and remains consistent; ensure rename is applied to all usages, related comments, and any tests or computed properties that reference heighestYearInt.Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift (2)
132-136: Consider exposing state via view model outputs rather than direct property access.Directly accessing
viewModel.requestproperties couples the VC tightly to the view model's internal state. If the request structure changes, this code breaks. Consider adding a computed property or output that provides the current departure/arrival info.Also applies to: 152-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift` around lines 132 - 136, The VC is directly reading viewModel.request properties (e.g., viewModel.request.departureType, viewModel.request.departureCustomName) which couples CallVanPostViewController to the request internals; add outputs on the view model such as currentDeparture and currentArrival (or computed properties like departureInfo/arrivalInfo) that return the type and custom name, update CallVanPostViewController to use those outputs when creating CallVanPostPlaceBottomSheetView and in the other similar spots (lines ~152-156), and keep the onApplyButtonTapped callback unchanged so the VC consumes stable viewModel outputs instead of request internals.
119-121: Address the TODO: Wire the submit action.The post button is wired but the action is empty. This needs to send a submit input to the view model to complete the post creation flow.
Do you want me to help implement the submit action? You'll need to:
- Add a
.submitcase toCallVanPostViewModel.Input- Handle it in the view model's transform to call the post creation use case
- Send the input from this action handler
`@objc` private func postButtonTapped() { - // TODO: + inputSubject.send(.submit) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift` around lines 119 - 121, Add a .submit case to CallVanPostViewModel.Input and in CallVanPostViewModel.transform handle the .submit input by invoking the post-creation use case (call the existing create/post use case method, map loading/success/error to the view model outputs and UI states). Then wire CallVanPostViewController.postButtonTapped to send that input to the view model (e.g., call whatever input.Send/dispatch method your view model uses with .submit) so tapping the post button triggers the transform path that creates the post.Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift (3)
120-138: All bottom sheet action handlers are empty.Every
onMainButtonTappedclosure is{}. These are placeholders that need implementation for the bottom sheet actions to work (participate, cancel participation, close recruitment, re-recruit, complete usage).Would you like me to help implement these action handlers? Each case would typically send an input to the view model:
.참여하기: Call a participate use case.참여취소: Call a cancel participation use case- etc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift` around lines 120 - 138, The switch in CallVanListViewController that assigns onMainButtonTapped currently uses empty closures; replace each `{}` with a closure that calls the appropriate ViewModel input method (e.g., callVanViewModel.participate(...), callVanViewModel.cancelParticipation(...), callVanViewModel.closeRecruitment(...), callVanViewModel.recruitAgain(...), callVanViewModel.completeUsage(...)) using the current item/ride identifiers from the controller (e.g., selectedCallVan or item.id), perform UI updates on the main thread (dismiss sheet, update list) and surface errors via alerts or processLogger; keep the special-case defaultHeight adjustment for the 이용완료 case and ensure closures capture self weakly to avoid retain cycles.
163-165: EmptycallButtonTappedhandler.This method is bound to the collection view's
callButtonTappedPublisherbut has no implementation. Consider adding a TODO comment or implementing the call functionality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift` around lines 163 - 165, The callButtonTapped handler in CallVanListViewController is empty; implement it or add a TODO so the callButtonTappedPublisher has behavior. Update the private func callButtonTapped(postId: Int) to either call into your view model (e.g., CallVanListViewModel.placeCall(postId:)) or implement the UI flow: resolve the phone number for the given postId, present a confirmation alert, and then initiate the call (e.g., open a tel:// URL or use your call service) while handling errors; if you cannot implement now, add a clear TODO comment referencing callButtonTappedPublisher and the intended call flow.
83-88: Inline dependency creation breaks DI pattern.Dependencies like
MockFetchCallVanNotificationListUseCaseandCallVanPostViewModelare created inline rather than injected. This is inconsistent with the DI pattern shown inHomeViewController.navigateToCallVanList()(lines 645-655) where dependencies are properly constructed and injected.Consider injecting factory closures or a coordinator to maintain testability and consistency.
Also applies to: 98-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift` around lines 83 - 88, bellButtonTapped currently constructs MockFetchCallVanNotificationListUseCase, CallVanNotificationViewModel and CallVanNotificationViewController inline which breaks the DI pattern; instead add injectable factory closures or a coordinator property to CallVanListViewController (e.g. makeFetchCallVanNotificationListUseCase: () -> FetchCallVanNotificationListUseCase and makeCallVanNotificationViewController: (CallVanNotificationViewModel) -> UIViewController, or a coordinator method navigateToCallVanNotification(using:)), update the initializer to accept those factories, and replace the inline creations in bellButtonTapped (and the similar inline construction at the other occurrence around lines 98-100) to call the injected factories/coordinator so dependencies are created outside the view controller for consistent DI and testability.Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift (1)
15-15: Typo:Properteis→Properties.- // MARK: - Properteis + // MARK: - Properties🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift` at line 15, The MARK comment in CallVanPostTimeView.swift reads "Properteis" and should be corrected to "Properties"; update the comment string (search for "Properteis") to "Properties" in the CallVanPostTimeView.swift file and fix any other identical misspellings in the same file to keep section headers consistent (e.g., MARK: - Properties).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swift`:
- Around line 32-44: Add the missing CallVanPlace.tennisCourt option by creating
a CallVanFilterButton(title: CallVanPlace.tennisCourt.description) and inserting
it into one of the existing button arrays (either buttons1 or buttons) so the UI
exposes the "테니스장" choice; update the corresponding stack view
(buttonsStackView2 or the stack that hosts the array you modify) initialization
so it includes the new button and maintains layout/spacing consistent with the
other CallVanFilterButton instances.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift`:
- Around line 190-197: The label uses trimmed characters but the input limit
uses updatedText.count, causing mismatch; make the counting rule consistent by
using the same trimmed-count logic in updateEvidenceCount as the input limit (or
vice versa) — e.g., compute count from the same source/normalization used when
enforcing the limit (reference updateEvidenceCount, contextTextView,
contextCountLabel, maximumContextLength and the updatedText.count logic in the
input handler) and set contextCountLabel using that normalized count so both UI
and enforcement match.
---
Duplicate comments:
In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swift`:
- Around line 54-62: In bind(), currently selectedItemPublisher only updates
dateLabel and applyButtonTappedPublisher only calls dismissDateDropDownView(),
so dateChangedPublisher is never emitted; fix by capturing the last
selectedItems (e.g., a private lastSelectedItems property updated in the
selectedItemPublisher sink) and when applyButtonTappedPublisher fires, format
those lastSelectedItems into the same string used for dateLabel and send it on
dateChangedPublisher before calling dismissDateDropDownView(); alternatively, if
you can change dateDropDownView.applyButtonTappedPublisher to emit selectedItems
directly, have its sink format and send to dateChangedPublisher and then call
dismissDateDropDownView().
In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift`:
- Around line 60-76: The bind() currently updates amPmLabel and timeLabel when
timeDropDownView.selectedItemPublisher emits but never sends the selected value
to timeChangedPublisher on apply; fix by storing the last selectedItems in a
private property (e.g., lastSelectedTimeItems) inside the closure subscribed to
timeDropDownView.selectedItemPublisher, and then in the
timeDropDownView.applyButtonTappedPublisher sink convert that cached AM/PM +
hour+minute into 24-hour format (use AM/PM from selectedItems[0] and hour from
selectedItems[1]) and call timeChangedPublisher.send(formattedTime) before
calling dismissTimeDropDownView(); update references in bind(),
timeDropDownView.selectedItemPublisher,
timeDropDownView.applyButtonTappedPublisher, timeChangedPublisher, and
dismissTimeDropDownView() accordingly.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift`:
- Around line 145-150: handleSelectedImage currently creates imageData then
discards it; persist the picked image bytes into the view controller's state
(e.g., an array property like selectedImages or attachmentData), enforce
maxImagesCount before inserting, then trigger UI updates by reloading
evidenceImagesCollectionView and updating evidenceCountLabel, and finally
forward the new data via inputSubject.send(.uploadFile([...])) as before; update
references in handleSelectedImage and any related helpers so the collection view
datasource reads from that new state.
- Around line 73-80: reportButtonTapped currently just pops the VC and shows a
success toast without calling the view model; change it to call the viewModel's
reporting method (e.g., viewModel.submitReport or viewModel.reportEvidence) and
wait for its completion/result, then only call
navigationController?.popToViewController(...) and showToast on success; on
failure present an error (e.g., showToast with failure message) and do not pop.
Ensure the call is performed on the correct thread (use the view model's
completion handler or async/await and dispatch UI updates to the main thread)
and reference reportButtonTapped and the viewModel method when implementing.
---
Nitpick comments:
In
`@Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swift`:
- Line 14: The duplicate numeric literal [53, 31, 31] should be replaced with
the single source-of-truth property columnWidths to avoid inconsistency; locate
the hardcoded array usage (the second occurrence) and change it to reference the
existing columnWidths constant (same-scope or pass it into the method using it)
so all width logic uses columnWidths rather than a repeated literal.
- Line 72: Rename the misspelled variable heighestYearInt to highestYearInt
inside the KoinPickerDropDownViewDateDelegate (and/or any enclosing function
where it's declared), and update every reference to that identifier so the code
compiles and remains consistent; ensure rename is applied to all usages, related
comments, and any tests or computed properties that reference heighestYearInt.
In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift`:
- Around line 120-138: The switch in CallVanListViewController that assigns
onMainButtonTapped currently uses empty closures; replace each `{}` with a
closure that calls the appropriate ViewModel input method (e.g.,
callVanViewModel.participate(...), callVanViewModel.cancelParticipation(...),
callVanViewModel.closeRecruitment(...), callVanViewModel.recruitAgain(...),
callVanViewModel.completeUsage(...)) using the current item/ride identifiers
from the controller (e.g., selectedCallVan or item.id), perform UI updates on
the main thread (dismiss sheet, update list) and surface errors via alerts or
processLogger; keep the special-case defaultHeight adjustment for the 이용완료 case
and ensure closures capture self weakly to avoid retain cycles.
- Around line 163-165: The callButtonTapped handler in CallVanListViewController
is empty; implement it or add a TODO so the callButtonTappedPublisher has
behavior. Update the private func callButtonTapped(postId: Int) to either call
into your view model (e.g., CallVanListViewModel.placeCall(postId:)) or
implement the UI flow: resolve the phone number for the given postId, present a
confirmation alert, and then initiate the call (e.g., open a tel:// URL or use
your call service) while handling errors; if you cannot implement now, add a
clear TODO comment referencing callButtonTappedPublisher and the intended call
flow.
- Around line 83-88: bellButtonTapped currently constructs
MockFetchCallVanNotificationListUseCase, CallVanNotificationViewModel and
CallVanNotificationViewController inline which breaks the DI pattern; instead
add injectable factory closures or a coordinator property to
CallVanListViewController (e.g. makeFetchCallVanNotificationListUseCase: () ->
FetchCallVanNotificationListUseCase and makeCallVanNotificationViewController:
(CallVanNotificationViewModel) -> UIViewController, or a coordinator method
navigateToCallVanNotification(using:)), update the initializer to accept those
factories, and replace the inline creations in bellButtonTapped (and the similar
inline construction at the other occurrence around lines 98-100) to call the
injected factories/coordinator so dependencies are created outside the view
controller for consistent DI and testability.
In `@Koin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swift`:
- Around line 132-136: The VC is directly reading viewModel.request properties
(e.g., viewModel.request.departureType, viewModel.request.departureCustomName)
which couples CallVanPostViewController to the request internals; add outputs on
the view model such as currentDeparture and currentArrival (or computed
properties like departureInfo/arrivalInfo) that return the type and custom name,
update CallVanPostViewController to use those outputs when creating
CallVanPostPlaceBottomSheetView and in the other similar spots (lines ~152-156),
and keep the onApplyButtonTapped callback unchanged so the VC consumes stable
viewModel outputs instead of request internals.
- Around line 119-121: Add a .submit case to CallVanPostViewModel.Input and in
CallVanPostViewModel.transform handle the .submit input by invoking the
post-creation use case (call the existing create/post use case method, map
loading/success/error to the view model outputs and UI states). Then wire
CallVanPostViewController.postButtonTapped to send that input to the view model
(e.g., call whatever input.Send/dispatch method your view model uses with
.submit) so tapping the post button triggers the transform path that creates the
post.
In `@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swift`:
- Line 15: The MARK comment in CallVanPostTimeView.swift reads "Properteis" and
should be corrected to "Properties"; update the comment string (search for
"Properteis") to "Properties" in the CallVanPostTimeView.swift file and fix any
other identical misspellings in the same file to keep section headers consistent
(e.g., MARK: - Properties).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b3c4744-4cde-4f96-adc6-951369da7b48
📒 Files selected for processing (11)
Koin/Core/View/KoinPickerDropDownView/Delegates/KoinPickerDropDownViewDateDelegate.swiftKoin/Domain/Model/CallVan/CallVanState.swiftKoin/Presentation/CallVan/CallVanList/CallVanListViewController.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swiftKoin/Presentation/CallVan/CallVanPost/CallVanPostViewController.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostDateView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostTimeView.swiftKoin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift
- Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift
- Koin/Domain/Model/CallVan/CallVanState.swift
Comment on lines
+32
to
+44
| private let buttons1 = [ | ||
| CallVanFilterButton(title: CallVanPlace.frontGate.description), | ||
| CallVanFilterButton(title: CallVanPlace.backGate.description), | ||
| CallVanFilterButton(title: CallVanPlace.dormitoryMain.description), | ||
| CallVanFilterButton(title: CallVanPlace.dormitorySub.description) | ||
| ] | ||
|
|
||
| private let buttonsStackView2 = UIStackView() | ||
| private let buttons2 = [ | ||
| CallVanFilterButton(title: CallVanPlace.terminal.description), | ||
| CallVanFilterButton(title: CallVanPlace.station.description), | ||
| CallVanFilterButton(title: CallVanPlace.asanStation.description) | ||
| ] |
There was a problem hiding this comment.
Missing tennisCourt place option.
The CallVanPlace enum defines 9 cases including .tennisCourt ("테니스장"), but the UI only provides buttons for 8 places. This prevents users from selecting "테니스장" as a departure or arrival location.
Consider adding a button for tennisCourt to one of the button stacks.
🔧 Proposed fix
private let buttons1 = [
CallVanFilterButton(title: CallVanPlace.frontGate.description),
CallVanFilterButton(title: CallVanPlace.backGate.description),
+ CallVanFilterButton(title: CallVanPlace.tennisCourt.description),
CallVanFilterButton(title: CallVanPlace.dormitoryMain.description),
- CallVanFilterButton(title: CallVanPlace.dormitorySub.description)
+ CallVanFilterButton(title: CallVanPlace.dormitorySub.description),
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private let buttons1 = [ | |
| CallVanFilterButton(title: CallVanPlace.frontGate.description), | |
| CallVanFilterButton(title: CallVanPlace.backGate.description), | |
| CallVanFilterButton(title: CallVanPlace.dormitoryMain.description), | |
| CallVanFilterButton(title: CallVanPlace.dormitorySub.description) | |
| ] | |
| private let buttonsStackView2 = UIStackView() | |
| private let buttons2 = [ | |
| CallVanFilterButton(title: CallVanPlace.terminal.description), | |
| CallVanFilterButton(title: CallVanPlace.station.description), | |
| CallVanFilterButton(title: CallVanPlace.asanStation.description) | |
| ] | |
| private let buttons1 = [ | |
| CallVanFilterButton(title: CallVanPlace.frontGate.description), | |
| CallVanFilterButton(title: CallVanPlace.backGate.description), | |
| CallVanFilterButton(title: CallVanPlace.tennisCourt.description), | |
| CallVanFilterButton(title: CallVanPlace.dormitoryMain.description), | |
| CallVanFilterButton(title: CallVanPlace.dormitorySub.description) | |
| ] | |
| private let buttonsStackView2 = UIStackView() | |
| private let buttons2 = [ | |
| CallVanFilterButton(title: CallVanPlace.terminal.description), | |
| CallVanFilterButton(title: CallVanPlace.station.description), | |
| CallVanFilterButton(title: CallVanPlace.asanStation.description) | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swift`
around lines 32 - 44, Add the missing CallVanPlace.tennisCourt option by
creating a CallVanFilterButton(title: CallVanPlace.tennisCourt.description) and
inserting it into one of the existing button arrays (either buttons1 or buttons)
so the UI exposes the "테니스장" choice; update the corresponding stack view
(buttonsStackView2 or the stack that hosts the array you modify) initialization
so it includes the new button and maintains layout/spacing consistent with the
other CallVanFilterButton instances.
Comment on lines
+190
to
+197
| private func updateEvidenceCount() { | ||
| let count: Int | ||
| if contextTextView.textColor == UIColor.appColor(.gray) { | ||
| count = 0 | ||
| } else { | ||
| count = contextTextView.text.trimmingCharacters(in: .whitespacesAndNewlines).count | ||
| } | ||
| contextCountLabel.text = "\(count)/\(maximumContextLength)" |
There was a problem hiding this comment.
Use the same counting rule for the label and the input limit.
The limit is enforced with updatedText.count, but the label trims whitespace before counting. That can make the user hit the 1000-character cap while the UI still shows less than 1000/1000.
🛠️ Suggested fix
private func updateEvidenceCount() {
let count: Int
if contextTextView.textColor == UIColor.appColor(.gray) {
count = 0
} else {
- count = contextTextView.text.trimmingCharacters(in: .whitespacesAndNewlines).count
+ count = contextTextView.text.count
}
contextCountLabel.text = "\(count)/\(maximumContextLength)"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift`
around lines 190 - 197, The label uses trimmed characters but the input limit
uses updatedText.count, causing mismatch; make the counting rule consistent by
using the same trimmed-count logic in updateEvidenceCount as the input limit (or
vice versa) — e.g., compute count from the same source/normalization used when
enforcing the limit (reference updateEvidenceCount, contextTextView,
contextCountLabel, maximumContextLength and the updatedText.count logic in the
input handler) and set contextCountLabel using that normalized count so both UI
and enforcement match.
DTO 파일 분리, ViewModelProtocol 누락 수정
그림자 수정
존재하지 않는 값인 0 대신, 전체보기에 해당하는 1로 수정했습니다.
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Koin/Presentation/Home/Home/HomeViewController.swift (1)
147-149:⚠️ Potential issue | 🟡 MinorRemove debug print statements before production.
These print statements expose sensitive keychain data (access token, FCM token, access history ID) to the console. They should be removed or replaced with proper logging that respects privacy.
🔒 Proposed fix
- print(KeychainWorker.shared.read(key: .access)) - print(KeychainWorker.shared.read(key: .fcm)) - print(KeychainWorker.shared.read(key: .accessHistoryId))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/Home/Home/HomeViewController.swift` around lines 147 - 149, The three debug print statements in HomeViewController that call KeychainWorker.shared.read(key: .access), .fcm and .accessHistoryId should be removed or replaced with privacy-safe logs; locate the print calls in HomeViewController and either delete them or change them to non-sensitive checks (e.g., log presence/boolean or masked values) using a secure logger rather than printing raw keychain contents from KeychainWorker.shared.read.
♻️ Duplicate comments (2)
Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift (2)
151-156:⚠️ Potential issue | 🟠 MajorAdd accessibility metadata to icon-only contact buttons.
chatButton/callButtonare image-only right now, so VoiceOver won’t announce meaningful names at Line 151 and Line 154.Suggested fix
chatButton.do { $0.setImage(UIImage.appImage(asset: .callVanChat), for: .normal) + $0.accessibilityLabel = "채팅" + $0.accessibilityHint = "콜밴팟 채팅으로 이동" } callButton.do { $0.setImage(UIImage.appImage(asset: .callVanCall), for: .normal) + $0.accessibilityLabel = "전화" + $0.accessibilityHint = "전화 연결" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift` around lines 151 - 156, chatButton and callButton are image-only and lack accessibility metadata, so update CallVanListCollectionViewCell to make each button an accessibility element and set descriptive accessibilityLabel (e.g., "Chat with driver" / "Call driver") and optional accessibilityHint as appropriate; locate the chatButton and callButton configuration in CallVanListCollectionViewCell and add properties like isAccessibilityElement = true, accessibilityLabel = "<descriptive text>" and accessibilityHint/traits if needed so VoiceOver announces meaningful names for these icon-only buttons.
207-214:⚠️ Potential issue | 🟠 MajorPrevent chat/call button overlap when both flags are true.
At Line 207–214 both buttons are pinned to the same top/trailing anchors, so they render on top of each other when both are visible.
Suggested fix (stack view-based)
@@ - private let chatButton = UIButton() - private let callButton = UIButton() + private let chatButton = UIButton() + private let callButton = UIButton() + private let contactButtonStackView = UIStackView() @@ + contactButtonStackView.do { + $0.axis = .horizontal + $0.spacing = 4 + $0.alignment = .center + } @@ - [routeImageView, departureLabel, destinationLabel, labelsStackView, callVanButtonStackView, chatButton, callButton].forEach { + [routeImageView, departureLabel, destinationLabel, labelsStackView, callVanButtonStackView, contactButtonStackView].forEach { contentView.addSubview($0) } + [chatButton, callButton].forEach { + contactButtonStackView.addArrangedSubview($0) + } @@ - chatButton.snp.makeConstraints { - $0.top.equalToSuperview().offset(16.5) - $0.trailing.equalToSuperview().offset(-24) - } - callButton.snp.makeConstraints { - $0.top.equalToSuperview().offset(16.5) - $0.trailing.equalToSuperview().offset(-24) - } + contactButtonStackView.snp.makeConstraints { + $0.top.equalToSuperview().offset(16.5) + $0.trailing.equalToSuperview().offset(-24) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift` around lines 207 - 214, In CallVanListCollectionViewCell, the chatButton and callButton are constrained to the same top/trailing anchors causing overlap; change the layout so the two buttons are arranged horizontally (either by creating an action UIStackView or by constraining callButton.leading to chatButton.trailing with a fixed spacing) and then pin the stack (or chatButton) to the top/trailing anchors (top = 16.5, trailing = -24) so they never occupy the same position; update the constraints in the method where chatButton.snp.makeConstraints and callButton.snp.makeConstraints are set to reflect this new arrangement.
🧹 Nitpick comments (6)
Koin/Domain/Model/CallVan/CallVanPlace.swift (1)
10-20: Decouple enum identity from localized labels.On Line 10-Line 20, using Korean UI text as
rawValuecouples domain identifiers to presentation copy. This becomes brittle for localization and any API/storage mapping. Prefer stable raw identifiers and expose a separate display property for UI text.Proposed refactor
enum CallVanPlace: String, CallVanFilterState { - case all = "전체" - case frontGate = "정문" - case backGate = "후문" - case tennisCourt = "테니스장" - case dormitoryMain = "본관동" - case dormitorySub = "별관동" - case terminal = "천안터미널" - case station = "천안역" - case asanStation = "천안아산역" - case custom = "기타" + case all + case frontGate + case backGate + case tennisCourt + case dormitoryMain + case dormitorySub + case terminal + case station + case asanStation + case custom + + var title: String { + switch self { + case .all: return "전체" + case .frontGate: return "정문" + case .backGate: return "후문" + case .tennisCourt: return "테니스장" + case .dormitoryMain: return "본관동" + case .dormitorySub: return "별관동" + case .terminal: return "천안터미널" + case .station: return "천안역" + case .asanStation: return "천안아산역" + case .custom: return "기타" + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Domain/Model/CallVan/CallVanPlace.swift` around lines 10 - 20, CallVanPlace currently uses Korean UI strings as rawValue which couples domain identity to presentation; change the enum raw string values to stable identifiers (e.g., "all", "frontGate", "backGate", "tennisCourt", "dormitoryMain", "dormitorySub", "terminal", "station", "asanStation", "custom") and add a computed property (e.g., var displayName: String) that returns the localized Korean label via NSLocalizedString or a localization helper; update any serialization/lookup sites that consume CallVanPlace.rawValue to use the new identifiers (or map old stored values to new ones) and replace UI code that used rawValue for display to use CallVanPlace.displayName instead so identity and presentation are decoupled.Koin/Domain/Model/CallVan/CallVanReportRequest.swift (1)
15-18: Consider validation: at least one ofreasonCodeorcustomReasonshould be present.Both fields being optional could lead to empty report reasons. Consider either:
- Making
reasonCodenon-optional with.otheras default- Adding validation logic when constructing the request
💡 Alternative with required reasonCode
struct CallVanReportRequestReason { - var reasonCode: CallVanReportRequestReasonCode? + var reasonCode: CallVanReportRequestReasonCode var customReason: String? }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Domain/Model/CallVan/CallVanReportRequest.swift` around lines 15 - 18, CallVanReportRequestReason currently allows both reasonCode and customReason to be nil which permits empty reasons; update the model to enforce at least one is present by either making reasonCode non-optional with a default value (e.g., set reasonCode: CallVanReportRequestReasonCode = .other) or add a failable/throwing initializer on CallVanReportRequestReason that validates (and rejects/throws) when both reasonCode and customReason are nil; reference the struct CallVanReportRequestReason and fields reasonCode and customReason when implementing the change and ensure callers are updated to handle the new initializer or default.Koin/Presentation/Home/Home/HomeViewController.swift (1)
643-659: Mock use cases are used in production navigation code.
MockFetchCallVanListUseCaseis injected innavigateToCallVanList(). While this is acceptable for a UI-only PR, ensure a TODO or issue is tracked to replace these with real implementations (DefaultFetchCallVanListUseCase) before the feature is released.Similarly,
CallVanPostViewModel()is initialized with no dependencies, which may need real use cases for actual API interactions.Do you want me to open an issue to track replacing mock use cases with real implementations?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/Home/Home/HomeViewController.swift` around lines 643 - 659, In navigateToCallVanList() replace the test double by creating and injecting the real use case (use DefaultFetchCallVanListUseCase instead of MockFetchCallVanListUseCase, constructed with the appropriate service/repository like DefaultCallVanService or repository similar to DefaultUserRepository), and in navigateToCallVanPost() wire CallVanPostViewModel with its real dependencies (e.g., DefaultCreateCallVanUseCase or other real use cases) instead of a no-arg initializer; add a clear TODO/FIXME comment in both navigateToCallVanList() and navigateToCallVanPost() and open a tracking issue to ensure MockFetchCallVanListUseCase and the parameterless CallVanPostViewModel are replaced with DefaultFetchCallVanListUseCase and the real CallVanPost use case implementations before release.Koin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableViewCell.swift (1)
40-40: Rename typo’d parameter before it spreads further.
shoudHideSepearatorViewshould be renamed to something likeshouldHideSeparatorViewfor API clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableViewCell.swift` at line 40, Rename the misspelled parameter shoudHideSepearatorView to shouldHideSeparatorView in the CallVanDataTableViewCell.configure(participant:shoudHideSepearatorView:) method and update all internal references inside that function to the new parameter name; also update every call site, override or protocol declaration that passes or declares shoudHideSepearatorView so signatures remain consistent and the public API reflects the corrected name.Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift (2)
312-315: Avoid hard-coded reason count in layout assembly.Using
0..<3tightly couples layout logic to current data and can cause index-out-of-range if reason options change. Iterate overreasonButtons.indicesand guard separator access.♻️ Proposed refactor
- for index in 0..<3 { + for index in reasonButtons.indices { buttonsStackView.addArrangedSubview(reasonButtons[index]) - buttonsStackView.addArrangedSubview(separatorViews[index]) + if index < separatorViews.count { + buttonsStackView.addArrangedSubview(separatorViews[index]) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift` around lines 312 - 315, The loop currently uses a hard-coded range 0..<3 which can crash if reasonButtons changes; update the assembly to iterate over reasonButtons.indices and for each index add reasonButtons[index] to buttonsStackView and only add separatorViews[index] if separatorViews.indices.contains(index) (or otherwise guard availability) so separators stay in sync with reasonButtons; ensure you update the code paths that reference reasonButtons and separatorViews (symbols: buttonsStackView, reasonButtons, separatorViews) to use the safe indexed iteration.
193-194: Replace debug print in failure path.
assertionFailure(debug) and/or a structured logger.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift` around lines 193 - 194, In CallVanReportReasonViewController replace the debug-only print("\(`#function`) failed") in the failure branch with a proper failure/logging strategy: call assertionFailure("CallVanReportReasonViewController: \(`#function`) failed") for debug builds and also emit a structured log via your app logger (or os_log/Logger) with context (e.g., "CallVanReportReasonViewController.\(`#function`) failed", include any error info if available). Update the failure branch in the same method where print("\(`#function`) failed") appears to remove the print and add both assertionFailure and a structured logger call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Koin/Domain/UseCase/CallVan/FetchCallVanListUseCase.swift`:
- Line 50: The mock CallVanList is populated with 6 posts but hardcodes
totalCount: 0 and totalPage: 0; update the creation of callVanList
(CallVanList(posts: ...)) to compute realistic pagination metadata — set
totalCount to posts.count, currentPage to 1 (or derive from the request/input
page), and totalPage to 1 (or compute as Int(ceil(Double(totalCount) /
Double(pageSize))) if a page size exists) so the mock metadata matches the
actual posts.
In `@Koin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swift`:
- Around line 8-9: The file FetchCallVanNotificationListUseCase.swift contains
duplicate import statements for Foundation and Combine; remove the repeated
import lines so each module is imported only once (keep a single "import
Foundation" and a single "import Combine"), and ensure the rest of the file
(including the FetchCallVanNotificationListUseCase type and any related
functions) still compiles after cleaning up the duplicate imports.
- Line 47: Rename the misspelled enum case .paritipantJoined to
.participantJoined in the CallVanNotification enum and update all usages (e.g.,
the instance in FetchCallVanNotificationListUseCase where it is referenced as
type: .paritipantJoined) to the corrected .participantJoined; ensure you also
update any switch statements, Codable/raw-value mappings, or tests that
reference the old name so serialization and pattern matching continue to work.
In `@Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift`:
- Around line 99-112: Replace the hardcoded UI placeholders by binding to the
ViewModel: set titleLabel.text to format viewModel.callVanPost.title and
viewModel.callVanPost.departureTime (e.g., "\(viewModel.callVanPost.title)
\(viewModel.callVanPost.departureTime)") instead of "테니스장 - ...", and set
paritipantsLabel.text to
"\(viewModel.callVanPost.currentParticipants)/\(viewModel.callVanPost.maxParticipants)"
instead of "6/8"; keep peopleImageView configuration as-is but ensure these
assignments occur where the viewModel is available (e.g., in the view model
binding/refresh method or after viewModel is set) so the UI reflects actual data
from CallVanChatViewController's viewModel.callVanPost.
- Line 22: Rename the misspelled property paritipantsLabel to participantsLabel
and update every use site (references at/near the original symbol: lines 108,
149, 177-179 in the diff) to the new identifier; ensure the lazy var declaration
in CallVanChatViewController and any constraints, text assignments, or
accessibility references that use paritipantsLabel are changed to
participantsLabel so the code compiles and semantics remain the same.
- Around line 43-49: The viewModel property is injected but never observed; add
a bind method on CallVanChatViewController that subscribes to the viewModel's
outputs and forwards UI events to its inputs (e.g., observe message list,
loading/error states, and send user actions), then call bind() from viewDidLoad
(alongside configureView(), configureNavigationBar(), setDelegate()). Locate the
viewModel symbol and implement subscriptions to its publishers/closures (or
Rx/Combine streams) to update the UI and handle navigation/error presentation;
ensure you wire user input handlers (send message, typing, etc.) into viewModel
inputs so the controller reacts to ViewModel changes.
In
`@Koin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableView.swift`:
- Around line 69-72: In bind(_ cell: CallVanDataTableViewCell) avoid the retain
cycle by capturing the cell weakly in the sink closure; change the closure
capture list to include [weak self, weak cell] (or [weak cell, weak self]) and
inside the closure unwrap cell (e.g. guard let cell = cell else { return })
before calling self?.closeReportButton(cell); keep storing the returned
AnyCancellable into &cell.subscriptions as before so the cancellable is still
owned by the cell but the closure no longer strongly captures it.
In
`@Koin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableViewCell.swift`:
- Around line 40-47: The cell's configure(participant:shoudHideSepearatorView:)
currently binds new participant data but doesn't reset transient report UI state
from a reused cell; update the configure method to explicitly reset
report-related UI: set reportButton.isHidden to true (or to the correct default
visibility), clear any selected/highlighted state on reportButton and
threeCircleImageView (e.g., reportButton.isSelected = false / isHighlighted =
false and threeCircleImageView.layer/background highlight cleared), and ensure
threeCircleButton/threeCircleImageView visibility remains driven by
participant.isMe so no leftover expanded-report visuals persist when reusing the
cell.
- Around line 65-70: hitTest currently returns reportButton whenever the touch
point falls within its bounds but doesn't guard against the button being hidden
or transparent, causing invisible dead touch areas when closeReportButton()
animates alpha to 0 before setting isHidden; update hitTest(_:with:) to first
check reportButton.isHidden == false, reportButton.alpha > 0 (or alpha threshold
like 0.01) and reportButton.isUserInteractionEnabled before converting the point
and returning reportButton so touches are ignored while the button is invisible
or disabled.
In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift`:
- Around line 93-96: The searchButton is not wired up in setAddTargets(); update
the setAddTargets() method to addTarget for searchButton (e.g.,
searchButton.addTarget(self, action: `#selector`(searchButtonTapped), for:
.touchUpInside)) and ensure a corresponding `@objc` func searchButtonTapped(_
sender: UIButton) (or appropriate signature) exists in CallVanListViewController
to handle the tap; reference the existing setAddTargets(), writeButton,
filterButton, and add the searchButton wiring and handler implementation.
- Around line 103-106: filterButtonTapped currently creates a fresh
CallVanListRequest and a no-op onApplyButtonTapped, so chosen filters are never
applied; update filterButtonTapped to pass the current list request/state (e.g.,
the existing CallVanListRequest instance or current filter model on
CallVanListViewController) into CallVanListFilterViewController and provide a
real onApplyButtonTapped closure that updates the controller's request,
refreshes/reloads the list (or calls the existing load/refresh method), and
dismisses the BottomSheetViewController; locate symbols filterButtonTapped,
CallVanListFilterViewController(filter:onApplyButtonTapped:), CallVanListRequest
and the view controller's load/refresh method to implement this change.
- Around line 84-87: The code currently instantiates
MockFetchCallVanNotificationListUseCase directly when creating
CallVanNotificationViewModel and pushing CallVanNotificationViewController (and
similarly instantiates mocks around lines 113-116); replace direct mock
construction with dependency resolution so production navigation paths never
ship test doubles: obtain the use case from the app's DI/factory/provider (or
pass it through the coordinator/router) and inject it into
CallVanNotificationViewModel (instead of
MockFetchCallVanNotificationListUseCase), and do the same for the detail flow
(replace Mock*UseCase with the real FetchCallVanNotificationDetailUseCase
resolved from the same DI/factory or passed from the caller) so navigation uses
real implementations in runtime.
- Around line 119-135: The onMainButtonTapped closure in
showBottomSheet(isLoggedIn:state:) is left as a no-op for every CallVanState and
is never wired to the bottom-sheet button; update showBottomSheet to assign
meaningful closures per state (e.g., callVanViewModel.participate(post:),
cancelParticipation(post:), closeRecruitment(post:), reopenRecruitment(post:),
markAsUsed(post:) or invoke the existing delegate methods) using the current
post/ID and isLoggedIn flag, and ensure the bottom-sheet initializer (the place
that previously set the button target at creation) uses this onMainButtonTapped
closure when the primary button is tapped so the appropriate action executes for
states .참여하기, .참여취소, .마감하기, .재모집, and .이용완료.
In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanFilterButton.swift`:
- Around line 18-23: Add a shared appearance setup method (e.g.,
configureAppearance()) in CallVanFilterButton that sets the button title (using
setTitle(_:for:)) and border color (layer.borderColor = UIColor... .cgColor)
based on the current isSelected and filterState.selectedCount, then call this
method from the init(filterState:) after configuring cornerRadius/borderWidth
and from the isSelected didSet; update didSet to invoke configureAppearance() so
initial visual state is correct even if isSelected isn’t changed later.
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListFilterViewController.swift`:
- Around line 410-413: The current logic in the filter handling always appends
departure, causing duplicate entries and preventing deselection; update the
logic in the CallVanListFilterViewController where filter.departure and
departure are handled so that: if departure == .all then set filter.departure =
[.all]; otherwise, if filter.departure already contains departure remove that
value (and if the list becomes empty set it to [.all]), else remove any .all
from filter.departure and append departure; always remove duplicates before
appending (e.g., use removeAll { $0 == departure } or work with a Set) and apply
the same change to the analogous block that currently appends at lines similar
to 425-427.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift`:
- Around line 184-204: When a predefined reason is selected in
setReasonButtonSelected, clear the stale custom-reason UI and ViewModel state:
in setReasonButtonSelected (and not only setCustomReasonButtonSelected) set the
textView back to its placeholder and gray color, update the text count UI
accordingly, and send inputSubject.send(.updateCustomReason(nil)) before sending
inputSubject.send(.updateReasonCode(...)) so model and UI stay in sync; also
replace the debug print in the failing branch with assertionFailure("Invalid
reason button mapping") to surface mapping errors during development.
---
Outside diff comments:
In `@Koin/Presentation/Home/Home/HomeViewController.swift`:
- Around line 147-149: The three debug print statements in HomeViewController
that call KeychainWorker.shared.read(key: .access), .fcm and .accessHistoryId
should be removed or replaced with privacy-safe logs; locate the print calls in
HomeViewController and either delete them or change them to non-sensitive checks
(e.g., log presence/boolean or masked values) using a secure logger rather than
printing raw keychain contents from KeychainWorker.shared.read.
---
Duplicate comments:
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swift`:
- Around line 151-156: chatButton and callButton are image-only and lack
accessibility metadata, so update CallVanListCollectionViewCell to make each
button an accessibility element and set descriptive accessibilityLabel (e.g.,
"Chat with driver" / "Call driver") and optional accessibilityHint as
appropriate; locate the chatButton and callButton configuration in
CallVanListCollectionViewCell and add properties like isAccessibilityElement =
true, accessibilityLabel = "<descriptive text>" and accessibilityHint/traits if
needed so VoiceOver announces meaningful names for these icon-only buttons.
- Around line 207-214: In CallVanListCollectionViewCell, the chatButton and
callButton are constrained to the same top/trailing anchors causing overlap;
change the layout so the two buttons are arranged horizontally (either by
creating an action UIStackView or by constraining callButton.leading to
chatButton.trailing with a fixed spacing) and then pin the stack (or chatButton)
to the top/trailing anchors (top = 16.5, trailing = -24) so they never occupy
the same position; update the constraints in the method where
chatButton.snp.makeConstraints and callButton.snp.makeConstraints are set to
reflect this new arrangement.
---
Nitpick comments:
In `@Koin/Domain/Model/CallVan/CallVanPlace.swift`:
- Around line 10-20: CallVanPlace currently uses Korean UI strings as rawValue
which couples domain identity to presentation; change the enum raw string values
to stable identifiers (e.g., "all", "frontGate", "backGate", "tennisCourt",
"dormitoryMain", "dormitorySub", "terminal", "station", "asanStation", "custom")
and add a computed property (e.g., var displayName: String) that returns the
localized Korean label via NSLocalizedString or a localization helper; update
any serialization/lookup sites that consume CallVanPlace.rawValue to use the new
identifiers (or map old stored values to new ones) and replace UI code that used
rawValue for display to use CallVanPlace.displayName instead so identity and
presentation are decoupled.
In `@Koin/Domain/Model/CallVan/CallVanReportRequest.swift`:
- Around line 15-18: CallVanReportRequestReason currently allows both reasonCode
and customReason to be nil which permits empty reasons; update the model to
enforce at least one is present by either making reasonCode non-optional with a
default value (e.g., set reasonCode: CallVanReportRequestReasonCode = .other) or
add a failable/throwing initializer on CallVanReportRequestReason that validates
(and rejects/throws) when both reasonCode and customReason are nil; reference
the struct CallVanReportRequestReason and fields reasonCode and customReason
when implementing the change and ensure callers are updated to handle the new
initializer or default.
In
`@Koin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableViewCell.swift`:
- Line 40: Rename the misspelled parameter shoudHideSepearatorView to
shouldHideSeparatorView in the
CallVanDataTableViewCell.configure(participant:shoudHideSepearatorView:) method
and update all internal references inside that function to the new parameter
name; also update every call site, override or protocol declaration that passes
or declares shoudHideSepearatorView so signatures remain consistent and the
public API reflects the corrected name.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift`:
- Around line 312-315: The loop currently uses a hard-coded range 0..<3 which
can crash if reasonButtons changes; update the assembly to iterate over
reasonButtons.indices and for each index add reasonButtons[index] to
buttonsStackView and only add separatorViews[index] if
separatorViews.indices.contains(index) (or otherwise guard availability) so
separators stay in sync with reasonButtons; ensure you update the code paths
that reference reasonButtons and separatorViews (symbols: buttonsStackView,
reasonButtons, separatorViews) to use the safe indexed iteration.
- Around line 193-194: In CallVanReportReasonViewController replace the
debug-only print("\(`#function`) failed") in the failure branch with a proper
failure/logging strategy: call
assertionFailure("CallVanReportReasonViewController: \(`#function`) failed") for
debug builds and also emit a structured log via your app logger (or
os_log/Logger) with context (e.g.,
"CallVanReportReasonViewController.\(`#function`) failed", include any error info
if available). Update the failure branch in the same method where
print("\(`#function`) failed") appears to remove the print and add both
assertionFailure and a structured logger call.
In `@Koin/Presentation/Home/Home/HomeViewController.swift`:
- Around line 643-659: In navigateToCallVanList() replace the test double by
creating and injecting the real use case (use DefaultFetchCallVanListUseCase
instead of MockFetchCallVanListUseCase, constructed with the appropriate
service/repository like DefaultCallVanService or repository similar to
DefaultUserRepository), and in navigateToCallVanPost() wire CallVanPostViewModel
with its real dependencies (e.g., DefaultCreateCallVanUseCase or other real use
cases) instead of a no-arg initializer; add a clear TODO/FIXME comment in both
navigateToCallVanList() and navigateToCallVanPost() and open a tracking issue to
ensure MockFetchCallVanListUseCase and the parameterless CallVanPostViewModel
are replaced with DefaultFetchCallVanListUseCase and the real CallVanPost use
case implementations before release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4b47350-1e69-4d3d-bfea-153bb582d215
📒 Files selected for processing (22)
Koin/Apps/NotificationHandler.swiftKoin/Domain/Model/CallVan/CallVanListRequest.swiftKoin/Domain/Model/CallVan/CallVanPlace.swiftKoin/Domain/Model/CallVan/CallVanPostRequest.swiftKoin/Domain/Model/CallVan/CallVanRecruitmentState.swiftKoin/Domain/Model/CallVan/CallVanReportRequest.swiftKoin/Domain/UseCase/CallVan/FetchCallVanListUseCase.swiftKoin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swiftKoin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swiftKoin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableView.swiftKoin/Presentation/CallVan/CallVanData/Subviews/CallVanDataTableView/CallVanDataTableViewCell.swiftKoin/Presentation/CallVan/CallVanList/CallVanListViewController.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanFilterButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanButton.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionViewCell.swiftKoin/Presentation/CallVan/CallVanList/Subviews/CallVanListFilterViewController.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceBottomSheetView.swiftKoin/Presentation/CallVan/CallVanPost/Subviews/CallVanPostPlaceView.swiftKoin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swiftKoin/Presentation/Home/Home/HomeViewController.swiftkoin.xcodeproj/project.pbxproj
🚧 Files skipped from review as they are similar to previous changes (2)
- Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanButton.swift
- Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListCollectionView/CallVanListCollectionView.swift
| mainState: .마감하기, subState: nil, showChatButton: false, showCallButton: true) | ||
| ] | ||
|
|
||
| let callVanList = CallVanList(posts: posts, totalCount: 0, currentPage: 0, totalPage: 0) |
There was a problem hiding this comment.
Mock pagination metadata is inconsistent with actual data.
The CallVanList is created with totalCount: 0, totalPage: 0 but contains 6 posts. This could cause pagination UI issues. Update to reflect the actual mock data:
🔧 Proposed fix
- let callVanList = CallVanList(posts: posts, totalCount: 0, currentPage: 0, totalPage: 0)
+ let callVanList = CallVanList(posts: posts, totalCount: posts.count, currentPage: 1, totalPage: 1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let callVanList = CallVanList(posts: posts, totalCount: 0, currentPage: 0, totalPage: 0) | |
| let callVanList = CallVanList(posts: posts, totalCount: posts.count, currentPage: 1, totalPage: 1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Domain/UseCase/CallVan/FetchCallVanListUseCase.swift` at line 50, The
mock CallVanList is populated with 6 posts but hardcodes totalCount: 0 and
totalPage: 0; update the creation of callVanList (CallVanList(posts: ...)) to
compute realistic pagination metadata — set totalCount to posts.count,
currentPage to 1 (or derive from the request/input page), and totalPage to 1 (or
compute as Int(ceil(Double(totalCount) / Double(pageSize))) if a page size
exists) so the mock metadata matches the actual posts.
Comment on lines
+8
to
+9
| import Foundation | ||
| import Combine |
There was a problem hiding this comment.
Remove duplicate imports.
Foundation and Combine are imported twice in this file.
🧹 Proposed fix
import Foundation
import Combine
protocol FetchCallVanNotificationListUseCase {
func execute() -> AnyPublisher<[CallVanNotification], ErrorResponse>
}
-import Combine
-import Foundation
-
final class MockFetchCallVanNotificationListUseCase: FetchCallVanNotificationListUseCase {Also applies to: 15-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swift` around
lines 8 - 9, The file FetchCallVanNotificationListUseCase.swift contains
duplicate import statements for Foundation and Combine; remove the repeated
import lines so each module is imported only once (keep a single "import
Foundation" and a single "import Combine"), and ensure the rest of the file
(including the FetchCallVanNotificationListUseCase type and any related
functions) still compiles after cleaning up the duplicate imports.
| CallVanNotification( | ||
| id: 2, | ||
| postId: 3, | ||
| type: .paritipantJoined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the CallVanNotificationType enum definition to verify the spelling
rg -n "participantJoined|paritipantJoined" --type swiftRepository: BCSDLab/KOIN_iOS
Length of output: 268
Fix typo: .paritipantJoined should be .participantJoined.
The typo exists in both the enum definition (CallVanNotification.swift:44) and its usage here. Both locations need to be corrected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Domain/UseCase/CallVan/FetchCallVanNotificationListUseCase.swift` at
line 47, Rename the misspelled enum case .paritipantJoined to .participantJoined
in the CallVanNotification enum and update all usages (e.g., the instance in
FetchCallVanNotificationListUseCase where it is referenced as type:
.paritipantJoined) to the corrected .participantJoined; ensure you also update
any switch statements, Codable/raw-value mappings, or tests that reference the
old name so serialization and pattern matching continue to work.
| // MARK: - TitleView | ||
| private lazy var titleLabel = UILabel() | ||
| private lazy var peopleImageView = UIImageView() | ||
| private lazy var paritipantsLabel = UILabel() |
There was a problem hiding this comment.
Typo: "paritipantsLabel" should be "participantsLabel".
✏️ Proposed fix
- private lazy var paritipantsLabel = UILabel()
+ private lazy var participantsLabel = UILabel()Note: Update all references to this variable (lines 108, 149, 177-179) accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift` at
line 22, Rename the misspelled property paritipantsLabel to participantsLabel
and update every use site (references at/near the original symbol: lines 108,
149, 177-179 in the diff) to the new identifier; ensure the lazy var declaration
in CallVanChatViewController and any constraints, text assignments, or
accessibility references that use paritipantsLabel are changed to
participantsLabel so the code compiles and semantics remain the same.
Comment on lines
+43
to
+49
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
| configureView() | ||
| configureNavigationBar() | ||
| setDelegate() | ||
| hideKeyboardWhenTappedAround() | ||
| } |
There was a problem hiding this comment.
ViewModel is injected but never used.
The viewModel property is initialized but there's no bind() method or any subscription to transform outputs. The view controller won't react to any data from the ViewModel.
🔧 Suggested structure to add binding
override func viewDidLoad() {
super.viewDidLoad()
configureView()
configureNavigationBar()
setDelegate()
hideKeyboardWhenTappedAround()
+ bind()
}
+
+ private func bind() {
+ // TODO: Subscribe to viewModel outputs and update UI
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| override func viewDidLoad() { | |
| super.viewDidLoad() | |
| configureView() | |
| configureNavigationBar() | |
| setDelegate() | |
| hideKeyboardWhenTappedAround() | |
| } | |
| override func viewDidLoad() { | |
| super.viewDidLoad() | |
| configureView() | |
| configureNavigationBar() | |
| setDelegate() | |
| hideKeyboardWhenTappedAround() | |
| bind() | |
| } | |
| private func bind() { | |
| // TODO: Subscribe to viewModel outputs and update UI | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/CallVan/CallVanChat/CallVanChatViewController.swift` around
lines 43 - 49, The viewModel property is injected but never observed; add a bind
method on CallVanChatViewController that subscribes to the viewModel's outputs
and forwards UI events to its inputs (e.g., observe message list, loading/error
states, and send user actions), then call bind() from viewDidLoad (alongside
configureView(), configureNavigationBar(), setDelegate()). Locate the viewModel
symbol and implement subscriptions to its publishers/closures (or Rx/Combine
streams) to update the UI and handle navigation/error presentation; ensure you
wire user input handlers (send message, typing, etc.) into viewModel inputs so
the controller reacts to ViewModel changes.
Comment on lines
+103
to
+106
| @objc private func filterButtonTapped() { | ||
| let contentViewController = CallVanListFilterViewController(filter: CallVanListRequest(), onApplyButtonTapped: { _ in }) | ||
| let bottomSheetViewController = BottomSheetViewController(contentViewController: contentViewController, defaultHeight: 605 + view.safeAreaInsets.bottom) | ||
| present(bottomSheetViewController, animated: true) |
There was a problem hiding this comment.
Filter apply path is disconnected from list state.
Line 104 always opens with a fresh CallVanListRequest() and the apply callback is a no-op, so user-selected filters are never applied/persisted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift` around
lines 103 - 106, filterButtonTapped currently creates a fresh CallVanListRequest
and a no-op onApplyButtonTapped, so chosen filters are never applied; update
filterButtonTapped to pass the current list request/state (e.g., the existing
CallVanListRequest instance or current filter model on
CallVanListViewController) into CallVanListFilterViewController and provide a
real onApplyButtonTapped closure that updates the controller's request,
refreshes/reloads the list (or calls the existing load/refresh method), and
dismisses the BottomSheetViewController; locate symbols filterButtonTapped,
CallVanListFilterViewController(filter:onApplyButtonTapped:), CallVanListRequest
and the view controller's load/refresh method to implement this change.
Comment on lines
+119
to
+135
| private func showBottomSheet(isLoggedIn: Bool?, state: CallVanState) { | ||
| let onMainButtonTapped: ()->Void | ||
| var defaultHeight: CGFloat = 195 + view.safeAreaInsets.bottom | ||
| switch (isLoggedIn, state) { | ||
| case (true, .참여하기): | ||
| onMainButtonTapped = {} | ||
| case (false, .참여하기): | ||
| onMainButtonTapped = {} | ||
| case (_ , .참여취소): | ||
| onMainButtonTapped = {} | ||
| case (_ , .마감하기): | ||
| onMainButtonTapped = {} | ||
| case (_ , .재모집): | ||
| onMainButtonTapped = {} | ||
| case (_, .이용완료): | ||
| onMainButtonTapped = {} | ||
| defaultHeight = 255 + view.safeAreaInsets.bottom |
There was a problem hiding this comment.
Bottom-sheet primary actions are currently no-ops.
Line 124–135 sets onMainButtonTapped = {} for every state, and Line 146–151 never wires the tapped post/state into a real action. The sheet can display, but critical actions (참여/취소/마감/재모집/이용완료) won’t execute.
Also applies to: 146-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/CallVan/CallVanList/CallVanListViewController.swift` around
lines 119 - 135, The onMainButtonTapped closure in
showBottomSheet(isLoggedIn:state:) is left as a no-op for every CallVanState and
is never wired to the bottom-sheet button; update showBottomSheet to assign
meaningful closures per state (e.g., callVanViewModel.participate(post:),
cancelParticipation(post:), closeRecruitment(post:), reopenRecruitment(post:),
markAsUsed(post:) or invoke the existing delegate methods) using the current
post/ID and isLoggedIn flag, and ensure the bottom-sheet initializer (the place
that previously set the button target at creation) uses this onMainButtonTapped
closure when the primary button is tapped so the appropriate action executes for
states .참여하기, .참여취소, .마감하기, .재모집, and .이용완료.
Comment on lines
+18
to
+23
| init(filterState: CallVanFilterState) { | ||
| self.filterState = filterState | ||
| super.init(frame: .zero) | ||
| layer.cornerRadius = 17 | ||
| layer.borderWidth = 1 | ||
| } |
There was a problem hiding this comment.
Missing initial visual configuration for button appearance.
The didSet on isSelected handles title and border color, but didSet does not fire during initialization. If isSelected is never explicitly set (or remains at default false), the button will have no title text and an undefined border color.
While context shows parent controllers always set isSelected programmatically, adding initial configuration prevents latent bugs in future usage.
🛠️ Proposed fix to configure initial appearance
init(filterState: CallVanFilterState) {
self.filterState = filterState
super.init(frame: .zero)
layer.cornerRadius = 17
layer.borderWidth = 1
+ configureAppearance()
+ }
+
+ private func configureAppearance() {
+ let foregroundColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral500)
+ let borderColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral300)
+
+ setAttributedTitle(NSAttributedString(string: filterState.rawValue, attributes: [
+ .font: UIFont.appFont(.pretendardBold, size: 14),
+ .foregroundColor: foregroundColor
+ ]), for: .normal)
+
+ layer.borderColor = borderColor.cgColor
}Then update didSet to call the shared method:
override var isSelected: Bool {
didSet {
- let foregroundColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral500)
- let borderColor = isSelected ? UIColor.appColor(.new500) : UIColor.appColor(.neutral300)
-
- setAttributedTitle(NSAttributedString(string: filterState.rawValue, attributes: [
- .font : UIFont.appFont(.pretendardBold, size: 14),
- .foregroundColor : foregroundColor
- ]), for: .normal)
-
- layer.borderColor = borderColor.cgColor
+ configureAppearance()
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanFilterButton.swift`
around lines 18 - 23, Add a shared appearance setup method (e.g.,
configureAppearance()) in CallVanFilterButton that sets the button title (using
setTitle(_:for:)) and border color (layer.borderColor = UIColor... .cgColor)
based on the current isSelected and filterState.selectedCount, then call this
method from the init(filterState:) after configuring cornerRadius/borderWidth
and from the isSelected didSet; update didSet to invoke configureAppearance() so
initial visual state is correct even if isSelected isn’t changed later.
Comment on lines
+410
to
+413
| } else { | ||
| filter.departure = filter.departure.filter { $0 != .all } | ||
| filter.departure.append(departure) | ||
| } |
There was a problem hiding this comment.
Prevent duplicate place entries and support toggle-off behavior.
Line 412 and Line 426 always append, so repeated taps can produce duplicates (e.g. [.frontGate, .frontGate]) and users cannot deselect a non-.all place. This can send incorrect filter payloads.
💡 Suggested fix
`@objc` private func departureButtonTapped(_ sender: UIButton) {
guard let departureButton = sender as? CallVanFilterButton,
let departure = departureButton.filterState as? CallVanPlace else {
return
}
if departure == .all {
filter.departure = [.all]
} else {
- filter.departure = filter.departure.filter { $0 != .all }
- filter.departure.append(departure)
+ let wasSelected = departureButton.isSelected
+ filter.departure.removeAll { $0 == .all || $0 == departure }
+ if !wasSelected {
+ filter.departure.append(departure)
+ }
+ if filter.departure.isEmpty {
+ filter.departure = [.all]
+ }
}
}
@@
`@objc` private func arrivalButtonTapped(_ sender: UIButton) {
guard let arrivalButton = sender as? CallVanFilterButton,
let arrival = arrivalButton.filterState as? CallVanPlace else {
return
}
if arrival == .all {
filter.arrival = [.all]
} else {
- filter.arrival = filter.arrival.filter { $0 != .all }
- filter.arrival.append(arrival)
+ let wasSelected = arrivalButton.isSelected
+ filter.arrival.removeAll { $0 == .all || $0 == arrival }
+ if !wasSelected {
+ filter.arrival.append(arrival)
+ }
+ if filter.arrival.isEmpty {
+ filter.arrival = [.all]
+ }
}
}Also applies to: 425-427
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanList/Subviews/CallVanListFilterViewController.swift`
around lines 410 - 413, The current logic in the filter handling always appends
departure, causing duplicate entries and preventing deselection; update the
logic in the CallVanListFilterViewController where filter.departure and
departure are handled so that: if departure == .all then set filter.departure =
[.all]; otherwise, if filter.departure already contains departure remove that
value (and if the list becomes empty set it to [.all]), else remove any .all
from filter.departure and append departure; always remove duplicates before
appending (e.g., use removeAll { $0 == departure } or work with a Set) and apply
the same change to the analogous block that currently appends at lines similar
to 425-427.
Comment on lines
+184
to
+204
| private func setReasonButtonSelected(_ sender: CallVanReportReasonButton) { | ||
| reasonButtons.forEach { button in | ||
| button.isSelected = button === sender | ||
| } | ||
| customReasonButton.isSelected = false | ||
|
|
||
| if let reasonCode = CallVanReportRequestReasonCode(rawValue: sender.title) { | ||
| inputSubject.send(.updateReasonCode(reasonCode)) | ||
| } else { | ||
| print("\(#function) failed") | ||
| } | ||
| } | ||
|
|
||
| private func setCustomReasonButtonSelected() { | ||
| reasonButtons.forEach { button in | ||
| button.isSelected = false | ||
| } | ||
| customReasonButton.isSelected = true | ||
|
|
||
| inputSubject.send(.updateReasonCode(.other)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether customReason can be serialized/sent even when reasonCode != .other
# Expected: submission path should ignore customReason unless reasonCode == .other
rg -n -C3 'customReason|reasonCode|reasons|CallVanReportRequestReason' Koin/Presentation/CallVan/CallVanReport
fd 'CallVanReportViewModel.swift' --exec sed -n '1,260p' {}Repository: BCSDLab/KOIN_iOS
Length of output: 24870
🏁 Script executed:
#!/bin/bash
# Find submission code and how reportRequest/reportReason is serialized
rg -n 'nextButtonTapped|CallVanReportRequest|submit|send.*report|Codable|encode' Koin/Presentation/CallVan/CallVanReport --max-count=20
# Check CallVanReportRequest structure
fd 'CallVanReportRequest.swift' --exec head -60 {}
# Check where inputSubject connects to ViewModel
rg -n 'inputSubject\|viewModel\.transform\|inputSubject.send' Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swiftRepository: BCSDLab/KOIN_iOS
Length of output: 2100
🏁 Script executed:
#!/bin/bash
# Get the nextButtonTapped implementation
sed -n '179,220p' Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift
# Search for where reportReason is added to reportRequest
rg -n 'reasons\|reportReason|reportRequest\[' Koin/Presentation/CallVan/CallVanReport/CallVanReportViewModel.swift -A 2
# Search for API submission/service code
fd '\.swift' Koin/Presentation/CallVan --exec rg -l 'CallVanReportRequest.*send\|CallVanReportRequest.*Codable\|encode' {}
# Check if CallVanReportRequest implements Codable
rg -n 'extension CallVanReportRequest\|Codable\|Encodable' Koin/Presentation/CallVan/CallVanReport/ -A 5Repository: BCSDLab/KOIN_iOS
Length of output: 1762
🏁 Script executed:
#!/bin/bash
# Search for API submission across CallVan directory
rg -n 'CallVanReportRequest\|reportRequest\|reasons.*append\|submit\|API' Koin/Presentation/CallVan/ -t swift --max-count=30
# Find all swift files in CallVan to understand flow
fd '\.swift$' Koin/Presentation/CallVan/ | head -20
# Search for where reasons array is populated
rg -n 'reasons.*=\|\[reportReason\]' Koin/Presentation/CallVan/ -t swiftRepository: BCSDLab/KOIN_iOS
Length of output: 1711
🏁 Script executed:
#!/bin/bash
# Find CallVanReportEvidenceViewController
fd 'CallVanReportEvidenceViewController' Koin/
# Search in broader area for report submission
rg -n 'CallVanReport.*submit\|report.*API\|reasons.*append' Koin/ -t swift --max-count=20
# Look for Repository pattern
fd 'Repository' Koin/ | grep -i reportRepository: BCSDLab/KOIN_iOS
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Read CallVanReportEvidenceViewController
wc -l Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift
# Get the file content
cat Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift | head -250Repository: BCSDLab/KOIN_iOS
Length of output: 8507
🏁 Script executed:
#!/bin/bash
# Continue reading the rest of CallVanReportEvidenceViewController
tail -150 Koin/Presentation/CallVan/CallVanReport/CallVanReportEvidenceViewController.swift
# Search for where reportRequest.reasons is updated or built
rg -n 'reportRequest\|reasons.*=\|append.*reportReason' Koin/Presentation/CallVan/CallVanReport/ -A 3 -B 1
# Check full ViewModel to see how reasons are built
wc -l Koin/Presentation/CallVan/CallVanReport/CallVanReportViewModel.swift
tail -100 Koin/Presentation/CallVan/CallVanReport/CallVanReportViewModel.swiftRepository: BCSDLab/KOIN_iOS
Length of output: 8710
🏁 Script executed:
#!/bin/bash
# Search for where reasons array is populated or modified
rg -n 'reasons\s*=\|reasons\.append\|reasons\s*\+=' Koin/Presentation/CallVan/CallVanReport/ -A 2
# Search for submission code - API call, encode, serialize
rg -n 'submit\|encode\|APIService\|NetworkService\|request.*send' Koin/Presentation/CallVan/CallVanReport/ -t swift
# Check if there's a complete ViewModel implementation elsewhere
fd 'CallVanReportViewModel' Koin/
# Search the entire codebase for where reasons is used
rg -n '\.reasons' Koin/ -t swift --max-count=15Repository: BCSDLab/KOIN_iOS
Length of output: 128
Clear stale custom reason state when selecting a predefined reason.
When switching from .other to a predefined reason, only reasonCode updates; the previously entered custom text remains in ViewModel state while UI clears. This creates state divergence: if submission logic eventually populates the reasons array or serializes reportReason, stale customReason will be included despite UI intent.
Clear both model and UI state atomically on predefined selection:
- Set textView to placeholder and gray color
- Send
.updateCustomReason(nil)to clear ViewModel state - Update text count display
- Replace debug print with
assertionFailurefor invalid button mappings
Proposed changes
private func setReasonButtonSelected(_ sender: CallVanReportReasonButton) {
reasonButtons.forEach { button in
button.isSelected = button === sender
}
customReasonButton.isSelected = false
+ customReasonTextView.text = customReasonPlaceholder
+ customReasonTextView.textColor = UIColor.appColor(.gray)
+ updateTextCount()
+ inputSubject.send(.updateCustomReason(nil))
if let reasonCode = CallVanReportRequestReasonCode(rawValue: sender.title) {
inputSubject.send(.updateReasonCode(reasonCode))
} else {
- print("\(`#function`) failed")
+ assertionFailure("\(`#function`): invalid reason button title mapping")
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Koin/Presentation/CallVan/CallVanReport/CallVanReportReasonViewController.swift`
around lines 184 - 204, When a predefined reason is selected in
setReasonButtonSelected, clear the stale custom-reason UI and ViewModel state:
in setReasonButtonSelected (and not only setCustomReasonButtonSelected) set the
textView back to its placeholder and gray color, update the text count UI
accordingly, and send inputSubject.send(.updateCustomReason(nil)) before sending
inputSubject.send(.updateReasonCode(...)) so model and UI stay in sync; also
replace the debug print in the failing branch with assertionFailure("Invalid
reason button mapping") to surface mapping errors during development.
hgjwilly-koreatech
force-pushed
the
design/callvan-party
branch
from
e07014e to
800c902
Compare
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
1 participant
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.
#️⃣연관된 이슈
📝작업 내용
콜밴팟 UI 구현작업입니다.
스크린샷 (선택)
💬리뷰 요구사항(선택)
Summary by CodeRabbit
New Features
UI Enhancements
Extensions
Assets