[CIS-259] Implement teams #905

DominikBucher12 · 2021-03-22T17:23:32Z

What this PR do:

Adds support for user teams and channel team inside those payloads

How to test it

Run tests if teams property is parsed from payload.

Where you can start

UserPayload and ChannelListPayload, check if there is correct relationship between objects.

codecov · 2021-03-22T17:32:26Z

Codecov Report

Merging #905 (000e5bb) into main (1dec12d) will increase coverage by 1.90%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #905      +/-   ##
==========================================
+ Coverage   87.11%   89.01%   +1.90%     
==========================================
  Files         238      200      -38     
  Lines        9147     8130    -1017     
==========================================
- Hits         7968     7237     -731     
+ Misses       1179      893     -286

Flag	Coverage Δ
llc-tests	`89.01% <100.00%> (+1.90%)`	⬆️
llc-tests-ios12	`89.01% <100.00%> (+1.90%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...lient/Endpoints/Payloads/CurrentUserPayloads.swift	`92.30% <ø> (ø)`
...at/APIClient/Endpoints/Payloads/UserPayloads.swift	`100.00% <ø> (ø)`
Sources/StreamChat/Models/User.swift	`89.47% <ø> (ø)`
...Client/Endpoints/Payloads/ChannelListPayload.swift	`100.00% <100.00%> (ø)`
Sources/StreamChat/Database/DTOs/ChannelDTO.swift	`100.00% <100.00%> (ø)`
Sources/StreamChat/Database/DTOs/TeamDTO.swift	`100.00% <100.00%> (ø)`
Sources/StreamChat/Database/DTOs/UserDTO.swift	`96.00% <100.00%> (+0.08%)`	⬆️
Sources/StreamChat/Models/Channel.swift	`78.94% <100.00%> (ø)`
...sts/StreamChatTests/DummyData/DevicePayloads.swift
...ests/StreamChatTests/DummyData/MemberPayload.swift
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dec12d...000e5bb. Read the comment docs.

VojtaStavik

Ideally, the tests should be in the same commit as the implementation, so it's easy to verify what's happening there.

It seems there are no tests for DTOs, correct? I see only tests for the payload serialization.

Sources/StreamChat/APIClient/Endpoints/Payloads/ChannelListPayload.swift

Sources/StreamChat/Database/DTOs/ChannelDTO.swift

Sources/StreamChat/Database/DTOs/UserDTO.swift

nuno-vieira

Great Job 💪 Just added some questions 👍

Sources/StreamChat/Models/User.swift

Tests/StreamChatTests/MockEnpointResponses/CurrentUser.json

VojtaStavik

Almost there... just some nits

Sources/StreamChat/Database/DTOs/TeamDTO_Tests.swift

VojtaStavik · 2021-03-23T13:30:02Z

Sources/StreamChat/Database/DTOs/UserDTO.swift

+
+        let teamsArray: [TeamDTO] = try payload.teams.map {
+            let team = try saveTeam(teamId: $0)
+            team.users.insert(dto)


this is IMO not needed. The relationship is already set on L120

Okay this is probably my gap in CoreData knowledge... But now I see you are right. It handles automatically... Thanks

So will you remove it then?

Sources/StreamChat/Database/DTOs/ChannelDTO.swift

Tests/Shared/CustomAssertions/AssertAsync.swift

CHANGELOG.md

Sources/StreamChat/Database/DTOs/ChannelDTO.swift

VojtaStavik · 2021-03-24T09:10:41Z

Sources/StreamChat/Database/DTOs/TeamDTO_Tests.swift

+    func test_teamsForUser_areStoredAndLoadedFromDB() {
+        let teamIds: [TeamId] = [.unique, .unique, .unique]
+        let userId: UserId = .unique
+
+        let payload: UserPayload<NoExtraData> = .dummy(userId: userId, teams: teamIds)
+
+        // Asynchronously save the user payload to the db, which should save teams for user as well.
+        try! database.writeSynchronously { session in
+            try! session.saveUser(payload: payload)
+        }
+
+        // Load the user from the db and check the fields are correct
+        var loadedUserDTO: UserDTO? {
+            database.viewContext.user(id: userId)
+        }
+
+        XCTAssertEqual(payload.teams.count, loadedUserDTO?.teams?.count)
+        XCTAssertEqual(loadedUserDTO?.teams?.first?.users.first, loadedUserDTO)
+    }


This should be among UserDTO tests

VojtaStavik · 2021-03-24T09:11:04Z

Sources/StreamChat/Database/DTOs/TeamDTO_Tests.swift

+    func test_teamForChannel_isStoredAndLoadedFromDB() {
+        let teamId: TeamId = .unique
+        let channelId: ChannelId = .unique
+
+        let channelDetailPayload: ChannelDetailPayload<NoExtraData> = .dummy(cid: channelId, teamId: teamId)
+
+        // Asynchronously save the channel payload to the db, which should save the team for the channel as well.
+        try! database.writeSynchronously { session in
+            try! session.saveChannel(payload: channelDetailPayload, query: nil)
+        }
+
+        // Load the channel from the db and check the team is correct
+        var loadedChannelDTO: ChannelDTO? {
+            database.viewContext.channel(cid: channelId)
+        }
+
+        XCTAssertEqual(channelDetailPayload.team, loadedChannelDTO?.team?.id)
+        XCTAssertEqual(loadedChannelDTO?.team?.channels.first, loadedChannelDTO)
+    }


This should be among ChannelDTO tests

Moved... and deleted file 🧙🏻‍♂️

VojtaStavik · 2021-03-24T09:11:37Z

Sources/StreamChat/Database/DTOs/TeamDTO_Tests.swift

+        try! database.writeSynchronously { session in
+            try! session.saveUser(payload: payload)
+        }


If you mark the test throws you can change try! to try

VojtaStavik · 2021-03-24T09:12:19Z

Sources/StreamChat/Database/DTOs/TeamDTO_Tests.swift

+        var loadedUserDTO: UserDTO? {
+            database.viewContext.user(id: userId)
+        }


let loadedUser = try XCTUnwrap(database.viewContext.user(id: userId))

would work better here

Damn that copypasting... Changed.

VojtaStavik · 2021-03-24T09:20:11Z

Sources/StreamChat/Database/DTOs/UserDTO.swift

+
+        let teamsArray: [TeamDTO] = try payload.teams.map {
+            let team = try saveTeam(teamId: $0)
+            team.users.insert(dto)


So will you remove it then?

VojtaStavik · 2021-03-24T09:21:46Z

Tests/StreamChatTests/DummyData/ChannelPayload.swift

-    static func dummy(cid: ChannelId) -> ChannelDetailPayload {
+    static func dummy(cid: ChannelId, teamId: TeamId? = nil) -> ChannelDetailPayload {


Not sure if this is needed tbh :) Why no just assign .unique to the team variable like we do it with other things?

Yeah, I guess you are right, I wanted to have control over with which teams I creat and assign to dummy, but I guess it needs to be known only for cid. Thanks fixed.

VojtaStavik · 2021-03-24T09:23:51Z

Sources/StreamChat/Database/DTOs/UserDTO_Tests.swift

@@ -43,6 +44,8 @@ class UserDTO_Tests: XCTestCase {
            Assert.willBeEqual(payload.createdAt, loadedUserDTO?.userCreatedAt)
            Assert.willBeEqual(payload.updatedAt, loadedUserDTO?.userUpdatedAt)
            Assert.willBeEqual(payload.lastActiveAt, loadedUserDTO?.lastActivityAt)
+            Assert.willBeEqual(payload.teams.first, loadedUserDTO?.teams?.first?.id)
+            Assert.willBeEqual(loadedUserDTO?.teams?.first?.users.first, loadedUserDTO)


What exactly are you testing here? 🤯 This is already taken care of by CoreData, we don't need to test it explicitly.

I started to panic when I figured out there are relationships to the TeamDTO, so I started to manually assign the relationships and test it... Only to figure out I don't need to :D

VojtaStavik · 2021-03-24T09:24:13Z

Sources/StreamChat/Database/DTOs/UserDTO_Tests.swift

@@ -43,6 +44,8 @@ class UserDTO_Tests: XCTestCase {
            Assert.willBeEqual(payload.createdAt, loadedUserDTO?.userCreatedAt)
            Assert.willBeEqual(payload.updatedAt, loadedUserDTO?.userUpdatedAt)
            Assert.willBeEqual(payload.lastActiveAt, loadedUserDTO?.lastActivityAt)
+            Assert.willBeEqual(payload.teams.first, loadedUserDTO?.teams?.first?.id)


We can be more specific here a do something like:

Assert.willBeEqual(payload.teams.sorted(), loadedUserDTO?.teams?.map { $0.id }.sorted())

Thank you, much better..

VojtaStavik

Cool 🎉 There's are still some strange things happening in tests. Please fix and it's good to 🚢

VojtaStavik · 2021-03-24T09:26:07Z

CHANGELOG.md

@@ -20,6 +20,7 @@ _March 12, 2021_
 - Expose `membership` value on `ChatChannel` which contains information about the current user membership [#885](https://github.com/GetStream/stream-chat-swift/issues/885)
 - `ChatChannelMember` now contains channel-specific ban information: `isBannedFromChannel` and `banExpiresAt` [#885](https://github.com/GetStream/stream-chat-swift/issues/885)
 - Channel-specific ban events are handled and the models are properly updated [#885](https://github.com/GetStream/stream-chat-swift/pull/885)
+- Introduce support for [multitenancy](https://getstream.io/chat/docs/react/multi_tenant_chat/?language=swift) - `teams` for `User` and `team` for `Channel` are now exposed.


The link to the PR is missing (see other CHANGELOG items please)

Thanks, fixed❗️

Co-authored-by: Dominik Bucher <d.bucher@centrum.cz>

DominikBucher12 requested a review from VojtaStavik March 22, 2021 17:23

DominikBucher12 force-pushed the CIS-259-Multi_tenant branch from 926ff5c to 9c8030d Compare March 22, 2021 22:47

DominikBucher12 marked this pull request as ready for review March 23, 2021 09:37

DominikBucher12 force-pushed the CIS-259-Multi_tenant branch from 9c8030d to a420ccc Compare March 23, 2021 09:39

VojtaStavik suggested changes Mar 23, 2021

View reviewed changes

Sources/StreamChat/APIClient/Endpoints/Payloads/ChannelListPayload.swift Show resolved Hide resolved

Sources/StreamChat/Database/DTOs/ChannelDTO.swift Outdated Show resolved Hide resolved

Sources/StreamChat/Database/DTOs/UserDTO.swift Outdated Show resolved Hide resolved

DominikBucher12 force-pushed the CIS-259-Multi_tenant branch from a420ccc to 53b6893 Compare March 23, 2021 11:37

DominikBucher12 added the 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK label Mar 23, 2021

nuno-vieira approved these changes Mar 23, 2021

View reviewed changes

Sources/StreamChat/Models/User.swift Outdated Show resolved Hide resolved

Tests/StreamChatTests/MockEnpointResponses/CurrentUser.json Show resolved Hide resolved

VojtaStavik suggested changes Mar 23, 2021

View reviewed changes

DominikBucher12 force-pushed the CIS-259-Multi_tenant branch from 14272dc to 605a588 Compare March 23, 2021 20:58

VojtaStavik reviewed Mar 24, 2021

View reviewed changes

VojtaStavik approved these changes Mar 24, 2021

View reviewed changes

DominikBucher12 force-pushed the CIS-259-Multi_tenant branch from 605a588 to 2e30974 Compare March 24, 2021 11:09

DominikBucher12 and others added 4 commits March 24, 2021 13:12

Add willNotBeNil assertion to AssertAsync

bbfb45b

Add TeamDTO implementation

3310a78

Use synchronous write in DTO tests

c9379da

Parse teams data from user payloads

5a2cd8d

Co-authored-by: Dominik Bucher <d.bucher@centrum.cz>

VojtaStavik force-pushed the CIS-259-Multi_tenant branch from 2e30974 to b7ab3f3 Compare March 24, 2021 12:13

VojtaStavik and others added 4 commits March 24, 2021 14:09

Save and expose ChatUser.teams

8bf9311

Co-authored-by: Dominik Bucher <d.bucher@centrum.cz>

Parse team info from channel payloads

ed86c49

Co-authored-by: Dominik Bucher <d.bucher@centrum.cz>

Save and expose ChatChannel.team

dc787aa

Co-authored-by: Dominik Bucher <d.bucher@centrum.cz>

Update CHANGELOG

966312c

Co-authored-by: Dominik Bucher <d.bucher@centrum.cz>

VojtaStavik force-pushed the CIS-259-Multi_tenant branch from b7ab3f3 to 966312c Compare March 24, 2021 13:16

Ignore Tests folder for code coverage

000e5bb

VojtaStavik merged commit 0aeb995 into main Mar 24, 2021

VojtaStavik deleted the CIS-259-Multi_tenant branch March 24, 2021 15:58

VojtaStavik mentioned this pull request Mar 24, 2021

Add team filter keys #912

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CIS-259] Implement teams #905

[CIS-259] Implement teams #905

DominikBucher12 commented Mar 22, 2021 •

edited

codecov bot commented Mar 22, 2021 •

edited

VojtaStavik left a comment

nuno-vieira left a comment

VojtaStavik left a comment

VojtaStavik Mar 23, 2021

DominikBucher12 Mar 23, 2021

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021 •

edited

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

VojtaStavik Mar 24, 2021 •

edited

DominikBucher12 Mar 24, 2021

VojtaStavik left a comment

VojtaStavik Mar 24, 2021

DominikBucher12 Mar 24, 2021

		static func dummy(cid: ChannelId) -> ChannelDetailPayload {
		static func dummy(cid: ChannelId, teamId: TeamId? = nil) -> ChannelDetailPayload {

[CIS-259] Implement teams #905

[CIS-259] Implement teams #905

Conversation

DominikBucher12 commented Mar 22, 2021 • edited

What this PR do:

How to test it

Where you can start

codecov bot commented Mar 22, 2021 • edited

Codecov Report

VojtaStavik left a comment

Choose a reason for hiding this comment

nuno-vieira left a comment

Choose a reason for hiding this comment

VojtaStavik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VojtaStavik Mar 24, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VojtaStavik Mar 24, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VojtaStavik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DominikBucher12 commented Mar 22, 2021 •

edited

codecov bot commented Mar 22, 2021 •

edited

VojtaStavik Mar 24, 2021 •

edited

VojtaStavik Mar 24, 2021 •

edited