chat docs: fix missing chat kotlin/swift/jetpack doc #3098
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds multi-language examples for media sharing, message replies, and reactions in chat docs, and corrects a Swift API parameter label in the message history examples from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sacOO7
changed the title
sacOO7
force-pushed
the
fix/missing-chat-documentation
branch
from
207d1c0 to
58c0968
Compare
sacOO7
changed the title
sacOO7
requested review from
AndyTWF,
lawrence-forooghian,
ttypic and
umair-ably
sacOO7
changed the title
sacOO7
changed the title
sacOO7
changed the title
sacOO7
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/docs/chat/rooms/replies.mdx`:
- Around line 66-99: The React reply example uses "createdAt" for the reply
metadata while Swift/Kotlin/JS use "timestamp"; update the React example so the
metadata key is "reply.timestamp" (replace createdAt with timestamp) to match
other language examples and ensure cross-platform consistency, and while here
confirm that the Kotlin usage of replyToMessage.timestamp is in milliseconds (or
convert it to ms like Swift does) so all examples supply timestamp as epoch
milliseconds.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/pages/docs/chat/rooms/history.mdxsrc/pages/docs/chat/rooms/media.mdxsrc/pages/docs/chat/rooms/message-reactions.mdxsrc/pages/docs/chat/rooms/replies.mdx
🔇 Additional comments (11)
src/pages/docs/chat/rooms/history.mdx (1)
57-57: LGTM!The Swift parameter label correction from
withOptionstowithParamsaligns with the SDK documentation URL referenced on line 11 (history(withparams:)).src/pages/docs/chat/rooms/message-reactions.mdx (1)
318-352: LGTM!The Swift and Kotlin examples for removing message reactions are consistent with the existing send reaction patterns earlier in the file (lines 122-162) and mirror the JavaScript/React examples.
src/pages/docs/chat/rooms/replies.mdx (3)
170-202: LGTM!The
prepareReplyhelper functions are consistent across Swift, Kotlin, and Jetpack, properly extracting and formatting parent message data for reply metadata.
233-275: LGTM!The
fetchParentMessageimplementations properly handle serial extraction with appropriate error handling for each platform (Swift usesNSError, Kotlin throwsIllegalArgumentException).
337-427: LGTM!The display reply implementations correctly handle metadata extraction using platform-appropriate patterns (Swift pattern matching, Kotlin null-safe operators). The Jetpack Compose example properly manages subscription lifecycle with
DisposableEffect.src/pages/docs/chat/rooms/media.mdx (6)
80-148: LGTM!The
MediaDatastruct andonMediaAttachflow implementations are consistent across Swift, Kotlin, and Jetpack Compose, properly managing the media attachment state.Note: The
uploadMedia()function referenced in Swift/Kotlin/Jetpack is assumed to be a placeholder that users implement, matching the pattern established in the JavaScript example at lines 29-44.
214-329: LGTM!The send message implementations properly construct JSON metadata containing the media array using platform-appropriate patterns (Swift
JSONValueenums, KotlinbuildJsonObject/buildJsonArray).
366-450: LGTM!The
getValidMediaimplementations properly validate media IDs using regex and safely extract typed values from JSON metadata. The validation approach is consistent with security best practices mentioned in the documentation.
515-623: LGTM!The display implementations demonstrate platform-appropriate UI patterns. The Swift (UIKit) and Kotlin (Android Views) examples include helpful comments indicating where image loading libraries should be integrated, while the Jetpack Compose example shows a complete implementation using Coil's
AsyncImage.
669-780: LGTM!The
addMediaToMessageimplementations correctly handle metadata mutation by preserving existing metadata while adding new media entries. The Kotlin/Jetpack approach of rebuilding the metadata object maintains immutability.
829-947: LGTM!The
removeMediaFromMessageimplementations properly filter out the target media ID while preserving other metadata. Early returns for empty media arrays prevent unnecessary API calls.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
4 times, most recently
from
57c3e71 to
a72f25d
Compare
…lin snippets - Added uploadMedia method to swift/kotlin and jetpack
- Fixed method signatures of jetpack compose extensions - Removed jetpack alias reference to kotlin, instead will use native jetpack icon - Updated ably/ui to jetpack compatible latest version
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
a72f25d to
520c5c6
Compare
sacOO7
force-pushed
the
fix/missing-chat-documentation
branch
from
bef8be6 to
a6181a1
Compare
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
659f155 to
507bc43
Compare
Fix chat-js snippets
ttypic
left a comment
ttypic
left a comment
There was a problem hiding this comment.
On second thought, I approve because uploadMedia should be suspended if it's not mocked (hopefully LLMs won't pick it as right way to build jsons)
…compose stateful annotations
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/pages/docs/chat/rooms/message-reactions.mdx`:
- Around line 649-652: The snippet calls the throwing async method
room.messages.history(withParams:) without using the try keyword; update the
initialization of messages to use try await (e.g., var messages = try await
room.messages.history(withParams: .init(limit: 50))). Ensure this is inside an
async context and handle or propagate the potential error (using do/catch or
marking the enclosing function as throws) so the code compiles.
In `@src/pages/docs/chat/rooms/replies.mdx`:
- Around line 339-440: The Swift snippet uses the old callback-based
room.messages.subscribe { event in ... } pattern; update it to the async/await
API by calling try await room.messages.subscribe() to get a
messagesSubscription, then iterate with for await inside a Task to process each
Message (use the existing Message extension properties replySerial and
replyPreview), keep the same reply handling logic and append to localMessages
within the Task, and ensure you cancel/unsubscribe the messagesSubscription
where appropriate.
♻️ Duplicate comments (1)
src/pages/docs/chat/rooms/replies.mdx (1)
36-49: Align reply metadata key totimestampacross languages.
Swift/Kotlin/Jetpack usereply.timestampwhile the React examples still storecreatedAt, which breaks cross‑platform consistency. Also confirm KotlinMessage.timestampis already epoch ms (convert if needed).✏️ Suggested alignment
- createdAt: messageToReplyTo.createdAt.getTime(), + timestamp: messageToReplyTo.createdAt.getTime(),- createdAt: parentMessage.createdAt.getTime(), + timestamp: parentMessage.createdAt.getTime(),Ably Chat Kotlin Message timestamp units millisecondsAlso applies to: 66-102, 161-169, 172-204
| ```swift | ||
| // init messages, in practice this should be updated with a message subscription | ||
| var messages = (await room.messages.history(withOptions: .init(limit: 50))).items | ||
| var messages = (await room.messages.history(withParams: .init(limit: 50))).items | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ably Chat Swift messages.history throws API documentation
💡 Result:
Use the Chat docs — message history in the Swift SDK is accessed via the room message APIs (e.g. call historyBeforeSubscribe on a messages subscription to fetch prior messages). See Ably Chat Swift docs for examples and the API reference for the Chat SDK. (ably.com)
Citations:
🌐 Web query:
Ably Chat Swift API reference history method throws
💡 Result:
Short answer: history/historyBeforeSubscribe in Ably’s Swift Chat API is an async throwing call — it uses Swift concurrency and can throw network/permission/continuity errors, so you must call it with try/await and handle errors (e.g. missing history capability, channel not persisted, continuity/connection interruptions). See examples and error notes in the docs and FAQ:
- Example using try await and catch in the Swift Chat guide. [1]
- Notes that calls wait for continuity and may be affected by discontinuity. [2]
- History/persistence must be enabled (and your key/token must have history capability); lack of persistence or permissions will cause errors. [3]
If you want, paste the exact error message you see and I’ll point to the precise cause and fix.
Sources:
[1] Ably Chat — Getting started: Chat with Swift (example using try await subscription.historyBeforeSubscribe).
[2] MessageSubscription / historyBeforeSubscribe docs (continuity behaviour).
[3] Ably FAQ — How do I enable history / message persistence.
🏁 Script executed:
cat -n src/pages/docs/chat/rooms/message-reactions.mdx | sed -n '645,660p'Repository: ably/docs
Length of output: 162
Swift history call must use try keyword.
The history(withParams:) method is an async throwing call in Ably's Swift Chat API and can throw errors for network issues, missing permissions, or continuity problems. The code snippet must use try await to compile correctly.
🐛 Proposed fix
-var messages = (await room.messages.history(withParams: .init(limit: 50))).items
+var messages = (try await room.messages.history(withParams: .init(limit: 50))).items📝 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.
| ```swift | |
| // init messages, in practice this should be updated with a message subscription | |
| var messages = (await room.messages.history(withOptions: .init(limit: 50))).items | |
| var messages = (await room.messages.history(withParams: .init(limit: 50))).items |
🤖 Prompt for AI Agents
In `@src/pages/docs/chat/rooms/message-reactions.mdx` around lines 649 - 652, The
snippet calls the throwing async method room.messages.history(withParams:)
without using the try keyword; update the initialization of messages to use try
await (e.g., var messages = try await room.messages.history(withParams:
.init(limit: 50))). Ensure this is inside an async context and handle or
propagate the potential error (using do/catch or marking the enclosing function
as throws) so the code compiles.
maratal
left a comment
maratal
left a comment
There was a problem hiding this comment.
Minor suggestions re xcode unused result and ForEach iterator, which are better to be fixed before merge (check all sends/updates for unused results).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sacOO7
force-pushed
the
fix/missing-chat-documentation
branch
from
c959151 to
efefab5
Compare
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
507bc43 to
1d9d74d
Compare
Fix chat-swift doc snippets
# Conflicts: # src/data/languages/languageData.ts # src/pages/docs/chat/connect.mdx # src/pages/docs/chat/rooms/history.mdx # src/pages/docs/chat/rooms/index.mdx # src/pages/docs/chat/rooms/message-reactions.mdx # src/pages/docs/chat/rooms/messages.mdx # src/pages/docs/chat/rooms/occupancy.mdx # src/pages/docs/chat/rooms/presence.mdx # src/pages/docs/chat/rooms/typing.mdx
sacOO7
force-pushed
the
fix/missing-chat-documentation
branch
from
3e5442e to
bbafc53
Compare
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.