[ECO-5438] Add the soft delete behaviour and validation loosening - #330
Merged
Conversation
WalkthroughThis update refactors error handling in message and reaction event processing throughout the chat system. Throwing guards and do-catch blocks are replaced with logging, early returns, and default fallbacks. Several initializers and methods are made non-throwing, and public entity signatures are updated to use optional types and defaults, improving resilience to malformed or incomplete data. Changes
Sequence Diagram(s)sequenceDiagram
participant AblyChannel
participant DefaultMessages
participant Subscriber
AblyChannel->>DefaultMessages: Receive message event
DefaultMessages->>DefaultMessages: Validate fields (no throw)
alt Malformed or missing data
DefaultMessages->>DefaultMessages: Log and use default values
end
DefaultMessages->>Subscriber: Emit Message (with defaults if needed)
sequenceDiagram
participant AblyChannel
participant DefaultMessageReactions
participant Subscriber
AblyChannel->>DefaultMessageReactions: Receive reaction event
DefaultMessageReactions->>DefaultMessageReactions: Validate action and type
alt Unknown action/type
DefaultMessageReactions->>DefaultMessageReactions: Log and return early
else Valid event
DefaultMessageReactions->>Subscriber: Emit Reaction Event
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (9)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsSources/AblyChat/DefaultMessages.swift (11)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (34)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 13, 2025 21:05
e3dd0f4 to
c2101df
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 14, 2025 13:09
c2101df to
1b20ecc
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 14, 2025 13:35
1b20ecc to
1584625
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 14, 2025 13:39
1584625 to
24f4aa2
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 14, 2025 13:57
24f4aa2 to
4539d1a
Compare
Contributor
Author
|
Spec coverage fails because ably/specification#339 is still opened. |
maratal
marked this pull request as ready for review
July 14, 2025 14:06
maratal
requested review from
lawrence-forooghian,
ttypic and
umair-ably
and removed request for
lawrence-forooghian and
umair-ably
July 14, 2025 14:07
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Sources/AblyChat/DefaultMessages.swift (1)
115-124: Consider using the same Date instance for both timestamp fieldsThe default values are appropriate for handling incomplete data. However, for consistency, consider capturing the current date once and using it for both fields when timestamps are missing:
+ let defaultTimestamp = Date() let message = try Message( serial: message.serial ?? "", // CHA-M4k1 action: action, clientID: message.clientId ?? "", // CHA-M4k1 text: text, - createdAt: message.timestamp ?? Date(), // CHA-M4k4 + createdAt: message.timestamp ?? defaultTimestamp, // CHA-M4k4 metadata: metadata, headers: headers, version: message.version ?? "", // CHA-M4k1 - timestamp: message.timestamp ?? Date(), // CHA-M4k4, + timestamp: message.timestamp ?? defaultTimestamp, // CHA-M4k4, operation: message.operation?.toChatOperation() )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Sources/AblyChat/DefaultMessageReactions.swift(3 hunks)Sources/AblyChat/DefaultMessages.swift(1 hunks)Sources/AblyChat/DefaultRoomReactions.swift(1 hunks)Sources/AblyChat/RoomReactionDTO.swift(3 hunks)Tests/AblyChatTests/DefaultMessagesTests.swift(3 hunks)Tests/AblyChatTests/DefaultRoomReactionsTests.swift(3 hunks)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift(1 hunks)Tests/AblyChatTests/RoomReactionDTOTests.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 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.
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.
Sources/AblyChat/DefaultRoomReactions.swift (10)
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The `Occupancy` protocol in the Ably Chat Swift SDK is annotated with `@MainActor`, making all conforming types like `DefaultOccupancy` automatically main actor-isolated without requiring explicit annotations on their methods.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/ContentView.swift:94-99
Timestamp: 2025-06-14T21:47:20.509Z
Learning: In the Ably Chat Swift example app, using fatalError for missing clientID is intentional and appropriate since it represents a programmer error rather than a recoverable runtime condition. The clientID is considered an essential configuration requirement.
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.
Learnt from: maratal
PR: ably/ably-chat-swift#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.
Learnt from: maratal
PR: ably/ably-chat-swift#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.
Tests/AblyChatTests/RoomReactionDTOTests.swift (9)
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Example/AblyChatExample/Mocks/MockRealtime.swift:348-384
Timestamp: 2025-06-14T16:19:29.327Z
Learning: The MockRealtime class in Example/AblyChatExample/Mocks/MockRealtime.swift is specifically for example app construction, not for testing. The actual test mocks are located in the Tests directory. If example app mocks are unused, fatalError calls are appropriate to catch accidental usage.
Learnt from: maratal
PR: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: The ably-chat-swift project uses Swift Testing framework (`#expect` macros) rather than XCTest for test assertions.
Learnt from: maratal
PR: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: The ably-chat-swift project uses Swift Testing framework (`#expect` macros) rather than XCTest for test assertions.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:28:19.696Z
Learning: The project uses Swift Testing framework with `#expect` macros for assertions, not XCTest framework with expectations and fulfillment.
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.
Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (13)
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/Mocks/MockRealtime.swift:348-384
Timestamp: 2025-06-14T16:19:29.327Z
Learning: The MockRealtime class in Example/AblyChatExample/Mocks/MockRealtime.swift is specifically for example app construction, not for testing. The actual test mocks are located in the Tests directory. If example app mocks are unused, fatalError calls are appropriate to catch accidental usage.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomReactions.swift:42-56
Timestamp: 2025-05-16T21:35:46.634Z
Learning: In the AblyChat framework, all subscription callbacks are annotated with @MainActor, ensuring they always execute on the main actor thread without requiring additional Task wrappers.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on `self`. Even with `[weak self]` capture, the continuation will still be resumed properly.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/DefaultOccupancy.swift:66-69
Timestamp: 2025-05-12T21:02:25.928Z
Learning: SubscriptionHandle.unsubscribe is already marked with @MainActor annotation, so there's no need to wrap code in MainActor.run when calling it.
Sources/AblyChat/DefaultMessageReactions.swift (9)
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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.
Learnt from: maratal
PR: ably/ably-chat-swift#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.
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.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The `Occupancy` protocol in the Ably Chat Swift SDK is annotated with `@MainActor`, making all conforming types like `DefaultOccupancy` automatically main actor-isolated without requiring explicit annotations on their methods.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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.
Sources/AblyChat/DefaultMessages.swift (13)
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on `self`. Even with `[weak self]` capture, the continuation will still be resumed properly.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomReactions.swift:42-56
Timestamp: 2025-05-16T21:35:46.634Z
Learning: In the AblyChat framework, all subscription callbacks are annotated with @MainActor, ensuring they always execute on the main actor thread without requiring additional Task wrappers.
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.
Tests/AblyChatTests/DefaultRoomReactionsTests.swift (13)
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/Mocks/MockRealtime.swift:348-384
Timestamp: 2025-06-14T16:19:29.327Z
Learning: The MockRealtime class in Example/AblyChatExample/Mocks/MockRealtime.swift is specifically for example app construction, not for testing. The actual test mocks are located in the Tests directory. If example app mocks are unused, fatalError calls are appropriate to catch accidental usage.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: The ably-chat-swift project uses Swift Testing framework (`#expect` macros) rather than XCTest for test assertions.
Learnt from: maratal
PR: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: The ably-chat-swift project uses Swift Testing framework (`#expect` macros) rather than XCTest for test assertions.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#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.
Sources/AblyChat/RoomReactionDTO.swift (4)
Learnt from: maratal
PR: ably/ably-chat-swift#293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.
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.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.
Tests/AblyChatTests/DefaultMessagesTests.swift (15)
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/Mocks/MockRealtime.swift:348-384
Timestamp: 2025-06-14T16:19:29.327Z
Learning: The MockRealtime class in Example/AblyChatExample/Mocks/MockRealtime.swift is specifically for example app construction, not for testing. The actual test mocks are located in the Tests directory. If example app mocks are unused, fatalError calls are appropriate to catch accidental usage.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#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: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on `self`. Even with `[weak self]` capture, the continuation will still be resumed properly.
Learnt from: maratal
PR: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: The ably-chat-swift project uses Swift Testing framework (`#expect` macros) rather than XCTest for test assertions.
Learnt from: maratal
PR: ably/ably-chat-swift#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: ably/ably-chat-swift#293
File: Example/AblyChatExample/ContentView.swift:94-99
Timestamp: 2025-06-14T21:47:20.509Z
Learning: In the Ably Chat Swift example app, using fatalError for missing clientID is intentional and appropriate since it represents a programmer error rather than a recoverable runtime condition. The clientID is considered an essential configuration requirement.
🧬 Code Graph Analysis (1)
Sources/AblyChat/RoomReactionDTO.swift (1)
Sources/AblyChat/JSONCodable.swift (2)
optionalStringValueForKey(145-159)optionalObjectValueForKey(73-87)
🔇 Additional comments (19)
Tests/AblyChatTests/RoomReactionDTOTests.swift (1)
18-19: Test correctly reflects the new optional behavior.The test update properly validates that
RoomReactionDTO.Datacan now be initialized with an empty dictionary, resulting inname == nilinstead of throwing an error. This aligns with the validation loosening approach implemented in the related source files.Sources/AblyChat/DefaultRoomReactions.swift (3)
75-75: Good fallback for missing clientId.Using an empty string default for missing
clientIdallows the reaction to be processed while still indicating an unknown client. The spec referenceCHA-ER4e3confirms this is the intended behavior.
79-79: Appropriate fallback for missing reaction name.Defaulting to an empty string when
dto.nameis nil ensures reactions can still be processed even with incomplete data. This aligns with the validation loosening approach and spec pointCHA-ER4e1.
82-82: Reasonable timestamp fallback.Using the current date as a fallback for missing
timestampis a sensible approach that maintains reaction processing while providing a meaningful default. The spec referenceCHA-ER4e4confirms this behavior.Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
149-149: Improved subscription tracking consistency.Moving the subscription tracking outside the conditional ensures that subscriptions are always recorded, regardless of whether an immediate message is emitted. This improves test reliability and supports scenarios where both immediate and subsequent messages need to be handled consistently.
Sources/AblyChat/DefaultMessageReactions.swift (4)
87-87: Good documentation with spec reference.Adding the
CHA-MR6a1comment clarifies the fallback behavior for missing summary JSON, improving code documentation.
112-112: Clear documentation of event type logic.The
CHA-MR7b1comment helpfully documents the logic for determining reaction event types based on annotation actions.
113-113: Excellent improvement: graceful handling of unknown reaction types.This change prevents reactions with unrecognized types from being silently discarded. Using
.distinctas the default fallback is a reasonable choice that allows the reaction to be processed while maintaining expected behavior. This aligns well with the validation loosening approach.
126-126: Good documentation additions.The
CHA-MR7b3comments improve code clarity by documenting the fallback behavior for missing client IDs and reaction names.Also applies to: 133-133
Sources/AblyChat/RoomReactionDTO.swift (4)
7-7: Consistent optional name implementation.Making the
nameproperty optional enables more permissive handling of reaction data, aligning with the overall validation loosening approach.
22-22: Proper interface update.The computed property correctly reflects the optional nature of the underlying
namefield, maintaining consistency across the DTO interface.
44-44: Appropriate JSON decoding change.Switching to
optionalStringValueForKeycorrectly implements the optional name behavior, allowing JSON objects without the "name" key to be decoded successfully.
50-50: Good backward compatibility consideration.Using
name ?? ""ensures that nil names are serialized as empty strings, maintaining backward compatibility with systems expecting string values while supporting the new optional behavior.Sources/AblyChat/DefaultMessages.swift (2)
85-86: Good change to handle unsupported actions gracefullyThe switch from throwing errors to logging and returning aligns well with the PR's goal of making the code more tolerant. This prevents message processing from failing due to unsupported actions while maintaining observability through debug logging.
89-111: Well-structured handling of message data with appropriate defaultsThe implementation correctly differentiates between delete and non-delete actions, providing empty values for deletes and safe defaults for other actions. The consistent use of default values (
?? [:]and?? "") ensures robustness when processing malformed messages.Tests/AblyChatTests/DefaultRoomReactionsTests.swift (2)
35-72: Test updates correctly reflect the new reaction formatThe changes properly update the test to use "name" instead of "type" for reactions, and the addition of the callback counter improves test reliability by verifying the expected number of invocations.
74-109: Important fix: Corrected event name for reaction simulationGood catch on line 107 - using
RoomReactionEvents.reaction.rawValueinstead of a chat message event ensures the test correctly simulates a malformed reaction event. The callback counter addition and key rename maintain consistency with the new format.Tests/AblyChatTests/DefaultMessagesTests.swift (2)
363-371: Good naming consistency improvementThe rename from
subscriptionHandletosubscriptionimproves readability and consistency with other tests.
380-441: Excellent test coverage for the new malformed event handling behaviorThe updated test effectively validates that:
- Valid messages are processed correctly with all fields intact
- Malformed messages are still emitted but with sensible defaults (empty strings/dictionaries)
This aligns perfectly with the PR's goal of making the system more tolerant of incomplete data while maintaining observability.
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 14, 2025 17:00
4539d1a to
7092f9a
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 14, 2025 17:05
7092f9a to
7c46c08
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 18, 2025 13:31
7c46c08 to
32b50ae
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 18, 2025 13:38
32b50ae to
8e0e78c
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 18, 2025 13:41
8e0e78c to
af1c2f5
Compare
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 18, 2025 13:47
af1c2f5 to
eac37eb
Compare
maratal
changed the title
…ification#339). Fix room reaction tests.
maratal
force-pushed
the
321-soft-deletes-and-validation-loosening
branch
from
July 19, 2025 16:22
eac37eb to
6e8bc65
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #321
Add the soft delete behaviour and validation loosening
Fix room reaction tests.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
New Features