Add Message Reminders #3623
Add Message Reminders #3623
Conversation
Generated by 🚫 Danger |
SDK Size
|
SDK Performance
|
SDK Size
|
DemoApp/Screens/AppConfigViewController/AppConfigViewController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/APIClient/Endpoints/Payloads/MessagePayloads.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Copilot reviewed 35 out of 55 changed files in this pull request and generated no comments.
Files not reviewed (20)
- DemoApp/Screens/AppConfigViewController/AppConfigViewController.swift: Language not supported
- DemoApp/Screens/DemoAppTabBarController.swift: Language not supported
- DemoApp/Shared/DemoUsers.swift: Language not supported
- DemoApp/Shared/StreamChatWrapper.swift: Language not supported
- DemoApp/StreamChat/Components/DemoChatMessageActionsVC.swift: Language not supported
- DemoApp/StreamChat/Components/DemoChatMessageContentView.swift: Language not supported
- DemoApp/StreamChat/Components/DemoChatMessageLayoutOptionsResolver.swift: Language not supported
- DemoApp/StreamChat/DemoAppCoordinator+DemoApp.swift: Language not supported
- Sources/StreamChat/APIClient/Endpoints/EndpointPath+OfflineRequest.swift: Language not supported
- Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift: Language not supported
- Sources/StreamChat/APIClient/Endpoints/MessageEndpoints.swift: Language not supported
- Sources/StreamChat/APIClient/Endpoints/Payloads/MessagePayloads.swift: Language not supported
- Sources/StreamChat/ChatClient+Environment.swift: Language not supported
- Sources/StreamChat/ChatClient.swift: Language not supported
- Sources/StreamChat/ChatClientFactory.swift: Language not supported
- Sources/StreamChat/Controllers/CurrentUserController/CurrentUserController.swift: Language not supported
- Sources/StreamChat/Controllers/MessageController/MessageController.swift: Language not supported
- Sources/StreamChat/Database/DTOs/MessageDTO.swift: Language not supported
- Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift: Language not supported
- Sources/StreamChat/Database/DatabaseSession.swift: Language not supported
nuno-vieira
force-pushed
the
add/snooze-messages
branch
from
60495fe to
1eca9b2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
Sources/StreamChat/Repositories/RemindersRepository.swift (1)
80-108:⚠️ Potential issueLocal database write errors are still swallowed – caller may hang indefinitely
A previous review already pointed this out.
If the optimistic local write throws, thecompletionpassed intocreateReminderis never invoked. The UI will spin forever.- } completion: { _ in + } completion: { dbError in + if let dbError = dbError { + completion(.failure(dbError)) + return + }Apply the same pattern to
updateReminderanddeleteReminder.
🧹 Nitpick comments (1)
Sources/StreamChat/Repositories/RemindersRepository.swift (1)
41-55: Consider surfacing partial-save failures to the caller
session.saveRemindercan throw.
Currently every failure is silently eaten and the reminder is omitted from the resulting array, while thecompletionis still called with.success.
If a reminder is missing in the UI the user has no clue something went wrong.If silent dropping is intentional please confirm. Otherwise, wrap the
compactMapin aResultand propagate an aggregated error (or at least log at error level).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Sources/StreamChat/APIClient/ChatRemoteNotificationHandler.swift(2 hunks)Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift(1 hunks)Sources/StreamChat/Repositories/RemindersRepository.swift(1 hunks)StreamChat.xcodeproj/project.pbxproj(64 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift
- StreamChat.xcodeproj/project.pbxproj
nuno-vieira
force-pushed
the
add/snooze-messages
branch
from
97e0650 to
9741b8a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift (1)
88-90:⚠️ Potential issueCritical compilation error: Invalid Core Data API usage
The method
NSEntityDescription.insertNewObject(into:for:)does not exist in Apple's Core Data framework and will cause a compilation error.Apply this fix to use the correct Core Data API:
- let request = fetchRequest(messageId: messageId) - let new = NSEntityDescription.insertNewObject(into: context, for: request) + let new = NSEntityDescription.insertNewObject( + forEntityName: MessageReminderDTO.entityName, + into: context + ) as! MessageReminderDTOStreamChat.xcodeproj/project.pbxproj (1)
1677-1690: Duplicate PBXBuildFile entries still presentThe duplicate entries for Message Reminder feature files identified in the previous review are still present in this code segment, which will cause build issues.
🧹 Nitpick comments (1)
Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift (1)
147-153: Proper cleanup but potential optimization opportunityThe delete method correctly removes the relationship and deletes the DTO. However, it could be optimized by directly loading the reminder DTO instead of going through the message relationship.
Consider this more direct approach:
func deleteReminder(messageId: MessageId) { - let message = message(id: messageId) - guard let reminderDTO = message?.reminder else { + guard let reminderDTO = MessageReminderDTO.load(messageId: messageId, context: self) else { return } delete(reminderDTO) - message?.reminder = nil + reminderDTO.message.reminder = nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
DemoAppPush/NotificationService.swift(1 hunks)Sources/StreamChat/APIClient/ChatRemoteNotificationHandler.swift(2 hunks)Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift(1 hunks)Sources/StreamChat/Repositories/RemindersRepository.swift(1 hunks)StreamChat.xcodeproj/project.pbxproj(64 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/StreamChat/APIClient/ChatRemoteNotificationHandler.swift
- Sources/StreamChat/Repositories/RemindersRepository.swift
🔇 Additional comments (11)
Sources/StreamChat/Database/DTOs/MessageReminderDTO.swift (3)
22-29: Efficient sorting helper implementationThe
willSave()override efficiently maintains thesortingRemindAtproperty for consistent sorting behavior whenremindAtis nil. The conditional check prevents unnecessary property updates.
39-69: Comprehensive query handling with proper fallbackThe
remindersFetchRequestmethod properly handles dynamic sorting from the query with a sensible default fallback. The switch statement covers all expected sorting keys with graceful handling of unknown keys.
135-135:❌ Incorrect review comment
Potential data inconsistency: ID assignment mismatch
The reminder ID is being set to
payload.messageIdinstead ofpayload.id, which could cause data inconsistency if the reminder ID and message ID are meant to be different entities.Please verify if this is intentional or if it should be:
- reminderDTO.id = payload.messageId + reminderDTO.id = payload.id
🏁 Script executed:
#!/bin/bash # Check if ReminderPayload has both id and messageId properties ast-grep --pattern 'struct ReminderPayload { $$$ }'Length of output: 55
🏁 Script executed:
#!/bin/bash # Locate the definition of ReminderPayload in the codebase rg -n "struct ReminderPayload" . # Fallback: check for class-based or typealias occurrences rg -n "class ReminderPayload" . # Inspect payload property usages to see which fields exist rg -n "payload.id" . rg -n "payload.messageId" .Length of output: 8239
🏁 Script executed:
#!/bin/bash # Show ReminderPayload class definition rg -n "class ReminderPayload" -A 20 Sources/StreamChat/APIClient/Endpoints/Payloads/ReminderPayloads.swift # Show tests for ReminderPayload rg -n "class ReminderPayload_Tests" -A 50 Tests/StreamChatTests/APIClient/Endpoints/Payloads/ReminderPayloads_Tests.swift # Show MessageReminderDTO class definition and key properties rg -n "class MessageReminderDTO" -A 20 Sources/StreamChat/Database/DTOs/MessageReminderDTO.swiftLength of output: 4386
Incorrect suggestion:
ReminderPayloadhas no separateidfield
TheReminderPayloadmodel only includesmessageId(noidproperty), and the API’s JSON uses"message_id"—there is no"id"key for a reminder. AssigningreminderDTO.id = payload.messageIdis therefore intentional and correct. No change needed.Likely an incorrect or invalid review comment.
DemoAppPush/NotificationService.swift (2)
99-101: Proper early return for reminder notificationsThe special handling for
.reminderDuemessage types with early return preserves the original notification content, which is appropriate for reminder notifications that likely have pre-formatted content.
112-118: Robust fallback handling for reminder notificationsThe fallback logic properly checks the stream payload for reminder due event types, ensuring reminder notifications are handled consistently even when they don't match the expected message notification format.
StreamChat.xcodeproj/project.pbxproj (6)
4429-4466: Comprehensive Message Reminder feature file organizationThe file organization for the Message Reminder feature looks well-structured with proper separation of concerns:
- Controllers and their Combine extensions
- Queries and DTOs for data management
- Endpoints and payloads for API communication
- Events and middleware for real-time updates
- Repository for data persistence
- Comprehensive test coverage and mocks
- Demo UI integration
This follows good architectural patterns for the StreamChat SDK.
5606-5606: Well-organized group structure for new featureThe Message Reminder files are properly organized into logical groups:
- Events grouped with other event types
- DTOs with other data transfer objects
- Queries with other query types
- Middleware with other event middlewares
- Payloads with other payload types
- Endpoints with other API endpoints
- Models with other domain models
- Controllers with other controller types
This maintains consistency with the existing project structure.
Also applies to: 5650-5650, 5732-5740, 5815-5815, 5837-5837, 5899-5899, 6008-6008, 6056-6056
8862-8879: Proper controller grouping structureThe MessageReminderListController files are properly grouped with separate directories for the main implementation and tests, following the established pattern in the project.
8906-8916: Comprehensive JSON test resources for reminder eventsGood coverage of reminder event types with dedicated JSON files:
- ReminderCreated.json
- ReminderUpdated.json
- ReminderDeleted.json
- ReminderDue.json
This ensures proper testing of all reminder event scenarios.
2496-2497:✅ Verification successful
Verify the Filter file renaming impact
The renaming from
Filter+ChatChannel.swifttoFilter+predicate.swiftsuggests the filter functionality is being extended beyond channels. Ensure this change is properly reflected throughout the codebase and doesn't break existing channel-specific filter usage.
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining references to the old filename or channel-specific filter usage that might be affected # Search for old filename references rg -i "filter.*channel\.swift|filterchannel" --type swift # Search for imports or references that might need updating rg "import.*Filter.*Channel|Filter.*Channel" --type swift # Check for any channel-specific filter methods that might be affected ast-grep --pattern 'extension Filter { $$$ }'Length of output: 11727
🏁 Script executed:
#!/bin/bash # Verify that the old file reference was removed and the new one added in the Xcode project rg 'Filter\+ChatChannel.swift' -n StreamChat.xcodeproj/project.pbxproj rg 'Filter\+predicate.swift' -n StreamChat.xcodeproj/project.pbxprojLength of output: 871
Filter File Rename Verified
I confirmed there are no lingering references to
Filter+ChatChannel.swiftin the Xcode project or Swift sources—only the newFilter+predicate.swiftis listed inproject.pbxproj. No further updates are needed.
15568-15570:✅ Verification successful
Verify atlantis package version compatibility
The atlantis package dependency is being updated from an exact version (1.18.0) to a minimum version requirement (1.28.0) with up-to-next-major-version. This is a significant version jump that should be verified for compatibility.
🌐 Web query:
What are the breaking changes between atlantis 1.18.0 and 1.28.0? Are there any compatibility issues or new requirements?💡 Result:
Based on the search results, I can provide information about the changes between Atlantis 1.18.0 and 1.28.0, focusing on the ProxymanApp/Atlantis repository which appears to be the most relevant to your query.
Breaking Changes and Updates in Atlantis 1.18.0 to 1.28.0
The most significant changes between versions 1.18.0 and 1.28.0 of the ProxymanApp/Atlantis tool revolve around connectivity improvements, bug fixes, and feature additions. Here's a detailed breakdown:
Major Changes
Version 1.28.0 (Released April 19, 2025): Fixed connection issues on macOS 15.4 by implementing modern networking APIs[1][6]
- Replaced deprecated NetService with NWBrowser and NWConnection
- Improved connectivity for iOS Simulators with direct connection to Proxyman App
- Maintained compatibility with iOS devices using Bonjour Service
Version 1.27.0: Updated with a new SwiftUI Sample App and revised documentation[6]
Version 1.26.0: Addressed duplicated requests issue when using custom URLProtocols[6]
Version 1.25.1 & 1.25.2: Fixed crashes when using Atlantis with AVAggregateAssetDownloadTask and updated Podspec[6]
Version 1.24.0: Added Privacy Manifest support[6]
Version 1.23.0: Added option to opt-out of Method Swizzling on WebSocket (WS/WSS) connections[6]
Version 1.22.0: Fixed crashes when intercepting URLSessionUploadTask with nil Data[6]
Compatibility Considerations
- Atlantis 1.28.0 is backward-compatible with Proxyman 5.18.0 or older versions[6]
- However, it's recommended to use Atlantis 1.28.0 with Proxyman 5.19.0 or later for optimal performance with the modern APIs[6]
WebSocket Support
The tool provides built-in support for capturing WebSocket traffic from
URLSessionWebsocketTaskbut doesn't natively work with third-party WebSocket libraries such as Starscream that don't useURLSessionWebsocketTaskunder the hood[1]Manual Request/Response Handling
For applications using C++ Network libraries or other non-URLSession networking stacks, Atlantis provides functionality to manually add requests and responses for inspection:
Atlantis.add(request: request, response: response, responseBody: responseData)This allows capturing network traffic from non-standard networking implementations[1]
The changes from 1.18.0 to 1.28.0 represent a series of incremental improvements rather than radical breaking changes, with the most significant update being the connectivity fixes for macOS 15.4 in the latest release.
Citations:
- 1: https://github.com/ProxymanApp/atlantis
- 2: https://github.com/runatlantis/atlantis/blob/main/CHANGELOG.md
- 3: https://www.runatlantis.io/docs/terraform-versions
- 4: https://specter2.fandom.com/wiki/Changelog/2023
- 5: https://formulae.brew.sh/formula/
- 6: https://github.com/ProxymanApp/atlantis/releases
- 7: https://www.atlantiswordprocessor.com/en/help/system_requirements.htm
- 8: https://www.scribd.com/document/551715438/Brew-Formula
- 9: https://www.cisa.gov/news-events/bulletins/sb22-213
- 10: https://www.minecraftforum.net/forums/support/server-support-and/3149244-server-crashing-you-get-the-point-im-sick-of-this
Atlantis dependency version bump is backward-compatible
We’ve verified that upgrading the Atlantis dependency from 1.18.0 to “^1.28.0” introduces only incremental improvements (macOS 15.4 connectivity fixes, bug fixes, new features) with no breaking API changes.
• Location: StreamChat.xcodeproj/project.pbxproj (lines 15568–15570)
• No removed or replaced APIs between 1.18.0 and 1.28.0
• Atlantis 1.28.0 remains compatible with Proxyman 5.18.0+, though Proxyman 5.19.0+ is recommended for optimal performanceNo further action required.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
DemoApp/Screens/DemoReminderListVC.swift (3)
186-208: Adddeinitto stop the timer and avoid a retain-cycle leak
refreshTimerholds a strong reference to itstarget(self).
If the VC is popped while the timer is still scheduled, both will leak.
A simpledeinit { stopRefreshTimer() }will guarantee cleanup.
381-384: Handle theErrorreturned bysynchronizeThe closure parameter is an
Error?, but it is ignored.
Surface the error to the user (alert / banner) or at least log it so failures are discoverable during QA.
491-505:activeControlleris recreated but never synchronisedAfter swapping in a fresh controller you immediately read
activeController.reminders.
Without asynchronize()call the list will stay empty until the user manually refreshes.
CallactiveController.synchronize()right after setting the delegate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
DemoApp/Screens/DemoReminderListVC.swift(1 hunks)DemoApp/StreamChat/Components/DemoChatMessageActionsVC.swift(3 hunks)DemoAppPush/NotificationService.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- DemoAppPush/NotificationService.swift
- DemoApp/StreamChat/Components/DemoChatMessageActionsVC.swift
🔇 Additional comments (1)
DemoApp/Screens/DemoReminderListVC.swift (1)
460-465: Race condition when switching filters during pagination
reminders = Array(activeController.reminders)assumes thatactiveControllerdid not change during an on-going pagination request (seeisPaginatingReminders).
If the user changes the filter while a page is loading,loadMoreReminders’s completion will mutateisPaginatingRemindersof the old controller, potentially leaving the new one blocked.Consider scoping
isPaginatingRemindersper-controller or cancelling outstanding loads when switching filters.
# Conflicts: # StreamChat.xcodeproj/project.pbxproj
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
12-15:⚠️ Fix list indentation
Nested sub-bullets should be indented with 2 spaces to comply with markdown style guidelines.Proposed diff:
- - Add `ChatMessageController.createReminder()` - - Add `ChatMessageController.updateReminder()` - - Add `ChatMessageController.deleteReminder()` - - Add `MessageReminderListController` and `MessageReminderListQuery` + - Add `ChatMessageController.createReminder()` + - Add `ChatMessageController.updateReminder()` + - Add `ChatMessageController.deleteReminder()` + - Add `MessageReminderListController` and `MessageReminderListQuery`🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
12-12: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)Sources/StreamChat/APIClient/Endpoints/Payloads/MessagePayloads.swift(5 hunks)Sources/StreamChat/Controllers/MessageController/MessageController.swift(3 hunks)Sources/StreamChat/Database/DTOs/MessageDTO.swift(3 hunks)Sources/StreamChat/Database/DatabaseSession.swift(2 hunks)StreamChat.xcodeproj/project.pbxproj(64 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- Sources/StreamChat/APIClient/Endpoints/Payloads/MessagePayloads.swift
- Sources/StreamChat/Database/DatabaseSession.swift
- Sources/StreamChat/Controllers/MessageController/MessageController.swift
- StreamChat.xcodeproj/project.pbxproj
- Sources/StreamChat/Database/DTOs/MessageDTO.swift
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
12-12: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test LLC (Debug)
- GitHub Check: Automated Code Review
- GitHub Check: Metrics
🔇 Additional comments (3)
CHANGELOG.md (3)
10-10: ✅ Correct: Document Filter.isNil addition
The newFilter.isNilAPI is accurately reflected in the changelog under the added section.
11-11: ✅ Document Message Reminders
The top-levelAdd Message Remindersentry correctly surfaces the new feature for users.
1-15: ❓ Document file rename
The PR also renamesFilter+ChatChannel.swifttoFilter+predicate. Consider adding a “Renamed” or “Changed” entry in the changelog so consumers are aware of the updated filename and import paths.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
12-12: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
nuno-vieira
force-pushed
the
add/snooze-messages
branch
from
8bb1b97 to
3f59a94
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
TestTools/StreamChatTestTools/TestData/DummyData/XCTestCase+Dummy.swift (1)
282-305: Consider adding the messageRemindersEnabled flag for consistency.The
dummyPayloadWithNoExtraDatafunction creates aChannelConfiginline but doesn't include themessageRemindersEnabledflag, which could lead to inconsistent test behavior depending on which dummy data function is used.For consistency, consider adding the flag here as well:
config: .init( reactionsEnabled: true, typingEventsEnabled: true, readEventsEnabled: true, connectEventsEnabled: true, uploadsEnabled: true, repliesEnabled: true, quotesEnabled: true, searchEnabled: true, mutesEnabled: true, urlEnrichmentEnabled: true, + messageRemindersEnabled: true, messageRetention: "1000", maxMessageLength: 100,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
TestTools/StreamChatTestTools/TestData/DummyData/XCTestCase+Dummy.swift(1 hunks)Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift(1 hunks)Tests/StreamChatTests/WebSocketClient/Events/ReminderEvents_Tests.swift(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/StreamChatTests/WebSocketClient/Events/ReminderEvents_Tests.swift
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Metrics
🔇 Additional comments (1)
TestTools/StreamChatTestTools/TestData/DummyData/XCTestCase+Dummy.swift (1)
142-142: LGTM! Test data correctly enables message reminders feature.The addition of
messageRemindersEnabled: trueto the default channel configuration ensures that test scenarios properly reflect the new message reminders capability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
6-9: Document theFilter+predicatefile rename.The PR renames
Filter+ChatChannel.swifttoFilter+predicate.swiftto generalize filter extensions beyond channels, but the changelog only lists the addition ofFilter.isNil. Please add a "🔄 Changed" entry under the Upcoming## StreamChatsection to reflect this rename. For example:### 🔄 Changed - Renamed `Filter+ChatChannel.swift` → `Filter+predicate.swift` to extend filter helpers beyond channels
10-13: Fix markdown list indentation for nested items.The nested bullets under
- Add Message Reminders [#3623]are indented by 3 spaces, but markdownlint expects 2 spaces for nested list items.
Apply this diff:- - Add `ChatMessageController.createReminder()` - - Add `ChatMessageController.updateReminder()` - - Add `ChatMessageController.deleteReminder()` - - Add `MessageReminderListController` and `MessageReminderListQuery` + - Add `ChatMessageController.createReminder()` + - Add `ChatMessageController.updateReminder()` + - Add `ChatMessageController.deleteReminder()` + - Add `MessageReminderListController` and `MessageReminderListQuery`🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
12-12: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)DemoAppPush/NotificationService.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- DemoAppPush/NotificationService.swift
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
10-10: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
12-12: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
testableapple
added
🧪 QAing
and removed
🤞 Ready For QA
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
8-9: Document file rename as part of PR. The PR also renamesFilter+ChatChannel.swifttoFilter+predicateto extend its reuse beyond channels. Please add a bullet under this section to capture that breaking/user-facing change.
10-13: Fix sub-list indentation. The nested bullets under "Add Message Reminders" use 3 spaces but should follow the project's 2-space indentation per list level (MD007). For example:- - Add `ChatMessageController.createReminder()` + - Add `ChatMessageController.createReminder()`🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
12-12: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
10-10: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
12-12: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Automated Code Review
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Metrics
🔇 Additional comments (1)
CHANGELOG.md (1)
7-7: Changelog section header formatting is consistent. Using### ✅ Addedaligns with the style of other release entries and clearly denotes the added features.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
DemoApp/Screens/DemoReminderListVC.swift (4)
186-208: Timer leaks if the VC is deallocated while hiddenThis issue was previously identified. Add cleanup in
deinitfor safety:deinit { stopRefreshTimer() }
381-384: Surface synchronisation errors to the userThis issue was previously identified.
controller.synchronize { _ in }ignores theError?. Handle the callback and show an alert or retry banner if the network call fails.
386-395: Propagate pagination errors to the UIThis issue was previously identified.
loadMoreRemindersswallows theError?returned byloadMoreReminders { … }. Handle the error and inform the user of pagination failures.
491-505: 🛠️ Refactor suggestionControllers recreated but not synchronised and incomplete filter handling
This partially duplicates a previous issue - controllers are recreated but
synchronize()is not called. Additionally, the logic only handlesupcoming,overdue, andscheduledfilters but ignoresallandlaterfilters when updating with new date.private func updateReminderListsWithNewNowDate() { + allRemindersController = FilterOption.all.makeController(client: currentUserController.client) upcomingRemindersController = FilterOption.upcoming.makeController(client: currentUserController.client) overdueRemindersController = FilterOption.overdue.makeController(client: currentUserController.client) scheduledRemindersController = FilterOption.scheduled.makeController(client: currentUserController.client) - if selectedFilter == .upcoming { + laterRemindersController = FilterOption.later.makeController(client: currentUserController.client) + + switch selectedFilter { + case .all: + activeController = allRemindersController + case .upcoming: activeController = upcomingRemindersController - } else if selectedFilter == .overdue { + case .overdue: activeController = overdueRemindersController - } else if selectedFilter == .scheduled { + case .scheduled: activeController = scheduledRemindersController - } else { - return + case .later: + activeController = laterRemindersController } + activeController.delegate = self + activeController.synchronize() updateRemindersData() }
🧹 Nitpick comments (2)
DemoApp/Screens/DemoReminderListVC.swift (2)
409-411: Fix suspicious calculation for 1-minute reminder.The calculation
1.05 * 60for a 1-minute reminder seems incorrect. This results in 63 seconds instead of 60 seconds. If the intention is to add a small buffer, this should be documented or use a more explicit approach.- alert.addAction(UIAlertAction(title: "Remind in 1 Minutes", style: .default) { [weak self] _ in - let date = Date().addingTimeInterval(1.05 * 60) + alert.addAction(UIAlertAction(title: "Remind in 1 Minute", style: .default) { [weak self] _ in + let date = Date().addingTimeInterval(60) // 1 minute self?.updateReminderDate(for: reminder, newDate: date) })
703-762: Consider simplifying time formatting logic.The time formatting logic works correctly but has duplicate rounding calculations. Consider extracting the common rounding logic to reduce code duplication.
+ private func roundTimeInterval(_ timeInterval: TimeInterval) -> TimeInterval { + let roundedMinutes = ceil(timeInterval / 60 - 0.5) + return max(roundedMinutes * 60, 60) // Minimum 1 minute + } + private func formatOverdueTime(timeInterval: TimeInterval) -> String { - // Round to the nearest minute (30 seconds or more rounds up) - let roundedMinutes = ceil(timeInterval / 60 - 0.5) - let roundedInterval = roundedMinutes * 60 - - // If less than a minute, show "1 min" instead of "0 min" - if roundedInterval == 0 { - return "Overdue by 1 min" - } + let roundedInterval = roundTimeInterval(timeInterval) let formatter = DateComponentsFormatter() // ... rest of the formatting logic } private func formatDueTime(timeInterval: TimeInterval) -> String { - // Round to the nearest minute (30 seconds or more rounds up) - let roundedMinutes = ceil(timeInterval / 60 - 0.5) - let roundedInterval = roundedMinutes * 60 - - // If less than a minute, show "1 min" instead of "0 min" - if roundedInterval == 0 { - return "1m" - } + let roundedInterval = roundTimeInterval(timeInterval) + + if roundedInterval == 60 { + return "1m" + } let formatter = DateComponentsFormatter() // ... rest of the formatting logic }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
DemoApp/Screens/DemoReminderListVC.swift(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Automated Code Review
- GitHub Check: Metrics
🔇 Additional comments (3)
DemoApp/Screens/DemoReminderListVC.swift (3)
398-459: Well-implemented user interaction handlers.The error handling, user avatar interaction, and edit reminder options are well-designed and provide good user experience. The alert presentation with proper popover configuration for iPad is a nice touch.
510-581: Solid table view implementation with good UX.The table view implementation includes proper error handling for deletion, well-configured swipe actions, and appropriate navigation to channels with pagination parameters. The scroll-based pagination trigger is also well-implemented.
9-765: Well-architected reminder list implementation.This is a comprehensive and well-structured implementation of the reminder list feature. The architecture demonstrates good separation of concerns, proper use of delegation patterns, and follows iOS best practices. The filter system is particularly well-designed with dedicated controllers for performance optimization.
|
🔗 Issue Links
IOS-696
🎯 Goal
Add message reminders feature. This will allow users to save messages for later and get notified about them.
📝 Summary
Docs PR: https://github.com/GetStream/docs-content/pull/260
New APIs:
Filter.isNilto make it easier to filter nil valuesChatMessageController.createReminder()ChatMessageController.updateReminder()ChatMessageController.deleteReminder()MessageReminderListControllerDemo App new features:
Bonus:
Filter+ChatChannel.swifttoFilter+predicateso that we can start using it for other data and not only channels.🎨 Showcase
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-03-28.at.16.01.16.mp4
🧪 Manual Testing Notes
In order to test the feature, you need to change the API Key to Frankfurt C2 Staging:
Saving a message as a reminder in the message list:
Saving a message for later in the message list:
Removing a reminder in the message list:
Updating a reminder in the message list:
Updating a saved for later message in the message list:
Fetching and filtering all reminders:
Deleting a message with a reminder:
Reminder when is overdue should be removed from upcoming:
Reminder Due Push Notification:
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit