[Feat] 알림 페이지 UI 구현 및 로직 연결 #153
Conversation
# Conflicts: # Wable-iOS.xcodeproj/project.pbxproj
- 15자가 넘어갈 경우, ...으로 대체하는 메서드
- Mock 유즈케이스로 테스트
WalkthroughThis pull request reorganizes the notification handling components across the project. It renames and adds Swift files, updates model types and mapping functions, and adjusts method signatures to use more descriptive names. The changes extend the UI with a paginated notification page controlled by a segmented control, incorporate reactive programming with Combine in the view models, and update layout and constraint settings in several views. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NPC as NotificationPageViewController
participant SC as SegmentedControl
participant ANC as ActivityNotificationViewController
participant INC as InformationNotificationViewController
participant AM as AmplitudeManager
U->>NPC: Tap segmented control (switch view)
NPC->>SC: Update selected segment
alt User selects Activity
NPC->>ANC: Display Activity Notifications
NPC->>AM: Log Activity event
else User selects Information
NPC->>INC: Display Information Notifications
NPC->>AM: Log Information event
end
sequenceDiagram
participant VM as NotificationViewModel
participant UC as NotificationUseCaseImpl
participant Repo as NotificationRepositoryImpl
participant API as Network API
VM->>UC: Request notifications with cursor
UC->>Repo: Fetch notifications data
Repo->>API: API call for notifications
API--)Repo: Return notification data
Repo--)UC: Deliver mapped notifications
UC--)VM: Return notifications via publisher
Assessment against linked issues
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (21)
Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotiTriggerType+.swift (2)
74-80: Documentation mismatch for googleFormTypes.The comment for
googleFormTypesincorrectly states "알림 셀을 눌렀을 때, 글쓰기 페이지로 이동해야 하는 경우를 정의합니다" (defining cases where tapping the cell should navigate to a writing page), but the actual purpose is to navigate to a Google Form as correctly stated in the later part of the comment.- /// 알림 셀을 눌렀을 때, 글쓰기 페이지로 이동해야 하는 경우를 정의합니다. + /// 알림 셀을 눌렀을 때, 구글폼으로 이동해야 하는 경우를 정의합니다.
10-84: Consider using nested types for constants as suggested in PR objectives.The current implementation uses static properties, which works well. However, as mentioned in the PR objectives, it might be beneficial to use nested types for constants to better organize related values.
You could refactor this to use a nested structure like:
extension TriggerType.ActivityNotification { enum InteractionType { static let profileInteraction: Set<TriggerType.ActivityNotification> = [ .commentLike, .contentLike, // ... ] static let content: Set<TriggerType.ActivityNotification> = [ // ... ] static let writeContent: Set<TriggerType.ActivityNotification> = [ // ... ] static let googleForm: Set<TriggerType.ActivityNotification> = [ // ... ] } }This would allow for more organized access like
TriggerType.ActivityNotification.InteractionType.profileInteraction.Wable-iOS/Presentation/Notification/Page/View/Subview/NotificationPageView.swift (4)
17-19: Consider making UI components private unless needed externally.The
navigationViewis declared without an access modifier, making it accessible to other classes. Unless this view needs to be accessed or modified from outside this class, consider making it private to improve encapsulation.- let navigationView = NavigationView(type: .page(type: .detail, title: "알림")).then { + private let navigationView = NavigationView(type: .page(type: .detail, title: "알림")).then { $0.configureView() }
21-23: Consider making segmentedControl private and using localized strings.The
segmentedControlhas two potential improvements:
- Make it private with a public getter method if external access is needed
- Use localized strings for segment items instead of hardcoded Korean text
- let segmentedControl = WableSegmentedControl(items: ["활동", "정보"]).then { + private let segmentedControl = WableSegmentedControl(items: [LocalizedString.activity, LocalizedString.information]).then { $0.selectedSegmentIndex = 0 }
27-27: Consider making pagingContainerView private with a dedicated method for child view controller containment.Since this view will likely be used to contain child view controllers, consider making it private and providing a method for the parent view controller to add child view controllers to it.
- let pagingContainerView = UIView() + private let pagingContainerView = UIView()
58-81: Implementation looks good, consider using constants for fixed dimensions.The constraints implementation is well-structured and clear. For better maintainability, consider extracting magic numbers like 56, 48, and 1 to constants or a design system.
+ private enum Layout { + static let navigationHeight: CGFloat = 56 + static let segmentHeight: CGFloat = 48 + static let dividerHeight: CGFloat = 1 + } func setupConstraint() { navigationView.snp.makeConstraints { make in make.top.horizontalEdges.equalTo(safeArea) - make.adjustedHeightEqualTo(56) + make.adjustedHeightEqualTo(Layout.navigationHeight) } segmentedControl.snp.makeConstraints { make in make.top.equalTo(navigationView.snp.bottom) make.horizontalEdges.equalToSuperview() - make.adjustedHeightEqualTo(48) + make.adjustedHeightEqualTo(Layout.segmentHeight) } segmentDivisionLine.snp.makeConstraints { make in make.top.equalTo(segmentedControl.snp.bottom) make.horizontalEdges.equalToSuperview() - make.height.equalTo(1) + make.height.equalTo(Layout.dividerHeight) } // ... }Wable-iOS/Presentation/WableComponent/SegmentedControl/WableSegmentedControl.swift (3)
16-18: Consider using Auto Layout instead of frame-based positioningUsing frame-based positioning might cause layout issues on different device sizes. Consider using Auto Layout constraints for the underline view for better responsiveness and maintainability.
- private lazy var underline = UIView(backgroundColor: .purple50).then { - $0.frame.size = CGSize(width: Constant.underlineWidth, height: Constant.underlineHeight) - } + private lazy var underline = UIView(backgroundColor: .purple50).then { + $0.translatesAutoresizingMaskIntoConstraints = false + } // Then in configureAppearance(): + NSLayoutConstraint.activate([ + underline.heightAnchor.constraint(equalToConstant: Constant.underlineHeight), + underline.widthAnchor.constraint(equalToConstant: Constant.underlineWidth), + underline.bottomAnchor.constraint(equalTo: bottomAnchor) + ])
71-81: Add a method to handle segment selection changesCurrently, the underline updates in layoutSubviews, but there's no explicit method to handle segment selection changes. Consider adding a valueChanged action method and connecting it to the segmented control.
+ override init(items: [Any]?) { super.init(items: items) configureAppearance() updateUnderlinePosition(animated: false) + addTarget(self, action: #selector(segmentValueChanged), for: .valueChanged) } + @objc private func segmentValueChanged() { + updateUnderlinePosition(animated: true) + }
12-104: Add accessibility supportThe custom segmented control doesn't include any accessibility enhancements. Consider adding proper accessibility labels and hints to improve the experience for users with disabilities.
func configureAppearance() { addSubview(underline) let image = UIImage() setBackgroundImage(image, for: .normal, barMetrics: .default) setBackgroundImage(image, for: .selected, barMetrics: .default) setBackgroundImage(image, for: .highlighted, barMetrics: .default) setDividerImage(image, forLeftSegmentState: .selected, rightSegmentState: .normal, barMetrics: .default) + // Set accessibility traits + accessibilityTraits = .tabBar + + // Set accessibility for each segment + for i in 0..<numberOfSegments { + if let title = titleForSegment(at: i) { + setAccessibilityLabel(title, forSegmentAt: i) + } + } // Rest of the configuration code... }Wable-iOS/Presentation/Notification/Information/Model/InfoNotificationType+.swift (1)
10-21: Consider using localization for notification messagesWhile the implementation of the message extension is clean, the strings are hardcoded in Korean. For better maintainability and potential future internationalization, consider using NSLocalizedString.
extension InfoNotificationType { var message: String { switch self { case .gameDone: - return "오늘 경기가 끝났어요. 결과를 확인해 보세요!" + return NSLocalizedString("info_notification_game_done", + comment: "Message shown when a game is finished") case .gameStart: - return "이제 곧 경기가 시작해요! 얼른 치킨 시키고 보러 갈까요?" + return NSLocalizedString("info_notification_game_start", + comment: "Message shown when a game is about to start") case .weekDone: - return "이번 주 경기가 끝났어요. 다음주에 진행될 경기 일정이 나왔으니 확인해 보세요!" + return NSLocalizedString("info_notification_week_done", + comment: "Message shown when weekly games are finished") } } }Then create a Localizable.strings file with these keys and their translations.
Wable-iOS/Presentation/WableComponent/Cell/NotificationCell.swift (3)
18-23: Check user interaction settings for profileImageViewThe profileImageView is initialized with
isUserInteractionEnabled = false, but it needs to be interactive to handle tap gestures. While this is addressed later in theprofileImageViewDidTapActionproperty's didSet, it might be clearer to set it to true by default if tap actions are expected.
90-106: Document the URL string constants used for default profile imagesThe method uses hardcoded strings like "PURPLE", "BLUE", and "GREEN" as URL values to determine which default image to use. Consider adding a comment explaining this design choice, or better yet, use an enum for these special cases.
func configureDefaultProfileImage(imageURL: URL?) -> UIImage? { guard let absoluteString = imageURL?.absoluteString else { return nil } + // Special URL string constants used for default profile images switch absoluteString { case "PURPLE": return .imgProfileBlue case "BLUE": return .imgProfileBlue case "GREEN": return .imgProfileGreen default: return nil } }
72-85: Consider truncating long content textSince the content label is limited to 3 lines, it would be beneficial to use the newly added
truncatedmethod from the String extension to ensure the text fits appropriately.func configure( imageURL: URL?, content: String, time: String ) { if let defaultProfileImage = configureDefaultProfileImage(imageURL: imageURL) { profileImageView.image = defaultProfileImage } else { profileImageView.kf.setImage(with: imageURL, placeholder: UIImage(resource: .imgProfileSmall)) } - contentLabel.text = content + contentLabel.text = content.truncated(toLength: 100) timeLabel.text = time }Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift (2)
159-161: Fix typo in method nameThere's a typo in the method name:
backButtoDidTapshould bebackButtonDidTap.-@objc func backButtoDidTap() { +@objc func backButtonDidTap() { navigationController?.popViewController(animated: true) }
131-134: Update target-action method reference to match the fixed method nameIf you fix the typo in the
backButtoDidTapmethod name, you'll need to update this reference as well.func setupAction() { - navigationView.backButton.addTarget(self, action: #selector(backButtoDidTap), for: .touchUpInside) + navigationView.backButton.addTarget(self, action: #selector(backButtonDidTap), for: .touchUpInside) segmentedControl.addTarget(self, action: #selector(segmentedControlDidChange(_:)), for: .valueChanged) }Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotiViewModel.swift (1)
139-139: Consider enhanced error handling or logging.
.replaceError(with: [])safely avoids crashes but may silently discard important errors. Consider adding a logging mechanism or user-facing error feedback if relevant.Wable-iOS/Presentation/Notification/Activity/View/ActivityNotiViewController.swift (1)
220-221: Resolve or clarify TODO comments.
These lines indicate pending screen transition features. Consider removing them if completed or opening an issue if additional tasks remain.Do you want me to open a separate issue to track these transitions or provide an implementation stub?
Also applies to: 229-230, 248-249
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 220-220: TODOs should be resolved (상세 게시물로 이동)
(todo)
Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift (2)
143-143: Replace unused closure parameter with_for clarity.
TheindexPathparameter in the cell registration closure is never referenced. SwiftLint flags this as unused.-let cellRegistration = CellRegistration<NotificationCell, Item> { cell, indexPath, item in +let cellRegistration = CellRegistration<NotificationCell, Item> { cell, _, item in🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 143-143: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
199-200: Address the TODO regarding navigation.
The comment suggests navigating based on the selected notification'sitem. Consider implementing or logging the user’s action if it’s not yet ready.Shall I generate a helper method to handle the appropriate screen transition?
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotiViewModel.swift (2)
95-102: Consider logging or surfacing errors.
.replaceError(with: [])silently discards errors. Depending on user requirements, you might want to log or handle the error to provide better visibility into underlying issues.
111-115: Check for constant duplication across view models.If
Constant.defaultItemsCountPerPageandConstant.initialCursormatch those inActivityNotiViewModel, consider moving them to a shared configuration or global constants file to avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
Wable-iOS.xcodeproj/project.pbxproj(19 hunks)Wable-iOS/Data/Mapper/NotificationMapper.swift(1 hunks)Wable-iOS/Domain/Entity/Notification.swift(1 hunks)Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift(1 hunks)Wable-iOS/Presentation/Helper/Extension/String+.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotiTriggerType+.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotification+.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/View/ActivityNotiViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotiViewModel.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/Model/InfoNotificationType+.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotiViewModel.swift(1 hunks)Wable-iOS/Presentation/Notification/NotificationEmpty.swift(0 hunks)Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Page/View/Subview/NotificationPageView.swift(1 hunks)Wable-iOS/Presentation/Overview/News/ViewModel/NewsViewModel.swift(3 hunks)Wable-iOS/Presentation/Overview/Notice/View/Cell/NoticeCell.swift(1 hunks)Wable-iOS/Presentation/Overview/Notice/View/NoticeViewController.swift(0 hunks)Wable-iOS/Presentation/Overview/Notice/ViewModel/NoticeViewModel.swift(3 hunks)Wable-iOS/Presentation/WableComponent/Cell/NotificationCell.swift(1 hunks)Wable-iOS/Presentation/WableComponent/SegmentedControl/WableSegmentedControl.swift(1 hunks)
💤 Files with no reviewable changes (2)
- Wable-iOS/Presentation/Notification/NotificationEmpty.swift
- Wable-iOS/Presentation/Overview/Notice/View/NoticeViewController.swift
🧰 Additional context used
🧬 Code Definitions (3)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotiViewController.swift (1)
Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotiViewModel.swift (1)
transform(38-131)
Wable-iOS/Presentation/WableComponent/Cell/NotificationCell.swift (2)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotiViewController.swift (3)
setupView(119-127)setupConstraint(129-142)setupAction(173-175)Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift (1)
setupAction(131-134)
Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotiViewModel.swift (3)
Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift (1)
viewDidLoad(76-87)Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotiViewModel.swift (3)
transform(38-131)fetchNotifications(137-141)isLastPage(143-145)Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (2)
fetchInformationNotifications(30-33)fetchInformationNotifications(72-91)
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotiViewController.swift
[Warning] 220-220: TODOs should be resolved (상세 게시물로 이동)
(todo)
[Warning] 229-229: TODOs should be resolved (게시물 작성하기로 이동)
(todo)
[Warning] 248-248: TODOs should be resolved (유저 프로필로 이동)
(todo)
Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
[Warning] 143-143: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 197-197: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (31)
Wable-iOS/Presentation/Overview/News/ViewModel/NewsViewModel.swift (3)
58-58: Constant Naming Consistency in Pagination CheckThe usage of the renamed constant (
defaultItemsCountPerPage) in the pagination check is clear and consistent with the intended functionality. The logic correctly determines if the fetched news array is empty or has fewer items than the expected count.
83-84: Re-Verified Pagination Logic in HandleEventsThe updated pagination logic in the second
handleEventsclosure (line 84) properly applies the new constant to decide if the last page has been reached. This consistent approach ensures that both initial load and pagination follow the same rules.
110-112: Enhanced Clarity via Constant RenamingRenaming the constant from
defaultNewsCountPerPagetodefaultItemsCountPerPageimproves readability and reusability by making it more generic in context. The change is well-applied in its definition and subsequent usage.Wable-iOS/Presentation/Overview/Notice/View/Cell/NoticeCell.swift (1)
78-78: Improved spacing in notification cell layout.The increased insets from 4pt to 16pt create a more consistent and spacious layout that aligns with iOS design standards. This change enhances content readability and provides better visual separation between the cell content and its boundaries.
Also applies to: 84-84, 89-89
Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotiTriggerType+.swift (3)
11-32: Good documentation and organization of notification types.The documentation for
profileInteractionTypesis comprehensive and clearly explains the purpose of this set. The organization of notification types into logical groups with detailed explanations will help developers understand when and how to use them.
34-61: Well-structured content types with clear use case documentation.The
contentTypesset is well-organized and includes thorough documentation about when to navigate to content detail pages. The comprehensive list covers various interaction scenarios which will be valuable for maintaining consistent navigation throughout the app.
63-72: Clear implementation for write content navigation.The
writeContentTypesset is appropriately documented and implements the requirements for navigating to content creation pages. Even though it currently only contains one type, the structure allows for easy expansion in the future.Wable-iOS/Presentation/Overview/Notice/ViewModel/NoticeViewModel.swift (3)
56-59: Consistent Pagination Check with Renamed ConstantThe pagination logic in the initial load branch correctly uses the renamed constant. The expression
news.isEmpty || news.count < Constant.defaultItemsCountPerPage
ensures that if the returned items are fewer than the expected page size or empty, the view model correctly signals that no further pages exist.
80-83: Consistent Application of Renamed Constant in Load More FlowHere too, the pagination check correctly employs
Constant.defaultItemsCountPerPageto determine the last page condition in the “load more” branch. This consistency across the two data-loading paths helps maintain correct pagination behavior.
107-111: Clear and Consistent Naming of Pagination ConstantRenaming the constant from its former name to
defaultItemsCountPerPageimproves clarity, indicating that the value now more generically represents the number of items per page rather than just "news" items. This change is in line with the broader context of handling multiple item types.Wable-iOS/Presentation/Notification/Page/View/Subview/NotificationPageView.swift (3)
31-36: The initialization pattern is well-implemented.The view initialization follows good practices by separating setup logic into dedicated methods, making the code more maintainable and readable.
38-41: Good practice marking the NSCoder initializer as unavailable.This ensures the view is only created programmatically and not from Interface Builder, which is appropriate for this type of custom view.
47-56: The view setup is well-structured.Good job using a helper method for setting up the view and organizing subviews. The
addSubviewsmethod (which appears to be a custom extension) makes the code cleaner and more readable.Wable-iOS.xcodeproj/project.pbxproj (3)
216-216: Well-structured notification UI implementation with clean architecture patterns.The project file changes show a comprehensive implementation of the notification UI with a well-organized architecture. The changes include:
- A page view controller with segmented control to switch between notification types
- Separate view controllers for Activity and Information notifications
- View models for both notification types
- Extension files for model types
- Reusable UI components (cells, segmented control)
This structure follows good iOS architecture patterns with clear separation of concerns.
Also applies to: 219-219, 230-233, 249-253, 469-472, 483-486
1430-1446: Good organization of notification feature components.The project structure for notifications is properly organized into logical groups:
- The main Notification group contains Page, Activity, and Information subgroups
- Each subgroup follows a consistent pattern with Model, ViewModel, and View folders
- The structure promotes maintainability and clean component separation
This organization will make it easier for team members to navigate and extend the notification feature.
Also applies to: 1519-1569
1117-1118: Good reusability through custom UI components.The implementation includes reusable UI components like
NotificationCellandWableSegmentedControlthat can potentially be used in other parts of the app. Placing these components in a dedicated section of the project demonstrates good architectural decision-making.Also applies to: 1544-1550
Wable-iOS/Data/Mapper/NotificationMapper.swift (1)
37-37: Property name update for improved clarityThe parameter has been renamed from
texttotargetContentTextin the ActivityNotification initialization, which better describes the purpose of this field and aligns with the corresponding property rename in the Notification.swift domain entity.Wable-iOS/Presentation/WableComponent/SegmentedControl/WableSegmentedControl.swift (1)
12-39: Well-structured custom segmented control implementationThe custom segmented control implementation with an animated underline indicator is clean and well-organized. The class is appropriately marked as
finaland has good separation of concerns.Wable-iOS/Domain/Entity/Notification.swift (1)
17-17: Property name update for improved clarityThe property has been renamed from
texttotargetContentText, which better describes the purpose of this field by clarifying that it contains text related to the target content of the notification.Wable-iOS/Presentation/Helper/Extension/String+.swift (1)
55-72: Well-implemented string truncation utility!The truncated method is cleanly implemented with good documentation. It will be useful for handling text in UI elements like notifications where content may need to be shortened.
Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotification+.swift (1)
10-43: LGTM! Comprehensive notification message handling.The extension provides a well-structured computed property that converts notification types into human-readable messages. Good use of the guard statement to handle nil cases, and the switch statement covers all notification types with properly formatted messages.
Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotiViewModel.swift (3)
14-16: Constructor usage is clear and straightforward.
No issues identified; good job cleanly injecting theNotificationUseCasedependency.
38-81: Well-structured transform method handling pagination and UI updates.
This code effectively handles refreshes, loading states, and pagination using Combine. Good job merging the signals and updating with minimal duplication.
143-145:Details
❓ Verification inconclusive
Verify partial-result handling from the server.
TheisLastPagelogic checks whethernotifications.count < defaultItemsCountPerPage. Consider confirming that the server consistently returns full pages or zero when there are no further results to avoid missing intermediate partial pages.
🏁 Script executed:
#!/bin/bash # Searching for server-side pagination logic that might return partial results rg "func.*fetchActivityNotifications" -A 10Length of output: 3694
Attention: Confirm API Pagination Behavior
The current
isLastPageimplementation assumes that a page is only partial (or complete) if the number of notifications is less thanConstant.defaultItemsCountPerPage. However, our search inNotificationUseCase.swift(specifically infetchActivityNotifications(for:)) suggests that the server-side logic is structured to either return a full page of notifications or none at all via an empty publisher when the pagination range is invalid. There is no explicit handling for cases where the API might return a non-full (but non-empty) page.
- Action: Verify the API contract or server behavior to ensure that it never returns a partial page of notifications. If partial pages are possible, we should adjust the pagination logic accordingly.
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotiViewController.swift (2)
103-112: Scroll-based prefetch approach is correct.
TriggeringwillDisplayLastItemSubjectfive items ahead is a typical, user-friendly approach for infinite scrolling.
170-210: Great integration of Combine outputs with UI elements.
Mapping the loading and notification state streams to the collection view refresh control and loading indicator is neat and maintainable.Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (3)
11-14: Protocol design looks good.The
NotificationUseCaseprotocol cleanly describes methods for fetching both activity and information notifications using Combine. No issues detected in the interface.
23-28: Confirm intent when discarding the repository check result.The line
_ = notificationRepository.checkNotification()discards any returned result. IfcheckNotification()returns important data or a publisher requiring subscription, ensure it's correctly handled or intentionally ignored.
36-125: Mock implementation is appropriate for testing.The forced unwrap of
rangeis guarded by the precedingnilcheck, and the random delays plus varied mock data are suitable for simulating realistic scenarios. Everything is consistent for demonstration or testing purposes.Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotiViewModel.swift (2)
11-17: Dependency injection is clear.Injecting
NotificationUseCaseintoInformationNotiViewModelpromotes testability and decoupling from the underlying repository. This is a good design choice.
34-92: Reactive flow appears correct.The
transformmethod appropriately handles the lifecycle of notifications, merges refresh triggers, manages loading states, and appends additional notifications when nearing the end of the list. This aligns well with Combine best practices.
| func setupPageController() { | ||
| addChild(pageViewController) | ||
|
|
||
| pagingContainerView.addSubview(pagingView) | ||
| pagingView.snp.makeConstraints { make in | ||
| make.edges.equalToSuperview() | ||
| } | ||
|
|
||
| pageViewController.didMove(toParent: self) | ||
|
|
||
| pageViewController.delegate = self | ||
| pageViewController.dataSource = self | ||
|
|
||
| pageViewController.setViewControllers([viewControllers[currentIndex]], direction: .forward, animated: false) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complete the view controller containment pattern
The current implementation adds the page view controller as a child but doesn't follow the complete containment pattern. Make sure to add the page view controller's view to the view hierarchy properly.
func setupPageController() {
addChild(pageViewController)
pagingContainerView.addSubview(pagingView)
pagingView.snp.makeConstraints { make in
make.edges.equalToSuperview()
}
pageViewController.didMove(toParent: self)
pageViewController.delegate = self
pageViewController.dataSource = self
pageViewController.setViewControllers([viewControllers[currentIndex]], direction: .forward, animated: false)
}Committable suggestion skipped: line range outside the PR's diff.
| let useCase = MockNotificationUseCaseImpl() | ||
| let activityNotiViewController = ActivityNotiViewController(viewModel: .init(useCase: useCase)) | ||
|
|
||
| let informationNotiViewController = InformationNotiViewController(viewModel: .init(useCase: useCase)) | ||
|
|
||
| viewControllers.append(activityNotiViewController) | ||
| viewControllers.append(informationNotiViewController) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace mock implementation with dependency injection
Using MockNotificationUseCaseImpl() directly in the controller ties it to test data. Consider implementing proper dependency injection to allow switching between mock and real implementations.
func setupViewControllers() {
- let useCase = MockNotificationUseCaseImpl()
+ // TODO: Replace with dependency injection
+ let useCase: NotificationUseCase = isRunningTests ? MockNotificationUseCaseImpl() : RealNotificationUseCaseImpl()
let activityNotiViewController = ActivityNotiViewController(viewModel: .init(useCase: useCase))
let informationNotiViewController = InformationNotiViewController(viewModel: .init(useCase: useCase))
viewControllers.append(activityNotiViewController)
viewControllers.append(informationNotiViewController)
}📝 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.
| let useCase = MockNotificationUseCaseImpl() | |
| let activityNotiViewController = ActivityNotiViewController(viewModel: .init(useCase: useCase)) | |
| let informationNotiViewController = InformationNotiViewController(viewModel: .init(useCase: useCase)) | |
| viewControllers.append(activityNotiViewController) | |
| viewControllers.append(informationNotiViewController) | |
| func setupViewControllers() { | |
| // TODO: Replace with dependency injection | |
| let useCase: NotificationUseCase = isRunningTests ? MockNotificationUseCaseImpl() : RealNotificationUseCaseImpl() | |
| let activityNotiViewController = ActivityNotiViewController(viewModel: .init(useCase: useCase)) | |
| let informationNotiViewController = InformationNotiViewController(viewModel: .init(useCase: useCase)) | |
| viewControllers.append(activityNotiViewController) | |
| viewControllers.append(informationNotiViewController) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift (2)
197-197: Consider prefixing the closure parameter with_if it's unused.Since
itemis not yet used, SwiftLint flags this. If you plan on using it soon for navigation logic, it’s fine to keep it, but otherwise rename it to_ item.🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 197-197: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
199-199: Address the TODO.You have a
TODOindicating a pending navigation flow. Stale TODO comments can lead to confusion.Would you like me to open a ticket or help implement the navigation flow for these items?
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotiTriggerType+.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotiTriggerType+.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Notification/Information/View/InformationNotiViewController.swift
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
[Warning] 197-197: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (4)
Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift (4)
17-22: Good use of a property observer for page changes.Your logic in
didSetensures thattrackPageChangeEventonly triggers when the index actually changes, preventing redundant analytics calls. Nice job!
100-109: Consider replacing mock use case with dependency injection.Using
MockNotificationUseCaseImpl()directly here permanently ties this controller to test data. A production environment or a more flexible architecture typically benefits from injecting the appropriate implementation at runtime.
111-125: Well-structured view controller containment.Your approach—calling
addChild(pageViewController), adding its view as a subview with constraints, and then callingdidMove(toParent:)—completes the standard containment pattern properly. This addresses past feedback on partial containment patterns. Good job!
159-174: Avoid potential crashes with unowned references in animation completions.Using
[unowned self]here can result in a crash if the closure outlives the view controller. Consider[weak self]for a safer approach unless you guarantee the controller's lifetime.If you need help verifying that the closure always deallocates before the view controller, let me know, and I can generate a targeted script or show you how to track reference lifetimes.
youz2me
left a comment
youz2me
left a comment
There was a problem hiding this comment.
고생하셨습니다 ~!
파일 네이밍이 현재
InformationNoti
InfoNotification
ActivityNoti
ActivityNotification
...
이런 식으로 약어를 사용하거나 사용하지 않는 부분이 보이는데 이 부분 통일하면 좋을 것 같습니다! 개인적으로는 프로젝트 전체로 봤을 때는 약어를 사용하지 않는 게 조금 더 통일성이 있을 것 같다는 생각이 드네요 ㅎㅎ
나머지는 리뷰 남긴 부분 확인 부탁드려요 ~! 1700자 ... 고생 많으셨습니다 ㅜㅜ
| .receive(on: DispatchQueue.main) | ||
| .sink { [weak self] item in | ||
|
|
||
| // TODO: Item에 따른 화면 이동 |
There was a problem hiding this comment.
빠르게 구현해보겠습니다 .....................
| extension InfoNotificationType { | ||
| var message: String { | ||
| switch self { | ||
| case .gameDone: | ||
| return "오늘 경기가 끝났어요. 결과를 확인해 보세요!" | ||
| case .gameStart: | ||
| return "이제 곧 경기가 시작해요! 얼른 치킨 시키고 보러 갈까요?" | ||
| case .weekDone: | ||
| return "이번 주 경기가 끝났어요. 다음주에 진행될 경기 일정이 나왔으니 확인해 보세요!" | ||
| } | ||
| } | ||
| } |
| enum Constant { | ||
| static let defaultItemsCountPerPage: Int = 15 | ||
| static let initialCursor: Int = -1 | ||
| } |
There was a problem hiding this comment.
저도 이런 부분은 리터럴로 빼도 괜찮을 것 같다는 생각이 드네요!!
| /// 알림 셀에서 프로필 이미지 뷰를 눌렀을 때, 상호작용이 필요한 경우를 정의합니다. | ||
| /// | ||
| /// 이 Set에 포함된 알림 유형들은 사용자가 프로필 이미지를 탭했을 때 | ||
| /// 해당 사용자의 프로필로 이동하거나 추가 정보를 표시하는 등의 | ||
| /// 인터랙션이 필요한 알림 유형들입니다. | ||
| /// | ||
| /// - 포함된 알림 유형: | ||
| /// - `.commentLike`: 댓글에 좋아요를 받은 경우 | ||
| /// - `.contentLike`: 게시물에 좋아요를 받은 경우 | ||
| /// - `.comment`: 게시물에 댓글을 받은 경우 | ||
| /// - `.childComment`: 댓글에 대댓글을 받은 경우 | ||
| /// - `.childCommentLike`: 대댓글에 좋아요를 받은 경우 | ||
| /// | ||
| /// - Note: 이 Set에 포함되지 않은 알림 유형(예: 시스템 알림)은 | ||
| /// 프로필 이미지 탭 시 아무런 동작을 수행하지 않습니다. | ||
| static let profileInteractionTypes: Set<TriggerType.ActivityNotification> = [ | ||
| .commentLike, | ||
| .contentLike, | ||
| .comment, | ||
| .childComment, | ||
| .childCommentLike | ||
| ] | ||
|
|
||
| /// 알림 셀을 눌렀을 때, 게시물 상세 페이지로 이동해야 하는 경우를 정의합니다. | ||
| /// | ||
| /// 이 Set에 포함된 알림 유형들은 셀을 탭했을 때 관련된 게시물로 이동해야 하는 | ||
| /// 인터랙션이 필요한 알림 유형들입니다. | ||
| /// | ||
| /// - 포함된 알림 유형: | ||
| /// - `.contentLike`: 게시물에 좋아요를 받은 경우 | ||
| /// - `.comment`: 게시물에 댓글을 받은 경우 | ||
| /// - `.commentLike`: 댓글에 좋아요를 받은 경우 | ||
| /// - `.popularContent`: 어제 가장 인기 있었던 글의 경우 | ||
| /// - `.popularWriter`: 인기글로 선정된 경우 | ||
| /// - `.childComment`: 댓글에 대댓글을 받은 경우 | ||
| /// - `.childCommentLike`: 대댓글에 좋아요를 받은 경우 | ||
| /// - `.contentGhost`: 작성한 게시물로 인해 투명도가 낮아진 경우 | ||
| /// - `.commentGhost`: 작성한 댓글로 인해 투명도가 낮아진 경우 | ||
| /// - `.beGhost`: 작성 제한이 되버린 경우 |
github-project-automation
bot
moved this from Ready for Review
to In Review
in Wable-iOS
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (13)
Wable-iOS/Presentation/Notification/Information/Model/InformationNotificationType+.swift (1)
10-21: Consider localizing these strings
The hard-coded strings might be better served by a localization approach (e.g., using Localizable.strings).Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift (1)
197-202: Resolve or track the TODO for item-based navigation.
The comment indicates a missing navigation flow. To keep the codebase maintainable, consider implementing or removing the TODO.Do you want me to open a new issue to handle this navigation flow?
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
[Warning] 197-197: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift (1)
220-250: Multiple TODOs flag missing navigations.
You have placeholders for viewing details, composing a new post, and showing the user profile. These are key features that should either be implemented or tracked in an issue ticket.Would you like me to open separate issues to track these pending navigations for better clarity?
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 220-220: TODOs should be resolved (상세 게시물로 이동)
(todo)
[Warning] 229-229: TODOs should be resolved (게시물 작성하기로 이동)
(todo)
[Warning] 248-248: TODOs should be resolved (유저 프로필로 이동)
(todo)
Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotificationTriggerType+.swift (2)
34-61: Consider a more descriptive name forcontentTypes.The current name
contentTypesis somewhat generic. Since this set represents notification types that require navigation to a post's detail page, a more specific name likepostDetailNavigationTypeswould better communicate its purpose.-static let contentTypes: Set<TriggerType.ActivityNotification> = [ +static let postDetailNavigationTypes: Set<TriggerType.ActivityNotification> = [Don't forget to update references to this property elsewhere in the codebase.
74-83: Consider documenting the GoogleForm URL source.The
googleFormTypesset is well-defined, but it might be helpful to include information about where the Google Form URL is obtained from or how it's configured in the system.Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift (3)
38-59: Enhance error handling for user feedback.The transform method currently doesn't provide any error feedback to the user interface. Consider adding an error output publisher.
struct Output { let notifications: AnyPublisher<[ActivityNotification], Never> let isLoading: AnyPublisher<Bool, Never> let isLoadingMore: AnyPublisher<Bool, Never> let content: AnyPublisher<Int, Never> let writeContent: AnyPublisher<Void, Never> let googleForm: AnyPublisher<Void, Never> let user: AnyPublisher<Int, Never> + let error: AnyPublisher<Error, Never> } func transform(input: Input, cancelBag: CancelBag) -> Output { let notificationsSubject = CurrentValueSubject<[ActivityNotification], Never>([]) let isLoadingSubject = CurrentValueSubject<Bool, Never>(false) let isLoadingMoreSubject = CurrentValueSubject<Bool, Never>(false) let isLastPageSubject = CurrentValueSubject<Bool, Never>(false) + let errorSubject = PassthroughSubject<Error, Never>() // Update the fetchNotifications call to handle errors // ... return Output( notifications: notificationsSubject.eraseToAnyPublisher(), isLoading: isLoadingSubject.eraseToAnyPublisher(), isLoadingMore: isLoadingMoreSubject.eraseToAnyPublisher(), content: content, writeContent: writeContent, googleForm: googleForm, user: user, + error: errorSubject.eraseToAnyPublisher() ) }
136-146: Ensure error logging for silently handled errors.The
fetchNotificationsmethod silently replaces errors with an empty array. Consider adding logging to track these errors even if they're not shown to the user.func fetchNotifications(for lastItemID: Int) -> AnyPublisher<[ActivityNotification], Never> { return useCase.fetchActivityNotifications(for: lastItemID) + .handleEvents(receiveCompletion: { completion in + if case let .failure(error) = completion { + print("Failed to fetch notifications: \(error)") + // Or use a proper logging framework + } + }) .replaceError(with: []) .eraseToAnyPublisher() }
150-154: Consider making constants configurable for testing.The hardcoded constants could be made configurable for easier testing with different pagination sizes.
private extension ActivityNotificationViewModel { enum Constant { - static let defaultItemsCountPerPage: Int = 15 - static let initialCursor: Int = -1 + static var defaultItemsCountPerPage: Int = 15 + static var initialCursor: Int = -1 } }Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (3)
36-70: Consider making mock delay configurable and adding error simulation.The mock implementation uses a random delay, which is good for testing variability, but consider making it configurable for predictable testing scenarios. Also, consider adding error simulation capabilities to test error handling.
struct MockNotificationUseCaseImpl: NotificationUseCase { - private var randomDelaySecond: Double { .random(in: 0.3...1.0) } + private var randomDelaySecond: Double + private var shouldSimulateError: Bool + + init(randomDelayRange: ClosedRange<Double> = 0.3...1.0, shouldSimulateError: Bool = false) { + self.randomDelaySecond = .random(in: randomDelayRange) + self.shouldSimulateError = shouldSimulateError + } func fetchActivityNotifications(for lastItemID: Int) -> AnyPublisher<[ActivityNotification], WableError> { + if shouldSimulateError { + return Fail(error: WableError.unknown) + .delay(for: .seconds(randomDelaySecond), scheduler: RunLoop.main) + .eraseToAnyPublisher() + } + let range = getPaginationRange(for: lastItemID) // Rest of the implementation remains the same
71-91: Duplicate error simulation pattern for information notifications.Similar to the previous comment, consider adding error simulation for the
fetchInformationNotificationsmethod.
95-106: Extract magic numbers to named constants.The magic numbers (15, 30, 33) used in the pagination ranges could be extracted to named constants for better clarity and maintainability.
+private enum PaginationConstants { + static let firstPageStart = 1 + static let firstPageEnd = 15 + static let secondPageStart = 16 + static let secondPageEnd = 30 + static let thirdPageStart = 31 +} private func getPaginationRange(for lastItemID: Int, maxPage: Int = 33) -> ClosedRange<Int>? { switch lastItemID { case -1: - return 1...15 + return PaginationConstants.firstPageStart...PaginationConstants.firstPageEnd case 15: - return 16...30 + return PaginationConstants.secondPageStart...PaginationConstants.secondPageEnd case 30: - return 31...maxPage + return PaginationConstants.thirdPageStart...maxPage default: return nil } }Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotificationViewModel.swift (2)
98-102: Improve error handling.Currently, errors from the use case are silently replaced with an empty array. Consider providing more explicit error handling or at least logging the errors for debugging purposes.
func fetchNotifications(for lastItemID: Int) -> AnyPublisher<[InformationNotification], Never> { return useCase.fetchInformationNotifications(for: lastItemID) + .handleEvents(receiveCompletion: { completion in + if case let .failure(error) = completion { + // Log error or handle it more explicitly + WableLogger.error("Failed to fetch notifications: \(error)") + } + }) .replaceError(with: []) .eraseToAnyPublisher() }
27-32: Consider adding error state output.The current output doesn't include a way to communicate errors to the UI. Consider adding an error publisher to provide feedback to users when network requests fail.
struct Output { let notifications: AnyPublisher<[InformationNotification], Never> let selectedNotification: AnyPublisher<InformationNotification, Never> let isLoading: AnyPublisher<Bool, Never> let isLoadingMore: AnyPublisher<Bool, Never> + let error: AnyPublisher<Error?, Never> }You would then need to update the transform method to handle and publish errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
Wable-iOS.xcodeproj/project.pbxproj(21 hunks)Wable-iOS/Data/Mapper/NotificationMapper.swift(2 hunks)Wable-iOS/Data/RepositoryImpl/NotificationRepositoryImpl.swift(1 hunks)Wable-iOS/Domain/Entity/Notification.swift(2 hunks)Wable-iOS/Domain/Enum/InformationNotificationType.swift(2 hunks)Wable-iOS/Domain/RepositoryInterface/NotificationRepository.swift(1 hunks)Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotificationTriggerType+.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/Model/InformationNotificationType+.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotificationViewModel.swift(1 hunks)Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Wable-iOS/Domain/Enum/InformationNotificationType.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Wable-iOS/Domain/Entity/Notification.swift
- Wable-iOS/Data/Mapper/NotificationMapper.swift
🧰 Additional context used
🧬 Code Definitions (4)
Wable-iOS/Domain/RepositoryInterface/NotificationRepository.swift (1)
Wable-iOS/Data/RepositoryImpl/NotificationRepositoryImpl.swift (1)
fetchInfoNotifications(18-25)
Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift (2)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift (5)
collectionView(94-96)collectionView(98-112)viewDidLoad(77-88)setupBinding(181-252)applySnapshot(258-264)Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotificationViewModel.swift (1)
transform(34-92)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift (2)
Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift (11)
collectionView(93-95)collectionView(97-111)viewDidLoad(76-87)setupView(117-125)setupConstraint(127-140)setupDataSource(142-159)setupAction(161-163)setupDelegate(165-167)setupBinding(169-210)collectionViewDidRefresh(228-230)applySnapshot(216-222)Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift (1)
transform(38-131)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (1)
Wable-iOS/Data/RepositoryImpl/NotificationRepositoryImpl.swift (3)
checkNotification(27-34)fetchUserNotifications(36-43)fetchInfoNotifications(18-25)
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
[Warning] 197-197: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift
[Warning] 220-220: TODOs should be resolved (상세 게시물로 이동)
(todo)
[Warning] 229-229: TODOs should be resolved (게시물 작성하기로 이동)
(todo)
[Warning] 248-248: TODOs should be resolved (유저 프로필로 이동)
(todo)
🔇 Additional comments (42)
Wable-iOS/Domain/RepositoryInterface/NotificationRepository.swift (1)
12-12:Details
✅ Verification successful
Verify old references
Ensure all references to the oldInfoNotificationtype are updated toInformationNotification.Please verify with the following script:
🏁 Script executed:
#!/bin/bash rg "InfoNotification"Length of output: 2294
Review Verification: All References Updated to InformationNotification
After running the verification script, all instances that previously referenced the old
InfoNotificationtype have been correctly updated toInformationNotification. This includes the interface inWable-iOS/Domain/RepositoryInterface/NotificationRepository.swiftas well as its implementations and related mapping logic. No further changes are necessary.Wable-iOS/Data/RepositoryImpl/NotificationRepositoryImpl.swift (1)
18-25: New type usage looks good
This update from[InfoNotification]to[InformationNotification]appears consistent across the codebase.Wable-iOS/Presentation/Notification/Page/View/NotificationPageViewController.swift (2)
102-106: Replace mock implementation with DI
You are directly instantiatingMockNotificationUseCaseImpl()here. Consider using dependency injection to switch between production and mock implementations more seamlessly.
167-173: Potential crash risk with unowned self
If the view controller is deallocated before the transition finishes,unowned selfmight cause a runtime crash.Can you confirm that this usage is safe in all scenarios?
Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift (7)
14-28: Well-structured class definition and type aliases.
These enumerations and type aliases improve code clarity, especially by differentiating each usage in the collection view data source.
31-48: UI components are set up cleanly.
Properties likecollectionView,emptyLabel, andloadingIndicatorare declared with appropriate styling. This layout is consistent with standard UIKit practices.
54-60: Combine subjects properly encapsulate user-driven events.
Exposing these subjects (didLoadSubject,didRefreshSubject, etc.) supports a clean reactive approach in the rest of the file.
76-88: Comprehensive setup inviewDidLoad.
The systematic initialization (view, constraint, data source, action, delegate, binding) is well-organized. Additionally, callingdidLoadSubject.send()is a good way to trigger data loading in a reactive context.
92-112: Delegate methods are handled correctly.
UsingdidSelectItemAtto route selection events viadidSelectSubjectis a neat way to keep the logic reactive. Also, the pagination trigger logic inwillDisplay cellensures additional data is fetched without blocking the main thread.
169-210: Binding logic properly connects the view model outputs.
The closure-based sinks neatly handle loading states, refresh control, and empty label logic. This approach simplifies the view’s code by letting the view model drive changes.🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 199-199: TODOs should be resolved (Item에 따른 화면 이동)
(todo)
[Warning] 197-197: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
216-222: Diffable data source snapshot updates are succinct and efficient.
Applying new items to the.mainsection viaapplySnapshotis straightforward and prevents UI inconsistencies.Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift (7)
15-29: Clear class organization and type aliases.
Defining theItem,DataSource, andSnapshotaliases in one place helps maintain code clarity and consistency while working with diffable data sources.
32-48: Good use of UI components and refresh controls.
The scroll behavior and refresh setup align well with iOS design, enhancing the user’s experience for pulling to refresh.
54-61: Reactive properties and subjects are well-defined.
They capture user interactions (didSelectItemSubject,profileImageViewDidTapSubject) effectively, promoting a clean separation of concerns.
77-88: Lifecycle method properly triggers reactive loading.
CallingdidLoadSubject.send()after all setup completes ensures the view model can proceed with initial data fetching.
93-112: Delegate callbacks for selection and pagination are well-designed.
The approach of alerting thewillDisplayLastItemSubjectavoids complicated timers or watchers, making it a performant way to handle incremental loading.
147-162: Cell registration shows good usage of dynamic data.
Conditionally binding profile tap actions based on notification type is a neat pattern, enhancing reusability.
256-264: Diffable data source updates are consistent with best practices.
Applying snapshots helps avoid concurrency issues compared to manual collection view updates.Wable-iOS.xcodeproj/project.pbxproj (10)
216-216: File reference for NotificationPageViewController added.
This entry correctly adds theNotificationPageViewController.swiftfile to the build phase.
219-219: File reference for InformationNotificationViewController added.
This entry correctly adds theInformationNotificationViewController.swiftfile to the build phase.
230-233: File references for Notification UI elements added.
These entries correctly addNotificationPageView.swift,WableSegmentedControl.swift,NotificationCell.swift, andActivityNotificationViewController.swiftto the build phase.
261-261: File reference for InformationNotificationType.swift added.
This entry looks consistent with the new notification-related files.
469-469: New PBXFileReference for NotificationPageViewController.swift.
Consistent reference to the same file mentioned in the PBXBuildFile section.
472-472: New PBXFileReference for InformationNotificationViewController.swift.
Matches the PBXBuildFile entry and ensures the file is recognized in the project structure.
483-486: New PBXFileReferences for NotificationPageView, WableSegmentedControl, NotificationCell, and ActivityNotificationViewController.
All references are accurately declared in the Sources group.
501-505: File references for additional notification models and view models.
These entries accurately include:
ActivityNotificationViewModel.swiftInformationNotificationViewModel.swiftInformationNotificationType+.swiftActivityNotification+.swiftActivityNotificationTriggerType+.swift
514-514: File reference for InformationNotificationType.swift reaffirmed.
No issues noted; aligns with the new notification system changes.
1117-1117: Added NotificationCell to the “Cell” group.
This ensures thatNotificationCell.swiftis also logically organized in the correct group.Wable-iOS/Presentation/Notification/Activity/Model/ActivityNotificationTriggerType+.swift (2)
10-32: Well-documented and organized extensions for notification types.The extension defining
profileInteractionTypesis well-structured with clear documentation explaining the purpose of this set. The comprehensive comments detailing each notification type and their interaction behaviors provide excellent context for developers.
63-72: Clear categorization for write content navigation types.This set is well-defined with proper documentation explaining its purpose for navigating to the content creation page.
Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift (5)
11-17: LGTM: Clean ViewModel initialization.The basic class structure with dependency injection through the constructor is well-implemented.
19-36: Well-structured Input/Output definitions.The Input and Output structures clearly define the communication channels between the view and view model, following good MVVM practices.
60-82: Well-implemented pagination with debounce.The pagination logic is well-implemented with proper debounce handling to prevent rapid requests, and good filtering to avoid unnecessary requests. The loading state management is also handled effectively.
98-115: Good handling of specialized notification types.The filtering and processing of different notification types (writeContent and googleForm) is clean and follows a consistent pattern.
83-96:Details
✅ Verification successful
Make sure TriggerType.ActivityNotification.contentTypes exists.
The code references
TriggerType.ActivityNotification.contentTypesbut if you renamed it as suggested earlier, make sure to update this reference as well.
🏁 Script executed:
#!/bin/bash # Description: Check if the contentTypes property is used elsewhere in the codebase rg "TriggerType.ActivityNotification.contentTypes" --type swiftLength of output: 238
TriggerType.ActivityNotification.contentTypes Reference Verified
The search confirms that the property is still referenced exactly as
TriggerType.ActivityNotification.contentTypesin the file, meaning it exists in the codebase. If you previously renamed this property, please update the reference here accordingly. Otherwise, no further action is necessary.Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (3)
11-14: Well-defined protocol with clear method signatures.The
NotificationUseCaseprotocol clearly defines the contract for fetching both activity and information notifications with appropriate return types.
16-34: Consider adding MARK directives for better code organization.Adding MARK directives would improve the organization and readability of this code section.
+// MARK: - NotificationUseCase Implementation final class NotificationUseCaseImpl: NotificationUseCase { private let notificationRepository: NotificationRepository init(notificationRepository: NotificationRepository) { self.notificationRepository = notificationRepository } + // MARK: - Protocol Methods func fetchActivityNotifications(for lastItemID: Int) -> AnyPublisher<[ActivityNotification], WableError> { _ = notificationRepository.checkNotification() return notificationRepository.fetchUserNotifications(cursor: lastItemID) .eraseToAnyPublisher() } func fetchInformationNotifications(for lastItemID: Int) -> AnyPublisher<[InformationNotification], WableError> { return notificationRepository.fetchInfoNotifications(cursor: lastItemID) .eraseToAnyPublisher() } }
23-28:Details
❓ Verification inconclusive
Verify the purpose of the discarded checkNotification() result.
The result of
notificationRepository.checkNotification()is discarded with_ =. If this is intentional (e.g., just marking notifications as checked without caring about the result), consider adding a comment explaining the purpose or handling any potential errors.
🏁 Script executed:
#!/bin/bash # Description: Check what checkNotification() does and how it's used elsewhere # Check the implementation of checkNotification() rg -A 5 "func checkNotification\(\)" --type swift # Check other places where checkNotification() is called rg "checkNotification\(\)" --type swift | grep -v "_ ="Length of output: 1449
Clarify Side-Effect Trigger & Publisher Subscription
The call to
notificationRepository.checkNotification()returns anAnyPublisher<Void, WableError>, and it now appears that it’s intended solely to trigger a side effect (marking notifications as checked) without caring for its result. However, since Combine publishers are cold by default, simply creating the publisher (and discarding it with_ =) will not start the network call unless it’s subscribed to—either explicitly or via some internal mechanism. Please verify that this behavior is intentional. If it is, consider adding an inline comment to clarify that:
- The call is meant only to trigger the side effect.
- Any errors are either handled upstream or intentionally ignored because they’re noncritical.
- The publisher is either auto-subscribed (if using a hot publisher) or that subscription is managed elsewhere.
Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotificationViewModel.swift (4)
11-17: Great setup with dependency injection.The class is properly marked as
finaland follows good dependency injection practices by accepting the use case through the initializer, which enhances testability.
19-33: Well-structured Input/Output definitions.The Input and Output structures are clean and follow reactive programming best practices with Combine. The publishers are appropriately typed and named according to their purpose.
111-116: Good use of namespaced constants.Using an enum for namespacing constants is a clean Swift pattern. The constants are well-defined and properly encapsulated within the ViewModel.
58-65: Good pagination implementation.The debounce and filtering logic for pagination is well implemented, preventing unnecessary network calls while ensuring a smooth user experience.
| private enum Constant { | ||
| static let imageURLText: String = "https://private-user-images.githubusercontent.com/80394340/349682631-566a0a8c-c673-4650-b9f4-3b74d7443aa9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDMzODUwNjMsIm5iZiI6MTc0MzM4NDc2MywicGF0aCI6Ii84MDM5NDM0MC8zNDk2ODI2MzEtNTY2YTBhOGMtYzY3My00NjUwLWI5ZjQtM2I3NGQ3NDQzYWE5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAzMzElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMzMxVDAxMzI0M1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRiMjJlZDIyOGU2M2E3NTBiMGQyMjUyNWI0MGQxYTk0ZGVkZmIyNWY2ZjY0YjVmZTQxNzdiMzQ0NzkxNTMzNmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.fslk0G5432-vBjha8bXJ6OAcCOusEowIPST_de3arwU" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace long GitHub URL with a more reliable placeholder.
The hardcoded GitHub URL is lengthy and might expire. Consider using a more permanent placeholder service URL.
private enum Constant {
- static let imageURLText: String = "https://private-user-images.githubusercontent.com/80394340/349682631-566a0a8c-c673-4650-b9f4-3b74d7443aa9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDMzODUwNjMsIm5iZiI6MTc0MzM4NDc2MywicGF0aCI6Ii84MDM5NDM0MC8zNDk2ODI2MzEtNTY2YTBhOGMtYzY3My00NjUwLWI5ZjQtM2I3NGQ3NDQzYWE5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAzMzElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMzMxVDAxMzI0M1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRiMjJlZDIyOGU2M2E3NTBiMGQyMjUyNWI0MGQxYTk0ZGVkZmIyNWY2ZjY0YjVmZTQxNzdiMzQ0NzkxNTMzNmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.fslk0G5432-vBjha8bXJ6OAcCOusEowIPST_de3arwU"
+ static let imageURLText: String = "https://picsum.photos/200"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private enum Constant { | |
| static let imageURLText: String = "https://private-user-images.githubusercontent.com/80394340/349682631-566a0a8c-c673-4650-b9f4-3b74d7443aa9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDMzODUwNjMsIm5iZiI6MTc0MzM4NDc2MywicGF0aCI6Ii84MDM5NDM0MC8zNDk2ODI2MzEtNTY2YTBhOGMtYzY3My00NjUwLWI5ZjQtM2I3NGQ3NDQzYWE5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAzMzElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMzMxVDAxMzI0M1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRiMjJlZDIyOGU2M2E3NTBiMGQyMjUyNWI0MGQxYTk0ZGVkZmIyNWY2ZjY0YjVmZTQxNzdiMzQ0NzkxNTMzNmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.fslk0G5432-vBjha8bXJ6OAcCOusEowIPST_de3arwU" | |
| } | |
| private enum Constant { | |
| static let imageURLText: String = "https://picsum.photos/200" | |
| } |
| let selectedNotification = input.didSelectItem | ||
| .filter { $0 < notificationsSubject.value.count } | ||
| .map { notificationsSubject.value[$0] } | ||
| .eraseToAnyPublisher() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential race condition in notifications access.
When mapping from index to notification, there's a potential race condition if the notifications array changes between the filter and map operations.
let selectedNotification = input.didSelectItem
- .filter { $0 < notificationsSubject.value.count }
- .map { notificationsSubject.value[$0] }
+ .compactMap { index -> InformationNotification? in
+ let notifications = notificationsSubject.value
+ guard index < notifications.count else { return nil }
+ return notifications[index]
+ }
.eraseToAnyPublisher()📝 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.
| let selectedNotification = input.didSelectItem | |
| .filter { $0 < notificationsSubject.value.count } | |
| .map { notificationsSubject.value[$0] } | |
| .eraseToAnyPublisher() | |
| let selectedNotification = input.didSelectItem | |
| .compactMap { index -> InformationNotification? in | |
| let notifications = notificationsSubject.value | |
| guard index < notifications.count else { return nil } | |
| return notifications[index] | |
| } | |
| .eraseToAnyPublisher() |
| .handleEvents(receiveOutput: { [weak self] notifications in | ||
| isLoadingMoreSubject.send(false) | ||
| isLastPageSubject.send(self?.isLastPage(notifications) ?? true) | ||
| }) | ||
| .filter { !$0.isEmpty } | ||
| .sink { notifications in | ||
| var currentItems = notificationsSubject.value | ||
| currentItems.append(contentsOf: notifications) | ||
| notificationsSubject.send(currentItems) | ||
| } | ||
| .store(in: cancelBag) |
There was a problem hiding this comment.
Fix inconsistent weak self usage.
Similar to the previous issue, there's a weak self capture in the handleEvents closure, but implicit strong references to self in subsequent operations. This should be consistent throughout the chain.
.handleEvents(receiveOutput: { [weak self] notifications in
isLoadingMoreSubject.send(false)
isLastPageSubject.send(self?.isLastPage(notifications) ?? true)
})
.filter { !$0.isEmpty }
-.sink { notifications in
+.sink(receiveValue: { [weak self] notifications in
+ guard let self = self else { return }
var currentItems = notificationsSubject.value
currentItems.append(contentsOf: notifications)
notificationsSubject.send(currentItems)
-}
+})
.store(in: cancelBag)| .handleEvents(receiveOutput: { [weak self] notifications in | ||
| isLoadingSubject.send(false) | ||
| isLastPageSubject.send(self?.isLastPage(notifications) ?? true) | ||
| }) | ||
| .sink { notificationsSubject.send($0) } | ||
| .store(in: cancelBag) |
There was a problem hiding this comment.
Fix potential memory leak in closure.
There's a weak self capture in the handleEvents closure, but then the subsequent sink closure uses the implicit strong reference to self. This could lead to memory management issues if self is deallocated between these operations.
.handleEvents(receiveOutput: { [weak self] notifications in
isLoadingSubject.send(false)
isLastPageSubject.send(self?.isLastPage(notifications) ?? true)
})
-.sink { notificationsSubject.send($0) }
+.sink(receiveValue: { notifications in
+ notificationsSubject.send(notifications)
+})
.store(in: cancelBag)Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (1)
126-128:⚠️ Potential issueReplace long GitHub URL with a more reliable placeholder
The hardcoded GitHub URL contains a JWT token which presents a security risk and might expire. Consider using a more permanent placeholder service URL.
private enum Constant { - static let imageURLText: String = "https://private-user-images.githubusercontent.com/80394340/349682631-566a0a8c-c673-4650-b9f4-3b74d7443aa9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDMzODUwNjMsIm5iZiI6MTc0MzM4NDc2MywicGF0aCI6Ii84MDM5NDM0MC8zNDk2ODI2MzEtNTY2YTBhOGMtYzY3My00NjUwLWI5ZjQtM2I3NGQ3NDQzYWE5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAzMzElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMzMxVDAxMzI0M1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRiMjJlZDIyOGU2M2E3NTBiMGQyMjUyNWI0MGQxYTk0ZGVkZmIyNWY2ZjY0YjVmZTQxNzdiMzQ0NzkxNTMzNmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.fslk0G5432-vBjha8bXJ6OAcCOusEowIPST_de3arwU" + static let imageURLText: String = "https://picsum.photos/200" }🧰 Tools
🪛 Gitleaks (8.21.2)
127-127: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🧹 Nitpick comments (2)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (2)
27-32: Consider handling the result ofcheckNotification()You're calling
notificationRepository.checkNotification()but discarding the result with_ =. This could silently ignore any errors. Consider either:
- Properly handling errors that might occur
- Adding a comment explaining why the result is intentionally ignored
func fetchActivityNotifications(for lastItemID: Int) -> AnyPublisher<[ActivityNotification], WableError> { - _ = notificationRepository.checkNotification() + // First check notification status, then fetch notifications + return notificationRepository.checkNotification() + .flatMap { _ in + return notificationRepository.fetchUserNotifications(cursor: lastItemID) + } - return notificationRepository.fetchUserNotifications(cursor: lastItemID) - .eraseToAnyPublisher() + .eraseToAnyPublisher() }
99-110: Extract magic numbers as constantsThe pagination values (15, 30, etc.) are hardcoded in the switch statement. Consider extracting these as named constants to improve readability and maintainability.
+private enum PaginationConstants { + static let firstPageSize = 15 + static let secondPageSize = 15 + static let defaultMaxPage = 33 +} -private func getPaginationRange(for lastItemID: Int, maxPage: Int = 33) -> ClosedRange<Int>? { +private func getPaginationRange(for lastItemID: Int, maxPage: Int = PaginationConstants.defaultMaxPage) -> ClosedRange<Int>? { switch lastItemID { case -1: - return 1...15 + return 1...PaginationConstants.firstPageSize - case 15: + case PaginationConstants.firstPageSize: - return 16...30 + return (PaginationConstants.firstPageSize + 1)...(PaginationConstants.firstPageSize + PaginationConstants.secondPageSize) - case 30: + case PaginationConstants.firstPageSize + PaginationConstants.secondPageSize: - return 31...maxPage + return (PaginationConstants.firstPageSize + PaginationConstants.secondPageSize + 1)...maxPage default: return nil } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (1)
Wable-iOS/Data/RepositoryImpl/NotificationRepositoryImpl.swift (3)
checkNotification(27-34)fetchUserNotifications(36-43)fetchInfoNotifications(18-25)
🪛 Gitleaks (8.21.2)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift
127-127: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (3)
Wable-iOS/Domain/UseCase/Notification/NotificationUseCase.swift (3)
11-12: Good use of MARK commentsI see you've added MARK comments as suggested in a previous review. This improves code organization and readability.
13-38: Clean protocol and implementation designThe protocol and implementation follow good SOLID principles with clear separation of concerns. The use of Combine for asynchronous operations is well implemented.
42-97: Well-structured mock implementationThe mock implementation provides good test data with randomization and realistic scenarios. This will be helpful for UI development and testing before the backend is fully integrated.
👻 PULL REQUEST
📄 작업 내용
👀 기타 더 이야기해볼 점
🔗 연결된 이슈
Summary by CodeRabbit
ActivityNotificationViewControllerandInformationNotificationViewControllerclasses for better management of notifications.NotificationCellfor displaying individual notifications in a collection view format.