feat: Update conversation Proteus verification status (WPB-1789) #2157

borichellow · 2023-10-19T14:50:21Z

What's new in this PR?

Issues

When all the clients that belong to conversation are Proteus-verified, such a conversation should become a "Proteus verified" + need to add SystemMessage to that conversation informing user that conversation is Verified.

the same flow when conversation becomes "Proteus degraded".

Solutions

update Conversation DB: add column proteus_verification_status
update Conversation DB: add select method for getting conversationID with current proteus_verification_status and if that conversation should be verified
update UpdateClientVerificationStatusUseCase to use select from #2, update conversations if needed and add SystemMessages to it.

Needs releases with:

GitHub link to other pull request

CLAassistant · 2023-10-19T14:50:28Z

All committers have signed the CLA.

…tion_status

github-actions · 2023-10-20T08:51:04Z

Unit Test Results

      2 files - 461       2 suites - 461 3m 55s ⏱️ +21s
2 216 tests - 362 2 209 ✔️ - 265 7 💤 - 97 0 ❌ ±0

Results for commit 4f9df40. ± Comparison against base commit 2fb391f.

♻️ This comment has been updated with latest results.

datadog-wireapp · 2023-10-20T08:59:34Z

Datadog Report

Branch report: feat/update_conversation_proteus_verification_status
Commit report: 5b114d6

✅ kalium-jvm: 0 Failed, 0 New Flaky, 1 Passed, 0 Skipped, 2.02s Wall Time

codecov-commenter · 2023-10-20T09:54:13Z

Codecov Report

Merging #2157 (4f9df40) into develop (2fb391f) will decrease coverage by 0.01%.
The diff coverage is 50.81%.

@@              Coverage Diff              @@
##             develop    #2157      +/-   ##
=============================================
- Coverage      57.98%   57.97%   -0.01%     
  Complexity        21       21              
=============================================
  Files           1064     1064              
  Lines          40319    40414      +95     
  Branches        3733     3738       +5     
=============================================
+ Hits           23377    23430      +53     
- Misses         15330    15372      +42     
  Partials        1612     1612

Files	Coverage Δ
...ire/kalium/logic/data/conversation/Conversation.kt	`90.96% <100.00%> (+0.11%)`	⬆️
...lium/logic/data/conversation/ConversationMapper.kt	`59.13% <100.00%> (+0.55%)`	⬆️
...otlin/com/wire/kalium/logic/data/web/WebMappers.kt	`37.30% <100.00%> (+0.50%)`	⬆️
...ion/MLSConversationsVerificationStatusesHandler.kt	`97.36% <100.00%> (ø)`
...um/persistence/dao/conversation/ConversationDAO.kt	`100.00% <ø> (ø)`
...persistence/dao/conversation/ConversationEntity.kt	`98.24% <100.00%> (+0.03%)`	⬆️
...persistence/dao/conversation/ConversationMapper.kt	`98.91% <100.00%> (+0.02%)`	⬆️
...istence/dao/conversation/ConversationViewEntity.kt	`97.77% <100.00%> (+0.05%)`	⬆️
...tlin/com/wire/kalium/persistence/db/TableMapper.kt	`100.00% <100.00%> (ø)`
.../logic/data/conversation/ConversationRepository.kt	`60.74% <0.00%> (ø)`
... and 8 more

... and 11 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

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

mchenani · 2023-10-20T12:11:56Z

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/Conversation.kt

@@ -269,6 +275,11 @@ data class Conversation(
        )
    }

+    data class ProteusVerificationData(
+        val conversationId: ConversationId,
+        val currentVerificationStatus: VerificationStatus,


suggestion:

Suggested change

val currentVerificationStatus: VerificationStatus,

val verificationStatus: VerificationStatus,

mchenani · 2023-10-20T12:12:13Z

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/Conversation.kt

+    data class ProteusVerificationData(
+        val conversationId: ConversationId,
+        val currentVerificationStatus: VerificationStatus,
+        val isActuallyVerified: Boolean


question: what does this mean?

conversationId is conversationId, currentVerificationStatus is the verification status of the conversation that is currently stored in DB, isActuallyVerified Boolean if the conversation actually should be verified or not (calculated by checking all the clients of the conversation).
It's a good idea to add some docs :)

so isn't better to store the new updated value then return the aggregated value? still I think it's confusing that we're returning two values that one is kinda out dated.

mchenani · 2023-10-20T12:14:03Z

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt

@@ -281,6 +281,14 @@ interface ConversationRepository {
     * @return **true** if the protocol was changed or **false** if the protocol was unchanged.
     */
    suspend fun updateProtocolLocally(conversationId: ConversationId, protocol: Conversation.Protocol): Either<CoreFailure, Boolean>
+
+    suspend fun getConversationsProteusVerificationDataByClientId(


Suggested change

suspend fun getConversationsProteusVerificationDataByClientId(

suspend fun getConversationsProteusVerificationData(

mchenani · 2023-10-20T12:14:17Z

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt

+        clientId: ClientId
+    ): Either<StorageFailure, List<Conversation.ProteusVerificationData>>
+
+    suspend fun updateProteusVerificationStatuses(


nitpicking: https://www.grammar-monster.com/plurals/plural_of_status.htm#:~:text=The%20plural%20of%20%22status%22%20is,statuses%22%20since%20the%201960s.)

Suggested change

suspend fun updateProteusVerificationStatuses(

suspend fun updateProteusVerificationStatus(

…tion_status

mchenani · 2023-10-23T12:46:32Z

persistence/src/commonMain/db_user/com/wire/kalium/persistence/Members.sq

@@ -51,7 +51,8 @@ SELECT
    user_message_timer,
    archived,
    archived_date_time,
-    verification_status
+    verification_status,


tbh storing the mls verification status for members doesn't make sense. why we need it? it's already in the CoreCrypto we only need to fetch it. keeping it up to dated is risky.

mchenani

I how two concerns, please review and if necessary we can sync online.

…as possible to DB level

mchenani · 2023-10-25T15:20:17Z

persistence/src/commonMain/db_user/com/wire/kalium/persistence/Clients.sq

@@ -79,3 +79,32 @@ SELECT *
  WHERE user_id IN
  (SELECT user FROM Member WHERE conversation = :conversation_id AND user_id IN :userIdList)
  AND is_valid = 1;
+
+CREATE TRIGGER updateConversationProteusVerificationStatus


🤘🙌🔥

mchenani · 2023-10-25T15:21:17Z

persistence/src/commonMain/db_user/com/wire/kalium/persistence/Conversations.sq

@@ -400,3 +403,25 @@ DELETE FROM Message WHERE conversation_id = :conversationId;

 selectChanges:
 SELECT changes();
+
+CREATE TRIGGER addMessageAfterProteusVerificationStatusChange


mchenani

Great work man, awesome 🤩 🔥

…tion_status

borichellow added 2 commits October 19, 2023 16:08

feat: Update conversation Proteus verification status

551cbd8

Merge develop into feat/update_conversation_proteus_verification_status

1feeac9

borichellow added the DO NOT MERGE label Oct 19, 2023

borichellow requested review from typfel, MohamadJaara, vitorhugods, mchenani and Garzas October 19, 2023 14:50

borichellow self-assigned this Oct 19, 2023

github-actions bot added 🚨 Potential breaking changes 👕 size: XL type: feature ✨ labels Oct 19, 2023

borichellow mentioned this pull request Oct 19, 2023

feat: Show conversation Proteus verification status wireapp/wire-android#2350

Merged

borichellow added 3 commits October 20, 2023 11:13

Code style fix

be4775d

Merge branch 'develop' into feat/update_conversation_proteus_verifica…

240203d

…tion_status

Fix code stlye again

8c01cf4

borichellow added 2 commits October 20, 2023 12:14

Fixed test models

db2b45b

Fixed test models

bcfa5a7

mchenani reviewed Oct 20, 2023

View reviewed changes

Review fixes

ca7540b

borichellow requested a review from mchenani October 23, 2023 11:32

Merge branch 'develop' into feat/update_conversation_proteus_verifica…

c44dd41

…tion_status

mchenani reviewed Oct 23, 2023

View reviewed changes

mchenani requested changes Oct 23, 2023

View reviewed changes

borichellow added 2 commits October 25, 2023 18:07

feat: Update conversation Proteus verification status: modev as much …

83f97e8

…as possible to DB level

Merge develop into feat/update_conversation_proteus_verification_status

9fe06f0

borichellow requested a review from mchenani October 25, 2023 15:15

Reverted useles changes

9488aa9

mchenani reviewed Oct 25, 2023

View reviewed changes

mchenani approved these changes Oct 25, 2023

View reviewed changes

Fixed test

1c5ee36

Garzas approved these changes Oct 26, 2023

View reviewed changes

borichellow added 2 commits October 26, 2023 15:27

Fixed DB migration

d149e33

Merge branch 'develop' into feat/update_conversation_proteus_verifica…

4f9df40

…tion_status

borichellow added this pull request to the merge queue Oct 27, 2023

Merged via the queue into develop with commit 07301bd Oct 27, 2023
16 of 17 checks passed

borichellow deleted the feat/update_conversation_proteus_verification_status branch October 27, 2023 09:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update conversation Proteus verification status (WPB-1789) #2157

feat: Update conversation Proteus verification status (WPB-1789) #2157

borichellow commented Oct 19, 2023

CLAassistant commented Oct 19, 2023 •

edited

github-actions bot commented Oct 20, 2023 •

edited

datadog-wireapp bot commented Oct 20, 2023 •

edited

codecov-commenter commented Oct 20, 2023 •

edited

mchenani Oct 20, 2023

mchenani Oct 20, 2023

borichellow Oct 23, 2023

mchenani Oct 23, 2023

mchenani Oct 20, 2023

mchenani Oct 20, 2023 •

edited

mchenani Oct 23, 2023

mchenani left a comment

mchenani Oct 25, 2023

mchenani Oct 25, 2023

mchenani left a comment

	val currentVerificationStatus: VerificationStatus,
	val verificationStatus: VerificationStatus,

	suspend fun getConversationsProteusVerificationDataByClientId(
	suspend fun getConversationsProteusVerificationData(

	suspend fun updateProteusVerificationStatuses(
	suspend fun updateProteusVerificationStatus(

feat: Update conversation Proteus verification status (WPB-1789) #2157

feat: Update conversation Proteus verification status (WPB-1789) #2157

Conversation

borichellow commented Oct 19, 2023

What's new in this PR?

Issues

Solutions

CLAassistant commented Oct 19, 2023 • edited

github-actions bot commented Oct 20, 2023 • edited

Unit Test Results

datadog-wireapp bot commented Oct 20, 2023 • edited

Datadog Report

codecov-commenter commented Oct 20, 2023 • edited

Codecov Report

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

mchenani Oct 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mchenani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mchenani left a comment

Choose a reason for hiding this comment

CLAassistant commented Oct 19, 2023 •

edited

github-actions bot commented Oct 20, 2023 •

edited

datadog-wireapp bot commented Oct 20, 2023 •

edited

codecov-commenter commented Oct 20, 2023 •

edited

mchenani Oct 20, 2023 •

edited