컬렉션 API 연동 #272
컬렉션 API 연동 #272
Conversation
Summary of ChangesHello @pinocchio22, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 애플리케이션의 컬렉션 관리 기능을 대폭 확장하기 위해 컬렉션 API를 포괄적으로 통합합니다. 사용자 정의 컬렉션을 생성, 조회, 수정 및 삭제하는 기능과 함께, 북마크를 이러한 컬렉션과 연결하는 개선된 메커니즘을 도입합니다. 데이터, 도메인, 프레젠테이션 계층 전반에 걸쳐 광범위한 리팩토링이 이루어졌으며, 네트워크 엔드포인트, 리포지토리 구현, 유스케이스 및 UI 구성 요소가 업데이트되어 더욱 강력하고 기능이 풍부한 컬렉션 경험을 제공합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
컬렉션 API 연동을 위한 대규모 리팩토링 및 기능 추가 작업이 인상적입니다. 전반적으로 UseCase와 Repository를 분리하고, Reactor 패턴을 적극적으로 활용하여 코드 구조가 명확해지고 유지보수성이 향상되었습니다. 특히 뷰 컨트롤러 간의 데이터 전달 방식을 클로저에서 PublishSubject로 변경하여 반응형 프로그래밍의 이점을 잘 살린 점이 돋보입니다.
다만, 코드베이스 전반에 걸쳐 사용되지 않는 주석 처리된 코드 블록이 다수 발견되었습니다. 이는 코드 가독성을 저해하고 혼란을 야기할 수 있으므로, 사용 계획이 없는 코드라면 정리하는 것이 좋겠습니다. 또한, 일부 파일 이름에 오타가 있고, 잠재적인 버그가 의심되는 부분이 있어 해당 내용에 대한 리뷰 의견을 남겼습니다. 이 부분들을 수정하면 코드의 완성도가 더욱 높아질 것입니다.
| case .undoLastDeletedBookmark: | ||
| guard let lastDeleted = currentState.lastDeletedBookmark else { return .empty() } | ||
| guard let lastDeleted = currentState.lastDeletedBookmark, | ||
| let lastDeletedBookmarkId = currentState.lastDeletedBookmark?.bookmarkId else { return .empty() } | ||
| return setBookmarkUseCase.execute( | ||
| bookmarkId: lastDeleted.originalId, | ||
| isBookmark: .set(lastDeleted.type) | ||
| ) | ||
| // 북마크 다시 설정시 이전 collection을 전부 추적해야하고 새로 바뀐 북마크ID가 필요하여 현재는 원할하게 동작하지 않음 | ||
| .andThen(addCollectionAndBookmarkUseCase.execute(collectionIds: [currentState.collection.collectionId], bookmarkIds: [lastDeletedBookmarkId])) | ||
| .andThen( | ||
| Observable.concat([ | ||
| // 불러오기 | ||
| fetchCollectionUseCase.execute(id: currentState.collection.collectionId) | ||
| .map { .setItems($0) }, | ||
| .just(.setLastDeletedBookmark(nil)) | ||
| ]) | ||
| ) |
There was a problem hiding this comment.
undoLastDeletedBookmark 액션 처리 로직에 "현재는 원할하게 동작하지 않음"이라는 주석이 있습니다. 버그가 있는 기능이 포함된 것으로 보입니다. 이 기능이 의도대로 동작하지 않는다면, 문제를 해결하거나 혹은 해결 전까지 해당 기능을 비활성화하여 사용자에게 잘못된 경험을 주지 않도록 하는 것이 중요합니다. 특히 addCollectionAndBookmarkUseCase를 호출하는 부분이 문제의 원인일 수 있으니 검토가 필요해 보입니다.
| // public func addBookmarksToCollection(collectionId: Int, bookmarkIds: [Int]) -> Completable { | ||
| // let endPoint = CollectionEndPoint.addBookmarksToCollection(id: collectionId, body: AddBookmarkRequestBody(bookmarkIds: bookmarkIds)) | ||
| // return provider.requestData(endPoint: endPoint, interceptor: tokenInterceptor) | ||
| // .catch { error in | ||
| // if let netErr = error as? NetworkError { | ||
| // switch netErr { | ||
| // case let .statusError(code, body): | ||
| // return .error(DomainHTTPError.httpStatus(code: code, message: body)) | ||
| // default: | ||
| // return .error(DomainHTTPError.unknown) | ||
| // } | ||
| // } else { | ||
| // return .error(DomainHTTPError.unknown) | ||
| // } | ||
| // } | ||
| // } | ||
| // | ||
| // public func addCollectionsToBookmark(bookmarkId: Int, collectionIds: [Int]) -> Completable { | ||
| // let endPoint = CollectionEndPoint.addCollectionsToBookmark(id: bookmarkId, body: AddCollectionRequestBody(collectionIds: collectionIds)) | ||
| // return provider.requestData(endPoint: endPoint, interceptor: tokenInterceptor) | ||
| // } |
There was a problem hiding this comment.
주석 처리된 addBookmarksToCollection 및 addCollectionsToBookmark 함수가 있습니다. 현재 사용되지 않는 코드라면 가독성과 유지보수성 향상을 위해 삭제하는 것을 고려해 보세요.
MLS/Domain/Domain/UseCaseImpl/Collection/AddBookmarksToCollectionUseCaseImpl.swift
Show resolved
Hide resolved
MLS/Domain/Domain/UseCaseImpl/Collection/AddCollectionsToBookmarkUseCaseImpl.swift
Show resolved
Hide resolved
| import Foundation | ||
|
|
||
| public enum DomainHTTPError: Error, Equatable { | ||
| case httpStatus(code: Int, message: String?) | ||
| case network | ||
| case decode | ||
| case noData | ||
| case unknown | ||
| } |
There was a problem hiding this comment.
파일 이름에 오타가 있습니다. DomainHTTPError.swift.swift가 아닌 DomainHTTPError.swift로 수정해야 합니다. 파일 확장자가 중복되었습니다.
MLS/Domain/DomainInterface/UseCase/Collection/AddBookmarksToCollectionUseCase.swift
Show resolved
Hide resolved
MLS/Domain/DomainInterface/UseCase/Collection/AddCollectionsToBookmarkUseCase.swift
Show resolved
Hide resolved
| .subscribe { owner, scrolls in | ||
| guard let dataSource = owner.dataSource else { return } | ||
| guard owner.dataSource != nil else { return } | ||
| var snapshot = owner.dataSource.snapshot() | ||
| snapshot.deleteItems(snapshot.itemIdentifiers(inSection: .scrollCategories)) | ||
| snapshot.appendItems(scrolls.scrollTypes.map { .scrollCategories($0) }, toSection: .scrollCategories) |
There was a problem hiding this comment.
dataSource가 nil인지 확인한 후, 옵셔널 체이닝(?)이나 guard let 바인딩 없이 강제 언래핑(!)처럼 사용되고 있습니다. dataSource가 UICollectionViewDiffableDataSource의 인스턴스이므로 클래스 타입이라 참조 시점에 nil이 아님이 보장될 수 있지만, guard let dataSource = owner.dataSource else { return }과 같이 안전하게 옵셔널을 해제하는 것이 더 명확하고 안전한 코드 스타일입니다.
| .subscribe { owner, scrolls in | |
| guard let dataSource = owner.dataSource else { return } | |
| guard owner.dataSource != nil else { return } | |
| var snapshot = owner.dataSource.snapshot() | |
| snapshot.deleteItems(snapshot.itemIdentifiers(inSection: .scrollCategories)) | |
| snapshot.appendItems(scrolls.scrollTypes.map { .scrollCategories($0) }, toSection: .scrollCategories) | |
| .subscribe { owner, scrolls in | |
| guard let dataSource = owner.dataSource else { return } | |
| var snapshot = dataSource.snapshot() | |
| snapshot.deleteItems(snapshot.itemIdentifiers(inSection: .scrollCategories)) | |
| snapshot.appendItems(scrolls.scrollTypes.map { .scrollCategories($0) }, toSection: .scrollCategories) | |
📌 이슈
✅ 작업 사항