Renames RoomReaction.type to RoomReaction.name#326
Conversation
WalkthroughThis change refactors the codebase by renaming the "type" property related to reactions to "name" across all relevant models, DTOs, parameters, and test cases. It updates property names, initializer parameters, and JSON keys as needed, ensuring consistency throughout the implementation and tests. No logic or control flow is altered. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant RoomReactions
participant RoomReactionDTO
UI->>RoomReactions: send(params: SendReactionParams(name: ...))
RoomReactions->>RoomReactionDTO: init(name: ...)
RoomReactionDTO-->>RoomReactions: RoomReactionDTO instance
RoomReactions-->>UI: Reaction sent
RoomReactions->>UI: subscribe(callback)
RoomReactions->>RoomReactionDTO: parse incoming event (name: ...)
RoomReactionDTO-->>RoomReactions: RoomReactionDTO instance
RoomReactions-->>UI: callback(RoomReaction(name: ...))
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 (10)
⏰ 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). (5)
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
Tests/AblyChatTests/DefaultRoomReactionsTests.swift (1)
28-28: Update test to use"name"JSON key instead of"type"The
RoomReactionDTO.toJSONObjectimplementation now serializes the reaction’s identifier underJSONKey.name.rawValue(i.e."name"), not"type". Update the test assertion accordingly.• File:
Tests/AblyChatTests/DefaultRoomReactionsTests.swift
Line: 28- #expect(channel.publishedMessages.last?.data == ["type": "like", "metadata": ["someMetadataKey": "someMetadataValue"]]) + #expect(channel.publishedMessages.last?.data == ["name": "like", "metadata": ["someMetadataKey": "someMetadataValue"]])Tests/AblyChatTests/RoomReactionDTOTests.swift (2)
32-44: Fix test inconsistencies in JSON deserialization.The test has multiple issues:
- JSON input uses
"type": "someType"but expects DTO withname: "someName"- The values don't match: "someType" ≠ "someName"
Apply this diff to fix the test:
- let data = try RoomReactionDTO.Data( - jsonValue: [ - "type": "someType", - "metadata": [ - "someStringKey": "someStringValue", - "someNumberKey": 123, - ], - ] - ) - - #expect(data == .init(name: "someName", metadata: ["someStringKey": "someStringValue", "someNumberKey": 123])) + let data = try RoomReactionDTO.Data( + jsonValue: [ + "name": "someType", + "metadata": [ + "someStringKey": "someStringValue", + "someNumberKey": 123, + ], + ] + ) + + #expect(data == .init(name: "someType", metadata: ["someStringKey": "someStringValue", "someNumberKey": 123]))
55-65: Fix JSON serialization test values and keys.The test has two issues:
- Creates DTO with
name: "someName"but expects JSON with"type": "someType"- Uses different values that don't match
Apply this diff to fix the test:
- let data = RoomReactionDTO.Data(name: "someName", metadata: ["someStringKey": "someStringValue", "someNumberKey": 123]) + let data = RoomReactionDTO.Data(name: "someType", metadata: ["someStringKey": "someStringValue", "someNumberKey": 123]) #expect(data.toJSONValue == [ - "type": "someType", + "name": "someType", "metadata": [ "someStringKey": "someStringValue", "someNumberKey": 123, ], ])
🧹 Nitpick comments (1)
Sources/AblyChat/RoomReactions.swift (1)
14-14: Update documentation comments to reflect the parameter rename.Several documentation comments still reference the old
typeparameter name and should be updated to usename:
- Line 14: Change "containing
type" to "containingname"- Line 65: Change "Only
typeis mandatory" to "Onlynameis mandatory"- Line 69: Change "The type of the reaction" to "The name of the reaction"
Apply this diff to update the documentation:
- * - params: An object containing `type` and optional `headers` with `metadata`. + * - params: An object containing `name` and optional `headers` with `metadata`.- * Params for sending a room-level reactions. Only `type` is mandatory. + * Params for sending a room-level reactions. Only `name` is mandatory.- * The type of the reaction, for example an emoji or a short string such as "like". + * The name of the reaction, for example an emoji or a short string such as "like".Also applies to: 65-65, 69-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Example/AblyChatExample/ContentView.swift(3 hunks)Example/AblyChatExample/Mocks/Misc.swift(2 hunks)Example/AblyChatExample/Mocks/MockClients.swift(2 hunks)Sources/AblyChat/DefaultRoomReactions.swift(2 hunks)Sources/AblyChat/RoomReaction.swift(2 hunks)Sources/AblyChat/RoomReactionDTO.swift(3 hunks)Sources/AblyChat/RoomReactions.swift(2 hunks)Tests/AblyChatTests/DefaultRoomReactionsTests.swift(3 hunks)Tests/AblyChatTests/IntegrationTests.swift(1 hunks)Tests/AblyChatTests/RoomReactionDTOTests.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
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: 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/IntegrationTests.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#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#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#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#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#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.
Tests/AblyChatTests/DefaultRoomReactionsTests.swift (16)
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#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#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#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: 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#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: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#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/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:28:19.696Z
Learning: The project uses Swift Testing framework with `#expect` macros for assertions, not XCTest framework with expectations and fulfillment.
Example/AblyChatExample/ContentView.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#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#165
File: docs-coverage-report:5-8
Timestamp: 2024-12-10T01:59:12.404Z
Learning: In the AblyChat Swift project, documentation coverage should focus on public methods and properties. Private and internal entities do not require documentation.
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/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/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#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: 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.
Example/AblyChatExample/Mocks/Misc.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/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: 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#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: 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#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/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#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.
Sources/AblyChat/RoomReactions.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#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#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#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/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#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#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#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/DefaultRoomReactions.swift (7)
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#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: 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.
Tests/AblyChatTests/RoomReactionDTOTests.swift (10)
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: 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#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#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: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#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#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/RoomReaction.swift (6)
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#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.
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.
Example/AblyChatExample/Mocks/MockClients.swift (10)
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/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#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#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#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#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#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.
🧬 Code Graph Analysis (2)
Example/AblyChatExample/ContentView.swift (2)
Sources/AblyChat/DefaultRoomReactions.swift (2)
send(10-12)send(36-50)Example/AblyChatExample/Mocks/MockClients.swift (3)
send(158-173)send(245-262)send(318-329)
Sources/AblyChat/RoomReactionDTO.swift (1)
Sources/AblyChat/JSONCodable.swift (2)
stringValueForKey(130-140)optionalObjectValueForKey(73-87)
⏰ 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). (5)
- GitHub Check: check-documentation
- GitHub Check: Generate code coverage
- GitHub Check: generate-matrices
- GitHub Check: spec-coverage
- GitHub Check: lint
🔇 Additional comments (12)
Example/AblyChatExample/Mocks/Misc.swift (1)
71-71: LGTM! Consistent enum and property renaming.The changes correctly rename
ReactionTypetoReactionNameand update the property access fromtypetoname, maintaining consistency with the overall refactoring objective.Also applies to: 94-94
Tests/AblyChatTests/IntegrationTests.swift (1)
329-329: LGTM! Test updates correctly reflect the API changes.The integration test properly updates both the reaction sending parameter (
nameinstead oftype) and the assertion to access the renamed property, ensuring the test continues to verify the correct behavior.Also applies to: 335-335
Example/AblyChatExample/ContentView.swift (1)
86-86: LGTM! UI code correctly updated for the renamed enum and parameter.The changes consistently update the enum reference from
ReactionTypetoReactionNameand correctly use the newnameparameter in the reaction send call, maintaining the same functionality.Also applies to: 242-242, 424-424
Sources/AblyChat/RoomReactions.swift (1)
72-72: LGTM! Property and initializer correctly renamed.The core API changes properly rename the property and initializer parameter from
typetoname, maintaining the same functionality while aligning with the PR objectives.Also applies to: 101-102
Sources/AblyChat/DefaultRoomReactions.swift (1)
40-40: LGTM! Implementation correctly updated for the renamed properties.The changes properly update both the reaction sending and receiving logic to use the renamed
nameproperty instead oftype, maintaining consistency with the updated API while preserving all functionality.Also applies to: 85-85
Tests/AblyChatTests/DefaultRoomReactionsTests.swift (2)
17-21: LGTM! Test parameter construction updated correctly.The change from
type: "like"toname: "like"inSendReactionParamsinitialization aligns with the property renaming across the codebase.
57-57: LGTM! Test assertions updated consistently.The assertions correctly reference
event.reaction.nameinstead ofevent.reaction.type, maintaining consistency with the property renaming.Also applies to: 93-93
Sources/AblyChat/RoomReaction.swift (1)
22-22: LGTM! Property renaming implemented correctly.The property declaration and initializer parameter have been consistently updated from
typetoname. The assignment logic is correct and maintains the same functionality.Also applies to: 49-50
Example/AblyChatExample/Mocks/MockClients.swift (2)
318-326: LGTM! Mock reaction creation updated correctly.The
RoomReactioninitialization properly usesname: params.nameinstead oftype: params.type, maintaining consistency with the property renaming.
332-344: LGTM! Random reaction generation updated correctly.The change from
ReactionType.allCases.randomElement()!.emojitoReactionName.allCases.randomElement()!.emojialigns with the enum renaming and property updates.Sources/AblyChat/RoomReactionDTO.swift (2)
6-9: LGTM! DTO structure updated consistently.The property renaming from
typetonamehas been implemented correctly throughout the DTO structure, including the property declaration, initializer parameter, and computed property accessor.Also applies to: 17-24
37-54: LGTM! JSON serialization updated comprehensively.The JSON coding implementation has been consistently updated:
- JSON key enum case changed from
typetoname- Deserialization uses the correct
namekey- Serialization outputs the correct
namekeyThis confirms that the failing tests in
RoomReactionDTOTests.swiftneed to be updated to expect"name"keys instead of"type"keys in JSON.
0040a04 to
6d5212f
Compare
… expected realtime changes
6d5212f to
47820ac
Compare
Refactors the RoomReaction
typeproperty to bename, in line with expected realtime changesSummary by CodeRabbit
Refactor
Tests