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.

Sign up for GitHub

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

Jump to bottom

fix: Delete conversation Crash #WPB-14938 #3848

Merged
merged 4 commits into from
Jan 30, 2025
Merged

fix: Delete conversation Crash #WPB-14938 #3848

merged 4 commits into from
Jan 30, 2025
+68 −4

Conversation

borichellow
Copy link
Contributor

@borichellow borichellow commented Jan 29, 2025 •
edited by github-actions bot
Loading

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

What's new in this PR?

Issues

Crash when user trying to "leave and delete the group" or just "delete group locally"

Causes (Optional)

App runs DeleteConversationLocallyWorker to remove all the conversation data, but that worker doesn't implement fun getForegroundInfo()

Solutions

implement fun getForegroundInfo() in DeleteConversationLocallyWorker
also implemented that fun in PersistentWebsocketCheckWorker just in case

@borichellow borichellow self-assigned this Jan 29, 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 29, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 40 lines in your changes missing coverage. Please review.

Project coverage is 45.19%. Comparing base (c1ff13b) to head (ca6da3e).

Files with missing lines Patch % Lines
...rkmanager/worker/PersistentWebsocketCheckWorker.kt 0.00% 17 Missing ⚠️
...kmanager/worker/DeleteConversationLocallyWorker.kt 0.00% 15 Missing ⚠️
.../com/wire/android/workmanager/WireWorkerFactory.kt 0.00% 6 Missing ⚠️
...wire/android/notification/NotificationConstants.kt 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3848      +/-   ##
===========================================
- Coverage    45.28%   45.19%   -0.10%     
===========================================
  Files          491      491              
  Lines        16952    16988      +36     
  Branches      2844     2844              
===========================================
  Hits          7677     7677              
- Misses        8489     8525      +36     
  Partials       786      786
Files with missing lines Coverage Δ
...wire/android/notification/NotificationConstants.kt 9.52% <0.00%> (-1.01%) ⬇️
.../com/wire/android/workmanager/WireWorkerFactory.kt 0.00% <0.00%> (ø)
...kmanager/worker/DeleteConversationLocallyWorker.kt 0.00% <0.00%> (ø)
...rkmanager/worker/PersistentWebsocketCheckWorker.kt 27.27% <0.00%> (-25.67%) ⬇️

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 c1ff13b...ca6da3e. Read the comment docs.

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-staging-compat-pr-3848.apk is available for download

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-dev-debug-pr-3848.apk is available for download

saleniuk
saleniuk approved these changes Jan 29, 2025
View reviewed changes
app/src/main/kotlin/com/wire/android/workmanager/worker/DeleteConversationLocallyWorker.kt Outdated
.setAutoCancel(true)
.setSilent(true)
.setCategory(NotificationCompat.CATEGORY_SERVICE)
.setProgress(0, 0, true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: I think we don't need to show any progress on this notification

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

god damn copy-past 😅

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-staging-compat-pr-3848.apk is available for download

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-dev-debug-pr-3848.apk is available for download

@MohamadJaara MohamadJaara added this pull request to the merge queue Jan 30, 2025
Merged via the queue into develop with commit 5431bcb Jan 30, 2025
12 of 13 checks passed
@MohamadJaara MohamadJaara deleted the fix/delete_conversation_crash branch January 30, 2025 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saleniuk saleniuk saleniuk approved these changes

@Garzas Garzas Garzas approved these changes

@sbakhtiarov sbakhtiarov Awaiting requested review from sbakhtiarov

@yamilmedina yamilmedina Awaiting requested review from yamilmedina

@MohamadJaara MohamadJaara Awaiting requested review from MohamadJaara

Assignees

@borichellow borichellow

Labels
echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap. size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@borichellow @codecov-commenter @Garzas @saleniuk @MohamadJaara