feat: Include legal hold flag when sending/receiving messages (WPB-5442) #2279

ohassine · 2023-12-01T10:07:32Z

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³
The PR Description
- is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Description

For these types of messages Text, Asset, Knock, Location, Reaction and Composite a flag of legal hold status should be included in the message.

I am adding in this PR this status to message object, so we can send/receive messages with correct legal hold status

Next PRs will be for getting correct legal hold status from DB when sending a message and updating received legal hold status in conversation table

PS: This PR should be review after #2276 as it contains some changes from it.

Needs releases with:

GitHub link to other pull request

Testing

Test Coverage (Optional)

I have added automated test to this contribution

PR Post Submission Checklist for internal contributors (Optional)

Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

…ation_table' into add_legal_hold_status_to_conversation_table

…clude_legal_hold_flag

github-actions · 2023-12-01T10:25:40Z

Test Results

1 901 tests - 784 1 871 ✔️ - 710 16s ⏱️ - 2m 38s
  350 suites - 130     30 💤 -   74
  350 files - 130       0 ❌ ±    0

Results for commit f83f78d. ± Comparison against base commit 4e75095.

This pull request removes 2685 and adds 1901 tests. Note that renamed tests count towards both.

PocIntegrationTest ‑ givenApiWhenGettingACMEDirectoriesThenReturnAsExpectedBasedOnNetworkState
PocIntegrationTest ‑ givenEmailAndPasswordWhenLoggingInThenRegisterClientAndLogout
com.wire.kalium.HttpClientConnectionSpecsTest ‑ givenTheHttpClientIsCreated_ThenEnsureOnlySupportedSpecsArePresent[jvm]
com.wire.kalium.api.base.authenticated.notification.AccessUpdateTest ‑ givenPayloadWithAccessRoleAndDeprecatedAccessRoleField_whenDecoding_thenDeprecatedFieldIsPreferred[jvm]
com.wire.kalium.api.base.authenticated.notification.AccessUpdateTest ‑ givenPayloadWithDeprecatedAccessRoleField_whenDecoding_thenSuccess[jvm]
com.wire.kalium.api.base.authenticated.notification.AccessUpdateTest ‑ givenPayload_whenDecoding_thenSuccess[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ givenNoLocationInHeader_whenCallingSendAcmeRequestApi_theResponseShouldBeConfigureCorrectly[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ givenNoNonce_whenCallingSendAcmeRequestApi_theResponseShouldBeMissingNonce[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ whenCallingGetACMEDirectoriesApi_theResponseShouldBeConfigureCorrectly[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ whenCallingGetACMENonceApi_theResponseShouldBeConfigureCorrectly[jvm]
…

com.wire.kalium.logic.cache.SelfConversationIdProviderTest ‑ givenFailure_thenErrorIsPropagated[iosX64]
com.wire.kalium.logic.cache.SelfConversationIdProviderTest ‑ givenMLSClientHasBeenRegistered_thenMLSAndProteusSelfConversationAreReturned[iosX64]
com.wire.kalium.logic.cache.SelfConversationIdProviderTest ‑ givenMLSClientHasNotBeenRegistered_thenProteusSelfConversationIsReturned[iosX64]
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenMLSClientWithE2EI_whenGettingE2EIClient_callsNewActivationEnrollment[iosX64]
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenMLSClientWithoutE2EI_whenGettingE2EIClient_callsNewRotateEnrollment[iosX64]
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenSelfUserNotFound_whenGettingE2EIClient_ReturnsError[iosX64]
com.wire.kalium.logic.configuration.ServerConfigMapperTest ‑ givenACommonApiVersion_whenMapping_thenValuesAreMappedCorrectly[iosX64]
com.wire.kalium.logic.configuration.ServerConfigMapperTest ‑ givenANetworkConfigEntity_whenMappingFromNetworkConfig_thenValuesAreMappedCorrectly[iosX64]
com.wire.kalium.logic.configuration.ServerConfigMapperTest ‑ givenAServerConfig_whenMappingToBackendConfig_thenValuesAreMappedCorrectly[iosX64]
com.wire.kalium.logic.configuration.ServerConfigMapperTest ‑ givenAServerConfig_whenMappingToNetworkConfig_thenValuesAreMappedCorrectly[iosX64]
…

This pull request removes 104 skipped tests and adds 30 skipped tests. Note that renamed tests count towards both.

PocIntegrationTest ‑ givenApiWhenGettingACMEDirectoriesThenReturnAsExpectedBasedOnNetworkState
PocIntegrationTest ‑ givenEmailAndPasswordWhenLoggingInThenRegisterClientAndLogout
com.wire.kalium.api.common.ACMEApiTest ‑ whenCallingSendChallengeRequestApi_theResponseShouldBeConfigureCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenAValidEmail_whenActivationEmailWIthCode_theRequestShouldBeConfiguredCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenAValidEmail_whenRegisteringAccountWithEMail_theRequestShouldBeConfiguredCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenAValidEmail_whenSendingActivationEmail_theRequestShouldBeConfiguredCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenActivationCodeFail_thenErrorIsPropagated[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenRegistrationFail_whenRegisteringAccountWithEMMail_thenErrorIsPropagated[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenSendActivationCodeFail_thenErrorIsPropagated[jvm]
com.wire.kalium.cryptography.CryptoUtilsTest ‑ givenDummyText_whenEncryptedAndDecryptedWithAES256_returnsOriginalText[js, browser]
…

com.wire.kalium.logic.data.reaction.ReactionRepositoryTest ‑ givenSelfUserReactionWasPersisted_whenObservingMessageReactions_thenShouldReturnReactionsPreviouslyStored[iosX64]
com.wire.kalium.logic.feature.backup.CreateBackupUseCaseTest ‑ givenSomeInvalidDBData_whenCreatingNonEncryptedBackup_thenTheRightErrorIsThrown[iosX64]
com.wire.kalium.logic.feature.backup.CreateBackupUseCaseTest ‑ givenSomeValidData_whenCreatingAnEncryptedBackup_thenTheFinalBackupFileIsCreatedCorrectly[iosX64]
com.wire.kalium.logic.feature.backup.CreateBackupUseCaseTest ‑ givenSomeValidData_whenCreatingNonEncryptedBackup_thenTheFinalBackupFileIsCreatedCorrectly[iosX64]
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenACorrectNonEncryptedBackupFileWithWrongAuthor_whenRestoring_thenTheCorrectErrorIsThrown[iosX64]
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenACorrectNonEncryptedBackupFileWithWrongMetadataFileName_whenRestoring_thenTheCorrectErrorIsThrown[iosX64]
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenACorrectNonEncryptedBackupFile_whenRestoring_thenTheBackupIsRestoredSuccessfully[iosX64]
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenACorrectlyEncryptedBackup_whenRestoringWithADBImportError_thenTheRightErrorIsThrown[iosX64]
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenACorrectlyEncryptedBackup_whenRestoringWithWrongPassword_thenTheRightErrorIsThrown[iosX64]
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenAValidEncryptedBackupFile_whenRestoring_thenTheBackupIsRestoredCorrectly[iosX64]
…

♻️ This comment has been updated with latest results.

codecov-commenter · 2023-12-01T10:37:13Z

Codecov Report

Merging #2279 (f83f78d) into develop (4e75095) will increase coverage by 0.60%.
The diff coverage is 77.21%.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop    #2279      +/-   ##
=============================================
+ Coverage      57.93%   58.53%   +0.60%     
  Complexity        21       21              
=============================================
  Files           1093     1093              
  Lines          41440    41492      +52     
  Branches        3826     3840      +14     
=============================================
+ Hits           24007    24287     +280     
+ Misses         15820    15579     -241     
- Partials        1613     1626      +13

Files	Coverage Δ
...in/com/wire/kalium/logic/data/asset/AssetMapper.kt	`39.74% <100.00%> (+0.38%)`	⬆️
...ire/kalium/logic/data/conversation/Conversation.kt	`91.12% <100.00%> (ø)`
...com/wire/kalium/logic/data/message/ProtoContent.kt	`100.00% <100.00%> (ø)`
...ver/conversation/message/NewMessageEventHandler.kt	`78.40% <ø> (ø)`
.../kalium/logic/feature/message/MLSMessageCreator.kt	`77.27% <40.00%> (-6.07%)`	⬇️
...um/logic/feature/message/MessageEnvelopeCreator.kt	`91.22% <57.14%> (-3.01%)`	⬇️
...re/kalium/logic/data/message/ProtoContentMapper.kt	`63.35% <89.09%> (+14.78%)`	⬆️
...otlin/com/wire/kalium/logic/data/web/WebMappers.kt	`37.12% <14.28%> (+0.40%)`	⬆️

... and 4 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 4e75095...f83f78d. Read the comment docs.

datadog-wireapp · 2023-12-01T10:41:08Z

Datadog Report

All test runs c64eb08 🔗

✅ 2 Total Test Services: 0 Failed, 0 with New Flaky, 2 Passed

Test Services

Service Name	Failed	Known Flaky	New Flaky	Passed	Skipped	Wall Time	Branch View
`kalium-ios`	0	0	0	1871	30	10.09s	Link
`kalium-jvm`	0	0	0	2584	104	7m 8.01s	Link

…_flag # Conflicts: # logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/Conversation.kt # logic/src/commonMain/kotlin/com/wire/kalium/logic/data/web/WebMappers.kt

persistence/src/commonMain/db_user/migrations/21.sqm

mchenani

isn't better with this much of todos merge it in an epic branch then merge that branch to develop all at once?

ohassine · 2023-12-07T09:04:35Z

isn't better with this much of todos merge it in an epic branch then merge that branch to develop all at once?

I don't like to have a big PR to review.
I addressed those TODO here #2286 #2289

ohassine added 8 commits November 29, 2023 11:43

feat: add legal hold status to conversation table

6519bd2

Merge branch 'develop' into add_legal_hold_status_to_conversation_table

7d7f5f6

feat: add legal hold status to conversation table

6c41b64

Merge remote-tracking branch 'origin/add_legal_hold_status_to_convers…

8205544

…ation_table' into add_legal_hold_status_to_conversation_table

feat: add missing params of legal hold

5ce36cd

chore: unit test

4f5a295

feat: include legal hold flag when sending/receiving messages

3c1c7e7

Merge branch 'develop' into include_legal_hold_flag

edf510f

github-actions bot assigned ohassine Dec 1, 2023

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

ohassine added 2 commits December 1, 2023 11:17

chore: detekt

a70b7e7

Merge remote-tracking branch 'origin/include_legal_hold_flag' into in…

6f9b8b0

…clude_legal_hold_flag

Merge remote-tracking branch 'origin/develop' into include_legal_hold…

83c5a66

…_flag # Conflicts: # logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/Conversation.kt # logic/src/commonMain/kotlin/com/wire/kalium/logic/data/web/WebMappers.kt

github-actions bot added 👕 size: L and removed 👕 size: XL labels Dec 1, 2023

Merge branch 'develop' into include_legal_hold_flag

b0d71c6

ohassine requested review from a team, typfel, yamilmedina, borichellow, vitorhugods and Garzas and removed request for a team December 4, 2023 14:54

Merge branch 'develop' into include_legal_hold_flag

cebcd2c

alexandreferris reviewed Dec 4, 2023

View reviewed changes

persistence/src/commonMain/db_user/migrations/21.sqm Outdated Show resolved Hide resolved

chore: revert changes

f6f3098

ohassine requested a review from alexandreferris December 4, 2023 16:15

chore: revert changes

b04cdd8

alexandreferris approved these changes Dec 4, 2023

View reviewed changes

ohassine added 2 commits December 5, 2023 12:19

Merge branch 'develop' into include_legal_hold_flag

1ec1caf

feat: legal hold status for ephemeral messages

0999a28

ohassine mentioned this pull request Dec 5, 2023

feat: Update legal hold status on message receive (WPB-5442) #2286

Merged

8 tasks

ohassine requested review from MohamadJaara and saleniuk December 6, 2023 09:10

Merge branch 'develop' into include_legal_hold_flag

d8494a5

mchenani reviewed Dec 6, 2023

View reviewed changes

Merge branch 'develop' into include_legal_hold_flag

f83f78d

saleniuk approved these changes Dec 7, 2023

View reviewed changes

ohassine added this pull request to the merge queue Dec 7, 2023

Merged via the queue into develop with commit edee554 Dec 7, 2023
17 checks passed

ohassine deleted the include_legal_hold_flag branch December 7, 2023 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Include legal hold flag when sending/receiving messages (WPB-5442) #2279

feat: Include legal hold flag when sending/receiving messages (WPB-5442) #2279

ohassine commented Dec 1, 2023 •

edited

Loading

github-actions bot commented Dec 1, 2023 •

edited

Loading

codecov-commenter commented Dec 1, 2023 •

edited

Loading

datadog-wireapp bot commented Dec 1, 2023 •

edited

Loading

mchenani left a comment

ohassine commented Dec 7, 2023

feat: Include legal hold flag when sending/receiving messages (WPB-5442) #2279

feat: Include legal hold flag when sending/receiving messages (WPB-5442) #2279

Conversation

ohassine commented Dec 1, 2023 • edited Loading

PR Submission Checklist for internal contributors

What's new in this PR?

Description

Testing

Test Coverage (Optional)

PR Post Submission Checklist for internal contributors (Optional)

PR Post Merge Checklist for internal contributors

References

github-actions bot commented Dec 1, 2023 • edited Loading

Test Results

codecov-commenter commented Dec 1, 2023 • edited Loading

Codecov Report

datadog-wireapp bot commented Dec 1, 2023 • edited Loading

Datadog Report

mchenani left a comment

Choose a reason for hiding this comment

ohassine commented Dec 7, 2023

ohassine commented Dec 1, 2023 •

edited

Loading

github-actions bot commented Dec 1, 2023 •

edited

Loading

codecov-commenter commented Dec 1, 2023 •

edited

Loading

datadog-wireapp bot commented Dec 1, 2023 •

edited

Loading