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

fix: self deleting ping has no sound (WPB-3175) #2269

Merged
merged 6 commits into from
Sep 26, 2023

Conversation

alexandreferris
Copy link
Contributor

@alexandreferris alexandreferris commented Sep 25, 2023

BugWPB-3175 No sound for Self-deleting pings


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?

Issues

Self-deleting pings (knock message) has no sound (both from notification or inside conversation)

Causes (Optional)

As it was being "hidden" behind self deleting message it wouldn't map correctly to Knock/Ping type

Solutions

Correctly map knock/ping from self deleting message to self deleting ping in order to play the sound on notification and inside conversation.

Testing

How to Test

Phone must NOT be on silent mode

  • On Conversation List:

  • Receive self deleting ping => should play a sound

  • Inside Conversation

  • Receive self deleting ping => should play a sound

Needs before:

[ ] - wireapp/kalium#2086

@github-actions
Copy link
Contributor

github-actions bot commented Sep 25, 2023

Test Results

613 tests  ±0   612 ✔️ ±0   9m 16s ⏱️ + 2m 59s
  88 suites ±0       1 💤 ±0 
  88 files   ±0       0 ±0 

Results for commit 2b20c2e. ± Comparison against base commit 3e508a1.

♻️ This comment has been updated with latest results.

@AndroidBob
Copy link
Collaborator

Build 954 failed.

@alexandreferris alexandreferris removed WIP Work In Progress DO NOT MERGE labels Sep 25, 2023
@alexandreferris alexandreferris requested review from a team, typfel, gongracr, MohamadJaara, mchenani, Garzas and yamilmedina and removed request for a team September 25, 2023 15:34
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #2269 (2b20c2e) into develop (3e508a1) will decrease coverage by 0.03%.
The diff coverage is 16.66%.

@@              Coverage Diff              @@
##             develop    #2269      +/-   ##
=============================================
- Coverage      40.39%   40.36%   -0.03%     
- Complexity      1011     1012       +1     
=============================================
  Files            318      318              
  Lines          11625    11635      +10     
  Branches        1542     1546       +4     
=============================================
+ Hits            4696     4697       +1     
- Misses          6489     6497       +8     
- Partials         440      441       +1     
Files Changed Coverage Δ
...android/notification/MessageNotificationManager.kt 0.00% <0.00%> (ø)
...ain/kotlin/com/wire/android/notification/Models.kt 0.00% <0.00%> (ø)
...re/android/notification/WireNotificationManager.kt 79.02% <66.66%> (-0.29%) ⬇️

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 3e508a1...2b20c2e. Read the comment docs.

@github-actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 966 succeeded.

The build produced the following APK's:

@alexandreferris alexandreferris added this pull request to the merge queue Sep 26, 2023
Merged via the queue into develop with commit 6c62bf9 Sep 26, 2023
12 checks passed
@alexandreferris alexandreferris deleted the fix/self_deleting_ping_sound branch September 26, 2023 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants