[Fix] 프로필 기능 개선 및 기타 오류 수정 #197
Conversation
- 뷰에서 디바운스 처리 후 뷰모델 요청
- 아무것도 하지 않는 방향으로 수정
|
Caution Review failedThe pull request is closed. ## Walkthrough
This update introduces extensive improvements and refactoring to the profile feature across the codebase. It adds async/await variants to repository interfaces and implementations, enhances the profile UI, introduces new moderation actions (report, ban, ghost), refactors view models for modularity and clarity, and updates navigation and gesture handling. Project structure and dependency injection are also reorganized.
## Changes
| Files/Groupings | Change Summary |
|------------------------------------------------------------------------------------------------------|---------------|
| Wable-iOS.xcodeproj/project.pbxproj | Renamed `ProfileSegmentKind.swift` to `ProfileSegment.swift` and moved `ProfileEditViewController.swift` into a new `Edit` group under `Profile`. |
| Wable-iOS/App/AppDelegate+InjectDependency.swift | Registered `GhostRepository` in dependency injection container. |
| Wable-iOS/Data/Mapper/CommentMapper.swift | Fixed date format strings for seconds in comment mapping methods. |
| Wable-iOS/Data/RepositoryImpl/CommentRepositoryImpl.swift<br>Wable-iOS/Domain/RepositoryInterface/CommentRepository.swift | Added async/await `deleteComment` method to repository and protocol; updated mock implementation. |
| Wable-iOS/Data/RepositoryImpl/ContentLikedRepositoryImpl.swift<br>Wable-iOS/Domain/RepositoryInterface/ContentLikedRepository.swift | Added async/await `createContentLiked` and `deleteContentLiked` methods to repository and protocol. |
| Wable-iOS/Data/RepositoryImpl/ContentRepositoryImpl.swift<br>Wable-iOS/Domain/RepositoryInterface/ContentRepository.swift | Added async/await `deleteContent` method to repository and protocol. |
| Wable-iOS/Data/RepositoryImpl/GhostRepositoryImpl.swift<br>Wable-iOS/Domain/RepositoryInterface/GhostRepository.swift | Added async/await `postGhostReduction` method to repository and protocol. |
| Wable-iOS/Data/RepositoryImpl/ReportRepositoryImpl.swift<br>Wable-iOS/Domain/RepositoryInterface/ReportRepository.swift | Added async/await `createReport` and `createBan` methods to repository, protocol, and mocks. |
| Wable-iOS/Presentation/Home/View/HomeViewController.swift | Changed injected use cases for profile view model; updated cell registration closure formatting. |
| Wable-iOS/Presentation/Overview/Detail/View/AnnouncementDetailViewController.swift<br>Wable-iOS/Presentation/Profile/AlarmSetting/View/AlarmSettingViewController.swift | Removed gesture recognizer delegate conformance and related methods; simplified gesture recognizer setup. |
| Wable-iOS/Presentation/Profile/Component/ProfileInfoCell.swift | Added level badge image view; adjusted layout constraints; fixed progress bar scaling and animation; streamlined edit button action handler. |
| Wable-iOS/Presentation/Profile/Component/ProfileSegmentedHeaderView.swift | Replaced target-action with Combine-based reactive handling for segmented control changes; renamed closure property. |
| Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift | Refactored to use Combine for willDisplay events; added delete confirmation action sheets; updated cell handlers and pagination logic. |
| Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift | Refactored to use repositories directly; removed Combine subjects; added async/await deletion and like toggle methods; centralized error handling. |
| Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift | Added Combine-based pagination; added contextual action sheets for report, ban, and ghost; improved cell handlers and UI feedback for reports. |
| Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift | Refactored to use repositories; removed Combine; added moderation and like toggle methods; centralized error handling; added completion flags. |
| Wable-iOS/Presentation/TabBar/TabBarController.swift | Updated dependency injection for profile and viewit controllers; disabled animation on tab setup. |
| Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift | Added profile tap relay; implemented explicit navigation logic for profile taps; added navigation helper methods. |
| Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift | Added user session awareness; added profile tap logic; simplified list update after banning actions. |
| Wable-iOS/Presentation/WableComponent/Cell/CommentCollectionViewCell.swift | Moved guard statement for `createdDate` to after more UI setup in `configureCell`. |
| Wable-iOS/Presentation/Profile/Model/ProfileEmptyCellItem.swift | Changed `segment` property type from `ProfileSegmentKind` to `ProfileSegment`. |
| Wable-iOS/Presentation/Profile/Model/ProfileSegment.swift | Renamed enum from `ProfileSegmentKind` to `ProfileSegment`. |
| Wable-iOS/Presentation/Profile/Model/ProfileViewItem.swift | Changed `currentSegment` property type from `ProfileSegmentKind` to `ProfileSegment`. |
| Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift | Updated `configure` method parameter type from `ProfileSegmentKind` to `ProfileSegment`. |
| Wable-iOS/Presentation/Profile/Other/View/Cell/OtherProfileEmptyCell.swift | Updated `configure` method parameter type from `ProfileSegmentKind` to `ProfileSegment`. |
| Wable-iOS/Resource/Assets.xcassets/Icon/ic_level_badge.imageset/Contents.json | Added new asset catalog JSON for level badge icon with multiple scale variants. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
actor User
participant MyProfileViewController
participant MyProfileViewModel
participant ContentRepository
participant CommentRepository
User->>MyProfileViewController: Tap like/delete on content/comment
MyProfileViewController->>MyProfileViewModel: Call toggleLike/delete method
MyProfileViewModel->>ContentRepository: (async) Toggle like/delete content
MyProfileViewModel->>CommentRepository: (async) Toggle like/delete comment
ContentRepository-->>MyProfileViewModel: Result/Error
CommentRepository-->>MyProfileViewModel: Result/Error
MyProfileViewModel-->>MyProfileViewController: Update UI state or error messagesequenceDiagram
actor User
participant OtherProfileViewController
participant OtherProfileViewModel
participant ReportRepository
participant GhostRepository
User->>OtherProfileViewController: Tap report/ban/ghost on content/comment
OtherProfileViewController->>OtherProfileViewModel: Call report/ban/ghost method
OtherProfileViewModel->>ReportRepository: (async) Report/Ban content/comment
OtherProfileViewModel->>GhostRepository: (async) Ghost content/comment
ReportRepository-->>OtherProfileViewModel: Result/Error
GhostRepository-->>OtherProfileViewModel: Result/Error
OtherProfileViewModel-->>OtherProfileViewController: Update UI state or show toast
Assessment against linked issues
Possibly related PRs
Poem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
Wable-iOS/App/AppDelegate+InjectDependency.swift (1)
67-69: Consider following the established dependency injection pattern.The
GhostRepositoryregistration doesn't follow the same pattern as other repositories in this file. Most repositories use a switch statement on theconfigparameter to provide mock implementations for debug builds.Consider updating to match the established pattern:
diContainer.register(for: GhostRepository.self) { config in - return GhostRepositoryImpl() + switch config { + case .debug: + return MockGhostRepository() // if available + case .release: + return GhostRepositoryImpl() + } }Or document why
GhostRepositorydoesn't need a mock implementation if that's intentional.Wable-iOS/Data/RepositoryImpl/GhostRepositoryImpl.swift (1)
39-60: Implementation is correct but consider reducing code duplication.The async implementation follows the correct pattern and properly handles errors. However, there's significant duplication with the Combine version.
Consider extracting the request construction into a private helper method:
+ private func createGhostReductionRequest( + alarmTriggerType: String, + alarmTriggerID: Int, + targetMemberID: Int, + reason: String + ) -> DTO.Request.UpdateGhost { + return DTO.Request.UpdateGhost( + alarmTriggerType: alarmTriggerType, + targetMemberID: targetMemberID, + alarmTriggerID: alarmTriggerID, + ghostReason: reason + ) + }Then use it in both methods to reduce duplication.
Wable-iOS/Data/RepositoryImpl/ContentRepositoryImpl.swift (1)
46-55: Implementation is correct and follows established patterns.The async
deleteContentmethod properly implements the async/await pattern with appropriate error handling usingErrorMapper.map.Similar to other repository implementations, consider extracting common logic to reduce duplication between the Combine and async versions:
+ private func performDeleteContent(contentID: Int) -> TargetType { + return .deleteContent(contentID: contentID) + }This would help maintain consistency and reduce the risk of divergence between the two implementations.
Wable-iOS/Data/RepositoryImpl/ContentLikedRepositoryImpl.swift (1)
55-64: Consistent async implementation for deleteContentLiked.The implementation follows the same pattern as the create method with proper error handling and mirroring of the Combine-based approach.
Consider adding documentation comments to indicate the relationship between the Combine and async variants, and establish a plan for eventual consolidation once the migration to async/await is complete to reduce maintenance overhead.
Wable-iOS/Data/RepositoryImpl/ReportRepositoryImpl.swift (1)
94-96: Improve mock repository async method implementationsThe mock async methods are currently empty stubs. Consider adding proper mock behavior for better testing:
- func createReport(nickname: String, text: String) async throws { - - } + func createReport(nickname: String, text: String) async throws { + try await Task.sleep(nanoseconds: UInt64(delaySeconds * 1_000_000_000)) + } - func createBan(memberID: Int, triggerType: TriggerType.Ban, triggerID: Int) async throws { - - } + func createBan(memberID: Int, triggerType: TriggerType.Ban, triggerID: Int) async throws { + try await Task.sleep(nanoseconds: UInt64(delaySeconds * 1_000_000_000)) + }Also applies to: 104-106
Wable-iOS/Data/RepositoryImpl/CommentRepositoryImpl.swift (1)
118-120: Consider improving mock error handlingThe mock implementation always throws
WableError.unknownError, which might not be ideal for all testing scenarios. Consider making the mock behavior configurable:- func deleteComment(commentID: Int) async throws { - throw WableError.unknownError - } + func deleteComment(commentID: Int) async throws { + // Add configurable success/failure behavior for testing + try await Task.sleep(nanoseconds: 100_000_000) // 0.1 second delay + }Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift (1)
299-316: Consider dependency injection for better testability.Creating all dependencies inline makes the code harder to test and maintain. Consider injecting these dependencies or using a factory pattern.
- func navigateToOtherProfile(for userID: Int) { - let otherProfileViewController = OtherProfileViewController( - viewModel: .init( - userID: userID, - fetchUserProfileUseCase: FetchUserProfileUseCaseImpl(), - checkUserRoleUseCase: CheckUserRoleUseCaseImpl( - repository: UserSessionRepositoryImpl( - userDefaults: UserDefaultsStorage( - jsonEncoder: JSONEncoder(), - jsonDecoder: JSONDecoder() - ) - ) - ) - ) - ) - - navigationController?.pushViewController(otherProfileViewController, animated: true) - } + func navigateToOtherProfile(for userID: Int) { + let viewModel = viewModelFactory.makeOtherProfileViewModel(userID: userID) + let otherProfileViewController = OtherProfileViewController(viewModel: viewModel) + navigationController?.pushViewController(otherProfileViewController, animated: true) + }Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift (1)
485-503: Consider refactoring to reduce code duplication.The delete methods for content and comments are nearly identical. Consider extracting a common method.
+ func presentDeleteActionSheet( + title: String, + message: String, + deleteAction: @escaping () -> Void + ) { + let actionSheet = WableSheetViewController(title: title, message: message) + let cancelAction = WableSheetAction(title: "취소", style: .gray) + let confirmAction = WableSheetAction(title: "삭제하기", style: .primary) { deleteAction() } + actionSheet.addActions(cancelAction, confirmAction) + present(actionSheet, animated: true) + } + func presentDeleteContentActionSheet(for contentID: Int) { - let actionSheet = WableSheetViewController(title: "게시글을 삭제하시겠어요?", message: "게시글이 영구히 삭제됩니다.") - let cancelAction = WableSheetAction(title: "취소", style: .gray) - let confirmAction = WableSheetAction(title: "삭제하기", style: .primary) { [weak self] in - self?.viewModel.deleteContent(for: contentID) - } - actionSheet.addActions(cancelAction, confirmAction) - present(actionSheet, animated: true) + presentDeleteActionSheet( + title: "게시글을 삭제하시겠어요?", + message: "게시글이 영구히 삭제됩니다.", + deleteAction: { [weak self] in self?.viewModel.deleteContent(for: contentID) } + ) } func presentDeleteCommentActionSheet(for commentID: Int) { - let actionSheet = WableSheetViewController(title: "댓글을 삭제하시겠어요?", message: "댓글이 영구히 삭제됩니다.") - let cancelAction = WableSheetAction(title: "취소", style: .gray) - let confirmAction = WableSheetAction(title: "삭제하기", style: .primary) { [weak self] in - self?.viewModel.deleteComment(for: commentID) - } - actionSheet.addActions(cancelAction, confirmAction) - present(actionSheet, animated: true) + presentDeleteActionSheet( + title: "댓글을 삭제하시겠어요?", + message: "댓글이 영구히 삭제됩니다.", + deleteAction: { [weak self] in self?.viewModel.deleteComment(for: commentID) } + ) }Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift (1)
161-172: Consider the performance impact of full list refresh.Changing from partial updates to full list refresh after ban actions ensures data consistency but may impact performance with large lists. Monitor the user experience and consider implementing incremental updates if performance becomes an issue.
Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift (1)
191-223: Consider adding completion feedback for ghost actionsThe ghost methods correctly implement the functionality but don't set any completion state. For consistency with report actions, consider setting
isGhostCompleted = trueafter successful ghosting to provide user feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
Wable-iOS.xcodeproj/project.pbxproj(2 hunks)Wable-iOS/App/AppDelegate+InjectDependency.swift(1 hunks)Wable-iOS/Data/Mapper/CommentMapper.swift(2 hunks)Wable-iOS/Data/RepositoryImpl/CommentRepositoryImpl.swift(2 hunks)Wable-iOS/Data/RepositoryImpl/ContentLikedRepositoryImpl.swift(2 hunks)Wable-iOS/Data/RepositoryImpl/ContentRepositoryImpl.swift(1 hunks)Wable-iOS/Data/RepositoryImpl/GhostRepositoryImpl.swift(1 hunks)Wable-iOS/Data/RepositoryImpl/ReportRepositoryImpl.swift(3 hunks)Wable-iOS/Domain/RepositoryInterface/CommentRepository.swift(1 hunks)Wable-iOS/Domain/RepositoryInterface/ContentLikedRepository.swift(1 hunks)Wable-iOS/Domain/RepositoryInterface/ContentRepository.swift(1 hunks)Wable-iOS/Domain/RepositoryInterface/GhostRepository.swift(1 hunks)Wable-iOS/Domain/RepositoryInterface/ReportRepository.swift(1 hunks)Wable-iOS/Presentation/Home/View/HomeViewController.swift(2 hunks)Wable-iOS/Presentation/Overview/Detail/View/AnnouncementDetailViewController.swift(2 hunks)Wable-iOS/Presentation/Profile/AlarmSetting/View/AlarmSettingViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/Component/ProfileInfoCell.swift(5 hunks)Wable-iOS/Presentation/Profile/Component/ProfileSegmentedHeaderView.swift(2 hunks)Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift(10 hunks)Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift(5 hunks)Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift(11 hunks)Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift(5 hunks)Wable-iOS/Presentation/TabBar/TabBarController.swift(2 hunks)Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift(5 hunks)Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift(5 hunks)Wable-iOS/Presentation/WableComponent/Cell/CommentCollectionViewCell.swift(1 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- Wable-iOS/Presentation/Profile/AlarmSetting/View/AlarmSettingViewController.swift
- Wable-iOS/Presentation/Overview/Detail/View/AnnouncementDetailViewController.swift
🔇 Additional comments (71)
Wable-iOS.xcodeproj/project.pbxproj (2)
1668-1675: Correctly added newEditPBXGroup for Profile editing
The new groupEdit(DE4EFCCF2DE0060700E4146C) encapsulatesProfileEditViewController.swiftas expected. Verify that thefileRef(DD51A44B2DD458A8004295B6) matches the actual file reference in thePBXFileReferencesection.
1734-1734: IncludedEditsubgroup under the Profile group
TheEditgroup is correctly inserted into theProfilePBXGroup’s children list betweenMyandAlarmSetting, reflecting the intended project structure.Wable-iOS/Presentation/Profile/AlarmSetting/View/AlarmSettingViewController.swift (3)
107-110: Verify unconditional gesture enabling doesn't break existing functionality.The change simplifies gesture recognizer setup by removing the delegate-based conditional logic, but unconditionally enabling the interactive pop gesture might introduce unintended behavior. The previous implementation likely had logic to prevent the gesture in certain states.
Please verify that:
- This change maintains the same functional behavior as the previous delegate-based implementation
- The gesture won't interfere with user interactions (e.g., during form input or critical operations)
- There are no edge cases where the gesture should be disabled
#!/bin/bash # Description: Search for any remaining gesture delegate usage or conditional gesture logic # Test: Look for patterns that might indicate where conditional gesture logic was previously used # Search for gesture recognizer delegate patterns rg -A 3 -B 3 "UIGestureRecognizerDelegate|gestureRecognizerShouldBegin|interactivePopGestureRecognizer.*delegate" # Search for conditional gesture enabling patterns rg -A 3 -B 3 "interactivePopGestureRecognizer.*isEnabled.*=" # Look for navigation controller count checks (common pattern for conditional gesture enabling) rg -A 3 -B 3 "navigationController.*viewControllers.*count"
109-109: Good simplification of gesture recognizer setup.The direct enabling of the interactive pop gesture recognizer simplifies the code by removing the need for delegate conformance and conditional logic. This improves maintainability.
However, please verify that unconditionally enabling the gesture doesn't cause issues in edge cases where the navigation stack might have only one view controller.
#!/bin/bash # Description: Search for any remaining gesture recognizer delegate implementations to ensure consistency # Expected: No remaining UIGestureRecognizerDelegate conformance in view controllers that were simplified echo "Checking for remaining UIGestureRecognizerDelegate conformance in the codebase..." rg -A 3 "UIGestureRecognizerDelegate" --type swift echo -e "\nChecking for gestureRecognizerShouldBegin implementations..." rg -A 5 "gestureRecognizerShouldBegin" --type swift
109-109: Verify navigation stack behavior with unconditional gesture enabling.The direct enabling of the interactive pop gesture recognizer removes the previous conditional logic that likely checked the navigation stack count. This could cause navigation issues if this view controller is ever presented as the root view controller, as users would be able to swipe back from the root, potentially causing unexpected behavior.
Please verify that this view controller is never presented as the root view controller in the navigation stack:
#!/bin/bash # Description: Search for instantiation patterns of AlarmSettingViewController to verify it's never used as root # Search for AlarmSettingViewController instantiation and navigation patterns rg -A 5 -B 5 "AlarmSettingViewController" --type swift # Look for direct presentation or setting as root view controller ast-grep --pattern 'navigationController?.setViewControllers([$$$], animated: $_)' ast-grep --pattern 'navigationController?.viewControllers = [$$$]'Wable-iOS/Presentation/Overview/Detail/View/AnnouncementDetailViewController.swift (6)
58-58: Consistent navigation setup pattern - verify edge case handling.Good addition of the
setupNavigationBar()call to ensure proper navigation configuration during view setup.
98-101: Verify unconditional gesture enabling aligns with user experience requirements.The implementation follows the same simplified pattern as
AlarmSettingViewController.swift, which is good for consistency. However, the same concern applies here regarding unconditional gesture enabling.Since this is an announcement detail view, users might be reading content and could accidentally trigger the gesture. Please ensure this doesn't negatively impact the reading experience.
58-58: Proper integration of navigation setup.Good placement of the
setupNavigationBar()call inviewDidLoad()following the standard view controller lifecycle pattern.
98-101: Consistent and clean navigation bar setup implementation.The
setupNavigationBar()method implementation is consistent with the pattern used inAlarmSettingViewController.swift. The method properly:
- Hides the navigation bar for custom navigation UI
- Enables the interactive pop gesture for intuitive navigation
This approach simplifies the gesture recognizer setup compared to the previous delegate-based conditional logic.
58-58: Good practice: Consistent navigation setup.The addition of
setupNavigationBar()call inviewDidLoad()maintains consistency with the navigation setup pattern used across the codebase.
98-101: Verify navigation stack behavior with unconditional gesture enabling.Similar to the other view controller in this PR, the new
setupNavigationBar()method unconditionally enables the interactive pop gesture recognizer. This removes previous safeguards that likely prevented swipe-back gestures when this view controller is the root view controller.While the
hidesBottomBarWhenPushed = truein the initializer suggests this is typically pushed onto a navigation stack, it's important to verify this view controller is never presented as the root to avoid navigation issues.Please verify the usage patterns of this view controller:
#!/bin/bash # Description: Search for AnnouncementDetailViewController instantiation and navigation patterns # Search for instantiation and navigation patterns rg -A 5 -B 5 "AnnouncementDetailViewController" --type swift # Look for any direct presentation patterns that might make it a root view controller ast-grep --pattern 'let $_ = AnnouncementDetailViewController()' ast-grep --pattern 'AnnouncementDetailViewController().present($_)'Wable-iOS/Domain/RepositoryInterface/ContentRepository.swift (1)
15-15: LGTM! Good API modernization approach.Adding an async/await variant alongside the existing Combine method provides flexibility for consumers to choose their preferred concurrency model. The method signature correctly follows Swift async conventions.
Wable-iOS/Domain/RepositoryInterface/CommentRepository.swift (1)
17-17: LGTM! Consistent with async/await modernization pattern.The async variant follows the same pattern established in other repositories, providing a modern alternative to the Combine-based API while maintaining backward compatibility.
Wable-iOS/Data/Mapper/CommentMapper.swift (2)
15-15: Critical bug fix for date parsing.Good catch! The original format
"yyyy-MM-dd HH:mm:dd"incorrectly useddd(day of month) for the seconds component. The corrected format"yyyy-MM-dd HH:mm:ss"properly usesssfor seconds.
57-57: Standardized seconds format for consistency.The change from
"yyyy-MM-dd HH:mm:SS"to"yyyy-MM-dd HH:mm:ss"standardizes the seconds format.SSrepresents fractional seconds whilessrepresents whole seconds, which is likely what the API returns.Wable-iOS/Domain/RepositoryInterface/GhostRepository.swift (1)
18-25: LGTM! Good async/await support addition.The new async throwing method provides a clean alternative to the Combine-based approach while maintaining the same interface contract. This pattern supports gradual migration to Swift concurrency.
Wable-iOS/Presentation/TabBar/TabBarController.swift (2)
74-81: LGTM! Dependency injection follows consistent pattern.The new
userSessionUseCasedependency injection is properly structured and aligns with the other use case dependencies in the view model.
155-155: Verify the impact of disabling tab bar setup animation.The change from
animated: truetoanimated: falsewill make the tab bar appear immediately without animation during setup. While this might improve perceived performance, ensure this aligns with the intended user experience.Consider documenting the reason for this change if it's intentional for better perceived performance or if it resolves a specific UI issue.
Wable-iOS/Domain/RepositoryInterface/ContentLikedRepository.swift (1)
14-14: Good addition of async/await API variants.Adding async/await versions alongside the existing Combine methods provides flexibility during the migration to Swift concurrency while maintaining backward compatibility. The method signatures are consistent and follow Swift concurrency best practices.
Also applies to: 16-16
Wable-iOS/Domain/RepositoryInterface/ReportRepository.swift (1)
14-14: Consistent async/await API addition.The addition of async/await variants maintains consistency with other repository interfaces in this PR, providing the same dual API pattern for gradual migration from Combine to Swift concurrency.
Also applies to: 16-16
Wable-iOS/Presentation/WableComponent/Cell/CommentCollectionViewCell.swift (1)
231-231: Minor optimization in guard positioning.Moving the guard statement after UI setup allows more cell configuration to occur before early exit. This ensures handlers and basic UI state are set even if
createdDateis nil, which could be beneficial for cell reuse scenarios.Wable-iOS/Data/RepositoryImpl/ContentLikedRepositoryImpl.swift (1)
32-44: Well-implemented async variant of createContentLiked.The async implementation correctly mirrors the Combine version with proper error handling via
ErrorMapper.map. The do-catch pattern follows Swift concurrency best practices.Wable-iOS/Presentation/Home/View/HomeViewController.swift (4)
178-181: LGTM: Improved closure readabilityThe multi-line closure syntax with explicit parameters enhances code readability without affecting functionality.
276-283: Verify consistency of dependency injection refactoringThe initialization of
OtherProfileViewControllerhas been refactored to useCheckUserRoleUseCaseImplinstead of the previous use cases. This appears to be part of a broader architectural change.Please verify that this change is consistent across the codebase and that
OtherProfileViewModelhas been updated to work with this new dependency structure:#!/bin/bash # Description: Verify OtherProfileViewModel constructor and CheckUserRoleUseCaseImpl usage # Search for OtherProfileViewModel initialization patterns ast-grep --pattern 'OtherProfileViewModel( $$$ )' # Search for CheckUserRoleUseCaseImpl usage across the codebase rg -A 3 -B 3 "CheckUserRoleUseCaseImpl"
178-181: LGTM: Improved code readabilityThe closure syntax change from single-line trailing closure to multi-line format with explicit parameters improves readability without altering functionality.
276-283: Verify that OtherProfileViewController works with the new use case configurationThe initialization of
OtherProfileViewController's view model was significantly changed - the previous use casesFetchUserContentUseCaseImplandFetchUserCommentListUseCaseImplwere removed and replaced with a singleCheckUserRoleUseCaseImpl. This could be a breaking change if the profile view controller depends on those specific use cases.Please run the following script to verify that
OtherProfileViewControllerandOtherProfileViewModelhave been updated to work with this new configuration:#!/bin/bash # Description: Check OtherProfileViewController and OtherProfileViewModel for use case dependencies # Search for references to the removed use cases in profile-related files echo "=== Checking for removed use case references ===" rg "FetchUserContentUseCaseImpl|FetchUserCommentListUseCaseImpl" --type swift # Search for OtherProfileViewModel constructor to see what dependencies it expects echo "=== Checking OtherProfileViewModel constructor ===" ast-grep --pattern 'init($$$) { $$$ }' --lang swift | rg -A 10 -B 5 "OtherProfileViewModel" # Check if CheckUserRoleUseCaseImpl is actually used in OtherProfileViewModel echo "=== Checking CheckUserRoleUseCaseImpl usage ===" rg "CheckUserRoleUseCaseImpl" --type swift -A 3 -B 3Wable-iOS/Data/RepositoryImpl/ReportRepositoryImpl.swift (5)
34-48: LGTM: Well-implemented async/await methodThe async version of
createReportproperly handles errors usingdo-catchwithErrorMapper.map()and maintains consistency with the existing Combine implementation.
65-80: LGTM: Consistent async/await implementationThe async version of
createBanfollows the same error handling pattern as thecreateReportmethod, maintaining consistency across the repository.
34-48: LGTM: Well-implemented async variantThe async
createReportmethod correctly follows the established pattern:
- Wraps the provider request in a do-catch block
- Uses the same request structure as the Combine variant
- Properly maps errors via
ErrorMapperThis addition supports modern Swift concurrency alongside existing Combine patterns.
65-80: LGTM: Consistent async implementationThe async
createBanmethod maintains consistency with the asynccreateReportimplementation and follows the same error handling pattern.
94-96: LGTM: Appropriate mock stubsThe empty async method stubs in
MockReportRepositoryare appropriate placeholders that maintain interface conformity without implementing actual functionality, which is expected behavior for mock repositories.Also applies to: 104-106
Wable-iOS/Presentation/Profile/Component/ProfileSegmentedHeaderView.swift (6)
19-19: LGTM: Improved property namingThe rename from
segmentDidChangeClosuretoonSegmentIndexChangedprovides better clarity about the property's purpose.
21-21: LGTM: Proper Combine lifecycle managementAdding
cancelBagand callingsetupAction()during initialization ensures proper subscription lifecycle management.Also applies to: 28-28
52-58: LGTM: Excellent Combine pattern implementationThe refactoring from target-action to Combine publisher provides several benefits:
- Debouncing prevents excessive calls during rapid segment changes
removeDuplicates()avoids unnecessary processing- Weak self capture prevents retain cycles
- More declarative and testable code
This is a great example of modernizing UI event handling with Combine.
19-19: LGTM: Improved naming conventionThe property rename from
segmentDidChangeClosuretoonSegmentIndexChangedfollows better Swift naming conventions and is more descriptive.
21-21: LGTM: Proper Combine lifecycle managementAdded
cancelBagproperty andsetupAction()call ensures proper subscription lifecycle management for the new reactive implementation.Also applies to: 28-28
52-58: LGTM: Excellent modernization to reactive patternThe new
setupAction()method represents a significant improvement:
- Replaces imperative target-action pattern with declarative Combine publisher
- Adds 300ms debounce to prevent rapid-fire events, improving performance
- Uses
removeDuplicates()to avoid unnecessary callbacks- Properly manages subscription lifecycle with
cancelBagThis change aligns with modern reactive programming patterns and provides better user experience.
Wable-iOS/Data/RepositoryImpl/CommentRepositoryImpl.swift (3)
65-74: LGTM: Consistent async/await implementationThe async version of
deleteCommentfollows the established pattern with proper error handling usingdo-catchandErrorMapper.map(). The implementation is consistent with other async methods in the repository.
65-74: LGTM: Consistent async implementationThe async
deleteCommentmethod follows the established pattern used throughout the codebase:
- Proper do-catch error handling
- Uses the same provider request as the Combine variant
- Correctly maps errors via
ErrorMapperThis addition supports the broader effort to modernize the data layer with Swift concurrency.
118-120: LGTM: Appropriate mock behaviorThe mock implementation correctly throws
WableError.unknownError, which is consistent with the expected behavior of a mock repository and provides predictable error handling for testing scenarios.Wable-iOS/Presentation/Profile/Component/ProfileInfoCell.swift (5)
30-34: LGTM! Good addition for the level badge UI.The implementation correctly adds the level badge image view with appropriate content mode and clipping settings.
78-78: Good improvement using.zeroproperty.Using
.zerois more idiomatic and clear thansetProgress(0.0, animated: false).
126-126: Good fix for progress bar normalization.The new calculation correctly normalizes the ghost value to the 0.0-1.0 range required by UIProgressView. This fixes a potential issue where negative ghost values would have resulted in invalid progress values.
188-197: Well-structured UI layout with the badge.The constraints properly position the level badge next to the profile image and align the level label and nickname label correctly with the new badge element.
Also applies to: 200-202
258-262: Clean action handler implementation.The simplified closure syntax with weak self capture is more concise and follows Swift best practices.
Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift (2)
36-36: Good addition for profile tap handling.The relay properly follows the reactive pattern used throughout the view controller.
119-123: Profile tap handling looks good.The implementation correctly sends the user ID through the relay for profile navigation.
Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift (4)
8-9: Good adoption of Combine for reactive programming.The use of PassthroughSubject for pagination events with debouncing is a good pattern to prevent excessive API calls.
Also applies to: 58-59
98-99: Good defensive programming with guard statement.The guard ensures only post sections trigger navigation, preventing potential crashes from invalid section access.
143-143: Good optimization for pagination trigger.Changing from 5 to 2 items before the end provides a smoother user experience by loading content earlier.
319-322: Excellent use of debouncing for pagination.The 1-second debounce prevents excessive API calls when scrolling quickly. This is a performance optimization best practice.
Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift (2)
16-16: Good addition of user session dependency.The dependency is properly injected and stored, following the existing pattern.
Also applies to: 23-23, 29-29
212-223: Well-implemented profile navigation logic.The logic correctly determines whether to navigate to the user's own profile (nil) or another user's profile based on ID comparison.
Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift (9)
8-8: Good architectural improvements!Adding Combine import and Int raw value conformance to the Section enum are positive changes that enable safer section handling and reactive programming patterns.
Also applies to: 18-18
54-54: Excellent implementation of debounced pagination!The use of PassthroughSubject with a 1-second debounce effectively prevents excessive API calls during rapid scrolling. The weak self capture in the sink closure properly avoids retain cycles.
Also applies to: 142-142, 178-182
96-96: Good safety improvement!Using
Section(rawValue:)with guard statement prevents potential crashes from invalid section indices.
207-233: Well-implemented moderation features with proper access control!The cell configuration correctly:
- Sets authorType to
.othersfor other users' profiles- Uses weak self references to prevent retain cycles
- Implements role-based access control where admins see additional "ban" option
- Presents appropriate action sheets based on user permissions
243-267: Consistent implementation for comment cells!The comment cell configuration mirrors the content cell pattern with proper authorType, weak self references, and role-based moderation options.
275-275: Better naming convention!The property name
onSegmentIndexChangedis more descriptive thansegmentDidChangeClosureand clearly indicates that it receives an index parameter.
352-356: Good user feedback implementation!The binding properly observes report completion and provides clear feedback to users with an informative toast message.
403-477: Well-structured moderation action methods!The implementation provides:
- Clear separation between content and comment actions
- Consistent action sheet presentation pattern
- Proper memory management with weak self references
- Appropriate titles and messages for each action type
540-571: Excellent constants organization!Centralizing UI strings in a nested Constants enum improves maintainability and makes future localization easier. The organization by feature (Report, Ban, Ghost, Cancel) provides clear structure.
Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift (6)
26-29: Excellent adoption of repository pattern with dependency injection!Using
@Injectedproperty wrapper for repositories improves testability and follows SOLID principles. This refactoring from use cases to repositories with DI is a positive architectural change.
50-52: Good simplification of segment handling!Directly updating the segment instead of using Combine subjects reduces complexity while maintaining functionality. This follows the KISS principle effectively.
83-105: Well-implemented delete functionality!The delete methods properly:
- Use async/await for repository calls
- Update UI state on MainActor for thread safety
- Handle errors through centralized error handling
- Remove items from local state after successful deletion
107-154: Comprehensive like/unlike implementation!The toggle methods effectively:
- Handle both like and unlike states
- Update local state on MainActor after successful API calls
- Include proper trigger types for notifications
- Maintain data consistency with immutable updates
164-247: Excellent async/await implementation!The fetch methods demonstrate:
- Effective use of structured concurrency with
async let- Consistent thread-safe UI updates with
MainActor.run- Proper task cancellation handling
- Clear separation between async operations and UI state updates
249-252: Clean error handling pattern!The
@MainActorannotated error handler ensures thread-safe error message updates and provides a single point for error processing.Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift (3)
16-32: Good additions for moderation features!The new published properties enable proper user feedback, and the repository dependencies follow the established dependency injection pattern consistently.
124-126: Clean role checking interface!The method provides a simple way for the view controller to check user permissions for moderation features.
128-155: Well-implemented reporting functionality!The report methods properly extract relevant data (nickname and text) and provide user feedback through the completion state.
| func showMyProfile() { | ||
| let myProfileTabIndex = 4 | ||
| navigationController?.tabBarController?.selectedIndex = myProfileTabIndex | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using an enum for tab indices instead of hard-coding.
The hard-coded tab index 4 is fragile and could break if the tab order changes. Consider defining an enum or constants for tab indices.
- func showMyProfile() {
- let myProfileTabIndex = 4
- navigationController?.tabBarController?.selectedIndex = myProfileTabIndex
- }
+ func showMyProfile() {
+ navigationController?.tabBarController?.selectedIndex = TabIndex.profile.rawValue
+ }Add this enum to your tab bar controller or a shared constants file:
enum TabIndex: Int {
case home = 0
case search = 1
case viewit = 2
case notification = 3
case profile = 4
}🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift around
lines 294 to 297, the tab index 4 is hard-coded when selecting the profile tab,
which is fragile. Define an enum representing the tab indices, such as TabIndex
with cases for each tab and their raw values, and replace the hard-coded 4 with
TabIndex.profile.rawValue to improve code clarity and maintainability.
| func banComment(for commentID: Int) { | ||
| Task { | ||
| do { | ||
| try await reportRepository.createBan( | ||
| memberID: userID, | ||
| triggerType: .content, | ||
| triggerID: commentID | ||
| ) | ||
|
|
||
| fetchViewItems(userID: userID, segment: item.currentSegment) | ||
|
|
||
| await MainActor.run { isGhostCompleted = true } | ||
| } catch { | ||
| await handleError(error: error) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix critical issues in banComment method
The banComment method has two bugs that need to be fixed:
- Line 178: Incorrect trigger type - should be
.commentinstead of.content - Line 184: Wrong completion flag - sets
isGhostCompletedinstead of a ban-specific flag
Apply this diff to fix the issues:
- try await reportRepository.createBan(
- memberID: userID,
- triggerType: .content,
- triggerID: commentID
- )
+ try await reportRepository.createBan(
+ memberID: userID,
+ triggerType: .comment,
+ triggerID: commentID
+ )
fetchViewItems(userID: userID, segment: item.currentSegment)
- await MainActor.run { isGhostCompleted = true }
+ // Remove this line or add a proper ban completion flag📝 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.
| func banComment(for commentID: Int) { | |
| Task { | |
| do { | |
| try await reportRepository.createBan( | |
| memberID: userID, | |
| triggerType: .content, | |
| triggerID: commentID | |
| ) | |
| fetchViewItems(userID: userID, segment: item.currentSegment) | |
| await MainActor.run { isGhostCompleted = true } | |
| } catch { | |
| await handleError(error: error) | |
| } | |
| } | |
| } | |
| func banComment(for commentID: Int) { | |
| Task { | |
| do { | |
| try await reportRepository.createBan( | |
| memberID: userID, | |
| triggerType: .comment, | |
| triggerID: commentID | |
| ) | |
| fetchViewItems(userID: userID, segment: item.currentSegment) | |
| // Remove this line or add a proper ban completion flag | |
| } catch { | |
| await handleError(error: error) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift
between lines 173 and 189, fix two bugs in the banComment method: change the
triggerType argument in the createBan call from .content to .comment, and
replace setting isGhostCompleted with the correct ban-specific completion flag
to reflect the ban action accurately.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift (1)
173-189:⚠️ Potential issueCritical bugs still present in banComment method - Same issues from previous review.
The banComment method still contains the two critical bugs identified in the previous review:
- Line 178: Incorrect trigger type - using
.contentinstead of.comment- Line 184: Wrong completion flag - setting
isGhostCompletedinstead of a ban-specific flagThese bugs will cause incorrect API calls and misleading UI state.
Apply this diff to fix the critical issues:
try await reportRepository.createBan( memberID: userID, - triggerType: .content, + triggerType: .comment, triggerID: commentID ) fetchViewItems(userID: userID, segment: item.currentSegment) - await MainActor.run { isGhostCompleted = true } + // Remove this line or add a proper ban completion flag if needed
🧹 Nitpick comments (2)
Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift (2)
89-89: Fix ternary operator usage for void functions.The static analysis correctly identified that using ternary operators to call void functions should be avoided for better readability.
Apply this diff to improve readability:
- isLiked ? contentInfo.like.unlike() : contentInfo.like.like() + if isLiked { + contentInfo.like.unlike() + } else { + contentInfo.like.like() + }And similarly for line 115:
- isLiked ? commentInfo.like.unlike() : commentInfo.like.like() + if isLiked { + commentInfo.like.unlike() + } else { + commentInfo.like.like() + }Also applies to: 115-115
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 89-89: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
191-223: Consider adding completion state tracking for ghost actions.The ghosting methods are well-implemented, but unlike reporting, they don't update the
isGhostCompletedflag consistently. Consider whether ghost actions should provide UI feedback similar to reporting.If ghost completion feedback is needed, add this after successful operations:
fetchViewItems(userID: userID, segment: item.currentSegment) + await MainActor.run { isGhostCompleted = true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Wable-iOS.xcodeproj/project.pbxproj(6 hunks)Wable-iOS/Presentation/Profile/Model/ProfileEmptyCellItem.swift(1 hunks)Wable-iOS/Presentation/Profile/Model/ProfileSegment.swift(1 hunks)Wable-iOS/Presentation/Profile/Model/ProfileViewItem.swift(1 hunks)Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift(1 hunks)Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift(5 hunks)Wable-iOS/Presentation/Profile/Other/View/Cell/OtherProfileEmptyCell.swift(1 hunks)Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- Wable-iOS/Presentation/Profile/Model/ProfileViewItem.swift
- Wable-iOS/Presentation/Profile/Model/ProfileSegment.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Wable-iOS.xcodeproj/project.pbxproj
🧰 Additional context used
🧬 Code Graph Analysis (2)
Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift (1)
Wable-iOS/Presentation/Profile/Other/View/Cell/OtherProfileEmptyCell.swift (1)
configure(32-35)
Wable-iOS/Presentation/Profile/Other/View/Cell/OtherProfileEmptyCell.swift (1)
Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift (1)
configure(43-58)
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift
[Warning] 89-89: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
[Warning] 115-115: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🔇 Additional comments (21)
Wable-iOS/Presentation/Profile/Model/ProfileEmptyCellItem.swift (2)
11-11: LGTM! Clean type rename as part of refactoring.The property type update from
ProfileSegmentKindtoProfileSegmentis consistent with the broader enum renaming refactoring effort mentioned in the summary. This maintains the same functionality while improving code clarity.
11-11:Details
✅ Verification successful
LGTM! Clean enum type refactoring.
The type change from
ProfileSegmentKindtoProfileSegmentimproves naming consistency. However, ensure this change has been applied consistently across the entire codebase to avoid compilation errors.
🏁 Script executed:
#!/bin/bash # Description: Verify that ProfileSegmentKind has been completely replaced with ProfileSegment across the codebase # Expected: No occurrences of ProfileSegmentKind should remain echo "Searching for any remaining ProfileSegmentKind references..." rg "ProfileSegmentKind" --type swift echo "Verifying ProfileSegment usage..." rg "ProfileSegment" --type swift -A 2Length of output: 4975
Enum refactoring complete and verified
Search for
ProfileSegmentKindreturned no hits, confirming it’s been fully replaced withProfileSegmentthroughout the codebase. All usages ofProfileSegmentare correct and consistent—no further action required.Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift (2)
43-43: Excellent refactoring! Parameter type updated consistently.The method signature update from
ProfileSegmentKindtoProfileSegmentmaintains the same interface behavior - the switch statement logic with.contentand.commentcases works identically. This demonstrates good refactoring practice where the type name is improved without breaking existing functionality.
43-58: Implementation looks correct and consistent.The parameter type change aligns with the enum refactoring from
ProfileSegmentKindtoProfileSegment. The method correctly handles both.contentand.commentcases with appropriate UI visibility logic and nickname handling.Wable-iOS/Presentation/Profile/Other/View/Cell/OtherProfileEmptyCell.swift (2)
32-32: Consistent type update across profile components.The parameter type change from
ProfileSegmentKindtoProfileSegmentis consistent with the same update inMyProfileEmptyCell.swift(line 43-58). Both empty cell implementations now use the renamed enum type while preserving identical logic for handling.contentand.commentcases. This demonstrates good consistency in the refactoring effort.
32-35: Implementation is correct and follows consistent patterns.The parameter type change from
ProfileSegmentKindtoProfileSegmentis properly implemented. The method correctly uses the enum to select appropriate empty state messages and handles the optional nickname with a sensible fallback ("알 수 없음").Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift (5)
16-17: LGTM: Good addition of completion state tracking.The new published properties
isReportCompletedandisGhostCompletedprovide clear UI feedback for async moderation actions.
26-32: Excellent refactoring to dependency injection pattern.The migration from direct use case dependencies to injected repositories improves testability and follows SOLID principles. The @injected pattern provides clean separation of concerns.
128-155: Well-implemented reporting functionality with proper error handling.The reporting methods correctly extract user information and handle async operations with proper error handling and UI state updates.
227-265: Excellent async/await implementation with proper concurrency.The refactored fetch method uses
async letfor concurrent operations and proper MainActor usage for UI updates. The error handling is centralized and clean.
317-321: Good centralized error handling approach.The
@MainActorannotation ensures thread-safe UI updates for error messages. This is a clean pattern for error handling across the view model.Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift (10)
26-29: LGTM: Clean dependency injection pattern.The migration from use cases to repositories using the
@Injectedproperty wrapper is well-structured and follows a consistent dependency injection pattern.
51-53: LGTM: Simple and correct segment change handling.The method correctly maps the selected index to a ProfileSegment enum and updates the current segment state.
70-81: LGTM: Improved pagination logic with direct method calls.The refactoring from Combine subjects to direct method calls makes the pagination logic more explicit and easier to follow. The switch statement properly handles different segment types.
107-128: LGTM: Well-implemented optimistic UI update with proper state capture.The method correctly captures the current like status before the async operation and handles both like/unlike scenarios properly. The MainActor usage for UI updates is appropriate.
130-155: LGTM: Consistent implementation with proper state management.The comment like toggle follows the same good pattern as content likes, with proper state capture and MainActor usage for UI updates. The inclusion of notification text is a nice touch.
158-196: LGTM: Clean async/await refactoring with proper concurrency handling.The method properly uses
async letfor concurrent execution of independent operations and correctly handles UI updates on the MainActor. The error handling and loading state management are well-implemented.
198-222: LGTM: Robust pagination implementation with task cancellation.The method includes proper task cancellation handling, loading state management, and MainActor usage for UI updates. The pagination logic is sound.
224-247: LGTM: Consistent pagination pattern for comments.This method follows the same robust pattern as the content pagination with proper error handling and concurrency management.
249-253: LGTM: Centralized error handling with MainActor annotation.The error handling method is properly annotated with
@MainActorsince it updates UI state, providing a clean centralized approach to error management.
83-93: 🛠️ Refactor suggestionPotential race condition in content deletion.
The current implementation looks up the index before performing the async deletion, which could lead to a race condition if the content list is modified between the index lookup and the deletion.
Consider capturing the index and validating it after the successful deletion:
func deleteContent(for contentID: Int) { Task { do { try await contentRepository.deleteContent(contentID: contentID) - guard let index = item.contentList.firstIndex(where: { $0.id == contentID }) else { return } - _ = await MainActor.run { item.contentList.remove(at: index) } + await MainActor.run { + if let index = item.contentList.firstIndex(where: { $0.id == contentID }) { + item.contentList.remove(at: index) + } + } } catch { await handleError(error: error) } } }Likely an incorrect or invalid review comment.
| func deleteComment(for commentID: Int) { | ||
| Task { | ||
| do { | ||
| try await commentRepository.deleteComment(commentID: commentID) | ||
| guard let index = item.commentList.firstIndex(where: { $0.comment.id == commentID }) else { return } | ||
| _ = await MainActor.run { item.commentList.remove(at: index) } | ||
| } catch { | ||
| await handleError(error: error) | ||
| } | ||
| .sink { [weak self] userID in | ||
| guard let self else { return } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same race condition issue in comment deletion.
This method has the same potential race condition as the content deletion method.
Apply the same fix pattern as suggested for content deletion:
func deleteComment(for commentID: Int) {
Task {
do {
try await commentRepository.deleteComment(commentID: commentID)
- guard let index = item.commentList.firstIndex(where: { $0.comment.id == commentID }) else { return }
- _ = await MainActor.run { item.commentList.remove(at: index) }
+ await MainActor.run {
+ if let index = item.commentList.firstIndex(where: { $0.comment.id == commentID }) {
+ item.commentList.remove(at: index)
+ }
+ }
} catch {
await handleError(error: 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.
| func deleteComment(for commentID: Int) { | |
| Task { | |
| do { | |
| try await commentRepository.deleteComment(commentID: commentID) | |
| guard let index = item.commentList.firstIndex(where: { $0.comment.id == commentID }) else { return } | |
| _ = await MainActor.run { item.commentList.remove(at: index) } | |
| } catch { | |
| await handleError(error: error) | |
| } | |
| .sink { [weak self] userID in | |
| guard let self else { return } | |
| } | |
| } | |
| func deleteComment(for commentID: Int) { | |
| Task { | |
| do { | |
| try await commentRepository.deleteComment(commentID: commentID) | |
| await MainActor.run { | |
| if let index = item.commentList.firstIndex(where: { $0.comment.id == commentID }) { | |
| item.commentList.remove(at: index) | |
| } | |
| } | |
| } catch { | |
| await handleError(error: error) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift around
lines 95 to 105, the deleteComment method has a race condition when removing a
comment from item.commentList after deletion. To fix this, ensure that the check
for the comment's index and its removal from the list happen atomically on the
MainActor to prevent concurrent modifications. Wrap both the index lookup and
removal inside a single await MainActor.run block to synchronize access to
item.commentList.
youz2me
left a comment
youz2me
left a comment
There was a problem hiding this comment.
굿!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 고생하셨습니닷. 조금만 더 화이팅 🙇♀️
| static func toDomain(_ response: [DTO.Response.FetchContentComments]) -> [ContentComment] { | ||
| let dateFormatter = DateFormatter() | ||
| dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:SS" | ||
| dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:ss" |
There was a problem hiding this comment.
헉 SS로 하지 않아도 상관 없나요? 뭔가 항상 대문자와 소문자를 교차로 사용했던 것 같은데 아니어도 괜찮은거였나 보군요 ,,,
There was a problem hiding this comment.
제가 알기로는 SS는 잘못된 표현 방식으로 알고 있습니다.
대문자로 표기되는 것들은 그 만의 형식?(24시간/12시간)이 있어서 사용하는 것으로 알고 있어요.
| let homeCellRegistration = CellRegistration<ContentCollectionViewCell, Content> { [weak self] cell, indexPath, item in | ||
| let homeCellRegistration = CellRegistration<ContentCollectionViewCell, Content> { | ||
| [weak self] cell, | ||
| indexPath, | ||
| item in |
There was a problem hiding this comment.
이 부분 들여쓰기 하는 거 혹시 맞을까요?? 제가 항상 들여쓰기를 안했어서 ㅎ ㅎ ..... 여쭤봅니닷.
There was a problem hiding this comment.
왜 이렇게 됐을까요?
여기는 제가 다시 수정하겠습니다.
| func setupDelegate() { | ||
| navigationController?.interactivePopGestureRecognizer?.delegate = self | ||
| func setupNavigationBar() { | ||
| navigationController?.navigationBar.isHidden = true | ||
| navigationController?.interactivePopGestureRecognizer?.isEnabled = true |
| private let levelBadgeImageView = UIImageView(image: .icLevelBadge).then { | ||
| $0.contentMode = .scaleAspectFill | ||
| $0.clipsToBounds = true | ||
| } |
There was a problem hiding this comment.
contentMode가 .scaleAspectFill이면 잘릴 수 있을 것 같기도 해서 .scaleAspectFit을 추천드려봅니닷.
| guard let userRole = self?.viewModel.checkUserRole(), | ||
| userRole != .owner | ||
| else { | ||
| return | ||
| } |
There was a problem hiding this comment.
헉 이렇게 하셨군요. 저도 홈 조만간 개선하도록 하겠습니다...
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Wable-iOS/Presentation/Home/View/HomeViewController.swift (1)
178-179: Fix unused parameter in closure to address SwiftLint warning.The closure syntax change improves readability, but the
indexPathparameter is unused in the closure body and should be replaced with_to follow Swift conventions.- let homeCellRegistration = CellRegistration<ContentCollectionViewCell, Content> { - [weak self] cell, indexPath, item in + let homeCellRegistration = CellRegistration<ContentCollectionViewCell, Content> { + [weak self] cell, _, item in🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 179-179: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Wable-iOS/Presentation/Home/View/HomeViewController.swift(2 hunks)
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Home/View/HomeViewController.swift
[Warning] 179-179: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (1)
Wable-iOS/Presentation/Home/View/HomeViewController.swift (1)
274-281: LGTM! Dependency injection refactoring aligns with architecture improvements.The replacement of
FetchUserContentUseCaseImplandFetchUserCommentListUseCaseImplwithCheckUserRoleUseCaseImplis consistent with the broader refactoring mentioned in the PR objectives. The dependency injection pattern is properly maintained with the new repository-based architecture.
…am-Wable/WABLE-iOS into fix/#196-profile-improvements
👻 PULL REQUEST
📄 작업 내용
📚 참고자료
🔗 연결된 이슈
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Style
Chores