[Feature] #805 - 앱잼템프 미션 상세뷰 + alert #806
Conversation
Summary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughisAppjam 플래그가 뷰모델·유스케이스·저장소로 전달되고, ListDetailModel에 선택적 profileInfo가 추가되어 조건부 프로필 섹션을 표시하며, 미완료 미션에 대해 비활성 사용자용 Alert 타입·문구·테마가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as MissionListVC / ListDetailVC
participant VM as ListDetailViewModel
participant UC as ListDetailUseCase
participant Repo as ListDetailRepository
participant API as StampService
Note over UI,VM: 사용자 상세 열기 또는 셀 탭
UI->>VM: request(missionId, username, isAppjam?)
VM->>UC: fetchListDetail(isAppjam, missionId, username)
UC->>Repo: fetchListDetail(isAppjam, missionId, username)
Repo->>API: API 호출 (StampService)
API-->>Repo: 응답 (데이터)
Repo-->>UC: 매핑된 ListDetailModel (profileInfo 포함 가능)
UC-->>VM: 퍼블리시(ListDetailModel)
VM->>UI: 바인딩 업데이트 (showProfileInfo / hideProfileInfo)
alt 미완료 & userType ∈ {inactive, visitor}
UI->>UI: AlertUtils.present(title: inactiveUserAlertTitle, desc: inactiveUserAlertDescription)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift (1)
108-116:runMissionDetailById에서isAppjam: true하드코딩 제거 필요이 메서드는 깊은 링크 핸들러에서 호출되는 공개 인터페이스이며, 앱잼 미션뿐만 아니라 일반 미션도 처리해야 합니다.
isAppjam을 파라미터로 받아 호출부에서 명시적으로 지정하세요. 현재 하드코딩된isAppjam: true는 모든 깊은 링크 네비게이션을 앱잼 미션으로 강제하므로 타입 불일치가 발생할 수 있습니다.
🧹 Nitpick comments (7)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
98-99: 파라미터 네이밍 개선 고려
cancelButtonColor(isNetworkErr:)메서드가 이제titleDescriptionSingleButton에도 사용되므로, 파라미터명isNetworkErr가 의미적으로 정확하지 않습니다.AlertVCTheme의 메서드 시그니처를isSingleButtonStyle등으로 변경하면 가독성이 향상됩니다.🔎 AlertVCTheme 리팩토링 제안
// AlertVCTheme.swift -func cancelButtonColor(isNetworkErr: Bool) -> UIColor { +func cancelButtonColor(isSingleButtonStyle: Bool) -> UIColor { switch self { case .main: - return isNetworkErr ? DSKitAsset.Colors.white.color : DSKitAsset.Colors.gray600.color + return isSingleButtonStyle ? DSKitAsset.Colors.white.color : DSKitAsset.Colors.gray600.color case .soptamp: - return isNetworkErr ? DSKitAsset.Colors.white.color : DSKitAsset.Colors.gray600.color + return isSingleButtonStyle ? DSKitAsset.Colors.white.color : DSKitAsset.Colors.gray600.color } }SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift (1)
821-834: 스택뷰 인덱스 하드코딩 주의
insertArrangedSubview(profileInfoView, at: 2)에서 인덱스가 하드코딩되어 있습니다. 스택뷰 구성이 변경되면 프로필 뷰가 잘못된 위치에 삽입될 수 있습니다. 현재 구조에서는 동작하지만, 유지보수를 위해 주석을 추가하거나 상대적 위치 기반 삽입을 고려해 주세요.SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (1)
20-20:hideBorder()중복 호출 제거
profileImageView에서hideBorder()가 초기화 시점(Line 20)과setUI()내부(Line 41)에서 두 번 호출되고 있습니다. 한 곳에서만 호출하면 됩니다.🔎 수정 제안
- private let profileImageView = CustomProfileImageView().hideBorder() + private let profileImageView = CustomProfileImageView()또는
setUI()내의 호출을 제거:private func setUI() { self.backgroundColor = .clear - profileImageView.hideBorder() - nameLabel.font = .SoptampFont.subtitle3Also applies to: 38-41
SOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swift (1)
49-57: 새로운ProfileInfo구조체 추가 확인프로필 정보를 위한 구조체가 적절하게 정의되었습니다. 다만,
ListDetailModel은 2-space 들여쓰기를 사용하고ProfileInfo는 4-space 들여쓰기를 사용하고 있어 파일 내 스타일이 일관되지 않습니다.🔎 들여쓰기 통일 제안
public struct ProfileInfo { - public let name: String - public let imageURL: String? - - public init(name: String, imageURL: String?) { - self.name = name - self.imageURL = imageURL - } + public let name: String + public let imageURL: String? + + public init(name: String, imageURL: String?) { + self.name = name + self.imageURL = imageURL + } }SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
38-40:NSError()직접 초기화 대신 도메인 에러 타입 사용 권장빈
NSError()생성은 디버깅 시 유용한 정보를 제공하지 않습니다. 도메인별 에러 타입을 정의하거나 최소한 의미 있는 에러 정보를 포함시키는 것이 좋습니다.🔎 수정 제안
guard let username else { - return Fail(error: NSError()).eraseToAnyPublisher() + return Fail(error: NSError(domain: "ListDetailRepository", code: -1, userInfo: [NSLocalizedDescriptionKey: "Username is required"])).eraseToAnyPublisher() }또는 별도의 도메인 에러 타입을 정의하여 사용:
enum ListDetailError: Error { case usernameMissing } // ... return Fail(error: ListDetailError.usernameMissing).eraseToAnyPublisher()SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (2)
104-108: 삼항 연산자로 void 함수 호출은 지양하세요.정적 분석 도구에서
void_function_in_ternary경고가 발생했습니다. 삼항 연산자 대신 if-else 문을 사용하면 코드 가독성이 향상됩니다.🔎 제안된 수정
.sink { owner, _ in - owner.isOtherUser - ? owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: owner.otherUserName) - : owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: nil) - + let username = owner.isOtherUser ? owner.otherUserName : nil + owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: username) + if owner.isOtherUser {
274-294: DEBUG 목 데이터가 향후 제거되어야 합니다.PR 목표에 명시된 대로 이 DEBUG 블록의 테스트용
ProfileInfo는 앱잼 미션 상세 API가 연결되면 제거해야 합니다.#if DEBUG로 감싸져 있어 프로덕션 빌드에는 영향이 없지만, 이 작업을 추적하기 위해 TODO 주석을 추가하는 것을 권장합니다.이 작업을 추적하기 위해 새로운 이슈를 생성해 드릴까요?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swift(1 hunks)SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift(1 hunks)SOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swift(3 hunks)SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift(1 hunks)SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift(2 hunks)SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertType.swift(1 hunks)SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift(3 hunks)SOPT-iOS/Projects/Features/StampFeature/Interface/Sources/StampFeatureBuildable.swift(1 hunks)SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swift(1 hunks)SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift(1 hunks)SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift(4 hunks)SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift(1 hunks)SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift(5 hunks)SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-07T07:54:51.219Z
Learnt from: yungu0010
Repo: sopt-makers/SOPT-iOS PR: 769
File: SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift:27-27
Timestamp: 2025-12-07T07:54:51.219Z
Learning: In SoptlogVC.swift, the isPokeEmpty field is intentionally hardcoded to true and not bound to the ViewModel because it's a temporary measure until the poke server 500 error is fixed. Once the server is stabilized, this field and the empty view implementation will be removed.
Applied to files:
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift
📚 Learning: 2025-09-23T04:48:57.229Z
Learnt from: dlwogus0128
Repo: sopt-makers/SOPT-iOS PR: 699
File: SOPT-iOS/Projects/Features/RootFeature/Sources/ApplicationCoordinator.swift:576-581
Timestamp: 2025-09-23T04:48:57.229Z
Learning: In SOPT-iOS coordinator refactoring: Legacy coordinators require addDependency() calls for proper lifecycle management, but the new coordinator architecture handles dependency management differently and does not require explicit addDependency() calls.
Applied to files:
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift
🧬 Code graph analysis (7)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (2)
SOPT-iOS/Projects/Modules/DSKit/Sources/Components/CustomProfileImageView.swift (1)
hideBorder(49-53)SOPT-iOS/Projects/Core/Sources/Extension/UIKit+/UIView+.swift (1)
addSubviews(13-15)
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift (3)
SOPT-iOS/Projects/Core/Sources/Literals/UserDefaultKeyLIst.swift (1)
getUserType(87-96)SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Coordinator/Router/LegacyRouter.swift (1)
presentAlertVC(281-298)SOPT-iOS/Projects/Features/StampFeature/Sources/RankingScene/CollectionView/Ranking/RankingSection.swift (1)
type(15-17)
SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift (2)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
fetchListDetail(36-51)SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
fetchListDetail(52-61)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift (1)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (1)
configure(78-86)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift (2)
cancelButtonColor(66-73)cancelButtonTitleColor(75-82)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (2)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
fetchListDetail(36-51)SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
fetchListDetail(52-61)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift (4)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/LegacyMissionDetailCoordinator.swift (1)
showMissionDetail(35-53)SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swift (1)
makeListDetailVC(34-54)SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/LegacyStampBuilder.swift (1)
makeListDetailVC(32-50)SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/Cells/MissionListCellType.swift (1)
toListDetailSceneType(117-119)
🪛 SwiftLint (0.57.0)
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift
[Warning] 483-483: TODOs should be resolved (- 앱잼안하는 활동유저의 경우대한 분기도 추가)
(todo)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift
[Warning] 39-39: Discouraged direct initialization of types that can be harmful
(discouraged_direct_init)
[Warning] 42-42: TODOs should be resolved (- 앱잼템프용 api로 변경)
(todo)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift
[Warning] 106-106: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift
[Warning] 119-119: TODOs should be resolved (- isAppjam 기본값 제거)
(todo)
🔇 Additional comments (18)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift (1)
118-120: Deprecation 경고와 TODO 확인
@available경고를 통해isAppjam파라미터 분기를 강제하려는 의도가 좋습니다. 다만 Line 88과 Line 236에서isAppjam없이 호출되어 deprecation 경고가 발생합니다. 의도된 동작이라면 괜찮지만, 추후 TODO에서 기본값을 제거할 때 이 호출부들도 명시적으로 수정이 필요합니다.SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertType.swift (1)
11-16: LGTM!
titleDescriptionSingleButtoncase가 기존 네이밍 컨벤션을 따르며 올바르게 추가되었습니다.AlertVC.swift에서 해당 case에 대한 처리가 적절히 구현되어 있습니다.SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (2)
114-122: LGTM!
cancelTitleswitch 문이 새 case를 올바르게 처리합니다..networkErr와.titleDescriptionSingleButton을 함께 그룹화하여 "OK" 버튼 텍스트를 표시하는 것이 적절합니다.
186-192: LGTM!싱글 버튼 레이아웃이
.titleDescriptionSingleButtoncase에 대해 올바르게 적용됩니다.SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift (2)
313-319: LGTM!
profileInfo의 존재 여부에 따라 프로필 뷰를 조건부로 표시/숨기는 로직이 올바르게 구현되었습니다. Optional binding을 통한 nil 체크가 적절합니다.
81-81: LGTM!
profileInfoView초기화가 다른 UI 컴포넌트들과 일관된 패턴으로 구현되었습니다.SOPT-iOS/Projects/Features/StampFeature/Interface/Sources/StampFeatureBuildable.swift (1)
22-24: API 불일치 우려는 기반이 없습니다.LegacyStampBuilder는StampFeatureBuildable이 아닌 별도의LegacyStampFeatureViewBuildable프로토콜을 준수합니다. 새로운StampFeatureBuildable프로토콜에isAppjam: Bool파라미터가 추가된 것은 레거시 프로토콜과 무관한 의도된 설계입니다.Likely an incorrect or invalid review comment.
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift (2)
471-486: 사용자 유형 분기 로직이 잘 구현되었습니다.미션 완료 여부와 사용자 유형에 따른 분기 처리가 명확합니다. TODO 주석에 명시된 대로 앱잼에 참여하지 않는 활동 유저에 대한 추가 분기가 필요합니다.
추후 구현 시
UserType에 앱잼 관련 상태를 추가하거나, 별도의 앱잼 활성 상태 체크 로직이 필요할 수 있습니다.
492-501: LGTM!
AlertUtils를 활용한 알림 표시 로직이 기존 패턴과 일관성 있게 구현되었습니다. I18N 문자열을 사용하여 지역화도 적절히 처리되었습니다.SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (1)
77-86: LGTM!
configure메서드의 이미지 URL 처리 로직이 적절합니다.nil또는 빈 문자열인 경우 플레이스홀더를 표시하는 방어적 처리가 잘 되어 있습니다.SOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swift (1)
21-22: LGTM!
profileInfo프로퍼티 추가와 이니셜라이저 변경이 적절합니다. 기본값nil을 사용하여 기존 코드와의 호환성을 유지한 점이 좋습니다.Also applies to: 32-34, 44-44
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
41-50: 앱잼 API 분기 구조 확인현재 두 분기가 동일한 API를 호출하고 있으며, TODO 주석에 앱잼용 API로 변경 예정임이 명시되어 있습니다. 앱잼 API가 준비되면 해당 분기를 업데이트해 주세요.
현재 중복 코드가 있지만, 향후 변경 의도가 명확하므로 구조는 적절합니다.
SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (2)
52-60: LGTM!
isAppjam파라미터가 Repository 레이어로 올바르게 전달되고 있습니다. 기존 에러 처리 및 데이터 전달 로직은 그대로 유지되어 있어 변경 범위가 최소화되었습니다.
14-15:isAppjam파라미터 일관성 검증 완료프로토콜 시그니처 변경이 모든 계층에서 일관되게 반영되었습니다:
ListDetailUseCase프로토콜:fetchListDetail(isAppjam: Bool?, missionId: Int, username: String?)✓DefaultListDetailUseCase구현: 파라미터를 저장소에 올바르게 전달 ✓ListDetailRepositoryInterface및ListDetailRepository: 모두 업데이트됨 ✓ListDetailViewModel: 메서드 호출 시isAppjam파라미터 정상 사용 ✓변경사항이 최소한이고 명확하며, 모든 구현체가 일관되게 업데이트되었습니다.
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swift (1)
131-134: LGTM!비활성 사용자 알림에 필요한 문자열 상수가 적절하게 추가되었습니다.
I18N.MissionList네임스페이스 내에 일관되게 배치되어 있습니다.SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (1)
35-36: LGTM!
isAppjam프로퍼티와 이니셜라이저 변경이 적절합니다. 기본값nil을 사용하여 기존 호출 코드와의 하위 호환성을 유지하고 있습니다.Also applies to: 74-91
SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift (1)
14-15: LGTM!
fetchListDetail시그니처에isAppjam: Bool?파라미터가 올바르게 추가되었습니다.ListDetailUseCase와ListDetailRepository구현체의 시그니처와 일치합니다. Repository 구현에서 앱잼 전용 API 연결은 남은 작업으로 표시되어 있음을 확인했습니다.SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swift (1)
34-54: LGTM!
makeListDetailVC메서드에isAppjam: Bool파라미터가 적절하게 추가되었고,ListDetailViewModel초기화 시 올바르게 전달됩니다.StampFeatureBuildable프로토콜 변경과 일관성 있게 구현되었습니다.
| private func hideProfileInfo() { | ||
| if contentStackView.arrangedSubviews.contains(profileInfoView) { | ||
| contentStackView.removeArrangedSubview(profileInfoView) | ||
| profileInfoView.removeFromSuperview() | ||
|
|
||
| profileInfoView.snp.removeConstraints() | ||
| } | ||
| profileInfoView.isHidden = true | ||
| } |
There was a problem hiding this comment.
snp.removeConstraints() 호출 불필요
removeFromSuperview() 호출 시 해당 뷰의 constraints가 자동으로 제거됩니다. snp.removeConstraints()는 SnapKit에서 제공하지 않는 API이며, 컴파일 에러가 발생할 수 있습니다.
🔎 수정 제안
private func hideProfileInfo() {
if contentStackView.arrangedSubviews.contains(profileInfoView) {
contentStackView.removeArrangedSubview(profileInfoView)
profileInfoView.removeFromSuperview()
-
- profileInfoView.snp.removeConstraints()
}
profileInfoView.isHidden = true
}📝 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.
| private func hideProfileInfo() { | |
| if contentStackView.arrangedSubviews.contains(profileInfoView) { | |
| contentStackView.removeArrangedSubview(profileInfoView) | |
| profileInfoView.removeFromSuperview() | |
| profileInfoView.snp.removeConstraints() | |
| } | |
| profileInfoView.isHidden = true | |
| } | |
| private func hideProfileInfo() { | |
| if contentStackView.arrangedSubviews.contains(profileInfoView) { | |
| contentStackView.removeArrangedSubview(profileInfoView) | |
| profileInfoView.removeFromSuperview() | |
| } | |
| profileInfoView.isHidden = true | |
| } |
🤖 Prompt for AI Agents
In
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift
around lines 836 to 844, remove the call to
profileInfoView.snp.removeConstraints() because SnapKit does not provide that
API and constraints are removed automatically on removeFromSuperview(); simply
delete that line and keep the existing arrangedSubview removal,
removeFromSuperview() call, and profileInfoView.isHidden = true to fix the
compile error.
| let model = tappedCell.model else { return } | ||
| onCellTap?(model, sceneType.usrename) | ||
| let userType = UserDefaultKeyList.Auth.getUserType() | ||
|
|
| } | ||
|
|
||
| private func showMissionDetail(_ model: MissionListModel, _ username: String?) { | ||
| @available(*, deprecated, message: "⚠️ 앱잼 템프인지 일반 미션인지 확인했나요? isAppjam 파라미터로 분기해주세요!") |
| self.view.backgroundColor = .clear | ||
| self.alertView.backgroundColor = alertTheme.backgroundColor | ||
| self.cancelButton.backgroundColor = alertTheme.cancelButtonColor(isNetworkErr: self.alertType == .networkErr) | ||
| self.cancelButton.backgroundColor = alertTheme.cancelButtonColor(isNetworkErr: self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton) |
There was a problem hiding this comment.
networkErr만 다루는게 아니라면 isNetworkErr 이름을 변경하는 것도 좋아보여요 !
There was a problem hiding this comment.
isSingleButtonAlert 요걸로 변경할게요!
hye0njuoo
force-pushed
the
feat/#805-mission-detail-alert
branch
from
1dbca0a to
b77049a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift:
- Around line 36-50: The fetchListDetail function has identical logic in the
isAppjam == true and else branches; remove the conditional and return a single
publisher: call stampService.fetchStampListDetail(missionId: missionId,
username: username), then .map { $0.toDomain() } and .eraseToAnyPublisher()
inside fetchListDetail, preserving the existing username defaulting behavior and
any guard checks.
In
@SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift:
- Around line 108-109: The call to alertTheme.cancelButtonTitleColor uses the
outdated parameter name isNetworkErr; update the API and call to use a single
consistent parameter name (isSingleButtonAlert) across AlertVC.swift and
AlertVCTheme.swift: rename the parameter in the
AlertVCTheme.cancelButtonTitleColor signature to isSingleButtonAlert (or add an
overload/forwarding parameter) and change the call in AlertVC (currently using
self.alertType == .networkErr || self.alertType ==
.titleDescriptionSingleButton) to pass isSingleButtonAlert: (self.alertType ==
.networkErr || self.alertType == .titleDescriptionSingleButton) so both
definition and caller use the same identifier.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift (1)
75-82:cancelButtonTitleColor의 파라미터명도 일관성 있게 변경 필요
cancelButtonColor는isSingleButtonAlert로 파라미터명을 변경했지만,cancelButtonTitleColor는 여전히isNetworkErr를 사용하고 있어 일관성이 떨어집니다. 두 함수 모두 동일한 조건(network error 또는 single button alert)을 다루므로 같은 파라미터명을 사용하는 것이 좋습니다.🔎 제안하는 수정안
- func cancelButtonTitleColor(isNetworkErr: Bool) -> UIColor { + func cancelButtonTitleColor(isSingleButtonAlert: Bool) -> UIColor { switch self { case .main: - return isNetworkErr ? DSKitAsset.Colors.black100.color : DSKitAsset.Colors.white.color + return isSingleButtonAlert ? DSKitAsset.Colors.black100.color : DSKitAsset.Colors.white.color case .soptamp: - return isNetworkErr ? DSKitAsset.Colors.black.color : DSKitAsset.Colors.white.color + return isSingleButtonAlert ? DSKitAsset.Colors.black.color : DSKitAsset.Colors.white.color } }그리고 AlertVC.swift의 호출부도 함께 업데이트:
self.cancelButton.setTitleColor(alertTheme.cancelButtonTitleColor(isSingleButtonAlert: self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton), for: .normal)SOPT-iOS/Projects/Features/StampFeature/Interface/Sources/StampFeatureBuildable.swift (1)
17-24: 프로토콜 시그니처 변경이 일부 구현체와 호출부에서 미대응
makeListDetailVC메서드에 추가된 필수 파라미터isAppjam: Bool이 다음 항목들에서 누락되었습니다:
LegacyStampBuilder.makeListDetailVC()-isAppjam: Bool파라미터 없음LegacyMissionDetailCoordinator의 호출부 -isAppjam파라미터 미전달StampBuilder와 StampCoordinator는 올바르게 업데이트되었지만, 레거시 코드경로에서 breaking change가 해결되지 않았습니다.
SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift (1)
19-22: 반환 타입 일관성을 맞춰 주세요.모든 메서드가 구현체에서 올바르게 구현되어 있으나, 반환 타입이 일관성이 없습니다.
putStamp와deleteStamp는Driver를 사용하고 있는 반면,clap과getClapList는AnyPublisher를 사용하고 있습니다. 동일한 프로토콜 내에서 반응형 타입을 통일하는 것을 권장합니다.
🧹 Nitpick comments (2)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (1)
106-107: Void 함수 호출 시 삼항 연산자 대신 if-else를 사용하세요.삼항 연산자는 값을 반환하는 표현식에 적합하며, Void 함수를 호출할 때는 if-else 문이 더 명확합니다. SwiftLint도 이를 경고하고 있습니다.
🔎 제안하는 리팩토링
- .sink { owner, _ in - owner.isOtherUser - ? owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: owner.otherUserName) - : owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: nil) + .sink { owner, _ in + if owner.isOtherUser { + owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: owner.otherUserName) + } else { + owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: nil) + }SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift (1)
15-15: 파라미터 타입과 순서 개선을 권장합니다.
fetchListDetail메서드의isAppjam파라미터가 필수 매개변수 앞에 배치되어 있고,Bool?타입을 사용하고 있습니다. 다음 점들을 검토하세요:
타입 선택: 현재 구현에서
isAppjam == true일 때와 그 외의 경우(nil/false)에서 동일한 API를 호출합니다.Bool?의 세 가지 상태가 모두 활용되지 않으므로,isAppjam: Bool = false로 변경하면 의도가 더 명확해집니다.파라미터 순서: 선택적 기능 플래그를 필수 매개변수 뒤에 배치하는 것이 관례입니다.
이 변경은 ListDetailRepositoryInterface, ListDetailUseCase 프로토콜, DefaultListDetailUseCase, ListDetailRepository에서 일관되게 적용되어야 합니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swiftSOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swiftSOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swiftSOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swiftSOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertType.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swiftSOPT-iOS/Projects/Features/StampFeature/Interface/Sources/StampFeatureBuildable.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swift
- SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift
- SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swift
- SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-23T04:48:57.229Z
Learnt from: dlwogus0128
Repo: sopt-makers/SOPT-iOS PR: 699
File: SOPT-iOS/Projects/Features/RootFeature/Sources/ApplicationCoordinator.swift:576-581
Timestamp: 2025-09-23T04:48:57.229Z
Learning: In SOPT-iOS coordinator refactoring: Legacy coordinators require addDependency() calls for proper lifecycle management, but the new coordinator architecture handles dependency management differently and does not require explicit addDependency() calls.
Applied to files:
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift
🧬 Code graph analysis (5)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift (3)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/LegacyMissionDetailCoordinator.swift (1)
showMissionDetail(35-53)SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swift (1)
makeListDetailVC(34-54)SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/LegacyStampBuilder.swift (1)
makeListDetailVC(32-50)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (3)
SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
fetchListDetail(52-61)SOPT-iOS/Projects/Modules/Networks/Sources/Service/StampService.swift (1)
fetchStampListDetail(31-33)SOPT-iOS/Projects/Data/Sources/Transform/ListDetailTransform.swift (2)
toDomain(15-27)toDomain(31-33)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift (2)
cancelButtonColor(66-73)cancelButtonTitleColor(75-82)
SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
fetchListDetail(36-51)
SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift (2)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
fetchListDetail(36-51)SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
fetchListDetail(52-61)
🪛 SwiftLint (0.57.0)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift
[Warning] 106-106: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift
[Warning] 119-119: TODOs should be resolved (- isAppjam 기본값 제거)
(todo)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift
[Warning] 39-39: Discouraged direct initialization of types that can be harmful
(discouraged_direct_init)
[Warning] 42-42: TODOs should be resolved (- 앱잼템프용 api로 변경)
(todo)
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift
[Warning] 451-451: TODOs should be resolved (- 앱잼안하는 활동유저의 경우대한 분기도 추가)
(todo)
🔇 Additional comments (12)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertType.swift (1)
14-14: 새로운 alert 타입 추가 확인
titleDescriptionSingleButton케이스가 적절하게 추가되었습니다. 관련 UI 로직도 AlertVC.swift에서 일관되게 처리되고 있습니다.SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (3)
98-98: LGTM: 단일 버튼 alert 처리 로직 정상 동작
isSingleButtonAlert조건을 사용하여.networkErr와.titleDescriptionSingleButton을 함께 처리하는 로직이 정확합니다.
115-122: 코드 가독성 개선이전 삼항 연산자 대신 switch 문을 사용하여 가독성이 향상되었습니다.
187-191: LGTM: 레이아웃 로직 정상 동작새로운
.titleDescriptionSingleButton타입이.networkErr와 동일한 단일 버튼 레이아웃을 사용하도록 올바르게 구현되었습니다.SOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swift (3)
21-21: LGTM: 프로필 정보 필드 추가
profileInfo옵셔널 필드가 적절하게 추가되었습니다. 옵셔널 타입을 사용하여 기존 코드와의 호환성을 유지하면서 앱잼 관련 프로필 정보를 지원할 수 있습니다.
32-34: 기본값을 통한 하위 호환성 유지
profileInfo파라미터에 기본값nil을 제공하여 기존 코드가 수정 없이 동작할 수 있도록 했습니다.
49-57: LGTM: ProfileInfo 구조체 구현프로필 정보를 담는 구조체가 깔끔하게 구현되었습니다.
name은 필수,imageURL은 옵셔널로 하여 이미지가 없는 경우도 처리할 수 있습니다.SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift (1)
118-120: Deprecation 전략이 적절합니다.기본값을 제공하면서 deprecation 메시지로 개발자에게 명시적인 분기를 유도하는 접근 방식이 좋습니다. 기존 호출 지점(예: Line 236)의 하위 호환성을 유지하면서 점진적으로 마이그레이션할 수 있습니다.
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (1)
274-294: DEBUG 전용 테스트 데이터 주입을 확인하세요.앱잼 기능 테스트를 위한 DEBUG 전용 profileInfo 주입이 적절하게 구현되어 있습니다. 앱잼 전용 API가 연결되면 이 코드를 실제 데이터로 교체해야 합니다.
실제 앱잼 API 연결 후 이 mock 데이터를 제거할 계획이 있는지 확인해주세요.
SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
15-15: LGTM!
isAppjam파라미터가 올바르게 리포지토리 계층으로 전달되고 있습니다. 단순 passthrough 구현이 적절합니다.Also applies to: 52-53
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift (2)
442-453: 사용자 유형 기반 분기 로직을 검증하세요.완료된 미션은 모든 사용자가 볼 수 있고, 미완료 미션은 활성 사용자만 접근할 수 있도록 하는 로직이 적절합니다. 다만 TODO 주석에 언급된 대로 "앱잼에 참여하지 않는 활동 유저"에 대한 추가 분기가 필요합니다.
다음 사항을 확인해주세요:
- 현재
UserType.active가 앱잼 참여 여부와 무관하게 모든 활성 사용자를 포함하는지 확인- 앱잼 비참여 활동 유저를 구분하는 로직 구현 계획 확인
460-469: LGTM!비활성 사용자를 위한 알림 표시 로직이 명확하고 적절하게 구현되어 있습니다. AlertUtils를 활용한 일관된 알림 표시 방식이 좋습니다.
| public func fetchListDetail(isAppjam: Bool?, missionId: Int, username: String?) -> AnyPublisher<ListDetailModel, Error> { | ||
| let username = username ?? UserDefaultKeyList.User.soptampName | ||
| guard let username else { | ||
| return Fail(error: NSError()).eraseToAnyPublisher() | ||
| } | ||
| return stampService.fetchStampListDetail(missionId: missionId, username: username) | ||
| .map { $0.toDomain() } | ||
| .eraseToAnyPublisher() | ||
| if isAppjam == true { | ||
| //TODO: - 앱잼템프용 api로 변경 | ||
| return stampService.fetchStampListDetail(missionId: missionId, username: username) | ||
| .map { $0.toDomain() } | ||
| .eraseToAnyPublisher() | ||
| } else { | ||
| return stampService.fetchStampListDetail(missionId: missionId, username: username) | ||
| .map { $0.toDomain() } | ||
| .eraseToAnyPublisher() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
중복 코드를 제거하고 단일 분기로 통합하세요.
현재 isAppjam == true와 else 분기가 완전히 동일한 로직을 실행하고 있습니다. 앱잼 전용 API가 구현될 때까지는 단일 분기로 통합하여 코드 중복을 제거하는 것이 좋습니다.
🔎 제안하는 리팩토링
public func fetchListDetail(isAppjam: Bool?, missionId: Int, username: String?) -> AnyPublisher<ListDetailModel, Error> {
let username = username ?? UserDefaultKeyList.User.soptampName
guard let username else {
return Fail(error: NSError()).eraseToAnyPublisher()
}
- if isAppjam == true {
- //TODO: - 앱잼템프용 api로 변경
- return stampService.fetchStampListDetail(missionId: missionId, username: username)
- .map { $0.toDomain() }
- .eraseToAnyPublisher()
- } else {
- return stampService.fetchStampListDetail(missionId: missionId, username: username)
- .map { $0.toDomain() }
- .eraseToAnyPublisher()
- }
+ //TODO: - isAppjam == true일 때 앱잼템프용 api로 변경
+ return stampService.fetchStampListDetail(missionId: missionId, username: username)
+ .map { $0.toDomain() }
+ .eraseToAnyPublisher()
}🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 39-39: Discouraged direct initialization of types that can be harmful
(discouraged_direct_init)
[Warning] 42-42: TODOs should be resolved (- 앱잼템프용 api로 변경)
(todo)
🤖 Prompt for AI Agents
In @SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift around
lines 36-50, The fetchListDetail function has identical logic in the isAppjam ==
true and else branches; remove the conditional and return a single publisher:
call stampService.fetchStampListDetail(missionId: missionId, username:
username), then .map { $0.toDomain() } and .eraseToAnyPublisher() inside
fetchListDetail, preserving the existing username defaulting behavior and any
guard checks.
There was a problem hiding this comment.
@yungu0010, 알겠습니다! 앱잼탬프 전용 API가 준비되면 isAppjam == true 분기에서 해당 API로 변경하실 계획이시군요. 향후 API 통합을 위해 미리 구조를 준비해두신 것이 합리적입니다. 👍
추후 앱잼 API가 준비되면 TODO 주석을 참고하여 업데이트하시면 될 것 같습니다!
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift
Outdated
Show resolved
Hide resolved
yungu0010
left a comment
yungu0010
left a comment
There was a problem hiding this comment.
고생하셨습니다! 코멘트 남긴 부분만 확인해주세요~!
| public func fetchListDetail(isAppjam: Bool?, missionId: Int, username: String?) -> AnyPublisher<ListDetailModel, Error> { | ||
| let username = username ?? UserDefaultKeyList.User.soptampName | ||
| guard let username else { | ||
| return Fail(error: NSError()).eraseToAnyPublisher() | ||
| } | ||
| return stampService.fetchStampListDetail(missionId: missionId, username: username) | ||
| .map { $0.toDomain() } | ||
| .eraseToAnyPublisher() | ||
| if isAppjam == true { | ||
| //TODO: - 앱잼템프용 api로 변경 | ||
| return stampService.fetchStampListDetail(missionId: missionId, username: username) | ||
| .map { $0.toDomain() } | ||
| .eraseToAnyPublisher() | ||
| } else { | ||
| return stampService.fetchStampListDetail(missionId: missionId, username: username) | ||
| .map { $0.toDomain() } | ||
| .eraseToAnyPublisher() | ||
| } |
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift
Outdated
Show resolved
Hide resolved
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
98-109: 중복된 boolean 표현식을 computed property로 추출하는 것을 권장합니다.
self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton표현식이 두 번 반복되고 있습니다. 이를 computed property로 추출하면 코드 중복을 제거하고 유지보수성을 향상시킬 수 있습니다.♻️ 제안하는 리팩토링
+ private var isSingleButtonAlert: Bool { + return self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton + } + private func setUI() { self.view.backgroundColor = .clear self.alertView.backgroundColor = alertTheme.backgroundColor - self.cancelButton.backgroundColor = alertTheme.cancelButtonColor(isSingleButtonAlert: self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton) + self.cancelButton.backgroundColor = alertTheme.cancelButtonColor(isSingleButtonAlert: self.isSingleButtonAlert) self.customButton.backgroundColor = alertTheme.customButtonColor self.titleLabel.font = .Main.headline2 self.descriptionLabel.font = .Main.body2 self.cancelButton.titleLabel?.font = .Main.caption3 self.customButton.titleLabel?.font = .Main.caption3 self.titleLabel.textColor = alertTheme.titleColor self.descriptionLabel.textColor = alertTheme.descriptionColor - self.cancelButton.setTitleColor(alertTheme.cancelButtonTitleColor(isSingleButtonAlert: self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton), + self.cancelButton.setTitleColor(alertTheme.cancelButtonTitleColor(isSingleButtonAlert: self.isSingleButtonAlert), for: .normal)이렇게 하면 switch 문 (lines 115-122)도 다음과 같이 단순화할 수 있습니다:
- let cancelTitle: String - switch self.alertType { - case .networkErr, .titleDescriptionSingleButton: - cancelTitle = I18N.Default.ok - default: - cancelTitle = I18N.Default.cancel - } - self.cancelButton.setTitle(cancelTitle, for: .normal) + self.cancelButton.setTitle(self.isSingleButtonAlert ? I18N.Default.ok : I18N.Default.cancel, for: .normal)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift
🧰 Additional context used
🧬 Code graph analysis (1)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/Components/HomeCategoryTagLabel.swift (1)
setTitleColor(45-49)
🔇 Additional comments (1)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
187-192: LGTM!
.titleDescriptionSingleButton과.networkErr를 함께 처리하는 레이아웃 로직이 올바르게 구현되었습니다. 두 alert 타입 모두 single button을 사용하므로 동일한 제약 조건을 공유하는 것이 적절합니다.
hye0njuoo
force-pushed
the
feat/#805-mission-detail-alert
branch
from
eb46f7b to
01351cb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (1)
98-109: 단일 버튼 alert 스타일링 로직 확인새로운 alert 타입과 기존 networkErr을 함께 처리하는 로직이 올바르게 구현되었습니다.
isSingleButtonAlert로 파라미터명이 통일되어 가독성이 개선되었습니다.♻️ 선택적 개선사항: 중복된 조건식 추출
동일한 조건식이 여러 곳에서 반복됩니다(라인 98, 108). computed property로 추출하면 유지보수성이 향상됩니다:
private var isSingleButtonAlert: Bool { return self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton }사용 시:
-self.cancelButton.backgroundColor = alertTheme.cancelButtonColor(isSingleButtonAlert: self.alertType == .networkErr || self.alertType == .titleDescriptionSingleButton) +self.cancelButton.backgroundColor = alertTheme.cancelButtonColor(isSingleButtonAlert: isSingleButtonAlert)SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (1)
20-20:hideBorder()중복 호출 제거Line 20에서
CustomProfileImageView().hideBorder()로 이미 호출되었고, Line 41에서 다시profileImageView.hideBorder()를 호출하고 있습니다. 중복 호출을 제거하세요.♻️ 제안하는 수정
private func setUI() { self.backgroundColor = .clear - profileImageView.hideBorder() - nameLabel.font = .SoptampFont.subtitle3Also applies to: 41-41
SOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swift (1)
49-57: 들여쓰기 일관성 유지 필요
ProfileInfo구조체가 4-space 들여쓰기를 사용하고 있지만,ListDetailModel은 2-space를 사용합니다. 프로젝트 전체 일관성을 위해 동일한 들여쓰기 스타일을 적용해 주세요.♻️ 제안하는 수정
public struct ProfileInfo { - public let name: String - public let imageURL: String? - - public init(name: String, imageURL: String?) { - self.name = name - self.imageURL = imageURL - } + public let name: String + public let imageURL: String? + + public init(name: String, imageURL: String?) { + self.name = name + self.imageURL = imageURL + } }SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
38-40:NSError()직접 초기화 개선 필요
NSError()를 domain과 code 없이 초기화하면 디버깅이 어렵습니다. 의미 있는 에러 정보를 제공하거나 커스텀 에러 타입을 사용하세요.♻️ 제안하는 수정
guard let username else { - return Fail(error: NSError()).eraseToAnyPublisher() + return Fail(error: NSError( + domain: "ListDetailRepository", + code: -1, + userInfo: [NSLocalizedDescriptionKey: "Username is required"] + )).eraseToAnyPublisher() }SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (1)
105-108: Void 함수에 삼항 연산자 사용 지양Void를 반환하는 함수 호출에 삼항 연산자를 사용하면 가독성이 떨어집니다. if/else 문으로 리팩토링하세요.
♻️ 제안하는 수정
.sink { owner, _ in - owner.isOtherUser - ? owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: owner.otherUserName) - : owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: nil) - + if owner.isOtherUser { + owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: owner.otherUserName) + } else { + owner.useCase.fetchListDetail(isAppjam: owner.isAppjam, missionId: owner.missionId, username: nil) + } + if owner.isOtherUser {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swiftSOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swiftSOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swiftSOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swiftSOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertType.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swiftSOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swiftSOPT-iOS/Projects/Features/StampFeature/Interface/Sources/StampFeatureBuildable.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swiftSOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift
🚧 Files skipped from review as they are similar to previous changes (5)
- SOPT-iOS/Projects/Domain/Sources/RepositoryInterface/ListDetailRepositoryInterface.swift
- SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampBuilder.swift
- SOPT-iOS/Projects/Features/StampFeature/Sources/Coordinator/StampCoordinator.swift
- SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/VC/ListDetailVC.swift
- SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift
🧰 Additional context used
🧬 Code graph analysis (4)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (2)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
fetchListDetail(36-51)SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
fetchListDetail(52-61)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (2)
SOPT-iOS/Projects/Modules/DSKit/Sources/Components/CustomProfileImageView.swift (1)
hideBorder(49-53)SOPT-iOS/Projects/Core/Sources/Extension/UIKit+/UIView+.swift (1)
addSubviews(13-15)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (3)
SOPT-iOS/Projects/Domain/Sources/UseCase/ListDetailUseCase.swift (1)
fetchListDetail(52-61)SOPT-iOS/Projects/Modules/Networks/Sources/Service/StampService.swift (1)
fetchStampListDetail(31-33)SOPT-iOS/Projects/Data/Sources/Transform/ListDetailTransform.swift (2)
toDomain(15-27)toDomain(31-33)
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift (2)
SOPT-iOS/Projects/Core/Sources/Literals/UserDefaultKeyLIst.swift (1)
getUserType(87-96)SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Coordinator/Router/LegacyRouter.swift (1)
presentAlertVC(281-298)
🪛 SwiftLint (0.57.0)
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift
[Warning] 106-106: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift
[Warning] 39-39: Discouraged direct initialization of types that can be harmful
(discouraged_direct_init)
[Warning] 42-42: TODOs should be resolved (- 앱잼템프용 api로 변경)
(todo)
SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift
[Warning] 452-452: TODOs should be resolved (- 앱잼안하는 활동유저의 경우대한 분기도 추가)
(todo)
🔇 Additional comments (15)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertType.swift (1)
14-14: 새로운 AlertType 케이스 추가 확인단일 버튼 alert을 위한 새로운 케이스가 명확하게 정의되었습니다. 네이밍이 직관적이고 기존 패턴과 일관성 있습니다.
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVC.swift (2)
115-122: 버튼 타이틀 로직 개선 확인삼항 연산자에서 switch문으로 변경하여 가독성과 확장성이 개선되었습니다. 단일 버튼 alert 케이스들이 명확하게 그룹화되어 있습니다.
187-192: 레이아웃 케이스 그룹화 확인새로운 alert 타입이 기존 단일 버튼 케이스와 올바르게 그룹화되어 동일한 레이아웃 제약조건을 적용받습니다.
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Alert/AlertVCTheme.swift (1)
66-82: 파라미터 네이밍 통일 완료
isNetworkErr에서isSingleButtonAlert로 변경하여 의미가 더 명확해졌습니다. 이제 여러 단일 버튼 alert 타입을 포괄하는 정확한 네이밍이 되었고, AlertVC.swift와 일관성이 확보되었습니다.SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/View/ProfileInfoView.swift (2)
47-49: 화살표 아이콘 방향 확인 필요
icLeftArrow에셋을 사용하고 있지만, 프로필 정보 우측에 표시되는 네비게이션 인디케이터로 보입니다. 아이콘 방향이 의도와 맞는지 확인해 주세요. 일반적으로 우측 화살표(icRightArrow또는icChevronRight)가 사용됩니다.
77-86: LGTM!
configure메서드가 optional URL을 적절히 처리하고 있으며, placeholder 이미지 fallback 로직이 잘 구현되어 있습니다.SOPT-iOS/Projects/Domain/Sources/Model/ListDetailModel.swift (1)
21-22: LGTM!
profileInfo필드가 optional로 추가되어 기존 코드와의 호환성이 잘 유지되며, 기본값nil을 통해 점진적 마이그레이션이 가능합니다.Also applies to: 32-34, 44-44
SOPT-iOS/Projects/Data/Sources/Repository/ListDetailRepository.swift (1)
41-50: 앱잼 API 분기 구조 확인이전 리뷰에서 논의된 대로, 앱잼 전용 API 연동을 위해 분기 구조를 미리 준비한 것으로 이해됩니다. TODO 주석이 있으니 앱잼 API 준비 시 업데이트해 주세요.
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swift (1)
131-133: LGTM!비활성 사용자 알림을 위한 문자열 리터럴이 적절하게 추가되었습니다. 네이밍이 명확하고 기존 컨벤션을 잘 따르고 있습니다.
SOPT-iOS/Projects/Features/StampFeature/Sources/ListDetailScene/ViewModel/ListDetailViewModel.swift (2)
274-294: DEBUG 모드 목 데이터 제거 계획 필요테스트용
ProfileInfo목 데이터가 DEBUG 블록 내에 하드코딩되어 있습니다. 앱잼 API 연동 완료 후 이 코드를 제거해야 합니다. TODO 주석을 추가하여 추적하는 것을 권장합니다.♻️ 제안하는 수정
#if DEBUG + // TODO: - 앱잼템프 API 연동 후 목 데이터 제거 if owner.isAppjam == true { let testProfileInfo = ProfileInfo( name: "성앱잼",
35-36: LGTM!
isAppjam파라미터가 ViewModel 전체에 올바르게 전파되고 있으며, 초기화 및 UseCase 호출에서 적절히 활용되고 있습니다.Also applies to: 77-78, 87-88
SOPT-iOS/Projects/Features/StampFeature/Interface/Sources/StampFeatureBuildable.swift (1)
22-24:isAppjam타입 일관성 확인 필요인터페이스에서는
isAppjam: Bool로 정의되어 있지만,ListDetailViewModel에서는isAppjam: Bool?로 정의되어 있습니다. 타입 불일치가 의도적인지 확인하고, 일관성을 위해 동일한 타입을 사용하는 것을 권장합니다.SOPT-iOS/Projects/Features/StampFeature/Sources/MissionListScene/VC/MissionListVC.swift (3)
24-25:isAppJam하드코딩 값 확인 필요
isAppJam이true로 하드코딩되어 있습니다. TODO 주석에 "화면 전환 시에 수정"이라고 되어 있으니, 실제 앱잼 여부를 판단하는 로직으로 대체될 예정인지 확인해 주세요.
441-454: 사용자 타입 분기 로직 구현 확인비완료 미션에 대한 사용자 타입별 분기 로직이 잘 구현되었습니다. TODO 주석에 명시된 "앱잼 안하는 활동유저" 케이스는 추후 구현 시
active케이스 내에서 추가 분기가 필요합니다.
461-470: LGTM!
showInactiveUserAlert()헬퍼 메서드가AlertUtils를 적절히 활용하여 구현되었으며, I18N 문자열 리터럴을 올바르게 사용하고 있습니다.
🌴 PR 요약
🌱 작업한 브랜치
🌱 PR Point
📌 참고 사항
showMissionDetail 함수 변경사항
기존
showMissionDetail함수는 미션 리스트에서 디테일 화면으로 전환할 때 사용되던 함수인데요.앱잼탬프 기능 추가로 인해 다음과 같은 분기 처리가 필요해졌습니다:
변경 내용
isAppjam파라미터를 추가하여 앱잼 여부를 구분할 수 있도록 수정했습니다.사용 방법
일반 솝탬프: 기존과 동일하게 사용 (default 값
false)앱잼탬프:
isAppjam: true로 설정isAppjam여부에서 따라서 레포지토리에서 분기해 적절한 api를 호출해줍니다.
주의사항
앱잼탬프 디테일로 진입할 때는 반드시
isAppjam: true를 명시해주세요. 그렇지 않으면 일반 솥탬프 API가 호출되어 프로필 정보가 표시되지 않습니다. 그래서 해당 함수에 임의로 deprecated추가해뒀습니다. 아마 노란색에러로 설명해줄거에요!왜? 이렇게 했냐면요
처음에는 Usertype으로 분기를 할려고 했는데, 일반 솝탬프도 존재하고, 앱잼탬프도 동시에 존재하는 구조다보니 유저타입으로 분기했을때, 해당 셀이 앱잼탬프인지 일반 솝탬프인지 구별할수 있는 방법이 없어 간단하게 호출부에서 파라미터 전달하는 방식으로 구현했습니다.
혹시 더 좋은 방법이 있다면 알려주세요~!!!
ListDetailVC
listdetailVC에서는 model에 있는 값을 바탕으로 프로필뷰를 표시할지 말지 정합니다. 앱잼탬프의 경우 기존 솝탬프와 다른게 앱잼팀, profileimage, profilename등의 필드가 추가로 전달되는데, 그값의 유무에 따라 뷰가 보이도록 했습니다.
alert조건
missonListVC에서 셀을 클릭할때 해당 유저 타입으로 alert를 호출할지 말지 결정합니다. usertype으로 분기가 필요한데, 아직 앱잼하는 유저에대한 분기가 정해지지 않아서 이부분은 TODO로 남겨뒀습니다.
남은 작업
📸 스크린샷
📮 관련 이슈