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

Bugfix/1724followup/improve notification sound handling #2566

Merged

Conversation

mahibi
Copy link
Collaborator

@mahibi mahibi commented Nov 14, 2022

followup of #2415

  • fix a bug that ringtone was played twice when device screen is off and call comes in
  • simplify ringtone handling

blocked by #2449
--> will resolve "Call requires API level 23 (current min is 21): android.app.NotificationManager#getActiveNotifications" error

@mahibi mahibi self-assigned this Nov 14, 2022
@mahibi mahibi added the 3. to review Waiting for reviews label Nov 14, 2022
@mahibi
Copy link
Collaborator Author

mahibi commented Dec 1, 2022

/rebase

@mahibi mahibi added this to the 15.1.0 milestone Dec 1, 2022
@nextcloud-command nextcloud-command force-pushed the bugfix/1724followup/improveNotificationSoundHandling branch from 4e7c0b3 to 4c4b440 Compare December 1, 2022 14:35
this commit removes the logic to play the ringtone in CallNotificationActivity. Playing ringtone should only be controlled by the notification channel from OS!

furthermore the checks if a call is stopped or is still ongoing etc was removed from CallNotificationActivity. Instead the CallNotificationActivity now is completely dependent on the notification. If the notification is canceled, the Activity stops. If the Notification is ongoing and hangup of accept call is clicked, then the notification is canceled (including the ringtone).

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
for example when call is hangup on mobile and immediately after on web, the loop in "checkIfCallIsActive" is still active and might trigger to send the "missed call" notification. Because of this, there is now another check if the "ongoing call" notification is still visible. It makes only sense to show the missed call notification, when the ongoing call notification is still visible.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@timkrueger timkrueger force-pushed the bugfix/1724followup/improveNotificationSoundHandling branch from 4c4b440 to 269844f Compare December 7, 2022 13:41
@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

Codacy

Lint

TypemasterPR
Warnings112112
Errors00

SpotBugs

CategoryBaseNew
Bad practice44
Correctness2828
Dodgy code233233
Experimental22
Internationalization66
Malicious code vulnerability4747
Performance2121
Security22
Total343343

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/2566-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

@timkrueger timkrueger merged commit 25c7d76 into master Dec 7, 2022
@delete-merged-branch delete-merged-branch bot deleted the bugfix/1724followup/improveNotificationSoundHandling branch December 7, 2022 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants