Skip to content
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

feat: Clear conversation content on all devices WPB-14938 #3235

Merged
merged 12 commits into from
Jan 23, 2025

Conversation

borichellow
Copy link
Contributor

@borichellow borichellow commented Jan 20, 2025

TaskWPB-14938 [Android] Add "Delete group and content on all devices" option when leaving a group

What's new in this PR?

  1. aligned all the "ClearContent" and "DeleteConversation" useCases to to re-use each other when needed instead of calling repository directly.
  2. updated ClearConversationContentUseCase to send a signal message with properly set needToRemoveLocally field which indicates when other clients should not just clear the content but fully remove the conversation.
  3. fixed ClearConversationContentHandler it was working wrong.
  4. added conversation to remove queue (save it in MetaDataDAO) for the case when event with MessageContent.Cleared comes before "UserLeft" event: we save ConversationId and check that queue in MemberLeaveEventHandler if self user left the conversation and that conversation is in queue to remove - delete the conversation.
  5. updated tests

@borichellow borichellow self-assigned this Jan 20, 2025
@echoes-hq echoes-hq bot added the echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap. label Jan 20, 2025
Copy link
Contributor

github-actions bot commented Jan 20, 2025

Test Results

3 408 tests  +3   3 300 ✅ +3   5m 29s ⏱️ -1s
  584 suites ±0     108 💤 ±0 
  584 files   ±0       0 ❌ ±0 

Results for commit dd3590a. ± Comparison against base commit c6d3335.

This pull request removes 7 and adds 10 tests. Note that renamed tests count towards both.
com.wire.kalium.logic.feature.conversation.DeleteConversationLocallyUseCaseTest ‑ givenDeleteLocalConversationInvoked_whenAssetClearIsUnsuccessful_thenErrorResultIsPropagated[jvm]
com.wire.kalium.logic.feature.conversation.DeleteConversationLocallyUseCaseTest ‑ givenDeleteLocalConversationInvoked_whenContentClearIsUnsuccessful_thenErrorResultIsPropagated[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherClient_whenMessageDoesNotNeedToBeRemovedAndUserIsNotPartOfConversation_thenContentNorConversationShouldBeRemoved[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherClient_whenMessageDoesNotNeedToBeRemovedAndUserIsPartOfConversation_thenContentShouldBeRemoved[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherClient_whenMessageNeedsToBeRemovedLocallyAndUserIsNotPartOfConversation_thenWholeConversationShouldBeDeleted[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherClient_whenMessageNeedsToBeRemovedLocallyAndUserIsPartOfConversation_thenOnlyContentShouldBeCleared[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromTheSameClient_whenHandleIsInvoked_thenContentNorConversationShouldBeRemoved[jvm]
com.wire.kalium.logic.feature.conversation.ClearConversationContentUseCaseTest ‑ givenClearAssetsFails_whenInvoking_thenCorrectlyPropagateFailure[jvm]
com.wire.kalium.logic.feature.conversation.DeleteConversationLocallyUseCaseTest ‑ givenDeleteLocalConversationInvoked_whenClearContentIsUnsuccessful_thenErrorResultIsPropagated[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherClient_whenMessageInSelfConversation_thenDoNothing[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherUserAndNeedToRemove_whenMessageNotInSelfConversation_thenWholeConversationShouldNotBeDeleted[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromOtherUser_whenMessageNotInSelfConversationAndNoNeedToRemove_thenOnlyClearContent[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromTheSelfUser_whenMessageInSelfConversationAndNoNeedToRemove_thenOnlyContentRemoved[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenMessageFromTheSelfUser_whenMessageNotInSelfConversation_thenContentNorConversationShouldBeRemoved[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenSelfSenderAndMessageInSelfConversation_whenNeedToRemoveAndConversationIsNotLeftYet_thenContentCleared[jvm]
com.wire.kalium.logic.sync.receiver.conversation.ClearConversationContentHandlerTest ‑ givenSelfSenderAndMessageInSelfConversation_whenNeedToRemoveAndLeftConversation_thenContentAndConversationRemoved[jvm]
com.wire.kalium.logic.sync.receiver.conversation.MemberLeaveEventHandlerTest ‑ givenDaoReturnsSuccessAndConversationInDeleteQueue_whenDeletingSelfMember_thenConversationDeleted[jvm]

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 20, 2025

@datadog-wireapp
Copy link

Datadog Report

Branch report: feat/clear_conversation_content_on_all_devices
Commit report: ce6774d
Test service: kalium-jvm

✅ 0 Failed, 3300 Passed, 108 Skipped, 1m 2.36s Total Time

@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2025

Codecov Report

Attention: Patch coverage is 64.21053% with 34 lines in your changes missing coverage. Please review.

Project coverage is 54.43%. Comparing base (c6d3335) to head (dd3590a).

Files with missing lines Patch % Lines
.../logic/data/conversation/ConversationRepository.kt 4.16% 23 Missing ⚠️
...um/logic/feature/conversation/ConversationScope.kt 0.00% 8 Missing ⚠️
...c/receiver/conversation/MemberLeaveEventHandler.kt 86.66% 0 Missing and 2 partials ⚠️
...re/conversation/ClearConversationContentUseCase.kt 96.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3235      +/-   ##
===========================================
- Coverage    54.44%   54.43%   -0.02%     
===========================================
  Files         1271     1271              
  Lines        37018    37062      +44     
  Branches      3753     3764      +11     
===========================================
+ Hits         20156    20174      +18     
- Misses       15445    15468      +23     
- Partials      1417     1420       +3     
Files with missing lines Coverage Δ
...e/conversation/DeleteConversationLocallyUseCase.kt 100.00% <100.00%> (ø)
...eceiver/handler/ClearConversationContentHandler.kt 100.00% <100.00%> (ø)
...re/conversation/ClearConversationContentUseCase.kt 96.77% <96.00%> (-3.23%) ⬇️
...c/receiver/conversation/MemberLeaveEventHandler.kt 93.93% <86.66%> (-2.62%) ⬇️
...um/logic/feature/conversation/ConversationScope.kt 0.00% <0.00%> (ø)
.../logic/data/conversation/ConversationRepository.kt 59.43% <4.16%> (-2.26%) ⬇️

... and 2 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 c6d3335...dd3590a. Read the comment docs.

@borichellow borichellow added this pull request to the merge queue Jan 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 23, 2025
@borichellow borichellow added this pull request to the merge queue Jan 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 23, 2025
@borichellow borichellow added this pull request to the merge queue Jan 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 23, 2025
@borichellow borichellow added this pull request to the merge queue Jan 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 23, 2025
@borichellow borichellow added this pull request to the merge queue Jan 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 23, 2025
@borichellow borichellow enabled auto-merge January 23, 2025 15:03
@borichellow borichellow added this pull request to the merge queue Jan 23, 2025
@datadog-wireapp
Copy link

Datadog Report

Branch report: feat/clear_conversation_content_on_all_devices
Commit report: cedd3ef
Test service: kalium-jvm

✅ 0 Failed, 3300 Passed, 108 Skipped, 59.25s Total Time

Merged via the queue into develop with commit 7456f22 Jan 23, 2025
23 checks passed
@borichellow borichellow deleted the feat/clear_conversation_content_on_all_devices branch January 23, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap. 🚨 Potential breaking changes 👕 size: XL type: feature ✨
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants