-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: self clients list is not updated when a new client event is received (WPB-5791) #2290
fix: self clients list is not updated when a new client event is received (WPB-5791) #2290
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, and one question!
@@ -253,6 +253,7 @@ class ClientDataSource( | |||
newClientEvent: Event.User.NewClient | |||
): Either<CoreFailure, Unit> = wrapStorageRequest { | |||
newClientDAO.insertNewClient(clientMapper.toInsertClientParam(selfUserID, newClientEvent)) | |||
clientDAO.insertClient(clientMapper.toInsertClientParam(selfUserID, newClientEvent)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between these two lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
different DB-tables where we save clients into.
NewClient
table containse only self-user clients that was added and user wasn't informed about it yet. So it's used for informing user about new devices by the dialog (kinda "there is a new device, was it you?").
And the Client
table - is a table for all the clients
logic/src/commonTest/kotlin/com/wire/kalium/logic/data/client/ClientRepositoryTest.kt
Outdated
Show resolved
Hide resolved
logic/src/commonMain/kotlin/com/wire/kalium/logic/data/client/ClientRepository.kt
Outdated
Show resolved
Hide resolved
Test Results2 310 tests - 375 2 244 ✔️ - 337 24s ⏱️ - 2m 30s Results for commit 5354485. ± Comparison against base commit 4e75095. This pull request removes 2685 and adds 2310 tests. Note that renamed tests count towards both.
This pull request removes 104 skipped tests and adds 66 skipped tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
Datadog ReportAll test runs ❌ 2 Total Test Services: 2 Failed, 0 with New Flaky, 0 Passed Test Services
❌ Failed Tests (4)
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #2290 +/- ##
=============================================
- Coverage 57.93% 57.93% -0.01%
Complexity 21 21
=============================================
Files 1093 1093
Lines 41440 41446 +6
Branches 3826 3827 +1
=============================================
+ Hits 24007 24010 +3
- Misses 15820 15823 +3
Partials 1613 1613
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
…:wireapp/kalium into fix/end_calls_when_self_added_new_device
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work as always, just need to cover with some sweet tests
logic/src/commonMain/kotlin/com/wire/kalium/logic/data/client/ClientRepository.kt
Outdated
Show resolved
Hide resolved
persistence/src/commonMain/db_user/com/wire/kalium/persistence/Clients.sq
Outdated
Show resolved
Hide resolved
persistence/src/commonMain/db_user/com/wire/kalium/persistence/Clients.sq
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please add one more test
given new client is added, then the same client is added to the clients table
What's new in this PR?
Issues
When I'm on a call in Proteus-verified conversation and I add a new client (login on new device). Conversation should degrade the verification status and end a call.
Causes (Optional)
a
NewClientEvent
handler doesn't save a new Client into the Client DB table, so the trigger for re-checking the conversation verification is not called -> conversation doesn't degrade -> call is not ended.Solutions
Save new client into Client DB table on every
NewClientEvent
.