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

Element Call ringing notifications #2978

Merged
merged 24 commits into from
Jun 10, 2024

Conversation

jmartinesp
Copy link
Member

@jmartinesp jmartinesp commented Jun 4, 2024

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • Handles the new CallNotify event type: when a push notification with an event of this type has a CallNotifyType.RING (DMs) and it was created <10s ago it'll make the phone ring. In any other cases, such as the room not being a DM or the call starting >= 10s ago it'll just add a new normal notification.
  • For this it creates a new notification channel and migrates the old one so it has a higher priority.
  • When the ringing should start we start the new IncomingCallForegroundService which will keep the app alive and display the ringing notification and, on a locked device, display the new incoming call screen.
  • If the ringing call goes unanswered for 15s it'll automatically stop and a new notification will be added so the user knows the call took place.
  • This CallNotify event will be sent to the other members of the room when a user joins a call.

Motivation and context

Closes #2894 .

Screenshots / GIFs

In the PR.

Tests

  • You'll need to install this same branch in 2 devices since the code that sends the CallNotify event type is also added here.
  • Create a DM with the 2 accounts and start a call. The other device should display a ringing call if the notification was received and processed quickly or a normal one otherwise. Check answering and hanging up from the ringing notification works.
  • Try locking the screen in one of the devices and start the call from the other, the new incoming call screen should be displayed. Both hanging up and answering should work here too.

Tested devices

  • Physical
  • Emulator
  • OS version(s): 14, 13

Checklist

@jmartinesp jmartinesp added Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. labels Jun 5, 2024
@github-actions github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Jun 5, 2024
Copy link
Contributor

github-actions bot commented Jun 5, 2024

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/cYo3g5

Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 82.50000% with 77 lines in your changes missing coverage. Please review.

Project coverage is 75.79%. Comparing base (ab0a9c3) to head (157ca22).
Report is 10 commits behind head on develop.

Files Patch % Lines
...roid/features/call/impl/ui/IncomingCallActivity.kt 0.00% 21 Missing ⚠️
...roid/features/call/impl/utils/ActiveCallManager.kt 84.37% 1 Missing and 9 partials ⚠️
...atures/call/impl/services/CallForegroundService.kt 0.00% 9 Missing ⚠️
...droid/features/call/impl/ui/ElementCallActivity.kt 0.00% 8 Missing ⚠️
...ndroid/features/call/impl/ui/IncomingCallScreen.kt 90.54% 0 Missing and 7 partials ⚠️
...impl/notifications/model/NotifiableMessageEvent.kt 14.28% 6 Missing ⚠️
...mpl/notifications/factories/NotificationCreator.kt 33.33% 2 Missing and 2 partials ⚠️
...all/impl/receivers/DeclineCallBroadcastReceiver.kt 25.00% 3 Missing ⚠️
...raries/matrix/api/timeline/item/event/EventType.kt 0.00% 2 Missing ⚠️
...es/push/impl/notifications/NotificationRenderer.kt 71.42% 2 Missing ⚠️
... and 5 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2978      +/-   ##
===========================================
+ Coverage    75.54%   75.79%   +0.24%     
===========================================
  Files         1589     1605      +16     
  Lines        37583    37983     +400     
  Branches      7298     7332      +34     
===========================================
+ Hits         28392    28789     +397     
+ Misses        5373     5354      -19     
- Partials      3818     3840      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmartinesp jmartinesp marked this pull request as ready for review June 5, 2024 16:13
@jmartinesp jmartinesp requested a review from a team as a code owner June 5, 2024 16:13
@jmartinesp jmartinesp requested review from ganfra and removed request for a team June 5, 2024 16:13
) {
ElementTheme {
@Composable
private fun IncomingCallScreen(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe extract to its own file?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't thought it was worth it as it's not likely that we'll use it anywhere else, but sure I can do it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, thanks, I like to have the files containing an Activity as short as possible.

…tyle` notifications.

This way they're part of the conversation and are useful for the conversation API.
Copy link
Member

@ganfra ganfra left a comment

Choose a reason for hiding this comment

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

I've no particular remark on the code, it looks good.
Lets try to fix the edge cases now!

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jmartinesp jmartinesp requested a review from ganfra June 10, 2024 07:24
And several other flags for retrocompatibility
@jmartinesp jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Jun 10, 2024
@github-actions github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Jun 10, 2024
@jmartinesp jmartinesp merged commit 30a1367 into develop Jun 10, 2024
23 of 24 checks passed
@jmartinesp jmartinesp deleted the feature/jme/element-call-ringing-with-telecom branch June 10, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Ringing for incoming call
3 participants