Conversation
- ReportEndpoint, ReportDTO, ReportRepository, ReportRepositoryProtocol - ReportDetail 불러오기 기능 구현
Walkthrough보고서 상세 조회 기능을 추가합니다. 새로운 DTO, 엔드포인트, 리포지토리 메서드를 정의하고, 프레젠테이션 레이어를 업데이트하여 reportId를 기반으로 상세 정보를 조회하고 사진과 진행 상태를 표시합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant RVC as ReportDetailViewController
participant VM as ReportDetailViewModel
participant Repo as ReportRepository
participant NS as NetworkService
participant API as API Server
User->>RVC: init(viewModel, reportId)
RVC->>RVC: viewDidLoad()
RVC->>VM: action(input: .fetchReportDetail(reportId))
VM->>Repo: fetchReportDetail(reportId)
Repo->>NS: request(ReportEndpoint.fetchReportDetail)
NS->>API: GET /api/v2/reports/{reportId}
API-->>NS: ReportDTO JSON
NS-->>Repo: ReportDTO
Repo->>Repo: dto.toReportEntity()
Repo-->>VM: ReportEntity
VM->>VM: map to ReportDetail
VM-->>RVC: publish reportDetail
RVC->>RVC: fillReportContent(reportDetail)
RVC->>RVC: configure ReportProgressView
RVC->>RVC: load & display photoUrls with Kingfisher
RVC-->>User: Display report details + photos + status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 2
🧹 Nitpick comments (6)
Projects/Domain/Sources/Protocol/Repository/ReportRepositoryProtocol.swift (1)
11-14:fetchReportDetail반환 타입(ReportEntity?) 의미를 주석에 명시하는 게 좋겠습니다메서드 추가 방향은 좋은데, 현재 주석에는 반환값이 optional 인 이유(예: 해당 ID 제보 미존재, 404 등)가 드러나지 않습니다.
nil이 의미하는 상태를 주석에 간단히 적어 두면 사용처에서 오해 없이 사용할 수 있을 것 같습니다.Projects/Presentation/Sources/Home/View/HomeViewController.swift (1)
667-676:reportId를 1로 하드코딩한 부분은 후속 작업 시 교체 필요현재는 항상
reportId: 1로ReportDetailViewController를 띄워서, 실제로 어떤 제보를 선택하든 동일한 상세만 조회하게 됩니다. PR 설명대로 API 연동 확인용 임시 구현이라면 괜찮지만,
- 나중에 실제 선택/생성된 제보 ID 를 넘기는 쪽으로 수정해야 하고
- 지금 있는 TODO 주석에 “임시로 1번 제보 상세로 이동 중” 같은 설명을 더 남겨 두면 다른 사람이 코드를 볼 때 혼동이 줄어들 것 같습니다.
Projects/DataSource/Sources/DTO/ReportDTO.swift (1)
1-38:ReportDTO매핑 기본값/이미지 필드 처리 개선 제안
ReportType(rawValue: reportCategory) ?? .transportation,ReportProgress(rawValue: reportStatus) ?? .received처럼 서버 값이 예상과 다를 때 조용히 기본값으로 떨어지는 구조입니다.
- 미정의 코드가 들어오면 실패로 처리해야 하는지, 아니면 현재처럼 “안전 기본값” 으로 두는 게 맞는지 도메인/백엔드와 한 번만 정리해 두는 게 좋겠습니다. (예: 알 수 없는 status 는 에러로 throw 하거나, 별도
.unknown케이스를 두는 등)
reportImageUrl(단일 URL) 은 선언되어 있지만, 실제 매핑에서는reportImageUrls만 사용하고 있어 단일 필드 값이 있다면 유실될 수 있습니다.
서버가 단일/배열 둘 중 하나만 내려줄 수 있다면, 아래처럼 fallback 으로 합치는 것도 고려해 볼 만합니다.struct ReportDTO: Decodable { let reportId: Int? let reportDate: String? let reportTitle: String let reportContent: String? let reportLocation: String let reportStatus: String let reportCategory: String - let reportImageUrl: String? - let reportImageUrls: [String]? + let reportImageUrl: String? + let reportImageUrls: [String]? @@ func toReportEntity() throws -> ReportEntity { guard let reportId else { throw NetworkError.decodingError } - return ReportEntity( + let photoUrls = reportImageUrls ?? (reportImageUrl.map { [$0] } ?? []) + + return ReportEntity( id: reportId, title: reportTitle, date: reportDate, type: ReportType(rawValue: reportCategory) ?? .transportation, progress: ReportProgress(rawValue: reportStatus) ?? .received, content: reportContent, location: LocationEntity( longitude: longitude, latitude: latitude, address: reportLocation), - photoUrls: reportImageUrls ?? []) + photoUrls: photoUrls) } }이 부분은 서버 응답 스펙에 따라 달라질 수 있으니, 실제 응답 예시와 함께 한 번만 검토해 보시면 좋겠습니다.
Projects/Domain/Sources/Entity/Enum/ReportProgress.swift (1)
8-12:ReportProgressrawValue 와 서버 상태 코드 계약 재확인 권장서버에서 내려주는 문자열과 직접 매핑하도록
StringrawValue 를 둔 건 DTO 변환에 잘 맞습니다. 다만:
- rawValue 가 모두 대문자(
ENTIRE,PENDING,IN_PROGRESS,COMPLETED) 로 고정되어 있어서- 서버에서 소문자나 다른 문자열을 보내면
ReportDTO쪽에서 기본값.received로 떨어질 수 있습니다.백엔드와 상태 코드 문자열이 1:1 로 정확히 맞는지, 혹은 알 수 없는 값에 대비해
.unknown같은 케이스를 두거나 에러를 던질지 정책을 한 번만 정리해 두면 좋겠습니다.Projects/DataSource/Sources/Endpoint/ReportEndpoint.swift (1)
12-52:baseURL/path조합 방식과 헤더를 기존 Endpoint 들과 맞추는지 확인 필요
baseURL+path구성현재 구현:
var baseURL: String { switch self { case .fetchReportDetail: return AppProperties.baseURL + "/api/v2/reports" } } var path: String { switch self { case .fetchReportDetail(let reportId): "\(baseURL)/\(reportId)" } }
NetworkService가 일반적인 패턴대로baseURL + path로 최종 URL 을 만든다면, 실제 요청 URL 이.../api/v2/reportshttps://.../api/v2/reports/1처럼 중복될 수 있습니다.기존 Endpoint 들(루틴/감정 등)에서 어떤 패턴을 쓰는지 확인하시고, 아래처럼 정리하는 것이 더 안전할 수 있습니다:
- var baseURL: String { - switch self { - case .fetchReportDetail: - return AppProperties.baseURL + "/api/v2/reports" - } - } + var baseURL: String { + AppProperties.baseURL + } @@ - var path: String { - switch self { - case .fetchReportDetail(let reportId): - "\(baseURL)/\(reportId)" - } - } + var path: String { + switch self { + case .fetchReportDetail(let reportId): + return "/api/v2/reports/\(reportId)" + } + }
- 헤더
지금은
accept: "*/*"로 두셨는데, 서버가 JSON 응답만 내려준다면application/json으로 좁히는 것도 고려해 볼 수 있습니다. (크게 문제는 아니지만, 명시해 두면 디버깅에 도움이 됩니다.)위 두 부분은 이 프로젝트의 기존 Endpoint 구현과 비교해서 일관성만 맞춰 주시면 될 것 같습니다.
Projects/DataSource/Sources/Repository/ReportRepository.swift (1)
11-20: Report 상세 조회 로직은 자연스럽지만report(reportEntity:)미구현 상태를 명시해 두는 편이 좋습니다Line 17-20의
fetchReportDetail(reportId:)는ReportEndpoint와ReportDTO.toReportEntity()를 잘 활용하고 있어서 흐름은 괜찮습니다.
다만 Line 13-15의report(reportEntity:)가 완전히 비어 있어, 나중에 UseCase에서 호출되더라도 조용히 아무 일도 일어나지 않는 상태라 디버깅 시 혼동될 수 있습니다.간단히 TODO라도 남겨 두면 의도가 더 분명해집니다.
- func report(reportEntity: ReportEntity) async { - - } + func report(reportEntity: ReportEntity) async { + // TODO: 제보 등록 API 연동 + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Projects/DataSource/Sources/DTO/ReportDTO.swift(1 hunks)Projects/DataSource/Sources/Endpoint/ReportEndpoint.swift(1 hunks)Projects/DataSource/Sources/Repository/ReportRepository.swift(1 hunks)Projects/Domain/Sources/Entity/Enum/ReportProgress.swift(1 hunks)Projects/Domain/Sources/Protocol/Repository/ReportRepositoryProtocol.swift(1 hunks)Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift(1 hunks)Projects/Presentation/Sources/Home/View/HomeViewController.swift(1 hunks)Projects/Presentation/Sources/Report/Model/ReportDetail.swift(1 hunks)Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift(4 hunks)Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift(2 hunks)Tuist.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: choijungp
Repo: YAPP-Github/Bitnagil-iOS PR: 30
File: Projects/DataSource/Sources/NetworkService/Plugin/RefreshTokenPlugin.swift:42-43
Timestamp: 2025-07-30T03:56:18.617Z
Learning: choijungp는 현재 테스트 단계에서 RefreshTokenPlugin의 토큰 갱신 로직 디버깅을 위해 액세스 토큰과 리프레시 토큰의 실제 값을 로그에 기록하는 것을 선호함.
📚 Learning: 2025-11-19T13:49:08.326Z
Learnt from: choijungp
Repo: YAPP-Github/Bitnagil-iOS PR: 68
File: Projects/Presentation/Sources/Report/View/ReportCompleteViewController.swift:108-172
Timestamp: 2025-11-19T13:49:08.326Z
Learning: ReportCompleteViewController에서 backgroudView가 fomoImageView와 의도적으로 겹치도록 설계됨. 이를 위해 backgroudView의 top 제약을 fomoImageView.snp.top (또는 fomoImageView)을 기준으로 설정하여 오프셋을 적용함.
Applied to files:
Projects/Presentation/Sources/Home/View/HomeViewController.swiftProjects/Presentation/Sources/Report/View/ReportDetailViewController.swift
📚 Learning: 2025-08-17T13:30:29.342Z
Learnt from: taipaise
Repo: YAPP-Github/Bitnagil-iOS PR: 51
File: Projects/Presentation/Sources/Common/Component/RoutineCardView.swift:76-80
Timestamp: 2025-08-17T13:30:29.342Z
Learning: In SnapKit, even height constraints (intrinsic constraints) should be applied after adding the view to its superview hierarchy. While simple height constraints might work before adding to superview, it's not guaranteed and goes against best practices. The recommended approach is to call addArrangedSubview first, then apply constraints.
Applied to files:
Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift
🧬 Code graph analysis (5)
Projects/Domain/Sources/Protocol/Repository/ReportRepositoryProtocol.swift (1)
Projects/DataSource/Sources/Repository/ReportRepository.swift (1)
fetchReportDetail(17-21)
Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift (1)
Projects/Shared/Sources/DIContainer/DIContainer.swift (1)
resolve(18-25)
Projects/DataSource/Sources/Endpoint/ReportEndpoint.swift (1)
Projects/DataSource/Sources/Repository/ReportRepository.swift (1)
fetchReportDetail(17-21)
Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift (2)
Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift (2)
action(31-36)fetchReportDetail(38-58)Projects/Presentation/Sources/Common/DesignSystem/Font/BitnagilFont.swift (1)
attributedString(50-67)
Projects/DataSource/Sources/Repository/ReportRepository.swift (3)
Projects/Domain/Sources/UseCase/Report/ReportUseCase.swift (1)
report(23-25)Projects/DataSource/Sources/NetworkService/NetworkService.swift (1)
request(25-48)Projects/DataSource/Sources/DTO/ReportDTO.swift (1)
toReportEntity(23-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
Tuist.swift (1)
5-5: Xcode 호환 버전을.all로 확장한 영향 확인특정 Xcode 버전에 맞춰 빌드/테스트를 돌리던 CI 설정이 있다면,
.all로 바뀐 것이 팀에서 정한 최소 지원 버전/배포 전략과 맞는지 한 번만 확인해 주세요. 설정 자체는 문제 없어 보입니다.Projects/Presentation/Sources/Report/Model/ReportDetail.swift (1)
13-17: 상태/사진 필드 추가 구조 적절도메인
ReportProgress와 사진 URL 배열을 non-optional 로 두어 View 단 바인딩이 단순해지고, DTO → 도메인 → 프레젠테이션 모델 흐름과 타입도 잘 맞습니다. 현재 형태 그대로 사용해도 무리 없어 보입니다.Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift (1)
136-141:ReportDetailViewModel의 DI 등록 구조 적절기존 ViewModel 들과 동일한 패턴으로
ReportRepositoryProtocol을 resolve 해서 주입하는 구조라 일관성과 가독성이 좋습니다. DataSource 쪽에서ReportRepositoryProtocol이 제대로 등록만 되어 있다면 런타임 이슈는 없을 것 같습니다.Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift (2)
29-47: Report 상세 섹션 구성과 타이틀 문구 변경이 자연스럽습니다
ReportDetailContentenum과makeReportContentView에서 각 섹션을 공통 컴포넌트로 만드는 방식은 재사용성이 좋아 보입니다. Line 44의 타이틀을"내 위치"→"신고 위치"로 바꾼 것도 상세 화면 맥락에 더 잘 맞는 네이밍이라 괜찮습니다.
별도 이슈는 없어 보입니다.Also applies to: 147-171
51-75:ReportProgressView사용과reportId기반 DI 구조가 잘 정리되어 있습니다
- Line 51에서
reportStatusView를ReportProgressView로 구체 타입으로 두고, Line 202에서configure(with:)로 설정하는 구조가 명확합니다.- Line 60-65의
init(viewModel:reportId:)로reportId를 함께 주입하고, Line 73에서viewDidLoad시점에 한 번만.fetchReportDetail(reportId:)를 트리거하는 흐름도 단순하고 이해하기 쉽습니다.현 상태로 충분히 괜찮습니다.
Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift (1)
12-29: ReportDetailViewModel에 Repository 주입 및 Input 설계가 명확합니다
- Line 13에서
Input.fetchReportDetail(reportId:)로reportId를 함께 넘기는 구조가 명시적이고 좋습니다.- Line 22-26에서
ReportRepositoryProtocol을 생성자 주입하고,Output을CurrentValueSubject기반으로 감싸AnyPublisher로 노출하는 패턴도 기존 아키텍처와 잘 맞아 보입니다.이 부분은 그대로 유지해도 될 것 같습니다.
| private func fillReportContent(reportDetail: ReportDetail?) { | ||
| guard let reportDetail else { return } | ||
|
|
||
| let photoView = UIView() | ||
| photoView.backgroundColor = BitnagilColor.gray30 | ||
| photoView.layer.masksToBounds = true | ||
| photoView.layer.cornerRadius = 9.25 | ||
| photoView.snp.makeConstraints { make in | ||
| make.size.equalTo(Layout.photoSize) | ||
| } | ||
| photoStackView.addArrangedSubview(photoView) | ||
|
|
||
| reportStatusView.configure(with: reportDetail.status) | ||
| dateLabel.text = reportDetail.date | ||
| titleContentLabel.text = reportDetail.title | ||
| categoryLabel.text = reportDetail.category.name | ||
| detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium).attributedString(text: reportDetail.description) | ||
| detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium) | ||
| .attributedString(text: reportDetail.description) | ||
| locationLabel.text = reportDetail.location | ||
|
|
||
| reportDetail.photoUrls.forEach { photoUrl in | ||
| let photoView = UIImageView() | ||
|
|
||
| if let url = URL(string: photoUrl) { | ||
| photoView.kf.setImage(with: url) | ||
| } | ||
| photoView.layer.masksToBounds = true | ||
| photoView.layer.cornerRadius = 9.25 | ||
| photoView.snp.makeConstraints { make in | ||
| make.size.equalTo(Layout.photoSize) | ||
| } | ||
| photoStackView.addArrangedSubview(photoView) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
fillReportContent에서 사진 뷰 중복 추가와 SnapKit 적용 순서 개선이 필요합니다
현재 구현에는 두 가지 우려가 있습니다.
-
중복 이미지 뷰 누적 가능성 (Line 210-222)
reportDetailPublisher가 여러 번 값을 방출하면(재시도, 재진입 등)fillReportContent가 반복 호출되는데, 기존photoStackView의arrangedSubviews를 정리하지 않고 새UIImageView만 계속 추가하고 있습니다. 이 경우 같은 사진이 여러 번 쌓이거나 메모리 사용이 불필요하게 늘어날 수 있습니다. -
SnapKit 제약을 superview에 추가하기 전에 설정 (Line 218-221)
photoView.snp.makeConstraints를 호출한 뒤에photoStackView.addArrangedSubview(photoView)를 호출하는 순서인데, 팀 내 SnapKit 가이드/learning에 따르면 뷰를 superview(혹은 stackView)에 먼저 추가한 뒤 제약을 잡는 패턴을 권장하고 있습니다. StackView의arrangedSubviews도 마찬가지로 추가 후 제약을 잡는 편이 안정적입니다. (Based on learnings)
아래처럼 기존 뷰를 먼저 비우고, addArrangedSubview 이후에 제약을 설정하는 쪽을 추천드립니다.
private func fillReportContent(reportDetail: ReportDetail?) {
guard let reportDetail else { return }
@@
detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium)
.attributedString(text: reportDetail.description)
locationLabel.text = reportDetail.location
-
- reportDetail.photoUrls.forEach { photoUrl in
- let photoView = UIImageView()
-
- if let url = URL(string: photoUrl) {
- photoView.kf.setImage(with: url)
- }
- photoView.layer.masksToBounds = true
- photoView.layer.cornerRadius = 9.25
- photoView.snp.makeConstraints { make in
- make.size.equalTo(Layout.photoSize)
- }
- photoStackView.addArrangedSubview(photoView)
- }
+ // 기존 이미지 정리
+ photoStackView.arrangedSubviews.forEach { $0.removeFromSuperview() }
+
+ reportDetail.photoUrls.forEach { photoUrl in
+ let photoView = UIImageView()
+ photoView.layer.masksToBounds = true
+ photoView.layer.cornerRadius = 9.25
+ photoView.contentMode = .scaleAspectFill
+
+ if let url = URL(string: photoUrl) {
+ photoView.kf.setImage(with: url)
+ }
+
+ photoStackView.addArrangedSubview(photoView)
+ photoView.snp.makeConstraints { make in
+ make.size.equalTo(Layout.photoSize)
+ }
+ }이제 파일의 실제 코드를 검증하겠습니다.
실제 코드를 확인하기 위해 코드베이스를 검색하겠습니다.
🏁 Script executed:
#!/bin/bash
# ReportDetailViewController 파일 내용 확인
find . -name "ReportDetailViewController.swift" -type f -exec cat {} \;Length of output: 8204
이제 UIStackView와 SnapKit의 제약 설정 순서에 대해 기술적으로 검증하겠습니다.
이제 중복 누적 문제를 더 자세히 검증하겠습니다.
이미지 뷰 중복 추가와 SnapKit 제약 순서 개선이 필요합니다
검증 결과, 현재 구현의 두 가지 문제가 확인되었습니다:
-
중복 이미지 뷰 누적 가능성
reportDetailPublisher가 여러 번 값을 방출할 수 있는 Combine 패턴(재시도, 에러 처리 등)을 사용하고 있습니다. 매번 publish할 때마다 fillReportContent가 호출되는데, 기존photoStackView의arrangedSubviews를 초기화하지 않고 새UIImageView만 계속 추가하고 있습니다. 같은 사진이 여러 번 쌓이거나 메모리가 낭비될 수 있습니다. -
SnapKit 제약을 arrangedSubview 추가 전에 설정
현재는photoView.snp.makeConstraints를 호출한 뒤에photoStackView.addArrangedSubview(photoView)를 호출하고 있습니다. UIStackView가 arrangedSubviews의 Auto Layout 제약을 자동으로 관리하기 때문에, 팀 내 learnings에 따르면 뷰를 먼저 추가한 후 제약을 잡는 패턴이 더 안정적입니다.
아래와 같이 기존 뷰를 먼저 비우고, addArrangedSubview 이후에 제약을 설정하는 것을 추천합니다:
private func fillReportContent(reportDetail: ReportDetail?) {
guard let reportDetail else { return }
@@
detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium)
.attributedString(text: reportDetail.description)
locationLabel.text = reportDetail.location
-
- reportDetail.photoUrls.forEach { photoUrl in
- let photoView = UIImageView()
-
- if let url = URL(string: photoUrl) {
- photoView.kf.setImage(with: url)
- }
- photoView.layer.masksToBounds = true
- photoView.layer.cornerRadius = 9.25
- photoView.snp.makeConstraints { make in
- make.size.equalTo(Layout.photoSize)
- }
- photoStackView.addArrangedSubview(photoView)
- }
+ // 기존 이미지 정리
+ photoStackView.arrangedSubviews.forEach { $0.removeFromSuperview() }
+
+ reportDetail.photoUrls.forEach { photoUrl in
+ let photoView = UIImageView()
+ photoView.layer.masksToBounds = true
+ photoView.layer.cornerRadius = 9.25
+ photoView.contentMode = .scaleAspectFill
+
+ if let url = URL(string: photoUrl) {
+ photoView.kf.setImage(with: url)
+ }
+
+ photoStackView.addArrangedSubview(photoView)
+ photoView.snp.makeConstraints { make in
+ make.size.equalTo(Layout.photoSize)
+ }
+ }🏁 Script executed:
#!/bin/bash
# ReportDetailViewController의 viewModel 동작 확인
# bind() 메서드와 publisher 호출 패턴 확인
grep -n "reportDetailPublisher" . -r --include="*.swift" -B2 -A2Length of output: 1648
🌐 Web query:
UIStackView arrangedSubviews SnapKit constraints order addArrangedSubview
💡 Result:
Short answer
- Add the view to the stack (addArrangedSubview/insertArrangedSubview) before you create constraints that reference the stack or other arranged subviews. The arranged subview must be in the view hierarchy to make meaningful constraints to its superview/peers. [1][2][3]
- Prefer setting constraints for the arranged subview’s internal content (its own subviews) — let UIStackView control the arranged subview’s position/size. Only add extra constraints that affect intrinsic sizing (width/height) or alignment when needed. [1][4]
- If you create constraints that relate two arranged subviews to each other or to the stack, ensure both are added to the stack first; call layoutIfNeeded() (or rely on Auto Layout updates) if you need immediate frames. [1][3]
Why
- UIStackView manages size/position of its arrangedSubviews; you should not fight it by constraining an arrangedSubview’s frame externally unless you’re changing intrinsic size (height/width) or alignment. [1]
- SnapKit will disable translatesAutoresizingMaskIntoConstraints when you install constraints, but constraints that reference a superview or sibling assume the views are in the same hierarchy — so add them to the stack before making such constraints. [2][3]
Minimal example (recommended pattern)
- Add the view to the stack, then constrain its internal contents:
let stack = UIStackView()
stack.axis = .vertical
view.addSubview(stack)
stack.snp.makeConstraints { make in
make.edges.equalToSuperview().inset(16)
}
let card = UIView()
stack.addArrangedSubview(card)
// constrain card's internal content (preferred)
let label = UILabel()
card.addSubview(label)
label.snp.makeConstraints { make in
make.edges.equalToSuperview().inset(8)
}
// if you must constrain card's size, do it after adding:
card.snp.makeConstraints { make in
make.height.equalTo(80) // acceptable: sets intrinsic size the stack will honor
}
Notes / pitfalls
- Don’t add subviews with addSubview instead of addArrangedSubview — they won’t be arranged by the stack. [4]
- Avoid constraining an arrangedSubview’s top/leading to the stack’s anchors to “position” it; change stack alignment/distribution or the subview’s intrinsic size instead.
- If you create constraints between two arranged subviews, add both to the stack first, then create constraints, and call layoutIfNeeded() if you need immediate layout results. [1][3]
Sources
- Apple Auto Layout Guide — Stack Views. [1]
- SnapKit docs. [2]
- Practical examples / community Q&A on using constraints with UIStackView and arrangedSubviews. [3][4]
이미지 뷰 중복 추가와 SnapKit 제약 순서 개선이 필요합니다
검증 결과, 현재 구현의 두 가지 문제가 모두 확인되었습니다:
-
중복 이미지 뷰 누적 가능성
ReportDetailViewModel에서reportDetailSubject를 사용한 Combine 패턴으로 publisher가 여러 번 값을 방출할 수 있습니다(재시도, 에러 처리 등). 매번 publish할 때마다fillReportContent가 호출되는데, 기존photoStackView의arrangedSubviews를 초기화하지 않고 새UIImageView만 계속 추가합니다. 같은 사진이 여러 번 쌓이거나 메모리가 불필요하게 낭비될 수 있습니다. -
SnapKit 제약을 arrangedSubview 추가 전에 설정
현재는photoView.snp.makeConstraints를 호출한 뒤에photoStackView.addArrangedSubview(photoView)를 호출하고 있습니다. Apple Auto Layout Guide와 SnapKit 문서에 따르면, arrangedSubview는 먼저 스택에 추가한 후 크기/높이 제약(intrinsic sizing)을 설정하는 것이 권장됩니다. UIStackView가 arrangedSubview의 위치/크기를 자동으로 관리하기 때문입니다. 팀 내 learnings(PR 46, 68)에서도 뷰를 먼저 추가한 후 제약을 잡는 패턴을 권장하고 있습니다.
아래와 같이 기존 뷰를 먼저 비우고, addArrangedSubview 이후에 제약을 설정하는 것을 추천합니다:
private func fillReportContent(reportDetail: ReportDetail?) {
guard let reportDetail else { return }
@@
detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium)
.attributedString(text: reportDetail.description)
locationLabel.text = reportDetail.location
-
- reportDetail.photoUrls.forEach { photoUrl in
- let photoView = UIImageView()
-
- if let url = URL(string: photoUrl) {
- photoView.kf.setImage(with: url)
- }
- photoView.layer.masksToBounds = true
- photoView.layer.cornerRadius = 9.25
- photoView.snp.makeConstraints { make in
- make.size.equalTo(Layout.photoSize)
- }
- photoStackView.addArrangedSubview(photoView)
- }
+ // 기존 이미지 정리
+ photoStackView.arrangedSubviews.forEach { $0.removeFromSuperview() }
+
+ reportDetail.photoUrls.forEach { photoUrl in
+ let photoView = UIImageView()
+ photoView.layer.masksToBounds = true
+ photoView.layer.cornerRadius = 9.25
+ photoView.contentMode = .scaleAspectFill
+
+ if let url = URL(string: photoUrl) {
+ photoView.kf.setImage(with: url)
+ }
+
+ photoStackView.addArrangedSubview(photoView)
+ photoView.snp.makeConstraints { make in
+ make.size.equalTo(Layout.photoSize)
+ }
+ }📝 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 fillReportContent(reportDetail: ReportDetail?) { | |
| guard let reportDetail else { return } | |
| let photoView = UIView() | |
| photoView.backgroundColor = BitnagilColor.gray30 | |
| photoView.layer.masksToBounds = true | |
| photoView.layer.cornerRadius = 9.25 | |
| photoView.snp.makeConstraints { make in | |
| make.size.equalTo(Layout.photoSize) | |
| } | |
| photoStackView.addArrangedSubview(photoView) | |
| reportStatusView.configure(with: reportDetail.status) | |
| dateLabel.text = reportDetail.date | |
| titleContentLabel.text = reportDetail.title | |
| categoryLabel.text = reportDetail.category.name | |
| detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium).attributedString(text: reportDetail.description) | |
| detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium) | |
| .attributedString(text: reportDetail.description) | |
| locationLabel.text = reportDetail.location | |
| reportDetail.photoUrls.forEach { photoUrl in | |
| let photoView = UIImageView() | |
| if let url = URL(string: photoUrl) { | |
| photoView.kf.setImage(with: url) | |
| } | |
| photoView.layer.masksToBounds = true | |
| photoView.layer.cornerRadius = 9.25 | |
| photoView.snp.makeConstraints { make in | |
| make.size.equalTo(Layout.photoSize) | |
| } | |
| photoStackView.addArrangedSubview(photoView) | |
| } | |
| } | |
| private func fillReportContent(reportDetail: ReportDetail?) { | |
| guard let reportDetail else { return } | |
| reportStatusView.configure(with: reportDetail.status) | |
| dateLabel.text = reportDetail.date | |
| titleContentLabel.text = reportDetail.title | |
| categoryLabel.text = reportDetail.category.name | |
| detailDescriptionLabel.attributedText = BitnagilFont(style: .body2, weight: .medium) | |
| .attributedString(text: reportDetail.description) | |
| locationLabel.text = reportDetail.location | |
| // 기존 이미지 정리 | |
| photoStackView.arrangedSubviews.forEach { $0.removeFromSuperview() } | |
| reportDetail.photoUrls.forEach { photoUrl in | |
| let photoView = UIImageView() | |
| photoView.layer.masksToBounds = true | |
| photoView.layer.cornerRadius = 9.25 | |
| photoView.contentMode = .scaleAspectFill | |
| if let url = URL(string: photoUrl) { | |
| photoView.kf.setImage(with: url) | |
| } | |
| photoStackView.addArrangedSubview(photoView) | |
| photoView.snp.makeConstraints { make in | |
| make.size.equalTo(Layout.photoSize) | |
| } | |
| } | |
| } |
| private func fetchReportDetail(reportId: Int) { | ||
| Task { | ||
| do { | ||
| if let reportEntity = try await reportRepository.fetchReportDetail(reportId: reportId) { | ||
| let reportDetail = ReportDetail( | ||
| date: reportEntity.date ?? "", | ||
| title: reportEntity.title, | ||
| status: reportEntity.progress, | ||
| category: reportEntity.type, | ||
| description: reportEntity.content ?? "", | ||
| location: reportEntity.location.address ?? "", | ||
| photoUrls: reportEntity.photoUrls) | ||
| reportDetailSubject.send(reportDetail) | ||
| } | ||
| reportDetailSubject.send(nil) | ||
| } catch { | ||
| reportDetailSubject.send(nil) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
fetchReportDetail(reportId:)가 항상 nil을 발행하는 버그와 Task 캡처 패턴을 함께 정리하는 것이 좋겠습니다
현재 구현에는 두 가지 문제가 있습니다.
-
성공 시에도 곧바로
nil을 발행하는 로직 (Line 41-55)if let reportEntity = try await reportRepository.fetchReportDetail(reportId: reportId) { // ... reportDetail 생성 reportDetailSubject.send(reportDetail) } reportDetailSubject.send(nil)
위 구조에서는 제보 조회에 성공해도
reportDetailSubject가 곧바로nil을 다시 발행합니다.
ReportDetailViewController.fillReportContent가guard let reportDetail else { return }로 시작하므로, 화면에서 데이터가 잠깐 세팅된 뒤 즉시 지워지는 효과가 나게 됩니다. -
Task 클로저에서의 self 캡처 (Line 38-40, 41-55)
Task { ... }내부에서reportRepository,reportDetailSubject를 사용하고 있으므로, 명시적으로self를 캡처하는 것이 안전합니다.- 화면이 내려간 뒤에도 Task가 살아 있을 수 있으므로
[weak self]+guard let self패턴을 사용하는 편이 메모리 및 크래시 측면에서 더 안전합니다.
아래처럼 성공/실패/데이터없음 케이스를 명확히 나누고, [weak self]를 적용하는 형태를 권장드립니다.
- private func fetchReportDetail(reportId: Int) {
- Task {
- do {
- if let reportEntity = try await reportRepository.fetchReportDetail(reportId: reportId) {
- let reportDetail = ReportDetail(
- date: reportEntity.date ?? "",
- title: reportEntity.title,
- status: reportEntity.progress,
- category: reportEntity.type,
- description: reportEntity.content ?? "",
- location: reportEntity.location.address ?? "",
- photoUrls: reportEntity.photoUrls)
- reportDetailSubject.send(reportDetail)
- }
- reportDetailSubject.send(nil)
- } catch {
- reportDetailSubject.send(nil)
- }
- }
- }
+ private func fetchReportDetail(reportId: Int) {
+ Task { [weak self] in
+ guard let self else { return }
+
+ do {
+ if let reportEntity = try await self.reportRepository.fetchReportDetail(reportId: reportId) {
+ let reportDetail = ReportDetail(
+ date: reportEntity.date ?? "",
+ title: reportEntity.title,
+ status: reportEntity.progress,
+ category: reportEntity.type,
+ description: reportEntity.content ?? "",
+ location: reportEntity.location.address ?? "",
+ photoUrls: reportEntity.photoUrls)
+ self.reportDetailSubject.send(reportDetail)
+ } else {
+ self.reportDetailSubject.send(nil)
+ }
+ } catch {
+ self.reportDetailSubject.send(nil)
+ }
+ }
+ }이렇게 하면:
- 성공 시에는
ReportDetail만 발행되고, - 데이터가 없거나 실패한 경우에만
nil이 발행되어 뷰에서 적절히 처리할 수 있습니다.
Based on the verification results, here is my rewritten review comment:
---
**`fetchReportDetail(reportId:)` 메서드의 nil 발행 로직 버그와 메모리 안전성 패턴을 함께 수정해주세요**
현재 구현에는 두 가지 문제가 있습니다.
1. **성공 시에도 곧바로 `nil`을 발행하는 로직 (Line 50)**
현재 코드 구조에서는 제보 조회에 성공해도 `reportDetailSubject`가 곧바로 `nil`을 다시 발행합니다. 성공/데이터없음/실패 케이스를 명확히 구분해야 합니다.
2. **Task 클로저에서의 self 캡처 (Line 39-56)**
`Task { ... }` 내부에서 `reportRepository`와 `reportDetailSubject`에 접근하므로, [weak self]를 사용하는 것이 필요합니다.
```diff
- private func fetchReportDetail(reportId: Int) {
- Task {
- do {
- if let reportEntity = try await reportRepository.fetchReportDetail(reportId: reportId) {
- let reportDetail = ReportDetail(
- date: reportEntity.date ?? "",
- title: reportEntity.title,
- status: reportEntity.progress,
- category: reportEntity.type,
- description: reportEntity.content ?? "",
- location: reportEntity.location.address ?? "",
- photoUrls: reportEntity.photoUrls)
- reportDetailSubject.send(reportDetail)
- }
- reportDetailSubject.send(nil)
- } catch {
- reportDetailSubject.send(nil)
- }
- }
- }
+ private func fetchReportDetail(reportId: Int) {
+ Task { [weak self] in
+ guard let self else { return }
+
+ do {
+ if let reportEntity = try await self.reportRepository.fetchReportDetail(reportId: reportId) {
+ let reportDetail = ReportDetail(
+ date: reportEntity.date ?? "",
+ title: reportEntity.title,
+ status: reportEntity.progress,
+ category: reportEntity.type,
+ description: reportEntity.content ?? "",
+ location: reportEntity.location.address ?? "",
+ photoUrls: reportEntity.photoUrls)
+ self.reportDetailSubject.send(reportDetail)
+ } else {
+ self.reportDetailSubject.send(nil)
+ }
+ } catch {
+ self.reportDetailSubject.send(nil)
+ }
+ }
+ }
🏁 Script executed:
#!/bin/bash
# Find ReportDetailViewModel.swift and verify the code
find . -name "ReportDetailViewModel.swift" -type f 2>/dev/null | head -5Length of output: 76
🏁 Script executed:
#!/bin/bash
# Extract and display the fetchReportDetail method around lines 38-57
if [ -f "Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift" ]; then
sed -n '38,57p' "Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift"
else
echo "File not found at expected path"
fiLength of output: 871
🏁 Script executed:
#!/bin/bash
# Search for ReportDetailViewController and fillReportContent method
find . -name "ReportDetailViewController.swift" -type f 2>/dev/nullLength of output: 76
🏁 Script executed:
#!/bin/bash
# Check reportDetailSubject type and usage
rg -n "reportDetailSubject" "Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift" -A 2 -B 2 2>/dev/null || echo "File not found or no matches"Length of output: 761
🏁 Script executed:
#!/bin/bash
# Look for fillReportContent in ReportDetailViewController
rg -n "fillReportContent\|reportDetailSubject" "Projects/Presentation/Sources/Report/View/Controller/" -A 3 -B 1 2>/dev/null | head -30Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for fillReportContent directly in ReportDetailViewController.swift
rg -n "fillReportContent" "Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift" -A 5 -B 2
Length of output: 661
🏁 Script executed:
#!/bin/bash
# Search for reportDetailPublisher subscription in ReportDetailViewController.swift
rg -n "reportDetailPublisher\|\.sink\|\.assign\|\.receive" "Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift" -B 2 -A 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the whole ReportDetailViewController to understand how reportDetailPublisher is used
cat "Projects/Presentation/Sources/Report/View/ReportDetailViewController.swift" | head -100Length of output: 3298
fetchReportDetail(reportId:)에서 성공 후 항상 nil을 발행하는 버그와 Task 메모리 안전성을 함께 수정해주세요
현재 구현에는 두 가지 문제가 있습니다.
-
성공 시에도 곧바로
nil을 발행하는 로직 (Line 52)
제보 조회 성공 후 reportDetail이 발행되지만, 즉시 line 52의reportDetailSubject.send(nil)이 조건 없이 실행되어 구독자(fillReportContent)가 nil을 받게 됩니다. 성공/데이터없음/실패 케이스를 명확히 구분해야 합니다. -
Task 클로저에서의 self 캡처 (Line 39-56)
reportRepository와reportDetailSubject에 접근하므로[weak self]패턴을 사용해야 합니다. ReportDetailViewController는 이 패턴을 올바르게 적용하고 있습니다.
- private func fetchReportDetail(reportId: Int) {
- Task {
- do {
- if let reportEntity = try await reportRepository.fetchReportDetail(reportId: reportId) {
- let reportDetail = ReportDetail(
- date: reportEntity.date ?? "",
- title: reportEntity.title,
- status: reportEntity.progress,
- category: reportEntity.type,
- description: reportEntity.content ?? "",
- location: reportEntity.location.address ?? "",
- photoUrls: reportEntity.photoUrls)
- reportDetailSubject.send(reportDetail)
- }
- reportDetailSubject.send(nil)
- } catch {
- reportDetailSubject.send(nil)
- }
- }
- }
+ private func fetchReportDetail(reportId: Int) {
+ Task { [weak self] in
+ guard let self else { return }
+
+ do {
+ if let reportEntity = try await self.reportRepository.fetchReportDetail(reportId: reportId) {
+ let reportDetail = ReportDetail(
+ date: reportEntity.date ?? "",
+ title: reportEntity.title,
+ status: reportEntity.progress,
+ category: reportEntity.type,
+ description: reportEntity.content ?? "",
+ location: reportEntity.location.address ?? "",
+ photoUrls: reportEntity.photoUrls)
+ self.reportDetailSubject.send(reportDetail)
+ } else {
+ self.reportDetailSubject.send(nil)
+ }
+ } catch {
+ self.reportDetailSubject.send(nil)
+ }
+ }
+ }🤖 Prompt for AI Agents
In Projects/Presentation/Sources/Report/ViewModel/ReportDetailViewModel.swift
around lines 38 to 57, the Task currently always sends nil after a successful
send and captures self strongly; change the Task to capture self weakly (Task {
[weak self] in ... }) and unwrap guard let self = self (or use optional
chaining) before accessing reportRepository/reportDetailSubject, and remove the
unconditional reportDetailSubject.send(nil) after the success branch — instead
send nil only in the else case when fetch returns no entity and inside the catch
block for failures.
| let latitude: Double? | ||
| let longitude: Double? | ||
|
|
||
| func toReportEntity() throws -> ReportEntity { |
There was a problem hiding this comment.
엔티티 변환 실패 가능성을 염두에 두셨군요!
추후에 다른 DTO -> Entity 에도 throw를 할 수 있게 하고, 따로 Error를 정의해서 로그를 찍는것도 좋을 것 같습니다!
2 participants
🌁 Background
제보 상세화면과 API를 연동했어용 ~
📱 Screenshot
👩💻 Contents
📝 Review Note
그 ... 제보하기가 아직 작동되지 않어가주구 ... 신뢰. 와 믿음. 을 갖고 될 것이라 생각하며 Repository를 만들었어요 ㅠㅠ !!!
문제가 있다면 추후 꼭 수정하겟심다 !!!!
📣 Related Issue
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.