[ECO-5478] feat: remove opinionated presence structure by ttypic · Pull Request #336 · ably/ably-chat-swift

ttypic · 2025-07-22T14:01:46Z

Removes the userCustomData key from presence and takes an unopinionated view on how presence should look.

Summary by CodeRabbit

Refactor
- Simplified presence data handling by removing the intermediate data transfer object, resulting in direct usage of presence data throughout the application.
Tests
- Updated test cases to match the new, simplified presence data structure.
- Removed tests related to the deleted data transfer object.
Chores
- Cleaned up supporting helper methods to align with the new data handling approach.
Documentation
- Corrected comment references related to realtime event handling in message processing and tests.

coderabbitai · 2025-07-22T14:01:53Z

Walkthrough

This change removes the intermediate PresenceDataDTO structure from presence data handling in the codebase. Presence data is now passed and processed directly as raw dictionaries, eliminating the extra encoding/decoding step. Corresponding tests and helper methods are updated to align with the new, unopinionated presence data structure.

Changes

Files/Paths	Change Summary
Sources/AblyChat/DefaultPresence.swift	Removed all usage of `PresenceDataDTO`, passing raw presence data directly; deleted decode helper.
Sources/AblyChat/PresenceDataDTO.swift	Deleted the `PresenceDataDTO` struct and its encoding/decoding logic.
Tests/AblyChatTests/DefaultPresenceTests.swift	Updated presence data assertions and expectations to use direct structure without `userCustomData`.
Tests/AblyChatTests/Helpers/Helpers.swift	Simplified data conversion helper to bypass `PresenceDataDTO`.
Tests/AblyChatTests/PresenceDataDTOTests.swift	Deleted tests for `PresenceDataDTO` encoding/decoding.
Sources/AblyChat/DefaultMessages.swift	Corrected comment tag from `(CHA-M5k)` to `(CHA-M4k)` without functional changes.
Tests/AblyChatTests/DefaultMessagesTests.swift	Corrected test comment annotation from `CHA-M5k` to `CHA-M4k`.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant DefaultPresence
    participant PresenceChannel

    Client->>DefaultPresence: enter/update/leave(presenceData)
    DefaultPresence->>PresenceChannel: enterClient/update/leave(presenceData)
    PresenceChannel-->>DefaultPresence: PresenceMessage(data)
    DefaultPresence-->>Client: presence event (with raw data)

Estimated code review effort

3 (~45 minutes)

Possibly related PRs

[ECO-5170] Improve API and internals for presence data #189: Introduces and relies on PresenceDataDTO for presence data, which is now being removed in this PR.
[ECO-5342] Refactor PresenceEvent #311: Refactors presence event/member models, affecting related presence data handling logic.

Suggested reviewers

lawrence-forooghian
maratal

Poem

A DTO hopped out of view,
Now presence is simple and true.
No more wrapping, no more fuss,
Just raw data—easy for us!
The code is lighter, tests are neat,
🐇 Hopping forward, feeling fleet!

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61a29d3 and 993898f.

📒 Files selected for processing (7)

Sources/AblyChat/DefaultMessages.swift (1 hunks)
Sources/AblyChat/DefaultPresence.swift (5 hunks)
Sources/AblyChat/PresenceDataDTO.swift (0 hunks)
Tests/AblyChatTests/DefaultMessagesTests.swift (1 hunks)
Tests/AblyChatTests/DefaultPresenceTests.swift (3 hunks)
Tests/AblyChatTests/Helpers/Helpers.swift (1 hunks)
Tests/AblyChatTests/PresenceDataDTOTests.swift (0 hunks)

🧠 Learnings (2)

📓 Common learnings

Learnt from: maratal
PR: ably/ably-chat-swift#165
File: Sources/AblyChat/Reaction.swift:14-14
Timestamp: 2024-12-10T01:59:02.065Z
Learning: For the 'ably-chat-swift' repository, documentation-only PRs should focus exclusively on public methods and properties. Avoid suggesting changes to internal comments or private methods in such PRs.

Sources/AblyChat/DefaultMessages.swift (10)

Learnt from: maratal
PR: #165
File: Sources/AblyChat/Reaction.swift:14-14
Timestamp: 2024-12-10T01:59:02.065Z
Learning: For the 'ably-chat-swift' repository, documentation-only PRs should focus exclusively on public methods and properties. Avoid suggesting changes to internal comments or private methods in such PRs.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionHandleStorage.swift:37-40
Timestamp: 2025-05-12T21:02:28.274Z
Learning: In the AblyChat Swift codebase, the SubscriptionHandle struct has an unsubscribe() method for terminating subscriptions, not a cancel() method.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/Mocks/MockClients.swift:0-0
Timestamp: 2025-05-16T21:04:26.244Z
Learning: In the ably-chat-swift project, mock implementations (like those in MockClients.swift) are intentionally kept simple, sometimes omitting parameter-based filtering behavior for testing simplicity.

Learnt from: maratal
PR: #249
File: Tests/AblyChatTests/DefaultMessagesTests.swift:45-48
Timestamp: 2025-05-22T19:17:21.392Z
Learning: The let doIt = {} closures in DefaultMessagesTests.swift are temporary workarounds for compiler crashes (related to issue #233) and will be removed once Xcode 16.3 is released, so they should not be flagged for missing async/throws annotations.

Learnt from: maratal
PR: #293
File: Tests/AblyChatTests/Mocks/MockRealtime.swift:41-47
Timestamp: 2025-06-14T15:18:17.427Z
Learning: In the MockRealtime class in Tests/AblyChatTests/Mocks/MockRealtime.swift, the body parameter in the request method is constrained to dictionary or array types, but currently only dictionary types are used. Arrays are not yet used for this method, so the current implementation assumes dictionary-only for simplicity.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:11-14
Timestamp: 2025-05-12T21:03:31.914Z
Learning: The Typing protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultTyping automatically main actor-isolated without requiring explicit annotations on their methods.

💤 Files with no reviewable changes (2)

Sources/AblyChat/PresenceDataDTO.swift
Tests/AblyChatTests/PresenceDataDTOTests.swift

✅ Files skipped from review due to trivial changes (1)

Tests/AblyChatTests/DefaultMessagesTests.swift

🚧 Files skipped from review as they are similar to previous changes (3)

Tests/AblyChatTests/Helpers/Helpers.swift
Tests/AblyChatTests/DefaultPresenceTests.swift
Sources/AblyChat/DefaultPresence.swift

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: maratal
PR: ably/ably-chat-swift#165
File: Sources/AblyChat/Reaction.swift:14-14
Timestamp: 2024-12-10T01:59:02.065Z
Learning: For the 'ably-chat-swift' repository, documentation-only PRs should focus exclusively on public methods and properties. Avoid suggesting changes to internal comments or private methods in such PRs.