[Feat] #202 - Sprint4 작업항목 플로우 구현, 인스타그램 스토리 공유#203
Merged
Conversation
- 사진 이동 || 복제 시 앨범 선택하는 재사용 뷰 리듀서
- 사진 이동 || 복제 시 앨범 선택하는 재사용 뷰
3 tasks
|
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:
Walkthrough앨범 선택 및 사진 가져오기 TCA 리듀서를 도입하고, 사진 복제/이동/가져오기/앨범 삭제/인스타그램 스토리 공유 흐름을 연결하는 뷰(AlbumSelectionView, PhotoImportView)와 Footer 스타일(ArchiveFooterStyle)을 추가해 여러 화면에 프레젠테이션 연동을 구현했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant View as Archive View
participant AlbumSelect as AlbumSelectionFeature
participant ArchiveClient as ArchiveClient
participant Origin as OriginFeature
User->>View: 복제/이동 요청 (선택 모드에서)
View->>AlbumSelect: present(photoIDs, purpose, currentAlbumId?)
activate AlbumSelect
AlbumSelect->>ArchiveClient: getFavoriteAlbumInfo()
ArchiveClient-->>AlbumSelect: favorite album
AlbumSelect->>ArchiveClient: getAlbumList()
ArchiveClient-->>AlbumSelect: album list
User->>AlbumSelect: 선택 후 확인
AlbumSelect->>Origin: delegate.didCompleteTask(message)
deactivate AlbumSelect
Origin->>ArchiveClient: duplicate/move API 호출
ArchiveClient-->>Origin: success/failure
Origin->>View: refresh UI / show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift (1)
233-240:⚠️ Potential issue | 🟠 Major다운로드 예외가 나면 로딩이 남습니다.
Line 238이 throw하면
downloadImageResponse가 오지 않아isLoading이 true에 남습니다. 실패 경로에서도 응답을 보내도록 감싸 주세요.🔧 제안 수정안
return .run { send in - let count = try await imageDownloadClient.downloadImages(urls: [url]) - await send(.downloadImageResponse(successCount: count)) + do { + let count = try await imageDownloadClient.downloadImages(urls: [url]) + await send(.downloadImageResponse(successCount: count)) + } catch { + await send(.downloadImageResponse(successCount: 0)) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 233 - 240, 현재 .onTapDownload 처리에서 imageDownloadClient.downloadImages가 throw하면 downloadImageResponse가 전송되지 않아 state.isLoading가 true로 남습니다; .onTapDownload 케이스 내부의 .run 블록에서 downloadImages 호출을 do-catch로 감싸고 성공 시 await send(.downloadImageResponse(successCount: count))를, 실패 시에도 await send(.downloadImageResponse(successCount: 0)) 또는 실패를 표현하는 별도 액션을 전송하도록 수정하세요; 관련 식별자: .onTapDownload, imageDownloadClient.downloadImages(urls:), downloadImageResponse(successCount:), state.isLoading.
🧹 Nitpick comments (12)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ZoomableImageView.swift (1)
24-29: 이미지 로드 실패 상태 UI를 분기하는 것을 권장합니다.현재는 로딩 placeholder는 있지만, 재시도 실패 후 사용자에게 실패 상태를 명확히 보여주지 못할 수 있습니다. 실패 오버레이(재시도 버튼 포함) 추가를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ZoomableImageView.swift` around lines 24 - 29, The current AsyncImage usage in ZoomableImageView (placeholder + .retry + .cancelOnDisappear) shows loading but has no explicit UI for the failure state; implement a failure overlay with a retry button. Replace the chained modifiers on AsyncImage with the AsyncImage initializer that provides an AsyncImagePhase closure (or switch on phase) inside ZoomableImageView to render: a ProgressView for .empty, the image view for .success, and a failure overlay (message + Retry button) for .failure; wire the Retry button to a small `@State` reload token (e.g., reloadToken or reloadCount) used to re-create/reload the AsyncImage (or call the same image-loading function) so pressing Retry re-attempts load. Ensure the overlay is visually distinct and respects cancelOnDisappear behavior.Neki-iOS/Shared/DesignSystem/Sources/Component/ToolBar/NekiToolBar.swift (1)
100-112: 툴바 아이템 간 크기 불일치 확인 필요.
Icon버튼의 프레임이 44x44로 변경되었으나, 같은 파일 내Back(52x52, Line 52)과Close(52x52, Line 61) 버튼은 여전히 52x52입니다. 이 불일치가 의도된 디자인인지 확인해 주세요.44pt는 Apple HIG 최소 터치 타겟 요구사항을 충족하지만, 동일 툴바 내에서 일관성을 유지하는 것이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Shared/DesignSystem/Sources/Component/ToolBar/NekiToolBar.swift` around lines 100 - 112, Icon button uses a 44x44 frame while Back and Close use 52x52, causing inconsistent toolbar item sizes; update the Icon struct (Icon: View) to use the same frame as Back and Close (52x52) and ensure its .contentShape(Rectangle()) remains applied, or if the design intends a smaller hit area, instead change Back and Close to 44x44 so all toolbar items (Icon, Back, Close) share the same frame and touch target; adjust any layout constants or tokens used by NekiToolBar to keep sizes consistent across these components.Neki-iOS/Core/Sources/ImagePicker/Data/DTO/PresignedURLRequestDTO.swift (1)
18-18: 한 줄에 여러 프로퍼티 선언은 가독성을 낮춥니다.
width,height,size를 한 줄에 선언하는 것보다 각각 별도 줄에 선언하면 가독성과 diff 추적이 개선됩니다.♻️ 개선된 포맷팅 제안
public struct PresignedURLRequestData: Encodable { public let filename: String public let contentType: String public let mediaType: String - public let width, height, size: Int? + public let width: Int? + public let height: Int? + public let size: Int? }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Core/Sources/ImagePicker/Data/DTO/PresignedURLRequestDTO.swift` at line 18, The single-line declaration "public let width, height, size: Int?" in PresignedURLRequestDTO reduces readability and makes diffs noisy; update the DTO so each property is declared on its own line (e.g., public let width: Int?; public let height: Int?; public let size: Int?) within the PresignedURLRequestDTO type to improve formatting and diff traceability while keeping the same access level and optional Int type for each property.Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/ArchiveRepository.swift (2)
33-36:PhotoUploadMethodenum 위치 검토 필요
PhotoUploadMethod가public으로 선언되어 다른 모듈에서도 사용될 것으로 보입니다. Repository 프로토콜 파일 내에 정의하는 것보다 별도의Enums또는Models파일로 분리하면 코드 구조가 더 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/ArchiveRepository.swift` around lines 33 - 36, PhotoUploadMethod가 public이라 다른 모듈에서도 사용될 가능성이 높으니 ArchiveRepository.swift에 두지 말고 별도의 파일(예: Enums 또는 Models 폴더)로 분리하세요: 새 파일에 enum PhotoUploadMethod: String, Sendable { case qr = "QR"; case direct = "DIRECT_UPLOAD" } 형태로 옮기고 기존 ArchiveRepository 및 다른 참조 지점에서 해당 타입을 import 또는 모듈 경로로 참조하도록 수정하며 접근 제어자(public)와 프로퍼티(rawValue, Sendable) 선언은 그대로 유지하세요.
13-13: 튜플 파라미터를 구조체로 리팩토링 고려
uploads: [(mediaID: Int, memo: String?, uploadMethod: PhotoUploadMethod)]튜플은 필드가 많아 가독성이 떨어질 수 있습니다. 타입 안전성과 명확성을 위해 전용 구조체로 추출하는 것을 고려해보세요.💡 구조체 추출 제안
public struct PhotoUploadItem: Sendable { public let mediaID: Int public let memo: String? public let uploadMethod: PhotoUploadMethod public init(mediaID: Int, memo: String?, uploadMethod: PhotoUploadMethod) { self.mediaID = mediaID self.memo = memo self.uploadMethod = uploadMethod } } // 사용: func registerPhoto(folderID: Int?, uploads: [PhotoUploadItem], favorite: Bool?) async throws🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/ArchiveRepository.swift` at line 13, The current method signature registerPhoto(folderID:uploads:favorite:) uses a tuple type for uploads which reduces readability and type-safety; replace the tuple with a dedicated value type by introducing a PhotoUploadItem struct (containing mediaID: Int, memo: String?, uploadMethod: PhotoUploadMethod) and change the method signature to accept uploads: [PhotoUploadItem]; update all callers and any conforming repository implementations to construct and pass PhotoUploadItem instances and adjust imports/usages of PhotoUploadMethod as needed.Neki-iOS/Features/Archive/Sources/Domain/Sources/Entities/PhotoEntity.swift (1)
14-14:isfavorite네이밍이 Swift 컨벤션과 다릅니다.기존 코드이지만, Swift 네이밍 컨벤션에 따르면
isFavorite(camelCase)가 더 적절합니다. 향후 리팩토링 시 고려해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Domain/Sources/Entities/PhotoEntity.swift` at line 14, Rename the property isfavorite in the PhotoEntity struct to follow Swift camelCase as isFavorite; update all references/usages (initializers, tests, view models, and any code accessing PhotoEntity.isfavorite) to use isFavorite, and if the type conforms to Codable or maps to external keys ensure you add or update a CodingKeys enum or mapping logic to preserve the external key name (e.g., "isfavorite") so serialization/deserialization remains unchanged.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift (1)
83-93: nil-coalescing으로 빈 클로저를 제공하면 핸들러 누락을 감지하기 어렵습니다.
onDuplicate ?? {}와onMove ?? {}는 핸들러가 제공되지 않았을 때 조용히 아무 동작도 하지 않습니다. 선택 모드에서 이 액션들이 필수라면, 옵셔널 대신 필수 파라미터로 변경하거나style == .selection일 때 핸들러 존재를 검증하는 것이 좋습니다.💡 선택 모드용 별도 이니셜라이저 제안
// 선택 모드 전용 이니셜라이저 추가 public static func selection( isEnabled: Bool = true, onDownload: `@escaping` () -> Void, onDelete: `@escaping` () -> Void, onDuplicate: `@escaping` () -> Void, onMove: `@escaping` () -> Void ) -> ArchiveImageFooter { ArchiveImageFooter( style: .selection, isEnabled: isEnabled, onDownload: onDownload, onDelete: onDelete, onDuplicate: onDuplicate, onMove: onMove ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift` around lines 83 - 93, The footer currently uses nil-coalescing to replace missing handlers (onDuplicate ?? {} and onMove ?? {}) which hides missing action wiring; update ArchiveImageFooter so that duplicate and move handlers are required for selection mode: either make onDuplicate and onMove non-optional when style == .selection by adding a selection-specific initializer (e.g., ArchiveImageFooter.selection(...) that accepts non-optional onDuplicate and onMove) or validate inside the view initializer that when style == .selection the optionals onDuplicate/onMove are non-nil and assert/throw/log if missing; then wire selectionButton to the non-optional handlers (or force-unwrapped after validation) instead of using {}.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Coordinator/ArchiveCoordinator.swift (1)
66-85: 앨범 라우팅 분기 로직을 헬퍼로 합치면 유지보수가 쉬워집니다.동일한
isFavorite분기가 3군데 반복되어 이후 수정 시 누락 위험이 있습니다.Also applies to: 77-85, 106-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Coordinator/ArchiveCoordinator.swift` around lines 66 - 85, The album routing is duplicated across handlers (.root(.albumTapped), .root(.afterUploadNavigateToAlbumDetail), and the other case at 106-113); extract the branch into a helper like appendAlbumRoute(for album: Album, to state: inout ArchiveCoordinator.State) (or a private method on ArchiveCoordinator) that does the isFavorite check and calls state.path.append(.favoriteAlbum(ArchiveFavoriteAlbumFeature.State(album: album))) or state.path.append(.albumDetail(ArchiveAlbumDetailFeature.State(album: album))). Replace the duplicated blocks in the handlers (.albumTapped, .afterUploadNavigateToAlbumDetail, and the third similar case) with a single call to this helper and keep existing returns (e.g., return .none) as before.Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift (1)
14-14: SwiftUI@State프로퍼티는private으로 선언해야 합니다.SwiftLint 경고에 따라
@State프로퍼티는 외부 접근을 방지하기 위해private으로 선언하는 것이 권장됩니다.♻️ 수정 제안
- `@State` var addAlbumSheetPresented: Bool = false + `@State` private var addAlbumSheetPresented: Bool = false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift` at line 14, The `@State` property addAlbumSheetPresented in AlbumSelectionView should be made private to satisfy SwiftLint and prevent external access; change the declaration of addAlbumSheetPresented to "private `@State` var addAlbumSheetPresented: Bool = false" and if other views need to control it, expose a Binding or initializer parameter instead of making the state public.Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAlbumDetailView.swift (2)
18-18: SwiftUI@State프로퍼티는private으로 선언해야 합니다.SwiftLint 경고에 따라
@State프로퍼티는private으로 선언하는 것이 권장됩니다.♻️ 수정 제안
- `@State` var deleteEntireAlbumSheetPresented: Bool = false + `@State` private var deleteEntireAlbumSheetPresented: Bool = false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAlbumDetailView.swift` at line 18, Make the `@State` property deleteEntireAlbumSheetPresented private to satisfy SwiftUI/SwiftLint conventions: change its declaration in ArchiveAlbumDetailView (the property named deleteEntireAlbumSheetPresented) to "@State private var deleteEntireAlbumSheetPresented: Bool = false". If other code currently accesses this state directly, update those callsites to use a Binding (pass $deleteEntireAlbumSheetPresented) or expose a computed Binding/closure so external code does not read the internal `@State` directly.
222-232: 앨범 삭제 버튼 색상 TODO에 대한 의견위험한 액션에 빨간색(
.primary500대신.red또는 destructive 색상)을 사용하는 것은 일반적인 UX 패턴입니다. 디자인 팀 답변을 기다리는 동안, 현재 구현도 문제없이 동작합니다.디자인 피드백 후 색상 변경이 필요하면 도움을 드릴 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAlbumDetailView.swift` around lines 222 - 232, The "앨범 삭제" button currently uses .foregroundStyle(.primary500); change this to the appropriate destructive color once design confirms (e.g., replace .foregroundStyle(.primary500) with the destructive color token or .red) in the Button label inside ArchiveAlbumDetailView's delete action (the Button that sets deleteEntireAlbumSheetPresented = true and sends .closeDropDownMenu) so the visual indicates a dangerous action; update only the foreground style/token when the design team provides the final color.Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchivePhotoDetailView.swift (1)
64-70:onChange에서 여러 액션을 순차 전송하는 패턴 검토
currentItemID변경 시 4개의 액션을 순차적으로 전송하고 있습니다. TCA에서는 동작하지만, 관련 상태 변경을 하나의 액션(예:.resetMemoState)으로 통합하면 리듀서에서 상태 변경을 원자적으로 처리할 수 있어 더 명확합니다.💡 리듀서에서 통합 액션 처리 예시
// View에서 .onChange(of: store.currentItemID) { _, _ in store.send(.onPhotoChanged) } // Feature에서 case .onPhotoChanged: state.showDropDownMenu = false state.isMemoVisible = false state.isMemoExpanded = false state.isMemoEditing = false return .none🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchivePhotoDetailView.swift` around lines 64 - 70, Consolidate the multiple sequential sends in the onChange handler into a single action so the reducer can atomically reset memo-related state: add a new action (e.g. .onPhotoChanged or .resetMemoState) to the feature's Action enum and implement its reducer branch to set showDropDownMenu = false, isMemoVisible = false, isMemoExpanded = false, and isMemoEditing = false (plus any other memo flags like toggleMemoExpanded handling). Then update the view's onChange(of: store.currentItemID) block to call store.send(.onPhotoChanged) instead of sending .closeDropDownMenu, .binding(...isMemoVisible...), .toggleMemoExpanded, and .binding(...isMemoEditing...), and retain the local focus flag update (isMemoEditingFocused = false) in the view if it remains a view-local state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Neki-iOS/Core/Sources/ImagePicker/Presentation/Extension/Data`+.swift:
- Around line 54-61: The current extraction of image dimensions from properties
(using kCGImagePropertyPixelWidth and kCGImagePropertyPixelHeight into
width/height) may return zero or negative values; update the logic in the
Data+.swift routine that reads these properties so it only returns a tuple when
both width and height are present and greater than zero (e.g., guard let w =
width, let h = height, w > 0, h > 0 else return nil), otherwise return nil so
downstream layout calculations never receive non-positive sizes.
In `@Neki-iOS/Core/Sources/ImagePicker/Presentation/ImagePickerFeature.swift`:
- Around line 150-168: The detectFormat(from:) function currently defaults
unknown formats to .jpeg; change it to only return explicit known formats (e.g.,
.png, .webp, .jpeg) and return a new .unknown/.unsupported case (or throw an
error) for all others so callers know the format is unsupported and can convert
or fail-safe. Update the ImageFileFormat enum to include .unknown or
.unsupported (or make detectFormat throwing), modify detectFormat to return that
value instead of defaulting to .jpeg, and audit callers that use detectFormat to
handle the new unknown/throwing outcome (perform conversion to JPEG or surface
an error before upload).
In
`@Neki-iOS/Features/Archive/Sources/Data/Sources/DefaultArchiveRepository.swift`:
- Around line 259-271: In updatePhotoMemo, do not substitute capturedAt with
Date() on cache-miss; instead retrieve the authoritative capturedAt from the
source or fail: when photoCache/favoritePhotoCache lookup for photoID returns
nil, call the appropriate endpoint (e.g., a photo/detail or
ArchiveEndpoint.getPhoto) to fetch the original createdAt and use that value in
UpdateMemoRequestDTO, or throw an error if that fetch returns not found; update
references to UpdateMemoRequestDTO and ArchiveEndpoint.updateMemo accordingly so
capturedAt reflects the real value rather than a generated timestamp.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageCard.swift`:
- Line 38: 현재 파일의 섹션 주석 형식이 SwiftLint 규칙과 일치하지 않습니다; 해당 주석 토큰을 찾아 "// MARK: -
Init" 형식으로 수정하세요 (문서 상의 기존 "//MARK: - Init"을 찾아 MARK 토큰과 하이픈 사이에 공백을 추가하여 업데이트).
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ZoomableImageView.swift`:
- Around line 41-45: The onEnded handler for the pinch gesture only calls
clampOffset(geo:) when scale <= 1.0, which allows offset to remain out-of-bounds
after a zoom-out; change the onEnded logic in the pinch gesture (the closure
that sets lastScale) to always call clampOffset(geo:) after resetting lastScale
so that offset is re-clamped regardless of the final scale value (refer to the
onEnded closure, lastScale, scale, and clampOffset(geo:) symbols).
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift`:
- Around line 237-257: The effect handling
albumSelection.presented(.delegate(delegateAction)) (specifically the
didSelectAlbum branch in ArchiveAlbumDetailFeature) currently ignores albumId
and state.selectedIDs and only sleeps then sends duplicatePhotosResponse or
movePhotosResponse; update the effect to capture the albumId and selectedIDs and
call the real move/duplicate API (or the existing interactor/service method)
with those parameters instead of Task.sleep, await the API result and send the
appropriate .duplicatePhotosResponse or .movePhotosResponse with
success/failure; ensure
state.albumSelection/state.selectionPurpose/state.isLoading are updated
consistently and propagate any API errors through the response actions so the
reducer can show error toasts.
- Around line 352-362: The current handler for
.photoImport(.presented(.delegate(delegateAction))) returns an immediate success
after a dummy delay and never uses photoIDs, so the UI shows import success
without actual replication; update the case for .didImportPhotos(photoIDs) to
either (A) call the real import API with the photoIDs (e.g., invoke the
project's importPhotos/importAssets API), await its result, then send
.importPhotosResponse(.success/ .failure) and only clear state.photoImport and
state.isLoading based on the actual response, or (B) if the backend is not
ready, disable this branch: do not set state.isLoading nor send a
success—instead set a flag or send an explicit
.importPhotosResponse(.failure(.notImplemented)) / show an unimplemented UI;
modify the return .run block (the async closure that currently uses Task.sleep
and send(.importPhotosResponse(.success(())))) to perform the chosen behavior
and ensure photoIDs is passed to the API call when implemented.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift`:
- Around line 188-206: The handler for case .didSelectAlbum in
ArchiveAllPhotosFeature.swift currently ignores the selected albumId and always
returns a fake success after a 1s delay (sending
.duplicatePhotosResponse/.movePhotosResponse), which falsely signals completion;
update this by either (A) wiring the actual albumId into your backend call
(e.g., call your archive API client or service with albumId when
state.selectionPurpose == .duplicate or move, await its result, then send the
real .duplicatePhotosResponse/.movePhotosResponse with the API result and ensure
state.isLoading and state.selectionPurpose are restored/cleared correctly), or
(B) if the API is not available yet, replace the fake success path with an
explicit not-implemented/failure path (send a failure case or dispatch a
.showNotImplementedAlert action), and ensure you set state.isLoading = false and
restore state.albumSelection/selectionPurpose so the UI reflects the
unimplemented state instead of a silent success; locate this logic around the
.albumSelection(.presented(.delegate(delegateAction))) branch and the
.didSelectAlbum handling to implement the fix.
- Around line 105-119: The run effect in the .onTapDownloadButton branch doesn't
handle thrown errors from imageDownloadClient.downloadImages, leaving
state.isLoading true; modify the effect started in onTapDownloadButton to catch
errors and always send a completion action (e.g., extend or overload
downloadImagesResponse to include a failure case or send a separate
.downloadImagesFailed) so that downloadImagesResponse (or the failure action) is
always dispatched; ensure handlers for downloadImagesResponse (or
.downloadImagesFailed) set state.isLoading = false, clear selection, and
dispatch the appropriate delegate toast.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swift`:
- Around line 186-204: The handler for
.albumSelection(.presented(.delegate(delegateAction))) currently ignores the
albumId from .didSelectAlbum(albumId) and unconditionally sends success after a
test sleep; change it to validate and use albumId (from .didSelectAlbum) and
avoid sending a fake .duplicatePhotosResponse(.success(())) or
.movePhotosResponse(.success(())) when no API is wired. Instead, either return
.none or send a failure result (e.g., .failure(.notImplemented)) and keep state
changes reversible (don't clear state.selectionPurpose or set isLoading true
before confirming an operation), or add a guard that blocks the branch unless a
real API client is present so the UI shows an appropriate error/toast rather
than silently faking success.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFeature.swift`:
- Around line 235-243: The current loop silently skips failed Data(contentsOf:)
reads and always sends send(.uploadSharedImagesResponse(.success(entities)))
even when entities.isEmpty; change this so after building entities you check if
entities.isEmpty and send a failure response (e.g.,
send(.uploadSharedImagesResponse(.failure(...))) or appropriate failure action)
so the UI shows a toast instead of doing nothing; also stop hardcoding format:
.jpeg when constructing ImageUploadEntity — infer the image format from the
source (e.g., derive from the file URL extension or inspect the data/mime type)
and set ImageUploadEntity(format: ...) accordingly; ensure you still call
send(.cleanSharedImages(appGroupID: appGroupID)) in both success and failure
paths.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`:
- Around line 121-130: The code in the .doneMemoEditing branch overwrites the
local memo immediately and ignores server errors by using try?, so failures show
as success in the UI; change this to perform the network call and handle its
response by dispatching a success/failure action instead of swallowing errors.
Specifically, in the case .doneMemoEditing handler (around state.photos[id:
photoID]?.memo and archiveClient.updatePhotoMemo), stop using try?, call
archiveClient.updatePhotoMemo inside the Effect returned and send a success
action that confirms the local update or a failure action that triggers a
rollback (restore previous memo), shows a failure toast, and sets
state.isMemoEditing/state.isMemoExpanded appropriately; add new actions like
.photoMemoUpdateSuccess(photoID, memo) and .photoMemoUpdateFailure(photoID,
previousMemo, error) and handle them in the reducer to perform rollback and show
the toast.
- Around line 177-203: The .didSelectAlbum(albumId) branch currently returns
.none and never closes the albumSelection sheet; update the handler in
ArchivePhotoDetailFeature (case
.albumSelection(.presented(.delegate(delegateAction)))) so that on
.didSelectAlbum you set state.albumSelection = nil to dismiss the sheet and then
either dispatch an effect to perform the add-to-album flow (e.g., return .run {
await send(.addToAlbumResponse(Result { /* call archiveClient.registerPhotos
with albumId/photoId */ })) }) or, if the server API is unavailable, explicitly
disable/lock the confirm action by setting a flag on state (e.g.,
state.albumSelection?.confirmDisabled = true) and still dismiss the sheet;
ensure you reference and update state.albumSelection and send the
.addToAlbumResponse action (or set the confirmDisabled flag) from the
.didSelectAlbum handler.
- Around line 253-258: The delete branch for the ArchivePhotoDetailFeature's
.onTapDelete currently masks failures because it uses try? then always sends
.deletePhotoResponse(.success(())); change the logic in the .onTapDelete handler
to perform a proper await call to archiveClient.deletePhotoList(photoIds: [id])
inside a do-catch (or capture the thrown error), and only send
.deletePhotoResponse(.success(())) if the call succeeds; on error send
.deletePhotoResponse(.failure(error)) (or map to the feature's failure type) so
the failure branch can run and the local list is not updated on server failure.
- Around line 219-229: The optimistic favorite toggle in
ArchivePhotoDetailFeature uses try? when calling archiveClient.toggleFavorite in
the .onTapFavorite effect so any network error is swallowed and
.toggleFavoriteResponse is never sent; change the effect to await the call
without try?, capture success/failure in a do/catch and send a
.toggleFavoriteResponse action (include photo id and result) from the effect so
the reducer can roll back the optimistic update on failure; update the reducer
branch for .toggleFavoriteResponse to accept the response payload and revert
state.photos[id: …].isFavorite when the network call failed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift`:
- Around line 105-148: When switching albums a previous fetch can complete and
overwrite the new album's photos; modify the fetch flow to either cancel
in-flight fetches on album change or tag responses with the requested album id
and ignore stale ones. Concretely: give the fetch effect a cancellation ID (e.g.
"FetchPhotosID") and make the .run effect in the .fetchPhotos case cancellable,
or capture the requested album id inside the effect and include it in the
.fetchPhotosResponse payload; in selectAlbum call cancel(id: FetchPhotosID) (or
dispatch a cancel effect) before sending .fetchPhotos, and in the
.fetchPhotosResponse handler verify the response's requested id matches
state.selectedAlbum?.id before mutating state.photos and toggling
isFetchingPhotos (if using tagged responses, drop mismatches).
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`:
- Around line 22-23: The loading message shown in AlbumSelectionView when the
condition store.isFetching && store.albums.isEmpty is true is misleading ("앨범에
추가하고 있어요." implies adding), so update the LoadingView call in AlbumSelectionView
to use a fetching-appropriate message such as "앨범을 불러오고 있어요." (i.e., replace the
string passed to LoadingView when evaluating store.isFetching &&
store.albums.isEmpty); ensure the change is applied where LoadingView(message:
...) is invoked so the UI correctly reflects fetching state.
In `@Neki-iOS/Info.plist`:
- Around line 5-6: The CFBundleIdentifier key in Info.plist is set to an empty
string which can override the build-time PRODUCT_BUNDLE_IDENTIFIER and break
signing/distribution; remove the <key>CFBundleIdentifier</key> entry (or replace
the empty <string> with $(PRODUCT_BUNDLE_IDENTIFIER) if an explicit value is
required) so the build settings supply the bundle identifier instead; locate the
CFBundleIdentifier entry in Info.plist and either delete that key/string pair or
set its value to $(PRODUCT_BUNDLE_IDENTIFIER).
---
Outside diff comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`:
- Around line 233-240: 현재 .onTapDownload 처리에서
imageDownloadClient.downloadImages가 throw하면 downloadImageResponse가 전송되지 않아
state.isLoading가 true로 남습니다; .onTapDownload 케이스 내부의 .run 블록에서 downloadImages 호출을
do-catch로 감싸고 성공 시 await send(.downloadImageResponse(successCount: count))를, 실패
시에도 await send(.downloadImageResponse(successCount: 0)) 또는 실패를 표현하는 별도 액션을 전송하도록
수정하세요; 관련 식별자: .onTapDownload, imageDownloadClient.downloadImages(urls:),
downloadImageResponse(successCount:), state.isLoading.
---
Nitpick comments:
In `@Neki-iOS/Core/Sources/ImagePicker/Data/DTO/PresignedURLRequestDTO.swift`:
- Line 18: The single-line declaration "public let width, height, size: Int?" in
PresignedURLRequestDTO reduces readability and makes diffs noisy; update the DTO
so each property is declared on its own line (e.g., public let width: Int?;
public let height: Int?; public let size: Int?) within the
PresignedURLRequestDTO type to improve formatting and diff traceability while
keeping the same access level and optional Int type for each property.
In `@Neki-iOS/Features/Archive/Sources/Domain/Sources/Entities/PhotoEntity.swift`:
- Line 14: Rename the property isfavorite in the PhotoEntity struct to follow
Swift camelCase as isFavorite; update all references/usages (initializers,
tests, view models, and any code accessing PhotoEntity.isfavorite) to use
isFavorite, and if the type conforms to Codable or maps to external keys ensure
you add or update a CodingKeys enum or mapping logic to preserve the external
key name (e.g., "isfavorite") so serialization/deserialization remains
unchanged.
In
`@Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/ArchiveRepository.swift`:
- Around line 33-36: PhotoUploadMethod가 public이라 다른 모듈에서도 사용될 가능성이 높으니
ArchiveRepository.swift에 두지 말고 별도의 파일(예: Enums 또는 Models 폴더)로 분리하세요: 새 파일에 enum
PhotoUploadMethod: String, Sendable { case qr = "QR"; case direct =
"DIRECT_UPLOAD" } 형태로 옮기고 기존 ArchiveRepository 및 다른 참조 지점에서 해당 타입을 import 또는 모듈
경로로 참조하도록 수정하며 접근 제어자(public)와 프로퍼티(rawValue, Sendable) 선언은 그대로 유지하세요.
- Line 13: The current method signature
registerPhoto(folderID:uploads:favorite:) uses a tuple type for uploads which
reduces readability and type-safety; replace the tuple with a dedicated value
type by introducing a PhotoUploadItem struct (containing mediaID: Int, memo:
String?, uploadMethod: PhotoUploadMethod) and change the method signature to
accept uploads: [PhotoUploadItem]; update all callers and any conforming
repository implementations to construct and pass PhotoUploadItem instances and
adjust imports/usages of PhotoUploadMethod as needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift`:
- Around line 83-93: The footer currently uses nil-coalescing to replace missing
handlers (onDuplicate ?? {} and onMove ?? {}) which hides missing action wiring;
update ArchiveImageFooter so that duplicate and move handlers are required for
selection mode: either make onDuplicate and onMove non-optional when style ==
.selection by adding a selection-specific initializer (e.g.,
ArchiveImageFooter.selection(...) that accepts non-optional onDuplicate and
onMove) or validate inside the view initializer that when style == .selection
the optionals onDuplicate/onMove are non-nil and assert/throw/log if missing;
then wire selectionButton to the non-optional handlers (or force-unwrapped after
validation) instead of using {}.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ZoomableImageView.swift`:
- Around line 24-29: The current AsyncImage usage in ZoomableImageView
(placeholder + .retry + .cancelOnDisappear) shows loading but has no explicit UI
for the failure state; implement a failure overlay with a retry button. Replace
the chained modifiers on AsyncImage with the AsyncImage initializer that
provides an AsyncImagePhase closure (or switch on phase) inside
ZoomableImageView to render: a ProgressView for .empty, the image view for
.success, and a failure overlay (message + Retry button) for .failure; wire the
Retry button to a small `@State` reload token (e.g., reloadToken or reloadCount)
used to re-create/reload the AsyncImage (or call the same image-loading
function) so pressing Retry re-attempts load. Ensure the overlay is visually
distinct and respects cancelOnDisappear behavior.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Coordinator/ArchiveCoordinator.swift`:
- Around line 66-85: The album routing is duplicated across handlers
(.root(.albumTapped), .root(.afterUploadNavigateToAlbumDetail), and the other
case at 106-113); extract the branch into a helper like appendAlbumRoute(for
album: Album, to state: inout ArchiveCoordinator.State) (or a private method on
ArchiveCoordinator) that does the isFavorite check and calls
state.path.append(.favoriteAlbum(ArchiveFavoriteAlbumFeature.State(album:
album))) or
state.path.append(.albumDetail(ArchiveAlbumDetailFeature.State(album: album))).
Replace the duplicated blocks in the handlers (.albumTapped,
.afterUploadNavigateToAlbumDetail, and the third similar case) with a single
call to this helper and keep existing returns (e.g., return .none) as before.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`:
- Line 14: The `@State` property addAlbumSheetPresented in AlbumSelectionView
should be made private to satisfy SwiftLint and prevent external access; change
the declaration of addAlbumSheetPresented to "private `@State` var
addAlbumSheetPresented: Bool = false" and if other views need to control it,
expose a Binding or initializer parameter instead of making the state public.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAlbumDetailView.swift`:
- Line 18: Make the `@State` property deleteEntireAlbumSheetPresented private to
satisfy SwiftUI/SwiftLint conventions: change its declaration in
ArchiveAlbumDetailView (the property named deleteEntireAlbumSheetPresented) to
"@State private var deleteEntireAlbumSheetPresented: Bool = false". If other
code currently accesses this state directly, update those callsites to use a
Binding (pass $deleteEntireAlbumSheetPresented) or expose a computed
Binding/closure so external code does not read the internal `@State` directly.
- Around line 222-232: The "앨범 삭제" button currently uses
.foregroundStyle(.primary500); change this to the appropriate destructive color
once design confirms (e.g., replace .foregroundStyle(.primary500) with the
destructive color token or .red) in the Button label inside
ArchiveAlbumDetailView's delete action (the Button that sets
deleteEntireAlbumSheetPresented = true and sends .closeDropDownMenu) so the
visual indicates a dangerous action; update only the foreground style/token when
the design team provides the final color.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchivePhotoDetailView.swift`:
- Around line 64-70: Consolidate the multiple sequential sends in the onChange
handler into a single action so the reducer can atomically reset memo-related
state: add a new action (e.g. .onPhotoChanged or .resetMemoState) to the
feature's Action enum and implement its reducer branch to set showDropDownMenu =
false, isMemoVisible = false, isMemoExpanded = false, and isMemoEditing = false
(plus any other memo flags like toggleMemoExpanded handling). Then update the
view's onChange(of: store.currentItemID) block to call
store.send(.onPhotoChanged) instead of sending .closeDropDownMenu,
.binding(...isMemoVisible...), .toggleMemoExpanded, and
.binding(...isMemoEditing...), and retain the local focus flag update
(isMemoEditingFocused = false) in the view if it remains a view-local state.
In `@Neki-iOS/Shared/DesignSystem/Sources/Component/ToolBar/NekiToolBar.swift`:
- Around line 100-112: Icon button uses a 44x44 frame while Back and Close use
52x52, causing inconsistent toolbar item sizes; update the Icon struct (Icon:
View) to use the same frame as Back and Close (52x52) and ensure its
.contentShape(Rectangle()) remains applied, or if the design intends a smaller
hit area, instead change Back and Close to 44x44 so all toolbar items (Icon,
Back, Close) share the same frame and touch target; adjust any layout constants
or tokens used by NekiToolBar to keep sizes consistent across these components.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2af9e1bb-4977-4795-aba7-3a1f405a829d
⛔ Files ignored due to path filters (7)
Neki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_download.imageset/download_disabled.svgis excluded by!**/*.svgNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_download_fill.imageset/download_abled.svgis excluded by!**/*.svgNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_duplicate.imageset/icon_duplicate.svgis excluded by!**/*.svgNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_duplicate_fill.imageset/icon_duplicate_fill.svgis excluded by!**/*.svgNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_move.imageset/icon_move.svgis excluded by!**/*.svgNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_move_fill.imageset/icon_move_fill.svgis excluded by!**/*.svgNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_note.imageset/icon_note.svgis excluded by!**/*.svg
📒 Files selected for processing (46)
Neki-iOS/APP/Sources/MainTab/MainTabCoordinator.swiftNeki-iOS/Core/Sources/ImagePicker/Data/DTO/PresignedURLRequestDTO.swiftNeki-iOS/Core/Sources/ImagePicker/Data/DefaultImageUploadRepository.swiftNeki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swiftNeki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadEntity.swiftNeki-iOS/Core/Sources/ImagePicker/Presentation/Extension/Data+.swiftNeki-iOS/Core/Sources/ImagePicker/Presentation/ImagePickerFeature.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/ArchiveEndpoint.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/DTO/PhotoListDTO.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/DTO/Request/UpdateMemoRequestDTO.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/DefaultArchiveRepository.swiftNeki-iOS/Features/Archive/Sources/Domain/Sources/Client/ArchiveClient.swiftNeki-iOS/Features/Archive/Sources/Domain/Sources/Entities/PhotoEntity.swiftNeki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/ArchiveRepository.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageCard.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ZoomableImageView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Coordinator/ArchiveCoordinator.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/Date+.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/SelectUploadAlbumFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Item/ArchiveImageItem.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAlbumDetailView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAllAlbumsView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAllPhotosView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveFavoriteAlbumView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchivePhotoDetailView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/PhotoImportView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/SelectUploadAlbumView.swiftNeki-iOS/Info.plistNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_download.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_download_fill.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_duplicate.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_duplicate_fill.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_move.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_move_fill.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Resources/Assets.xcassets/Common/icon_note.imageset/Contents.jsonNeki-iOS/Shared/DesignSystem/Sources/Component/ToolBar/NekiToolBar.swift
Comment on lines
+54
to
+61
| var width = properties[kCGImagePropertyPixelWidth] as? Int | ||
| var height = properties[kCGImagePropertyPixelHeight] as? Int | ||
|
|
||
| if let w = width, let h = height { | ||
| return (w, h) | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
이미지 크기 메타데이터의 유효값(양수) 검증이 필요합니다.
현재는 0 또는 음수 크기도 반환될 수 있어 후속 레이아웃 계산에 문제를 만들 수 있습니다.
🔧 제안 수정안
- var width = properties[kCGImagePropertyPixelWidth] as? Int
- var height = properties[kCGImagePropertyPixelHeight] as? Int
-
- if let w = width, let h = height {
- return (w, h)
- }
-
- return nil
+ guard
+ let width = properties[kCGImagePropertyPixelWidth] as? Int, width > 0,
+ let height = properties[kCGImagePropertyPixelHeight] as? Int, height > 0
+ else {
+ return nil
+ }
+ return (width, height)📝 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
| var width = properties[kCGImagePropertyPixelWidth] as? Int | |
| var height = properties[kCGImagePropertyPixelHeight] as? Int | |
| if let w = width, let h = height { | |
| return (w, h) | |
| } | |
| return nil | |
| guard | |
| let width = properties[kCGImagePropertyPixelWidth] as? Int, width > 0, | |
| let height = properties[kCGImagePropertyPixelHeight] as? Int, height > 0 | |
| else { | |
| return nil | |
| } | |
| return (width, height) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Neki-iOS/Core/Sources/ImagePicker/Presentation/Extension/Data`+.swift around
lines 54 - 61, The current extraction of image dimensions from properties (using
kCGImagePropertyPixelWidth and kCGImagePropertyPixelHeight into width/height)
may return zero or negative values; update the logic in the Data+.swift routine
that reads these properties so it only returns a tuple when both width and
height are present and greater than zero (e.g., guard let w = width, let h =
height, w > 0, h > 0 else return nil), otherwise return nil so downstream layout
calculations never receive non-positive sizes.
Comment on lines
+150
to
+168
| private static func detectFormat(from data: Data) -> ImageFileFormat { | ||
| guard data.count > 12 else { return .jpeg } | ||
| let header = data.prefix(12) | ||
| let firstByte = header[0] | ||
|
|
||
| // PNG 확인 (0x89로 시작) | ||
| if firstByte == 0x89 { | ||
| return .png | ||
| } | ||
|
|
||
| // WebP 확인 | ||
| // WebP 파일 구조: | ||
| // Offset 0-3: "RIFF" (0x52, 0x49, 0x46, 0x46) | ||
| // Offset 8-11: "WEBP" (0x57, 0x45, 0x42, 0x50) | ||
| if header[0] == 0x52 && header[1] == 0x49 && header[2] == 0x46 && header[3] == 0x46 && // "RIFF" | ||
| header[8] == 0x57 && header[9] == 0x45 && header[10] == 0x42 && header[11] == 0x50 { // "WEBP" | ||
| return .webp | ||
| } | ||
| return .jpeg |
There was a problem hiding this comment.
지원하지 않는 포맷을 .jpeg로 위장하지 마세요.
지금 분기는 PNG/WebP가 아니면 모두 .jpeg로 반환합니다. 원본이 HEIC 같은 포맷으로 들어오면 실제 바이트는 JPEG가 아닌데 메타데이터만 JPEG로 올라가서, 업로드 단계에서 잘못된 Content-Type/확장자 처리로 이어질 수 있습니다. 지원 포맷만 명시적으로 매핑하고, 나머지는 실제 변환 후 .jpeg를 설정하거나 실패로 처리하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Neki-iOS/Core/Sources/ImagePicker/Presentation/ImagePickerFeature.swift`
around lines 150 - 168, The detectFormat(from:) function currently defaults
unknown formats to .jpeg; change it to only return explicit known formats (e.g.,
.png, .webp, .jpeg) and return a new .unknown/.unsupported case (or throw an
error) for all others so callers know the format is unsupported and can convert
or fail-safe. Update the ImageFileFormat enum to include .unknown or
.unsupported (or make detectFormat throwing), modify detectFormat to return that
value instead of defaulting to .jpeg, and audit callers that use detectFormat to
handle the new unknown/throwing outcome (perform conversion to JPEG or surface
an error before upload).
Comment on lines
+259
to
+271
| func updatePhotoMemo(photoID: Int, memo: String) async throws { | ||
| var capturedAt = "" | ||
| if let cachedItem = photoCache.values.flatMap({ $0 }).first(where: { $0.photoID == photoID }) { | ||
| capturedAt = cachedItem.createdAt | ||
| } else if let cachedItem = favoritePhotoCache.first(where: { $0.photoID == photoID }) { | ||
| capturedAt = cachedItem.createdAt | ||
| } else { | ||
| capturedAt = Date().ISO8601Format() | ||
| } | ||
|
|
||
| let requestDTO = UpdateMemoRequestDTO(memo: memo, capturedAt: capturedAt) | ||
| let endpoint = ArchiveEndpoint.updateMemo(photoID: photoID, request: requestDTO) | ||
| let _ = try await networkProvider.request(endpoint: endpoint) |
There was a problem hiding this comment.
캐시 미스 시 capturedAt를 현재 시각으로 대체하면 안 됩니다.
메모만 수정하려는 요청인데, 캐시에 없는 사진은 Date().ISO8601Format()이 들어가서 촬영 시각이 임의로 바뀔 수 있습니다. 이 값이 정렬/표시 기준이면 메모 수정만으로 사진 순서나 날짜 표기가 흔들릴 수 있으니, 원본 값을 다시 조회하거나 없으면 실패로 처리하는 편이 맞습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Data/Sources/DefaultArchiveRepository.swift`
around lines 259 - 271, In updatePhotoMemo, do not substitute capturedAt with
Date() on cache-miss; instead retrieve the authoritative capturedAt from the
source or fail: when photoCache/favoritePhotoCache lookup for photoID returns
nil, call the appropriate endpoint (e.g., a photo/detail or
ArchiveEndpoint.getPhoto) to fetch the original createdAt and use that value in
UpdateMemoRequestDTO, or throw an error if that fetch returns not found; update
references to UpdateMemoRequestDTO and ArchiveEndpoint.updateMemo accordingly so
capturedAt reflects the real value rather than a generated timestamp.
| return nil | ||
| } | ||
|
|
||
| //MARK: - Init |
There was a problem hiding this comment.
MARK 주석 포맷을 SwiftLint 규칙에 맞춰주세요.
Line 38은 // MARK: - Init 형태로 수정하는 것이 필요합니다.
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 38-38: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageCard.swift`
at line 38, 현재 파일의 섹션 주석 형식이 SwiftLint 규칙과 일치하지 않습니다; 해당 주석 토큰을 찾아 "// MARK: -
Init" 형식으로 수정하세요 (문서 상의 기존 "//MARK: - Init"을 찾아 MARK 토큰과 하이픈 사이에 공백을 추가하여 업데이트).
Comment on lines
+41
to
+45
| .onEnded { _ in | ||
| lastScale = 1.0 | ||
| if scale <= 1.0 { | ||
| clampOffset(geo: geo) | ||
| } |
There was a problem hiding this comment.
축소 종료 시 오프셋 경계 재보정이 누락되어 있습니다.
Line 43 조건 때문에 scale > 1.0 상태에서 축소 종료하면, 기존 offset이 새 배율 기준 허용 범위를 벗어난 채 유지될 수 있습니다. 종료 시점에는 항상 clampOffset(geo:)를 호출하는 편이 안정적입니다.
제안 수정안
.gesture(
MagnifyGesture()
.onChanged { value in
let delta = value.magnification / lastScale
lastScale = value.magnification
scale = min(max(scale * delta, 0.8), 5.0) // 0.8 ~ 5배까지
}
.onEnded { _ in
lastScale = 1.0
- if scale <= 1.0 {
- clampOffset(geo: geo)
- }
+ clampOffset(geo: geo)
}
)📝 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
| .onEnded { _ in | |
| lastScale = 1.0 | |
| if scale <= 1.0 { | |
| clampOffset(geo: geo) | |
| } | |
| .onEnded { _ in | |
| lastScale = 1.0 | |
| clampOffset(geo: geo) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ZoomableImageView.swift`
around lines 41 - 45, The onEnded handler for the pinch gesture only calls
clampOffset(geo:) when scale <= 1.0, which allows offset to remain out-of-bounds
after a zoom-out; change the onEnded logic in the pinch gesture (the closure
that sets lastScale) to always call clampOffset(geo:) after resetting lastScale
so that offset is re-clamped regardless of the final scale value (refer to the
onEnded closure, lastScale, scale, and clampOffset(geo:) symbols).
Comment on lines
219
to
229
| case .onTapFavorite: | ||
| guard let item = state.currentItem else { return .none } | ||
| let newStatus = !item.isFavorite | ||
|
|
||
| state.photos[id: item.id]?.isFavorite = newStatus | ||
|
|
||
| return .run { [id = item.id, isFavorite = newStatus] send in | ||
| do { | ||
| try await archiveClient.toggleFavorite(photoID: id, request: isFavorite) | ||
| await send(.toggleFavoriteResponse(photoID: id, result: .success(()))) | ||
| } catch { | ||
| await send(.toggleFavoriteResponse(photoID: id, result: .failure(error))) | ||
| } | ||
| try? await archiveClient.toggleFavorite(photoID: id, request: isFavorite) | ||
| } | ||
|
|
||
| case .toggleFavoriteResponse(_, .success): | ||
| case .toggleFavoriteResponse: | ||
| return .none |
There was a problem hiding this comment.
즐겨찾기 토글 실패를 복구할 수 없게 됐습니다.
Line 225의 try? 때문에 실패가 사라지고, Line 228의 toggleFavoriteResponse 분기는 더 이상 도달하지 않습니다. 네트워크 실패 시 optimistic update가 그대로 남아 상세 화면 상태가 실제 서버 값과 어긋납니다.
🔧 제안 수정안
return .run { [id = item.id, isFavorite = newStatus] send in
- try? await archiveClient.toggleFavorite(photoID: id, request: isFavorite)
+ do {
+ try await archiveClient.toggleFavorite(photoID: id, request: isFavorite)
+ await send(.toggleFavoriteResponse(photoID: id, result: .success(())))
+ } catch {
+ await send(.toggleFavoriteResponse(photoID: id, result: .failure(error)))
+ }
}
-case .toggleFavoriteResponse:
+case let .toggleFavoriteResponse(photoID, .failure):
+ state.photos[id: photoID]?.isFavorite.toggle()
+ return .send(.delegate(.showToast(NekiToastItem("즐겨찾기 변경에 실패했어요", style: .error))))
+
+case .toggleFavoriteResponse:
return .none🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`
around lines 219 - 229, The optimistic favorite toggle in
ArchivePhotoDetailFeature uses try? when calling archiveClient.toggleFavorite in
the .onTapFavorite effect so any network error is swallowed and
.toggleFavoriteResponse is never sent; change the effect to await the call
without try?, capture success/failure in a do/catch and send a
.toggleFavoriteResponse action (include photo id and result) from the effect so
the reducer can roll back the optimistic update on failure; update the reducer
branch for .toggleFavoriteResponse to accept the response payload and revert
state.photos[id: …].isFavorite when the network call failed.
Comment on lines
253
to
258
| case .onTapDelete: | ||
| guard let id = state.currentItem?.id else { return .none } | ||
| return .run { send in | ||
| do { | ||
| try await archiveClient.deletePhotoList(photoIds: [id]) | ||
| await send(.deletePhotoResponse(.success(()))) | ||
| } catch { | ||
| await send(.deletePhotoResponse(.failure(error))) | ||
| } | ||
| try? await archiveClient.deletePhotoList(photoIds: [id]) | ||
| await send(.deletePhotoResponse(.success(()))) | ||
| } |
There was a problem hiding this comment.
삭제 실패도 성공으로 처리되고 있습니다.
Line 256의 try? 뒤에서 Line 257이 항상 success를 보내서, 서버 삭제가 실패해도 로컬 리스트에서는 사진이 사라집니다. 현재 .deletePhotoResponse(.failure) 분기는 사실상 도달하지 못합니다.
🔧 제안 수정안
return .run { send in
- try? await archiveClient.deletePhotoList(photoIds: [id])
- await send(.deletePhotoResponse(.success(())))
+ do {
+ try await archiveClient.deletePhotoList(photoIds: [id])
+ await send(.deletePhotoResponse(.success(())))
+ } catch {
+ await send(.deletePhotoResponse(.failure(error)))
+ }
}📝 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
| case .onTapDelete: | |
| guard let id = state.currentItem?.id else { return .none } | |
| return .run { send in | |
| do { | |
| try await archiveClient.deletePhotoList(photoIds: [id]) | |
| await send(.deletePhotoResponse(.success(()))) | |
| } catch { | |
| await send(.deletePhotoResponse(.failure(error))) | |
| } | |
| try? await archiveClient.deletePhotoList(photoIds: [id]) | |
| await send(.deletePhotoResponse(.success(()))) | |
| } | |
| case .onTapDelete: | |
| guard let id = state.currentItem?.id else { return .none } | |
| return .run { send in | |
| do { | |
| try await archiveClient.deletePhotoList(photoIds: [id]) | |
| await send(.deletePhotoResponse(.success(()))) | |
| } catch { | |
| await send(.deletePhotoResponse(.failure(error))) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`
around lines 253 - 258, The delete branch for the ArchivePhotoDetailFeature's
.onTapDelete currently masks failures because it uses try? then always sends
.deletePhotoResponse(.success(())); change the logic in the .onTapDelete handler
to perform a proper await call to archiveClient.deletePhotoList(photoIds: [id])
inside a do-catch (or capture the thrown error), and only send
.deletePhotoResponse(.success(())) if the call succeeds; on error send
.deletePhotoResponse(.failure(error)) (or map to the feature's failure type) so
the failure branch can run and the local list is not updated on server failure.
Comment on lines
+105
to
+148
| case .fetchPhotos: | ||
| guard !state.isFetchingPhotos else { return .none } | ||
| state.isFetchingPhotos = true | ||
| let targetFolderId = state.selectedAlbum?.id == -1 ? nil : state.selectedAlbum?.id | ||
| let sortOrder = "DESC" | ||
|
|
||
| return .run { [id = state.selectedAlbum?.id] send in | ||
| if id == -1 { | ||
| await send(.fetchPhotosResponse(Result { try await archiveClient.fetchFavoritePhotoList(20, sortOrder) })) | ||
| } else { | ||
| await send(.fetchPhotosResponse(Result { try await archiveClient.fetchPhotoList(folderId: targetFolderId, size: 20, sortOrder: sortOrder) })) | ||
| } | ||
| } | ||
|
|
||
| case .loadMorePhotos: | ||
| return .send(.fetchPhotos) | ||
|
|
||
| case let .fetchPhotosResponse(.success(entities)): | ||
| state.isFetchingPhotos = false | ||
| let currentAlbumId = state.selectedAlbum?.id | ||
| let newItems = entities.map { entity in | ||
| ArchiveImageItem(id: entity.photoID, imageURLString: entity.imageURL, isFavorite: entity.isfavorite, date: entity.createdAt.toISO8601Date(), folderId: currentAlbumId, memo: entity.memo ?? "", width: entity.width, height: entity.height) | ||
| } | ||
| state.photos = IdentifiedArray(uniqueElements: newItems) | ||
| return .none | ||
|
|
||
| case .fetchPhotosResponse(.failure): | ||
| state.isFetchingPhotos = false | ||
| return .none | ||
|
|
||
| case .toggleDropdown: | ||
| state.isDropdownOpen.toggle() | ||
| return .none | ||
|
|
||
| case .closeDropdown: | ||
| state.isDropdownOpen = false | ||
| return .none | ||
|
|
||
| case let .selectAlbum(album): | ||
| state.selectedIDs.removeAll() // TODO: - 현재는 앨범 변경 시 선택된 사진 해제. 피그마에 문의 남김. 추후 수정될 여지 있음 | ||
| state.selectedAlbum = album | ||
| state.isDropdownOpen = false | ||
| state.photos.removeAll() | ||
| return .send(.fetchPhotos) |
There was a problem hiding this comment.
앨범 전환 중에는 이전 fetch 응답을 버릴 수 있어야 합니다.
지금은 selectAlbum 직후에도 기존 요청이 살아 있으면 새 .fetchPhotos가 isFetchingPhotos guard에 막히고, 늦게 도착한 이전 응답이 그대로 state.photos를 덮습니다. 그래서 다른 앨범을 눌렀는데 직전 앨범 사진이 다시 보이는 race가 생깁니다. 응답에 요청한 album ID/token을 싣고 현재 선택과 일치할 때만 반영하거나, 전환 시 이전 effect를 cancel 해야 합니다.
패치 방향 예시
enum Action: BindableAction {
@@
- case fetchPhotosResponse(Result<[PhotoEntity], Error>)
+ case fetchPhotosResponse(requestedAlbumID: Int?, Result<[PhotoEntity], Error>)
@@
case .fetchPhotos:
guard !state.isFetchingPhotos else { return .none }
state.isFetchingPhotos = true
let targetFolderId = state.selectedAlbum?.id == -1 ? nil : state.selectedAlbum?.id
let sortOrder = "DESC"
- return .run { [id = state.selectedAlbum?.id] send in
- if id == -1 {
- await send(.fetchPhotosResponse(Result { try await archiveClient.fetchFavoritePhotoList(20, sortOrder) }))
+ return .run { [requestedAlbumID = state.selectedAlbum?.id] send in
+ if requestedAlbumID == -1 {
+ await send(.fetchPhotosResponse(requestedAlbumID: requestedAlbumID, Result { try await archiveClient.fetchFavoritePhotoList(20, sortOrder) }))
} else {
- await send(.fetchPhotosResponse(Result { try await archiveClient.fetchPhotoList(folderId: targetFolderId, size: 20, sortOrder: sortOrder) }))
+ await send(.fetchPhotosResponse(requestedAlbumID: requestedAlbumID, Result { try await archiveClient.fetchPhotoList(folderId: targetFolderId, size: 20, sortOrder: sortOrder) }))
}
}
@@
- case let .fetchPhotosResponse(.success(entities)):
+ case let .fetchPhotosResponse(requestedAlbumID, .success(entities)):
+ guard requestedAlbumID == state.selectedAlbum?.id else { return .none }
state.isFetchingPhotos = false
let currentAlbumId = state.selectedAlbum?.id
let newItems = entities.map { entity in
ArchiveImageItem(id: entity.photoID, imageURLString: entity.imageURL, isFavorite: entity.isfavorite, date: entity.createdAt.toISO8601Date(), folderId: currentAlbumId, memo: entity.memo ?? "", width: entity.width, height: entity.height)
}
state.photos = IdentifiedArray(uniqueElements: newItems)
return .none🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift`
around lines 105 - 148, When switching albums a previous fetch can complete and
overwrite the new album's photos; modify the fetch flow to either cancel
in-flight fetches on album change or tag responses with the requested album id
and ignore stale ones. Concretely: give the fetch effect a cancellation ID (e.g.
"FetchPhotosID") and make the .run effect in the .fetchPhotos case cancellable,
or capture the requested album id inside the effect and include it in the
.fetchPhotosResponse payload; in selectAlbum call cancel(id: FetchPhotosID) (or
dispatch a cancel effect) before sending .fetchPhotos, and in the
.fetchPhotosResponse handler verify the response's requested id matches
state.selectedAlbum?.id before mutating state.photos and toggling
isFetchingPhotos (if using tagged responses, drop mismatches).
Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift
Outdated
Show resolved
Hide resolved
Comment on lines
+5
to
+6
| <key>CFBundleIdentifier</key> | ||
| <string></string> |
There was a problem hiding this comment.
빈 CFBundleIdentifier 값은 빌드 문제를 일으킬 수 있습니다.
CFBundleIdentifier가 빈 문자열로 설정되어 있습니다. 일반적으로 이 값은 Xcode 빌드 설정의 $(PRODUCT_BUNDLE_IDENTIFIER)에서 자동으로 설정됩니다. 명시적인 빈 값이 빌드 타임 값을 덮어쓰면 앱 서명이나 배포 시 문제가 발생할 수 있습니다.
이 항목이 의도적으로 추가된 것인지 확인하고, 필요 없다면 제거하는 것을 권장합니다.
🔧 제안: 불필요하면 해당 항목 제거
- <key>CFBundleIdentifier</key>
- <string></string>📝 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
| <key>CFBundleIdentifier</key> | |
| <string></string> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Neki-iOS/Info.plist` around lines 5 - 6, The CFBundleIdentifier key in
Info.plist is set to an empty string which can override the build-time
PRODUCT_BUNDLE_IDENTIFIER and break signing/distribution; remove the
<key>CFBundleIdentifier</key> entry (or replace the empty <string> with
$(PRODUCT_BUNDLE_IDENTIFIER) if an explicit value is required) so the build
settings supply the bundle identifier instead; locate the CFBundleIdentifier
entry in Info.plist and either delete that key/string pair or set its value to
$(PRODUCT_BUNDLE_IDENTIFIER).
Member
There was a problem hiding this comment.
앞서 확인한 대로 스프린트4의 일부 태스크를 별도 Issue-PR에서 다루고, API 조율이 끝나거나 기디측 확인이 끝난 작업만을 해당 PR에서 다룹니다.
뷰 쪽 코드의 모든 수치들을 다 확인할 수가 없어서 대강만 훑어 봤는데 큰 문제는 없어보입니다. 남긴 라인별 코멘트만 확인해주세요! 아마 정 아닌것들은 QA 기간 때 레이아웃 변경 요청이 오겠죠?
본 PR에서는 앨범 복제/이동 기능 추가에 대해 전반적인 플로우만 정해둔 것으로 인지하고 있습니다.
작업 중 히스토리를 나름 추적하고 있었긴 했는데, 아무래도 각종 팀 내 컨택 채널이 너무 많아 구현담당자보다는 코드 이해도가 떨어질 수 밖에 없습니다. 그래서 질문 드립니다.
- AlbumSelectionFeature의 복제/이동 개념과 PhotoImportFeature의 복제/이동 개념이 서로 달라서 두 리듀서로 만든 건가요?
특정 앨범에서 다른 앨범으로 사진 추가하기VS특정 폴더 내에서 네키에 올라간 사진들 올리기- 위 두 피그마 상의 그... 개발 요청이 뭔가 이해가 안되는데, UI/UX 자체는 다르긴하다만은 두 개념이 무엇이 다른지??
- A 앨범에서 B로 사진 추가(이동? 복제?)라는 개념은 같지 않나...? 라는 생각
- 그리고 앨범과 폴더는 서로 다른 개념인건지? 이거 부르는 사람마다, 그리고 문서마다 칭하는 게 달라서 뭔가 맨날 헷갈림. 이미 앞선 수많은 PR에서도 똑같은 질문을 몇번째 한거같긴함 ㅋㅋㅋㅋ.....
Comment on lines
+10
to
+16
| extension Date { | ||
| /// Date를 "yyyy.MM.dd" 형태의 문자열로 변환합니다. | ||
| func toDotFormatString() -> String { | ||
| return DateFormatters.dotFormat.string(from: self) | ||
| } | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
p3)
날짜 형식 표현과 타입 전환에 대해 확장성을 조금 더 챙길 수도 있습니다.
DateFormat + Extension
Comment on lines
+110
to
+119
| case let .tapAlbum(id): | ||
| // 즐겨찾기 앨범은 선택 불가하게 | ||
| guard id != -1 else { return .none } | ||
|
|
||
| if state.selectedAlbumId == id { | ||
| state.selectedAlbumId = nil | ||
| } else { | ||
| state.selectedAlbumId = id | ||
| } | ||
| return .none |
Member
There was a problem hiding this comment.
p3)
'즐겨찾기 앨범의 식별자는 -1로 예약되어 있다'는 사실이 하드코딩으로 인해 감춰지지 않도록 동료 개발자에게 알리는 방법으로, 네이밍에 의미를 담은 지역상수를 선언하는 습관도 적절해보여요!
let isFavoriteAlbum: Bool = id == -1
Comment on lines
105
to
112
| case .onTapDownloadButton: | ||
| guard !state.selectedIDs.isEmpty else { return .none } | ||
|
|
||
| state.isLoading = true | ||
| let urls = state.selectedIDs.compactMap { state.photos[id: $0]?.imageURL } | ||
|
|
||
| return .run { send in | ||
| let count = try await imageDownloadClient.downloadImages(urls: urls) | ||
| await send(.downloadImagesResponse(successCount: count)) | ||
| } |
Member
There was a problem hiding this comment.
p3)
이 라인을 읽고 갑자기 궁금해졌는데, TCA의 .run 이펙트 내 코드는 언제나 백그라운드 스레드에서 동작함이 보장될까요?
만약 그렇다면 selectedIDs에서 imageURL들을 꺼내오는 고차함수들도 백그라운드 스레드에서 돌려서 메인 스레드의 부담을 최대한 덜 수 있지 않을까? 라는 생각이 들었어요.
Member
Author
There was a problem hiding this comment.
옹 확실히 이런 세세한 부분에서도 최적화를 할 수 있겠네요. 반영하겠습니다!
Member
There was a problem hiding this comment.
TCA의 .run 이펙트 내 코드는 언제나 백그라운드 스레드에서 동작함이 보장될까요?
어디까지나 이 추론이 맞다면! 얘기였어요~~~
Comment on lines
+176
to
+185
| case .onTapDuplicateButton: | ||
| state.selectionPurpose = .duplicate | ||
| state.albumSelection = AlbumSelectionFeature.State(uploadCount: state.selectedIDs.count) | ||
| return .none | ||
|
|
||
| case .onTapMoveButton: | ||
| state.selectionPurpose = .move | ||
| state.albumSelection = AlbumSelectionFeature.State(uploadCount: state.selectedIDs.count) | ||
| return .none | ||
|
|
Member
There was a problem hiding this comment.
p3)
혹시 사진 올리기 작업을 QR스캐너든 아카이빙 리듀서든 공통적으로 쓸 수 있도록 자식 리듀서로 추출한 것처럼,
사진 복제/이동 작업이 Archive~~~Feature라고 하는 리듀서들에서 똑같은 일을 한다면 역시 추출할 수 있나요?
TCA 공식문서에서도 그것이 성능상 유리하다고 권장하고 있으니까요!
만약 복제/이동 작업까지는 동일하여 추출했는데, 그걸 쓰는 상위 리듀서의 반응(액션 처리?)가 다르면 고것만 상위 리듀서의 별도 액션으로 Delegate 받을 수 있게??
Member
There was a problem hiding this comment.
p3)
PhotoUploadFeature가 사진 앱 같은 데서 이미지를 가져와 서버에 올려주는 기능이라면,
이 PhotoImportFeature는 이미 네키 앱(또는 서버)에 있는 이미지를 다른 앨범에 올리거나 하는 차이라고 생각하면 적절한가요?
Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift
Outdated
Show resolved
Hide resolved
Comment on lines
+60
to
+65
| .onTapGesture { | ||
| // 즐겨찾기 앨범은 선택하지 못하도록 예외 처리 | ||
| if !album.isFavorite { | ||
| store.send(.tapAlbum(album.id)) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
p3)
UI에서도 1차적으로 막고, Feature에서도 2차적으로 막은 것 너무 좋아요
Comment on lines
+56
to
+60
| .fullScreenCover(item: $store.scope(state: \.albumSelection, action: \.albumSelection)) { selectionStore in | ||
| NavigationStack { | ||
| AlbumSelectionView(store: selectionStore) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
p3)
Coordinator 통제를 벗어난 별도의 Navigation trace가 필요했나요?
Member
Author
There was a problem hiding this comment.
플로우 상 하나의 독립된 뷰가 아니라 액션 선택을 위한 분기점의 역할이라 이전 뷰에 의존적이라고 느껴지기도 했고,
CoordinatorView까지 데이터를 전달하고 다시 받는 과정이 너무 지저분하더라구요.
근데 NavigationStack을 넣어서 구현했던 이유가 분명 있었는데 기억이 안나네... 일단 지금 기준으로 지워도 문제 없이 작동하니까 NavigationStack은 지워두겠습니다!!
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift (4)
180-188:⚠️ Potential issue | 🟠 Major앨범 추가가 실제로는 수행되지 않습니다.
선택된
albumIds를 사용하지 않고 sleep 뒤 success만 보내고 있어서, 시트는 닫히지만 서버 상태는 바뀌지 않습니다. 실제 API를 연결하거나, 연동 전에는 이 경로를 막아두는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 180 - 188, The current didSelectAlbums branch doesn't use the selected album IDs and only waits then sends a success, so the server never updates; replace the placeholder run task with a real call to the API client using the selected IDs (reference state.albumSelection and the action .addToAlbumResponse) and await the network result, dispatching .addToAlbumResponse(.success/ .failure) accordingly, update state.isLoading and clear state.albumSelection on success, and handle errors by logging/setting an error state; if the backend is not ready, instead return .none (or guard and early-return .none) to prevent a misleading success.
213-218:⚠️ Potential issue | 🟠 Major즐겨찾기 실패를 복구할 수 없는 상태입니다.
optimistic update 뒤
try?로 오류를 삼켜서toggleFavoriteResponse실패 분기가 사실상 도달하지 않습니다.ArchiveAlbumDetailFeature처럼 성공/실패 응답을 보내고, 실패 시 로컬 상태를 되돌려 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 213 - 218, The optimistic favorite update swallows errors with try? inside the .run for ArchivePhotoDetailFeature so toggleFavoriteResponse never receives failures; change the .run block handling the toggleFavorite action to await archiveClient.toggleFavorite(photoID: id, request: isFavorite) without try?, capture the result (or throw) and send an explicit .toggleFavoriteResponse(success: Bool, id: id, previousStatus: !isFavorite) (mirroring ArchiveAlbumDetailFeature) so the reducer can revert local state on failure; ensure the case .toggleFavoriteResponse handles success/failure and restores the previous favorite flag on failure.
244-246:⚠️ Potential issue | 🔴 Critical삭제 실패도 성공으로 처리됩니다.
try?뒤에 항상.success를 보내서, 서버 삭제가 실패해도 상세 화면에서는 사진이 사라집니다. 여기서는 반드시 success/failure를 구분해 보내야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 244 - 246, The current delete flow in ArchivePhotoDetailFeature uses try? before calling archiveClient.deletePhotoList and unconditionally sends send(.deletePhotoResponse(.success(()))), so failures are treated as success; change this to a do/catch around archiveClient.deletePhotoList(photoIds: [id]) and only send .deletePhotoResponse(.success(())) on success, and send .deletePhotoResponse(.failure(error)) inside the catch (mapping or wrapping the thrown error as needed) so the reducer can distinguish success vs failure; update references to archiveClient.deletePhotoList and the send(.deletePhotoResponse(...)) calls accordingly.
121-130:⚠️ Potential issue | 🟠 Major메모 저장 실패를 아직도 무시하고 있습니다.
로컬 메모를 먼저 바꾼 뒤
try?로 서버 오류를 버리면, 저장 실패가 UI에서는 성공처럼 보이고 다음 재조회 때 값이 되돌아갑니다. 응답 액션을 추가해서 실패 토스트와 롤백을 처리해 주세요.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift (2)
351-360:⚠️ Potential issue | 🟠 Major가져오기 완료를 가짜 success로 처리하고 있습니다.
photoIDs를 사용하지 않고 delay 뒤.success를 보내서, 실제 복제가 없는데도 가져오기가 끝난 것처럼 보입니다. API가 준비되기 전까지는 이 흐름을 막거나 명시적인 실패/미구현 상태를 보여 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift` around lines 351 - 360, The temporary success path in the reducer handling .photoImport(.presented(.delegate(.didImportPhotos))) incorrectly ignores photoIDs and fakes completion; update the handler in ArchiveAlbumDetailFeature (the switch case for .photoImport .delegate delegateAction) to either call a placeholder error/ unimplemented response or propagate a "not implemented" failure instead of awaiting a dummy sleep and sending .importPhotosResponse(.success(())); use the available photo IDs from state.photoImport (or pass them into the run task) and if the real API is not ready, send .importPhotosResponse(.failure(.notImplemented)) or set state.photoImport to a distinct unimplemented/error state so the UI shows that import is pending/unavailable rather than falsely succeeding.
237-255:⚠️ Potential issue | 🟠 Major앨범 선택 후 성공 UI만 선반영되고 있습니다.
선택한
albumIds를 전혀 쓰지 않고 지연 뒤 success를 보내서, 실제 이동/복제가 없는데도 성공 토스트가 뜹니다. 실제 API 연동 전이라면 이 경로는 실패/미구현으로 처리하는 편이 맞습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift` around lines 237 - 255, The handler for case .albumSelection(.presented(.delegate(delegateAction))) and specifically the case .didSelectAlbums(albumIds) currently ignores albumIds and always sends a success response after a test delay; instead either wire albumIds into the real API call or, if the API is not yet implemented, send a proper failure/NotImplemented response: use the albumIds parameter in the duplicatePhotosResponse or movePhotosResponse run block (or return a .failure with a clear error value) and ensure state.albumSelection/selectionPurpose/isLoading are updated only based on the actual result from send(.duplicatePhotosResponse(...)) or send(.movePhotosResponse(...)) so the UI does not show false success toasts.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift (2)
105-113:⚠️ Potential issue | 🟠 Major다운로드 실패 경로가 아직 없습니다.
downloadImages가 throw 하면 완료 액션이 오지 않아isLoading이 계속true로 남고 선택 상태도 정리되지 않습니다. 실패 액션을 추가해서 성공/실패 모두에서 공통 정리를 해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift` around lines 105 - 113, The .onTapDownloadButton branch currently awaits imageDownloadClient.downloadImages and only sends .downloadImagesResponse on success, so if downloadImages throws state.isLoading stays true and selectedIDs aren’t cleared; wrap the await in a do/catch in the .run block (or use try? with explicit failure send) to send a failure action (e.g. .downloadImagesFailed(error:)) when an error occurs, and add reducer handling for both .downloadImagesResponse and .downloadImagesFailed to set state.isLoading = false and clear state.selectedIDs (and any other common cleanup). Ensure references: .onTapDownloadButton, imageDownloadClient.downloadImages(urls:), .downloadImagesResponse and the new .downloadImagesFailed action are updated accordingly.
189-206:⚠️ Potential issue | 🟠 Major이동/복제가 아직 실제로는 아무 것도 하지 않습니다.
선택된
albumIds를 사용하지 않고 1초 딜레이 뒤 success만 보내고 있어서, 토스트만 성공으로 보이고 서버 상태는 바뀌지 않습니다. 실제 API를 연결하거나, 연동 전이라면 명시적인 미구현/실패 경로로 막아두는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift` around lines 189 - 206, The current handler for case .albumSelection(.presented(.delegate(delegateAction))) merely waits 1s and sends success without using the selected album IDs or calling any API; fix it by extracting the selected album IDs from state.albumSelection (or the delegate payload) when handling .didSelectAlbums and then call the real API client (e.g., call your movePhotos/duplicatePhotos network methods) and await their result before sending .movePhotosResponse or .duplicatePhotosResponse; if the API client is not yet implemented, return a clear failure response (send .movePhotosResponse(.failure(.notImplemented)) / .duplicatePhotosResponse(.failure(...))) and set state.isLoading = false and clear state.albumSelection/state.selectionPurpose accordingly so the UI does not show a phantom success; reference functions/types: didSelectAlbums, state.selectionPurpose, state.albumSelection, duplicatePhotosResponse, movePhotosResponse.
🧹 Nitpick comments (1)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/String+.swift (1)
75-80:timeZone을 명시적으로 설정하는 것을 고려하세요.Line 75-80의
dotFormat은 명시된timeZone없이 시스템 기본 시간대를 사용합니다. 기기의 시간대 설정이 변경되면 동일한Date객체가 다르게 포맷될 수 있습니다. 일관된 날짜 표시를 원한다면timeZone을 명시적으로 설정하는 것이 좋습니다.💡 제안 수정안
static let dotFormat: DateFormatter = { let formatter = DateFormatter() formatter.dateFormat = "yyyy.MM.dd" formatter.locale = Locale(identifier: "ko_KR") + formatter.timeZone = TimeZone(identifier: "Asia/Seoul") return formatter }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/String`+.swift around lines 75 - 80, The DateFormatter dotFormat currently relies on the device default timezone; set an explicit timezone to ensure consistent formatting (e.g., for Korean locale use TimeZone(identifier: "Asia/Seoul") or another agreed-upon zone) by assigning formatter.timeZone inside the dotFormat initializer. Update the static let dotFormat block (symbol: dotFormat) to set formatter.timeZone to the chosen TimeZone so the same Date always formats identically across devices.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift`:
- Around line 108-110: Album 목록 조회 실패 분기(case .fetchAlbumsResponse(.failure)
inside AlbumSelectionFeature)가 실패를 조용히 무시하고 있어 사용자에게 피드백이 없습니다; 이 분기에서
state.isFetching = false를 유지하되 delegate(.showToast(...))를 호출해 실패 메시지(예: 네트워크
오류)를 보내고, 필요하면 재시도 신호(예: 보내진 액션으로 fetchAlbums 재시도)를 함께 트리거하도록 변경하세요 so that UI
shows a toast and optionally dispatches a retry action instead of leaving a
blank screen.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift`:
- Around line 290-297: The onTapDownloadButton handler currently calls
imageDownloadClient.downloadImages inside a Task and only sends
.downloadImagesResponse on success, so if downloadImages throws the
state.isLoading flag never gets cleared; modify the .onTapDownloadButton case to
await downloadImages inside a do/catch (or use Result) and always send a
response action on both paths (e.g., send .downloadImagesResponse(successCount:
count) on success and send a new failure action like
.downloadImagesFailed(error:) or .downloadImagesResponse(successCount: 0, error:
error) on catch), and ensure the reducer handles that failure action by setting
state.isLoading = false (and preserving/setting error state if you add one);
reference symbols: .onTapDownloadButton,
imageDownloadClient.downloadImages(urls:), .downloadImagesResponse (and add
.downloadImagesFailed if you choose that name).
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`:
- Around line 222-229: The .onTapDownload handler sets state.isLoading = true
then calls imageDownloadClient.downloadImages but if downloadImages throws the
existing .downloadImageResponse is never sent so isLoading never resets; wrap
the await call in a do-catch inside the .run closure (or use TaskResult) and
send a new failure action (e.g., .downloadImageFailed(error) or a
Result-wrapping .downloadImageResponse) from the catch path; then update the
reducer to handle that failure action (or the Result case) and set
state.isLoading = false there just like the success case so loading is cleared
on both success and error, referencing .onTapDownload, downloadImages,
downloadImageResponse (or the new failure action) in your changes.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`:
- Around line 61-65: The tap-block condition is inverted: currently it uses ||
so taps are allowed if either not-favorite OR not-current, which still sends tap
for favorite or current albums; change the guard to require both "not favorite"
AND "not current" before sending the action. Locate the onTapGesture use of
album.isFavorite, album.id and store.currentAlbumId and update the if to use &&
(or equivalently check that album.isFavorite || album.id == store.currentAlbumId
is false) before calling store.send(.tapAlbum(album.id)).
- Around line 115-129: The UI always shows "앨범에 추가"/"업로드" regardless of
selectionPurpose; update AlbumSelectionView to switch copy when selectionPurpose
== .move: change the header Text("앨범에 추가") to use a conditional label (e.g.,
"앨범으로 이동" when selectionPurpose == .move, otherwise "앨범에 추가") and change the
Button label text from "\(store.uploadCount)장 업로드" to a move-specific variant
(e.g., "\(store.uploadCount)장 이동" or "이동") when selectionPurpose == .move; keep
the same disabled logic (store.selectedAlbumIDs.isEmpty) and foregroundStyle
handling so enabled/disabled styling remains consistent.
---
Duplicate comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift`:
- Around line 351-360: The temporary success path in the reducer handling
.photoImport(.presented(.delegate(.didImportPhotos))) incorrectly ignores
photoIDs and fakes completion; update the handler in ArchiveAlbumDetailFeature
(the switch case for .photoImport .delegate delegateAction) to either call a
placeholder error/ unimplemented response or propagate a "not implemented"
failure instead of awaiting a dummy sleep and sending
.importPhotosResponse(.success(())); use the available photo IDs from
state.photoImport (or pass them into the run task) and if the real API is not
ready, send .importPhotosResponse(.failure(.notImplemented)) or set
state.photoImport to a distinct unimplemented/error state so the UI shows that
import is pending/unavailable rather than falsely succeeding.
- Around line 237-255: The handler for case
.albumSelection(.presented(.delegate(delegateAction))) and specifically the case
.didSelectAlbums(albumIds) currently ignores albumIds and always sends a success
response after a test delay; instead either wire albumIds into the real API call
or, if the API is not yet implemented, send a proper failure/NotImplemented
response: use the albumIds parameter in the duplicatePhotosResponse or
movePhotosResponse run block (or return a .failure with a clear error value) and
ensure state.albumSelection/selectionPurpose/isLoading are updated only based on
the actual result from send(.duplicatePhotosResponse(...)) or
send(.movePhotosResponse(...)) so the UI does not show false success toasts.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift`:
- Around line 105-113: The .onTapDownloadButton branch currently awaits
imageDownloadClient.downloadImages and only sends .downloadImagesResponse on
success, so if downloadImages throws state.isLoading stays true and selectedIDs
aren’t cleared; wrap the await in a do/catch in the .run block (or use try? with
explicit failure send) to send a failure action (e.g.
.downloadImagesFailed(error:)) when an error occurs, and add reducer handling
for both .downloadImagesResponse and .downloadImagesFailed to set
state.isLoading = false and clear state.selectedIDs (and any other common
cleanup). Ensure references: .onTapDownloadButton,
imageDownloadClient.downloadImages(urls:), .downloadImagesResponse and the new
.downloadImagesFailed action are updated accordingly.
- Around line 189-206: The current handler for case
.albumSelection(.presented(.delegate(delegateAction))) merely waits 1s and sends
success without using the selected album IDs or calling any API; fix it by
extracting the selected album IDs from state.albumSelection (or the delegate
payload) when handling .didSelectAlbums and then call the real API client (e.g.,
call your movePhotos/duplicatePhotos network methods) and await their result
before sending .movePhotosResponse or .duplicatePhotosResponse; if the API
client is not yet implemented, return a clear failure response (send
.movePhotosResponse(.failure(.notImplemented)) /
.duplicatePhotosResponse(.failure(...))) and set state.isLoading = false and
clear state.albumSelection/state.selectionPurpose accordingly so the UI does not
show a phantom success; reference functions/types: didSelectAlbums,
state.selectionPurpose, state.albumSelection, duplicatePhotosResponse,
movePhotosResponse.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`:
- Around line 180-188: The current didSelectAlbums branch doesn't use the
selected album IDs and only waits then sends a success, so the server never
updates; replace the placeholder run task with a real call to the API client
using the selected IDs (reference state.albumSelection and the action
.addToAlbumResponse) and await the network result, dispatching
.addToAlbumResponse(.success/ .failure) accordingly, update state.isLoading and
clear state.albumSelection on success, and handle errors by logging/setting an
error state; if the backend is not ready, instead return .none (or guard and
early-return .none) to prevent a misleading success.
- Around line 213-218: The optimistic favorite update swallows errors with try?
inside the .run for ArchivePhotoDetailFeature so toggleFavoriteResponse never
receives failures; change the .run block handling the toggleFavorite action to
await archiveClient.toggleFavorite(photoID: id, request: isFavorite) without
try?, capture the result (or throw) and send an explicit
.toggleFavoriteResponse(success: Bool, id: id, previousStatus: !isFavorite)
(mirroring ArchiveAlbumDetailFeature) so the reducer can revert local state on
failure; ensure the case .toggleFavoriteResponse handles success/failure and
restores the previous favorite flag on failure.
- Around line 244-246: The current delete flow in ArchivePhotoDetailFeature uses
try? before calling archiveClient.deletePhotoList and unconditionally sends
send(.deletePhotoResponse(.success(()))), so failures are treated as success;
change this to a do/catch around archiveClient.deletePhotoList(photoIds: [id])
and only send .deletePhotoResponse(.success(())) on success, and send
.deletePhotoResponse(.failure(error)) inside the catch (mapping or wrapping the
thrown error as needed) so the reducer can distinguish success vs failure;
update references to archiveClient.deletePhotoList and the
send(.deletePhotoResponse(...)) calls accordingly.
---
Nitpick comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/String`+.swift:
- Around line 75-80: The DateFormatter dotFormat currently relies on the device
default timezone; set an explicit timezone to ensure consistent formatting
(e.g., for Korean locale use TimeZone(identifier: "Asia/Seoul") or another
agreed-upon zone) by assigning formatter.timeZone inside the dotFormat
initializer. Update the static let dotFormat block (symbol: dotFormat) to set
formatter.timeZone to the chosen TimeZone so the same Date always formats
identically across devices.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7d0e2b33-5bda-448d-83b5-b2aaa70a7727
📒 Files selected for processing (12)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/Date+.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/String+.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAlbumDetailView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAllPhotosView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveFavoriteAlbumView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchivePhotoDetailView.swift
✅ Files skipped from review due to trivial changes (1)
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/Extension/Date+.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAllPhotosView.swift
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveFavoriteAlbumView.swift
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swift
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchivePhotoDetailView.swift
Comment on lines
+108
to
+110
| case .fetchAlbumsResponse(.failure): | ||
| state.isFetching = false | ||
| return .none |
There was a problem hiding this comment.
앨범 목록 조회 실패를 조용히 무시하면 빈 화면만 남습니다.
이 분기에서 토스트나 재시도 신호가 없어서, 네트워크 실패 시 사용자는 왜 아무 것도 보이지 않는지 알 수 없습니다. 이미 delegate(.showToast) 경로가 있으니 최소한 실패 피드백은 보내는 편이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift`
around lines 108 - 110, Album 목록 조회 실패 분기(case .fetchAlbumsResponse(.failure)
inside AlbumSelectionFeature)가 실패를 조용히 무시하고 있어 사용자에게 피드백이 없습니다; 이 분기에서
state.isFetching = false를 유지하되 delegate(.showToast(...))를 호출해 실패 메시지(예: 네트워크
오류)를 보내고, 필요하면 재시도 신호(예: 보내진 액션으로 fetchAlbums 재시도)를 함께 트리거하도록 변경하세요 so that UI
shows a toast and optionally dispatches a retry action instead of leaving a
blank screen.
Comment on lines
290
to
297
| case .onTapDownloadButton: | ||
| guard !state.selectedIDs.isEmpty else { return .none } | ||
| state.isLoading = true | ||
|
|
||
| let urls = state.selectedIDs.compactMap { state.photos[id: $0]?.imageURL } | ||
|
|
||
| return .run { send in | ||
| return .run { [photos = state.photos, selectedIDs = state.selectedIDs] send in | ||
| let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL } | ||
|
|
||
| let count = try await imageDownloadClient.downloadImages(urls: urls) | ||
| await send(.downloadImagesResponse(successCount: count)) |
There was a problem hiding this comment.
다운로드 실패 시 로딩이 풀리지 않습니다.
imageDownloadClient.downloadImages가 throw 하면 응답 액션이 오지 않아 isLoading이 계속 true로 남습니다. 여기에도 실패 액션을 추가해서 로딩 상태를 항상 정리해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift`
around lines 290 - 297, The onTapDownloadButton handler currently calls
imageDownloadClient.downloadImages inside a Task and only sends
.downloadImagesResponse on success, so if downloadImages throws the
state.isLoading flag never gets cleared; modify the .onTapDownloadButton case to
await downloadImages inside a do/catch (or use Result) and always send a
response action on both paths (e.g., send .downloadImagesResponse(successCount:
count) on success and send a new failure action like
.downloadImagesFailed(error:) or .downloadImagesResponse(successCount: 0, error:
error) on catch), and ensure the reducer handles that failure action by setting
state.isLoading = false (and preserving/setting error state if you add one);
reference symbols: .onTapDownloadButton,
imageDownloadClient.downloadImages(urls:), .downloadImagesResponse (and add
.downloadImagesFailed if you choose that name).
Comment on lines
222
to
229
| case .onTapDownload: | ||
| guard let url = state.currentItem?.imageURL else { | ||
| return .none | ||
| } | ||
|
|
||
| guard let url = state.currentItem?.imageURL else { return .none } | ||
| state.isLoading = true | ||
|
|
||
| return .run { send in | ||
| return .run { [url = url] send in | ||
| let count = try await imageDownloadClient.downloadImages(urls: [url]) | ||
| await send(.downloadImageResponse(successCount: count)) | ||
| } |
There was a problem hiding this comment.
다운로드 예외 시 로딩이 영구히 남습니다.
downloadImages가 throw 하면 downloadImageResponse가 오지 않아 isLoading이 false로 복구되지 않습니다. 실패 액션을 추가해서 성공/실패 모두에서 로딩을 정리해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`
around lines 222 - 229, The .onTapDownload handler sets state.isLoading = true
then calls imageDownloadClient.downloadImages but if downloadImages throws the
existing .downloadImageResponse is never sent so isLoading never resets; wrap
the await call in a do-catch inside the .run closure (or use TaskResult) and
send a new failure action (e.g., .downloadImageFailed(error) or a
Result-wrapping .downloadImageResponse) from the catch path; then update the
reducer to handle that failure action (or the Result case) and set
state.isLoading = false there just like the success case so loading is cleared
on both success and error, referencing .onTapDownload, downloadImages,
downloadImageResponse (or the new failure action) in your changes.
Comment on lines
+61
to
+65
| .onTapGesture { | ||
| // 즐겨찾기 앨범이랑 현재 앨범은 선택하지 못하도록 예외 처리 | ||
| if !album.isFavorite || !(album.id == store.currentAlbumId) { | ||
| store.send(.tapAlbum(album.id)) | ||
| } |
There was a problem hiding this comment.
탭 차단 조건이 의도와 반대로 작성됐습니다.
지금 식은 둘 중 하나만 아니어도 tapAlbum을 보내서, 주석과 달리 즐겨찾기 앨범/현재 앨범 탭이 reducer까지 전달됩니다. UI에서 1차 차단하려면 && 조건이어야 합니다.
수정 예시
- if !album.isFavorite || !(album.id == store.currentAlbumId) {
+ if !album.isFavorite && album.id != store.currentAlbumId {
store.send(.tapAlbum(album.id))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`
around lines 61 - 65, The tap-block condition is inverted: currently it uses ||
so taps are allowed if either not-favorite OR not-current, which still sends tap
for favorite or current albums; change the guard to require both "not favorite"
AND "not current" before sending the action. Locate the onTapGesture use of
album.isFavorite, album.id and store.currentAlbumId and update the if to use &&
(or equivalently check that album.isFavorite || album.id == store.currentAlbumId
is false) before calling store.send(.tapAlbum(album.id)).
Comment on lines
+115
to
+129
| Button { | ||
| store.send(.tapConfirm) | ||
| } label: { | ||
| Text("\(store.uploadCount)장 업로드") | ||
| .nekiFont(.body16SemiBold) | ||
| .foregroundStyle(store.selectedAlbumIDs.isEmpty ? .gray200 : .primary500) | ||
| } | ||
| .disabled(store.selectedAlbumIDs.isEmpty) | ||
| } | ||
| .frame(height: 54) | ||
| .padding(.horizontal, 20) | ||
|
|
||
| Text("앨범에 추가") | ||
| .nekiFont(.title20SemiBold) | ||
| .foregroundStyle(.gray900) |
There was a problem hiding this comment.
이 화면의 문구가 이동 플로우를 구분하지 못합니다.
selectionPurpose가 .move여도 제목과 CTA가 계속 “앨범에 추가”/“업로드”로 보입니다. 이동 플로우에서는 문구를 바꿔야 사용자가 실제 동작을 오해하지 않습니다.
수정 예시
- Text("\(store.uploadCount)장 업로드")
+ Text(store.selectionPurpose == .move ? "\(store.uploadCount)장 이동" : "\(store.uploadCount)장 업로드")
.nekiFont(.body16SemiBold)
.foregroundStyle(store.selectedAlbumIDs.isEmpty ? .gray200 : .primary500)
@@
- Text("앨범에 추가")
+ Text(store.selectionPurpose == .move ? "앨범으로 이동" : "앨범에 추가")
.nekiFont(.title20SemiBold)
.foregroundStyle(.gray900)📝 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
| Button { | |
| store.send(.tapConfirm) | |
| } label: { | |
| Text("\(store.uploadCount)장 업로드") | |
| .nekiFont(.body16SemiBold) | |
| .foregroundStyle(store.selectedAlbumIDs.isEmpty ? .gray200 : .primary500) | |
| } | |
| .disabled(store.selectedAlbumIDs.isEmpty) | |
| } | |
| .frame(height: 54) | |
| .padding(.horizontal, 20) | |
| Text("앨범에 추가") | |
| .nekiFont(.title20SemiBold) | |
| .foregroundStyle(.gray900) | |
| Button { | |
| store.send(.tapConfirm) | |
| } label: { | |
| Text(store.selectionPurpose == .move ? "\(store.uploadCount)장 이동" : "\(store.uploadCount)장 업로드") | |
| .nekiFont(.body16SemiBold) | |
| .foregroundStyle(store.selectedAlbumIDs.isEmpty ? .gray200 : .primary500) | |
| } | |
| .disabled(store.selectedAlbumIDs.isEmpty) | |
| } | |
| .frame(height: 54) | |
| .padding(.horizontal, 20) | |
| Text(store.selectionPurpose == .move ? "앨범으로 이동" : "앨범에 추가") | |
| .nekiFont(.title20SemiBold) | |
| .foregroundStyle(.gray900) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`
around lines 115 - 129, The UI always shows "앨범에 추가"/"업로드" regardless of
selectionPurpose; update AlbumSelectionView to switch copy when selectionPurpose
== .move: change the header Text("앨범에 추가") to use a conditional label (e.g.,
"앨범으로 이동" when selectionPurpose == .move, otherwise "앨범에 추가") and change the
Button label text from "\(store.uploadCount)장 업로드" to a move-specific variant
(e.g., "\(store.uploadCount)장 이동" or "이동") when selectionPurpose == .move; keep
the same disabled logic (store.selectedAlbumIDs.isEmpty) and foregroundStyle
handling so enabled/disabled styling remains consistent.
Member
Author
백엔드에서 앨범을 칭하는 이름 = 폴더 그런데 클라에서는 피그마에 맞춰 앨범이란 네이밍으로 Presentation을 짜다가 스웨거에 맞춰 DTO를 만들면서 Folder 라는 네이밍도 사용하다보니 헷갈려서 혼용하고 있는 상태가 됨. 알잘딱깔센이 필요한 지점입니다 |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift (4)
176-182:⚠️ Potential issue | 🟠 Major즐겨찾기 토글 실패 시 UI 상태가 서버와 불일치합니다.
try?로 에러를 무시하면서toggleFavoriteResponse액션을 보내지 않아, 네트워크 실패 시 optimistic update가 롤백되지 않습니다.🔧 수정 제안
case .onTapFavorite: guard let item = state.currentItem else { return .none } let newStatus = !item.isFavorite state.photos[id: item.id]?.isFavorite = newStatus return .run { [id = item.id, isFavorite = newStatus] send in - try? await archiveClient.toggleFavorite(photoID: id, request: isFavorite) + do { + try await archiveClient.toggleFavorite(photoID: id, request: isFavorite) + await send(.toggleFavoriteResponse(photoID: id, result: .success(()))) + } catch { + await send(.toggleFavoriteResponse(photoID: id, result: .failure(error))) + } } + +case let .toggleFavoriteResponse(photoID, .failure): + state.photos[id: photoID]?.isFavorite.toggle() + return .send(.delegate(.showToast(NekiToastItem("즐겨찾기 변경에 실패했어요", style: .error))))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 176 - 182, The optimistic favorite toggle in ArchivePhotoDetailFeature.onTapFavorite updates state immediately but swallows errors with try?, so failures never dispatch a response to rollback; modify the .onTapFavorite effect (the return .run block that calls archiveClient.toggleFavorite(photoID:request:)) to await the network call and send a toggleFavoriteResponse action containing the photo id and a success flag or error, and on failure dispatch the response that flips state back (use the captured id and isFavorite to indicate the intended new state and let the reducer handle rollback on failure instead of ignoring errors).
200-205:⚠️ Potential issue | 🔴 Critical삭제 실패도 성공으로 처리됩니다.
try?사용으로deletePhotoList실패가 무시되고 항상.success가 전송됩니다. 서버 삭제가 실패해도 로컬에서 사진이 제거되는 문제가 있습니다.🔧 수정 제안
case .onTapDelete: guard let id = state.currentItem?.id else { return .none } return .run { send in - try? await archiveClient.deletePhotoList(photoIds: [id]) - await send(.deletePhotoResponse(.success(()))) + do { + try await archiveClient.deletePhotoList(photoIds: [id]) + await send(.deletePhotoResponse(.success(()))) + } catch { + await send(.deletePhotoResponse(.failure(error))) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 200 - 205, The onTapDelete branch in ArchivePhotoDetailFeature currently uses try? which swallows errors and always sends .deletePhotoResponse(.success(())); change it to a do-catch around await archiveClient.deletePhotoList(photoIds: [id]) in the .onTapDelete effect, and on success send .deletePhotoResponse(.success(())) but on failure send .deletePhotoResponse(.failure(error)) (or map the thrown error to your feature's Failure type) so server deletion failures are propagated and handled instead of being treated as success.
116-125:⚠️ Potential issue | 🟠 Major메모 저장 실패를 무시하면 데이터 불일치가 발생합니다.
try?로 서버 오류를 무시하고 로컬 상태만 변경합니다. 저장 실패 시 UI는 성공처럼 보이지만, 다음 조회 시 값이 되돌아갈 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 116 - 125, The current doneMemoEditing handling uses try? await archiveClient.updatePhotoMemo(...) which ignores server errors after mutating state.photos[...] and causes silent divergence; capture the previous memo (e.g., let previousMemo = state.photos[id: photoID]?.memo) before mutating, remove the try?, perform the async update inside a do/catch in the .run block calling await archiveClient.updatePhotoMemo(photoID:memo:), and on catch send a failure action (e.g., .memoSaveFailed(photoID, previousMemo, error)) that reverts state.photos[id: photoID]?.memo = previousMemo and sets an error/UI flag (e.g., state.isMemoEditing or state.isMemoExpanded) so the UI reflects the failed save instead of silently succeeding.
187-193:⚠️ Potential issue | 🟠 Major다운로드 실패 시 로딩 상태가 복구되지 않습니다.
imageDownloadClient.downloadImages가 throw하면downloadImageResponse가 전송되지 않아isLoading이 계속 true로 남습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift` around lines 187 - 193, The .onTapDownload branch sets state.isLoading = true but if imageDownloadClient.downloadImages throws, no .downloadImageResponse is sent and isLoading remains true; wrap the download call inside a do/catch in the .run closure (or use Result/Task initializer) so you always send a response on both success and failure (e.g., send .downloadImageResponse(successCount: count) on success and send a failure response (or .downloadImageResponse(successCount: 0) until a dedicated failure case exists) in the catch), referencing ArchivePhotoDetailFeature, the .onTapDownload action, imageDownloadClient.downloadImages, and the downloadImageResponse action so the UI can set isLoading back to false.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift (1)
116-159:⚠️ Potential issue | 🟠 Major앨범 전환 시 이전 fetch 응답이 현재 앨범을 덮어쓸 수 있습니다.
selectAlbum액션에서state.photos.removeAll()후.fetchPhotos를 보내지만, 기존 요청이 완료되면fetchPhotosResponse가 새 앨범이 아닌 이전 앨범의 데이터로state.photos를 덮어씁니다.Effect cancellation ID를 사용하거나, 응답에 요청한 album ID를 포함시켜 현재 선택과 일치할 때만 반영하도록 수정이 필요합니다.
🔧 수정 방향 예시
+private enum CancelID { case fetchPhotos } + case .fetchPhotos: guard !state.isFetchingPhotos else { return .none } state.isFetchingPhotos = true let targetFolderId = state.selectedAlbum?.id == -1 ? nil : state.selectedAlbum?.id let sortOrder = "DESC" return .run { [id = state.selectedAlbum?.id] send in if id == -1 { await send(.fetchPhotosResponse(Result { try await archiveClient.fetchFavoritePhotoList(20, sortOrder) })) } else { await send(.fetchPhotosResponse(Result { try await archiveClient.fetchPhotoList(folderId: targetFolderId, size: 20, sortOrder: sortOrder) })) } } + .cancellable(id: CancelID.fetchPhotos, cancelInFlight: true) case let .selectAlbum(album): state.selectedIDs.removeAll() state.selectedAlbum = album state.isDropdownOpen = false state.photos.removeAll() + return .concatenate( + .cancel(id: CancelID.fetchPhotos), + .send(.fetchPhotos) + ) - return .send(.fetchPhotos)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift` around lines 116 - 159, When switching albums in selectAlbum, in-flight fetchPhotos effects can overwrite the new album's photos; modify the fetchPhotos effect (the .run block that sends .fetchPhotosResponse) to either attach and send the requested album id with the response or use a cancellable effect id, then cancel previous fetches when selectAlbum runs; specifically update fetchPhotos to include the selected album id in the Result (or call .cancellable(id: some FetchPhotosID) and in selectAlbum call .cancel(id: FetchPhotosID) before sending .fetchPhotos) and update fetchPhotosResponse handling to only apply entities when the response's album id matches state.selectedAlbum?.id (or when the effect wasn't canceled).Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift (2)
120-134:⚠️ Potential issue | 🟡 Minor이동/복제에 따른 UI 문구 분기가 필요합니다.
selectionPurpose가.move여도 제목과 버튼이 "앨범에 추가"/"업로드"로 표시됩니다. 이동 플로우에서는 "앨범으로 이동"/"(count)장 이동"으로 표시해야 사용자가 실제 동작을 정확히 인지할 수 있습니다.🔧 수정 제안
-Text("앨범에 추가") +Text(store.selectionPurpose == .move ? "앨범으로 이동" : "앨범에 추가") .nekiFont(.title20SemiBold) .foregroundStyle(.gray900) -Text("\(store.uploadCount)장 업로드") +Text(store.selectionPurpose == .move ? "\(store.uploadCount)장 이동" : "\(store.uploadCount)장 추가") .nekiFont(.body16SemiBold) .foregroundStyle(store.selectedAlbumIDs.isEmpty ? .gray200 : .primary500)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift` around lines 120 - 134, The title and confirm button text currently always read "앨범에 추가" and "\(store.uploadCount)장 업로드" even when the flow is a move; update the UI to branch on selectionPurpose (e.g., check selectionPurpose == .move) and when true change the title Text to "앨범으로 이동" and the Button label to "\(store.uploadCount)장 이동" while preserving the existing styling and the .disabled(store.selectedAlbumIDs.isEmpty) behavior; locate these changes around the Button and the Text("앨범에 추가") usages so the string variants are shown in the move vs add flows.
61-66:⚠️ Potential issue | 🟡 Minor탭 차단 조건이 의도와 다르게 동작합니다.
현재 조건
!album.isFavorite || !(album.id == store.currentAlbumId)는 OR 연산으로, 둘 중 하나만 만족해도 탭이 허용됩니다. 즐겨찾기 앨범과 현재 앨범 모두를 차단하려면 AND 조건이어야 합니다.🔧 수정 제안
.onTapGesture { // 즐겨찾기 앨범이랑 현재 앨범은 선택하지 못하도록 예외 처리 - if !album.isFavorite || !(album.id == store.currentAlbumId) { + if !album.isFavorite && album.id != store.currentAlbumId { store.send(.tapAlbum(album.id)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift` around lines 61 - 66, The tap-block condition is using OR so taps are allowed if either not favorite or not current album; change it to require both checks to be false before allowing a tap: in AlbumSelectionView.swift update the condition around album.isFavorite and album.id vs store.currentAlbumId to use AND (e.g. if !album.isFavorite && album.id != store.currentAlbumId) and then call store.send(.tapAlbum(album.id)) only when that combined condition is true.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift (1)
100-107:⚠️ Potential issue | 🟠 Major다운로드 실패 시
isLoading이 영구히 true로 남습니다.
imageDownloadClient.downloadImages가 throw하면downloadImagesResponse가 전송되지 않습니다. 이전 리뷰에서 지적된 문제가 아직 해결되지 않았습니다.🔧 수정 제안
case .onTapDownloadButton: guard !state.selectedIDs.isEmpty else { return .none } state.isLoading = true return .run { [photos = state.photos, selectedIDs = state.selectedIDs] send in - let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL } - let count = try await imageDownloadClient.downloadImages(urls: urls) - await send(.downloadImagesResponse(successCount: count)) + do { + let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL } + let count = try await imageDownloadClient.downloadImages(urls: urls) + await send(.downloadImagesResponse(successCount: count)) + } catch { + await send(.downloadImagesResponse(successCount: 0)) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift` around lines 100 - 107, The onTapDownloadButton effect can throw, leaving state.isLoading true because downloadImagesResponse isn't sent on error; update the .onTapDownloadButton .run block in ArchiveAllPhotosFeature (case .onTapDownloadButton) to wrap the try await imageDownloadClient.downloadImages call in a do-catch and ensure you always send a response (e.g., send .downloadImagesResponse with appropriate successCount or an error variant) in both success and catch paths, so the reducer that clears state.isLoading when handling .downloadImagesResponse will always run.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift (1)
115-117:⚠️ Potential issue | 🟠 Major앨범 목록 조회 실패 시 사용자 피드백이 없습니다.
네트워크 실패 시
isFetching만 false로 설정하고 에러 피드백 없이 종료됩니다. 사용자는 빈 화면만 보게 되며 왜 앨범이 표시되지 않는지 알 수 없습니다.🔧 수정 제안
case .fetchAlbumsResponse(.failure): state.isFetching = false - return .none + return .send(.delegate(.showToast(NekiToastItem("앨범을 불러오지 못했어요", style: .error))))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift` around lines 115 - 117, The failure arm for AlbumSelectionFeature's action .fetchAlbumsResponse currently only sets state.isFetching = false and returns .none; update that case to capture the failure (e.g., .fetchAlbumsResponse(.failure(let error))) and set a user-facing state field (create one if missing) such as state.fetchError or state.alert = AlertState(title: "Failed to load albums", message: error.localizedDescription) so the UI can show an error/alert, then set state.isFetching = false and return .none (or send a follow-up action to present the alert) — adjust names to match your state (e.g., state.fetchError or state.alert) and ensure AlbumSelectionFeature/Action/.fetchAlbumsResponse references are updated accordingly.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift (1)
220-227:⚠️ Potential issue | 🟠 Major다운로드 실패 시 로딩 상태가 복구되지 않습니다.
다른 Feature들과 동일하게
imageDownloadClient.downloadImages가 throw하면downloadImagesResponse가 전송되지 않습니다. 일관된 에러 처리 패턴 적용이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift` around lines 220 - 227, 현재 .onTapDownloadButton의 Task에서 imageDownloadClient.downloadImages가 throw하면 .downloadImagesResponse가 전송되지 않아 state.isLoading가 복구되지 않습니다; .run 블록 내부에서 downloadImages 호출을 do/catch로 감싸고, 성공 시 기존처럼 await send(.downloadImagesResponse(successCount: count))를 호출하며 실패(catch) 시에도 await send(.downloadImagesResponse(successCount: 0)) 또는 프로젝트의 에러 응답 케이스로 실패 이벤트를 전송하도록 변경해 state.isLoading이 reducer에서 항상 false로 복구되도록 하세요; 관련 식별자: case .onTapDownloadButton, imageDownloadClient.downloadImages(urls:), downloadImagesResponse(successCount:), state.isLoading, state.selectedIDs, state.photos.
🧹 Nitpick comments (2)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift (1)
122-125: 즐겨찾기 앨범 ID-1하드코딩에 대한 명시적 표현을 고려하세요.
id != -1조건이 즐겨찾기 앨범 예외 처리임이 명확하지 않습니다. 가독성을 위해 지역 상수나 명명된 조건을 사용하면 동료 개발자가 이해하기 쉽습니다.♻️ 예시
case let .tapAlbum(id): + let isFavoriteAlbum = id == -1 // 즐겨찾기 앨범이거나, 현재 진입한 앨범이면 선택 무시 - guard id != -1 else { return .none } + guard !isFavoriteAlbum else { return .none } guard id != state.currentAlbumId else { return .none }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift` around lines 122 - 125, The guard using a magic value (`id != -1`) in the .tapAlbum(id) case is unclear; replace the hardcoded -1 with a named constant or helper to express "favorite album" (e.g., define a local constant or AlbumConstants.favoriteId or a helper method isFavoriteAlbum(_:)) and use that in the guards alongside state.currentAlbumId so the intent in AlbumSelectionFeature.swift (the .tapAlbum(id) case) is explicit and readable.Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift (1)
172-183: 업로드 API 연동 전 placeholder 상태입니다.현재
tapUpload는 실제 API 호출 없이 1초 딜레이 후 성공을 반환합니다. TODO 주석대로 실제 API 연동이 필요하며, 연동 전까지는 사용자에게 미구현 상태임을 명시하는 것이 안전합니다.API 연동 코드를 작성해 드릴까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift` around lines 172 - 183, The tapUpload branch currently returns success after a placeholder 1s sleep; replace this with a real API call to archiveClient.duplicatePhoto (or the actual upload method) using the collected ids and targetAlbumId (handle targetId optional/unwrapping), await the call inside the .run task, catch and send a failure action (e.g., .taskFailed(error:)) on error and include error details in the sent message, and ensure state.isLoading is cleared (false) in both success and failure paths; keep the guard using state.isUploadEnabled and use state.selectedIDs/targetAlbumId within the task rather than capturing mutable state directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swift`:
- Around line 200-206: The .onTapDownloadButton run effect currently awaits
imageDownloadClient.downloadImages and if it throws the run will exit without
sending .downloadImagesResponse, leaving isLoading true; wrap the await call in
do/catch (or add try? and handle nil) and always send a .downloadImagesResponse
in both success and failure paths (e.g. send successCount: count on success and
successCount: 0 or an explicit failure case on error), updating the closure that
captures state.photos/selectedIDs and the send(.downloadImagesResponse(...))
invocation accordingly; apply the same pattern to the analogous case in
ArchiveAllPhotosFeature to ensure loading is cleared on errors.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`:
- Line 14: The `@State` property addAlbumSheetPresented in AlbumSelectionView is
not marked private; change its declaration (the addAlbumSheetPresented `@State`
var) to private to follow SwiftUI/SwiftLint conventions and update any internal
references within AlbumSelectionView to use the now-private property (if
external views need control, expose a Binding or a separate `@Binding` property or
callback instead of making the state non-private).
---
Duplicate comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift`:
- Around line 115-117: The failure arm for AlbumSelectionFeature's action
.fetchAlbumsResponse currently only sets state.isFetching = false and returns
.none; update that case to capture the failure (e.g.,
.fetchAlbumsResponse(.failure(let error))) and set a user-facing state field
(create one if missing) such as state.fetchError or state.alert =
AlertState(title: "Failed to load albums", message: error.localizedDescription)
so the UI can show an error/alert, then set state.isFetching = false and return
.none (or send a follow-up action to present the alert) — adjust names to match
your state (e.g., state.fetchError or state.alert) and ensure
AlbumSelectionFeature/Action/.fetchAlbumsResponse references are updated
accordingly.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swift`:
- Around line 220-227: 현재 .onTapDownloadButton의 Task에서
imageDownloadClient.downloadImages가 throw하면 .downloadImagesResponse가 전송되지 않아
state.isLoading가 복구되지 않습니다; .run 블록 내부에서 downloadImages 호출을 do/catch로 감싸고, 성공 시
기존처럼 await send(.downloadImagesResponse(successCount: count))를 호출하며 실패(catch)
시에도 await send(.downloadImagesResponse(successCount: 0)) 또는 프로젝트의 에러 응답 케이스로 실패
이벤트를 전송하도록 변경해 state.isLoading이 reducer에서 항상 false로 복구되도록 하세요; 관련 식별자: case
.onTapDownloadButton, imageDownloadClient.downloadImages(urls:),
downloadImagesResponse(successCount:), state.isLoading, state.selectedIDs,
state.photos.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swift`:
- Around line 100-107: The onTapDownloadButton effect can throw, leaving
state.isLoading true because downloadImagesResponse isn't sent on error; update
the .onTapDownloadButton .run block in ArchiveAllPhotosFeature (case
.onTapDownloadButton) to wrap the try await imageDownloadClient.downloadImages
call in a do-catch and ensure you always send a response (e.g., send
.downloadImagesResponse with appropriate successCount or an error variant) in
both success and catch paths, so the reducer that clears state.isLoading when
handling .downloadImagesResponse will always run.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swift`:
- Around line 176-182: The optimistic favorite toggle in
ArchivePhotoDetailFeature.onTapFavorite updates state immediately but swallows
errors with try?, so failures never dispatch a response to rollback; modify the
.onTapFavorite effect (the return .run block that calls
archiveClient.toggleFavorite(photoID:request:)) to await the network call and
send a toggleFavoriteResponse action containing the photo id and a success flag
or error, and on failure dispatch the response that flips state back (use the
captured id and isFavorite to indicate the intended new state and let the
reducer handle rollback on failure instead of ignoring errors).
- Around line 200-205: The onTapDelete branch in ArchivePhotoDetailFeature
currently uses try? which swallows errors and always sends
.deletePhotoResponse(.success(())); change it to a do-catch around await
archiveClient.deletePhotoList(photoIds: [id]) in the .onTapDelete effect, and on
success send .deletePhotoResponse(.success(())) but on failure send
.deletePhotoResponse(.failure(error)) (or map the thrown error to your feature's
Failure type) so server deletion failures are propagated and handled instead of
being treated as success.
- Around line 116-125: The current doneMemoEditing handling uses try? await
archiveClient.updatePhotoMemo(...) which ignores server errors after mutating
state.photos[...] and causes silent divergence; capture the previous memo (e.g.,
let previousMemo = state.photos[id: photoID]?.memo) before mutating, remove the
try?, perform the async update inside a do/catch in the .run block calling await
archiveClient.updatePhotoMemo(photoID:memo:), and on catch send a failure action
(e.g., .memoSaveFailed(photoID, previousMemo, error)) that reverts
state.photos[id: photoID]?.memo = previousMemo and sets an error/UI flag (e.g.,
state.isMemoEditing or state.isMemoExpanded) so the UI reflects the failed save
instead of silently succeeding.
- Around line 187-193: The .onTapDownload branch sets state.isLoading = true but
if imageDownloadClient.downloadImages throws, no .downloadImageResponse is sent
and isLoading remains true; wrap the download call inside a do/catch in the .run
closure (or use Result/Task initializer) so you always send a response on both
success and failure (e.g., send .downloadImageResponse(successCount: count) on
success and send a failure response (or .downloadImageResponse(successCount: 0)
until a dedicated failure case exists) in the catch), referencing
ArchivePhotoDetailFeature, the .onTapDownload action,
imageDownloadClient.downloadImages, and the downloadImageResponse action so the
UI can set isLoading back to false.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift`:
- Around line 116-159: When switching albums in selectAlbum, in-flight
fetchPhotos effects can overwrite the new album's photos; modify the fetchPhotos
effect (the .run block that sends .fetchPhotosResponse) to either attach and
send the requested album id with the response or use a cancellable effect id,
then cancel previous fetches when selectAlbum runs; specifically update
fetchPhotos to include the selected album id in the Result (or call
.cancellable(id: some FetchPhotosID) and in selectAlbum call .cancel(id:
FetchPhotosID) before sending .fetchPhotos) and update fetchPhotosResponse
handling to only apply entities when the response's album id matches
state.selectedAlbum?.id (or when the effect wasn't canceled).
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`:
- Around line 120-134: The title and confirm button text currently always read
"앨범에 추가" and "\(store.uploadCount)장 업로드" even when the flow is a move; update
the UI to branch on selectionPurpose (e.g., check selectionPurpose == .move) and
when true change the title Text to "앨범으로 이동" and the Button label to
"\(store.uploadCount)장 이동" while preserving the existing styling and the
.disabled(store.selectedAlbumIDs.isEmpty) behavior; locate these changes around
the Button and the Text("앨범에 추가") usages so the string variants are shown in the
move vs add flows.
- Around line 61-66: The tap-block condition is using OR so taps are allowed if
either not favorite or not current album; change it to require both checks to be
false before allowing a tap: in AlbumSelectionView.swift update the condition
around album.isFavorite and album.id vs store.currentAlbumId to use AND (e.g. if
!album.isFavorite && album.id != store.currentAlbumId) and then call
store.send(.tapAlbum(album.id)) only when that combined condition is true.
---
Nitpick comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swift`:
- Around line 122-125: The guard using a magic value (`id != -1`) in the
.tapAlbum(id) case is unclear; replace the hardcoded -1 with a named constant or
helper to express "favorite album" (e.g., define a local constant or
AlbumConstants.favoriteId or a helper method isFavoriteAlbum(_:)) and use that
in the guards alongside state.currentAlbumId so the intent in
AlbumSelectionFeature.swift (the .tapAlbum(id) case) is explicit and readable.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swift`:
- Around line 172-183: The tapUpload branch currently returns success after a
placeholder 1s sleep; replace this with a real API call to
archiveClient.duplicatePhoto (or the actual upload method) using the collected
ids and targetAlbumId (handle targetId optional/unwrapping), await the call
inside the .run task, catch and send a failure action (e.g.,
.taskFailed(error:)) on error and include error details in the sent message, and
ensure state.isLoading is cleared (false) in both success and failure paths;
keep the guard using state.isUploadEnabled and use
state.selectedIDs/targetAlbumId within the task rather than capturing mutable
state directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 885d4281-cc1e-4739-864a-c409d7e44694
📒 Files selected for processing (8)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/AlbumSelectionFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAlbumDetailFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveAllPhotosFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchivePhotoDetailFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/PhotoImportFeature.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/PhotoImportView.swift
✅ Files skipped from review due to trivial changes (1)
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/PhotoImportView.swift
Comment on lines
+200
to
206
| case .onTapDownloadButton: | ||
| guard !state.selectedIDs.isEmpty else { return .none } | ||
| return .run { [photos = state.photos, selectedIDs = state.selectedIDs] send in | ||
| let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL } | ||
| let count = try await imageDownloadClient.downloadImages(urls: urls) | ||
| await send(.downloadImagesResponse(successCount: count)) | ||
| } |
There was a problem hiding this comment.
다운로드 예외 시 로딩 상태가 복구되지 않습니다.
imageDownloadClient.downloadImages가 throw하면 downloadImagesResponse가 전송되지 않아 isLoading이 계속 true로 남습니다. (참고: ArchiveAllPhotosFeature에도 동일한 패턴이 있어 일관된 수정이 필요합니다.)
🔧 수정 제안
case .onTapDownloadButton:
guard !state.selectedIDs.isEmpty else { return .none }
+ state.isLoading = true
return .run { [photos = state.photos, selectedIDs = state.selectedIDs] send in
- let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL }
- let count = try await imageDownloadClient.downloadImages(urls: urls)
- await send(.downloadImagesResponse(successCount: count))
+ do {
+ let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL }
+ let count = try await imageDownloadClient.downloadImages(urls: urls)
+ await send(.downloadImagesResponse(successCount: count))
+ } catch {
+ await send(.downloadImagesResponse(successCount: 0))
+ }
}📝 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
| case .onTapDownloadButton: | |
| guard !state.selectedIDs.isEmpty else { return .none } | |
| return .run { [photos = state.photos, selectedIDs = state.selectedIDs] send in | |
| let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL } | |
| let count = try await imageDownloadClient.downloadImages(urls: urls) | |
| await send(.downloadImagesResponse(successCount: count)) | |
| } | |
| case .onTapDownloadButton: | |
| guard !state.selectedIDs.isEmpty else { return .none } | |
| state.isLoading = true | |
| return .run { [photos = state.photos, selectedIDs = state.selectedIDs] send in | |
| do { | |
| let urls = selectedIDs.compactMap { photos[id: $0]?.imageURL } | |
| let count = try await imageDownloadClient.downloadImages(urls: urls) | |
| await send(.downloadImagesResponse(successCount: count)) | |
| } catch { | |
| await send(.downloadImagesResponse(successCount: 0)) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFavoriteAlbumFeature.swift`
around lines 200 - 206, The .onTapDownloadButton run effect currently awaits
imageDownloadClient.downloadImages and if it throws the run will exit without
sending .downloadImagesResponse, leaving isLoading true; wrap the await call in
do/catch (or add try? and handle nil) and always send a .downloadImagesResponse
in both success and failure paths (e.g. send successCount: count on success and
successCount: 0 or an explicit failure case on error), updating the closure that
captures state.photos/selectedIDs and the send(.downloadImagesResponse(...))
invocation accordingly; apply the same pattern to the analogous case in
ArchiveAllPhotosFeature to ensure loading is cleared on errors.
| struct AlbumSelectionView: View { | ||
| @Bindable var store: StoreOf<AlbumSelectionFeature> | ||
|
|
||
| @State var addAlbumSheetPresented: Bool = false |
There was a problem hiding this comment.
SwiftUI @State 프로퍼티는 private으로 선언해야 합니다.
SwiftLint 규칙에 따라 @State 프로퍼티는 외부 접근을 방지하기 위해 private으로 선언하는 것이 권장됩니다.
🔧 수정 제안
-@State var addAlbumSheetPresented: Bool = false
+@State private var addAlbumSheetPresented: Bool = false📝 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
| @State var addAlbumSheetPresented: Bool = false | |
| `@State` private var addAlbumSheetPresented: Bool = false |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 14-14: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/AlbumSelectionView.swift`
at line 14, The `@State` property addAlbumSheetPresented in AlbumSelectionView is
not marked private; change its declaration (the addAlbumSheetPresented `@State`
var) to private to follow SwiftUI/SwiftLint conventions and update any internal
references within AlbumSelectionView to use the now-private property (if
external views need control, expose a Binding or a separate `@Binding` property or
callback instead of making the state non-private).
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift (2)
65-71: 스타일 분기 로직이 명확합니다.2가지 케이스만 있어서 현재 if/else도 충분하지만, 향후 스타일이 추가될 경우
switch문을 사용하면 컴파일러가 새로운 케이스 처리 누락을 잡아줄 수 있습니다.♻️ switch 문 사용 제안 (선택)
var body: some View { - if style == .selection { - selectionModeFooter - } else { - detailModeFooter + switch style { + case .selection: + selectionModeFooter + case .detail: + detailModeFooter } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift` around lines 65 - 71, Replace the current if/else in the body computed property with a switch on style to make branch handling exhaustive; locate the body property and switch on style, returning selectionModeFooter for .selection and detailModeFooter for the other case(s) so the compiler will warn if new ArchiveImageFooter.Style cases are added (refer to body, style, selectionModeFooter, detailModeFooter).
140-167: 스타일 적용 방식과 비활성화 색상이 일관되지 않습니다.두 가지 불일치가 있습니다:
.renderingMode(.template)사용 불일치: 다운로드 버튼(Line 142)은.foregroundStyle()만 사용하지만, 삭제 버튼(Line 165)은.renderingMode(.template)과 함께 사용합니다. 이미지 에셋 설정에 따라 색상 틴팅이 다르게 동작할 수 있습니다.비활성화 색상 불일치: 다운로드 버튼은
.gray200, 삭제 버튼은.gray100을 사용합니다.의도된 디자인이라면 무시해도 되지만, 그렇지 않다면 통일이 필요합니다.
♻️ 일관된 스타일 적용 제안
Button(action: onDownload) { Image(isEnabled ? .iconDownloadFill : .iconDownload) + .renderingMode(.template) .foregroundStyle(isEnabled ? .gray700 : .gray200) } .disabled(!isEnabled) // ... 중략 ... Button(action: onDelete) { Image(isEnabled ? .iconTrashFill : .iconTrash) .renderingMode(.template) - .foregroundStyle(isEnabled ? .gray700 : .gray100) + .foregroundStyle(isEnabled ? .gray700 : .gray200) } .disabled(!isEnabled)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift` around lines 140 - 167, The download and delete buttons are inconsistent: the onDownload Button (images iconDownloadFill/iconDownload) uses only .foregroundStyle(...) while the onDelete Button (images iconTrashFill/iconTrash) uses .renderingMode(.template) plus .foregroundStyle(...), and their disabled colors differ (.gray200 vs .gray100). Make them consistent by applying the same rendering mode policy and the same disabled color to both controls—either remove .renderingMode(.template) from the delete button or add it to the download button depending on your asset tinting, and use a single disabled foregroundStyle (e.g., .gray200 or whichever is the design token) for both the onDownload and onDelete Buttons so their visuals match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift`:
- Around line 65-71: Replace the current if/else in the body computed property
with a switch on style to make branch handling exhaustive; locate the body
property and switch on style, returning selectionModeFooter for .selection and
detailModeFooter for the other case(s) so the compiler will warn if new
ArchiveImageFooter.Style cases are added (refer to body, style,
selectionModeFooter, detailModeFooter).
- Around line 140-167: The download and delete buttons are inconsistent: the
onDownload Button (images iconDownloadFill/iconDownload) uses only
.foregroundStyle(...) while the onDelete Button (images iconTrashFill/iconTrash)
uses .renderingMode(.template) plus .foregroundStyle(...), and their disabled
colors differ (.gray200 vs .gray100). Make them consistent by applying the same
rendering mode policy and the same disabled color to both controls—either remove
.renderingMode(.template) from the delete button or add it to the download
button depending on your asset tinting, and use a single disabled
foregroundStyle (e.g., .gray200 or whichever is the design token) for both the
onDownload and onDelete Buttons so their visuals match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 63e5acd9-0705-438c-b3be-3a4bb000e8ab
📒 Files selected for processing (2)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAllPhotosView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Neki-iOS/Features/Archive/Sources/Presentation/Sources/View/ArchiveAllPhotosView.swift
Member
Remaked-Swain
left a comment
Remaked-Swain
left a comment
There was a problem hiding this comment.
본 PR에서는 앨범 내 사진 복제/이동에 대한 로직으로만 마치는 것으로 하고, 실제 API연결은 별도의 Issue로 뺀 것을 확인했습니다!
이전 PR과 이어서 보는 부분이라 새롭게 봐드릴 것은 없는 것 같고, '인스타그램 스토리 올리기' 관련해서 질문 몇가지 남겼습니다.
코멘트 응답만 받고 모두 검토되면 별도로 어프룹 사인을 드리겠습니다!
| return false | ||
| } | ||
| if !isShared { await send(.delegate(.showToast(NekiToastItem("인스타그램 앱이 설치되어 있지 않아요", style: .error)))) } | ||
| } |
Member
There was a problem hiding this comment.
p3)
질문거리 남깁니다~~!~!
- 저는 애로우 코드뎁스를 보면 참을 수 없는 병에 걸렸습니다. 혹시 가드문으로 개선할 수 있나요?
- 인스타그램 스토리로 이미지를 넘길 때 UIPasteboard를 사용하셨군요! UIActivityViewController 등등 다른 공유방식도 있는데 이 방식을 선택한 이유가 있을까요?
- UIPasteboard는 UIImage 타입, NSString으로 표현될 수 있는 타입(NSData, NSNumber, etc..)을 모두 다를 수 있습니다. 그런데
.pngData()로 바이너리로 변환한 이유가 무엇인가요? - 현재 로직은 '인스타그램이라는 외부 앱과 연동한다.' 그리고 'PNG 포맷을 다룬다.'에 초점이 맞춰져 있는 것 같습니다. 만약 추후에 카카오톡이나 X 등 다른 SNS 서비스로도 확장하는 상황임, 그리고 그 서비스는 png 확장자를 제공하지 않음을 가정하고 리팩토링 한다면 어떻게 할 수 있을까요?
- UIPasteboard.general 인스턴스에 setItems 메서드를 호출하며 옵션을 전달하고 있습니다. 이것이 무슨 의미인지 설명해주세요. (추가로, '60*5' 와 같은 값을 하드코딩 방지할 수 있나요?)
- 어쩌다가 SwiftUI 베이스의 TCA에서 기본적으로 제공하는 openURL 의존성 대신, UIApplication.shared 인스턴스를 직접 호출해 앱 라우팅 로직을 돌리게 되었는지, 그 히스토리가 궁금합니다!
- TCA의 장점을 제대로 살리려면 UIApplication 뿐만 아니라 UIPasteboard도 외부 의존성이고 이에 접근하는 동작도 외부 사이드이펙트이므로 Dependency로 등록하는 것이 좋을 것 같다고 생각합니다. 중간저장소로써 공유가 잘 되는지 클립보드를 mocking해서 테스트 해야하는 상황이라고 상상하면 이해하기에 도움이 될까요? 이 부분에 대해서는 어떤 견해이신가요? (만약 동의하신다면, 해당 UIPasteboardClient 또는 ~Service가 UIKit을 임포팅 해야하는 딜레마에 대해서도 생각해봐주시면 좋을 것 같아용)
p2)
- 이미지 형식이 다르다거나 하는 이슈로
image.pngData()가 실패하면false가 반환되고, 이 경우 인스타그램이 설치되어 있음에도? "인스타그램이 설치되어있지 않다"는 판정을 받을 수 있습니다. 에러 핸들링을 조금 더 명확하게 분기해볼까요?
Member
Author
There was a problem hiding this comment.
확인했습니다. 하지만 답변과 고민은 지금 마감기한과 중요도를 고려했을 때 우선순위가 높지 않아 후순위로 미루겠습니다. 특히나 인스타그램 공유 기능의 경우 본래 이번 스프린트 작업사항이 아닌데 오해하고 임시로 구현해둔 것이므로 더더욱 우선순위가 낮다고 여겨지네요!
Comment on lines
+10
to
+14
| public enum PhotoSelectionPurpose { | ||
| case duplicate | ||
| case move | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
p3)
Presentation/Components 보다는 Domain Layer에 위치하는 것이 아키텍처 규칙 상 적합해보입니다. 이미지를 다루는 방법에는 복제가 있고 이동이 있다! 라는 비즈니스 룰이라.
단, ArchiveFooterStyle이라는 타입은 Presentation Layer에 적절히 잘 만드신 것 같아요. 뷰의 레이아웃 스타일을 설명하는 타입에 불과하니까!
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift (1)
136-161:.renderingMode(.template)사용 및 비활성 색상 일관성 확인 필요.
- Line 137(다운로드 아이콘):
.foregroundStyle(...)단독 사용- Line 160-161(삭제 아이콘):
.renderingMode(.template)+.foregroundStyle(...)함께 사용또한 비활성 상태 색상이 다릅니다:
- 다운로드 아이콘:
.gray200- 삭제 아이콘:
.gray100selectionModeFooter아이콘들:.gray200의도된 디자인이라면 무시하셔도 됩니다. 아니라면 일관성을 위해 아래 수정을 고려해 주세요.
♻️ 일관성 개선 제안
Button(action: onDownload) { Image(isEnabled ? .iconDownloadFill : .iconDownload) + .renderingMode(.template) .foregroundStyle(isEnabled ? .gray700 : .gray200) } .disabled(!isEnabled) // ... 중간 코드 생략 ... Button(action: onDelete) { Image(isEnabled ? .iconTrashFill : .iconTrash) .renderingMode(.template) - .foregroundStyle(isEnabled ? .gray700 : .gray100) + .foregroundStyle(isEnabled ? .gray700 : .gray200) } .disabled(!isEnabled)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift` around lines 136 - 161, In ArchiveImageFooter.swift, make icon rendering and disabled color usage consistent: ensure Image(.iconDownload...) uses the same rendering mode and disabled foreground as the delete and other footer icons (e.g., apply .renderingMode(.template) if you rely on template rendering) and standardize the disabled color to .gray200 across the download, trash (currently using .gray100), and selectionModeFooter icons; update the Button/Image configurations for the download Image and the trash Image (and any other footer icons) to use the same combination of .renderingMode and .foregroundStyle so visual states are uniform.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift`:
- Around line 136-161: In ArchiveImageFooter.swift, make icon rendering and
disabled color usage consistent: ensure Image(.iconDownload...) uses the same
rendering mode and disabled foreground as the delete and other footer icons
(e.g., apply .renderingMode(.template) if you rely on template rendering) and
standardize the disabled color to .gray200 across the download, trash (currently
using .gray100), and selectionModeFooter icons; update the Button/Image
configurations for the download Image and the trash Image (and any other footer
icons) to use the same combination of .renderingMode and .foregroundStyle so
visual states are uniform.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c1703828-c4aa-4940-ab64-1773b3fb6848
📒 Files selected for processing (2)
Neki-iOS/Features/Archive/Sources/Domain/Sources/Entities/PhotoSelectionPurpose.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Components/ArchiveImageFooter.swift
✅ Files skipped from review due to trivial changes (1)
- Neki-iOS/Features/Archive/Sources/Domain/Sources/Entities/PhotoSelectionPurpose.swift
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
2 participants
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.
🌴 작업한 브랜치
feat/#202-sprint4✅ 작업한 내용
사진 이동/복제 플로우와 사진 가져오기(Import) 기능을 구현하고, 전반적인 사용성을 높이기 위한 UI/UX 개선 작업을 진행했습니다.
❗️PR Point
📸 스크린샷
ScreenRecording_04-03-2026.03-19-58_1-1.mov
📟 관련 이슈
Summary by CodeRabbit
New Features
UI/UX Improvements