✨feat: CustomMissionCell 삭제 기능 추가#63
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
이번 풀 리퀘스트는 MissionViewController에 컨텍스트 메뉴를 통한 미션 삭제 기능을 추가합니다. deleteMissionSubject를 도입하여 CoreDataManager를 통해 삭제를 수행하고 결과를 처리하도록 구현되었습니다. 리뷰 결과, 발생한 에러의 종류에 따라 더 구체적인 에러 메시지를 제공하도록 예외 처리 로직을 개선할 필요가 있으며, 시스템 이미지 이름의 오타("trah.fill" -> "trash.fill")로 인해 아이콘이 표시되지 않는 문제를 수정해야 합니다.
| try self.coreDataManager.deleteMissionEntity(of: mission.id) | ||
| self.initialLoadSubject.onNext(()) | ||
| } catch { | ||
| self.showErrorAlert(error: .saveFailed) |
Contributor
There was a problem hiding this comment.
deleteMissionEntity(of:) 메서드는 .saveFailed 외에도 다양한 CoreDataError를 발생시킬 수 있습니다. 예를 들어, fetchMissionEntity가 실패하면 .loadFailed나 .empty 오류가 발생할 수 있습니다. 현재 catch 블록은 모든 오류를 .saveFailed로 처리하여 사용자에게 부정확한 오류 정보를 보여줄 수 있습니다. 발생한 실제 오류를 전달하여 더 정확한 피드백을 제공해야 합니다.
if let coreDataError = error as? CoreDataManager.CoreDataError {
self.showErrorAlert(error: coreDataError)
} else {
self.showErrorAlert(error: .saveFailed)
}References
- Swallowed or mishandled errors that obscure failure causes should be classified as high severity. The current implementation misrepresents the error type, which can make debugging difficult and provide a poor user experience. (link)
- When displaying errors to the user, provide specific and meaningful messages based on the error type instead of a generic message.
| case .customMission(let mission) = item else { return nil } | ||
|
|
||
| return UIContextMenuConfiguration(identifier: nil, previewProvider: nil) { _ in | ||
| let delete = UIAction(title: "삭제", image: UIImage(systemName: "trah.fill"), attributes: .destructive) { [weak self] _ in |
Contributor
There was a problem hiding this comment.
UIImage의 시스템 이름에 오타가 있습니다. "trah.fill"이 아니라 "trash.fill"이어야 합니다. 이 오타 때문에 삭제 아이콘이 표시되지 않습니다.
Suggested change
| let delete = UIAction(title: "삭제", image: UIImage(systemName: "trah.fill"), attributes: .destructive) { [weak self] _ in | |
| let delete = UIAction(title: "삭제", image: UIImage(systemName: "trash.fill"), attributes: .destructive) { [weak self] _ in |
References
- Visible UI bugs should be classified as high severity. A typo causing a missing icon is a clear UI bug that degrades the user experience. (link)
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
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.
🎫 관련 이슈 (Linked Issues)
Closes #
🛠 작업 내용 (What I did)
💻 구현 상세 (Implementation Details)
스와이프, 길게 눌러서 삭제 Alert를 통한 기능 중 길게 눌러서 삭제를 선택함. (Compositional Layout에서는 Swipe 불가)
삭제 기능 동작 이후 기존에 사용하던 initialize를 사용하여 목록 동기화 (CoreData를 한번 더 부른다는 단점 존재)
추후 리팩토링 시 UICollectionViewLayoutListConfiguration을 통해 Swipe 기능 구현
추후 리팩토링 시 initialize 대신 Relay를 사용하여 불필요한 CoreData 호출 제거