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

Add permanent warning when notifications are not set up correctly #4476

Merged
merged 11 commits into from
Nov 26, 2024

Conversation

mahibi
Copy link
Collaborator

@mahibi mahibi commented Nov 22, 2024

Show a warning whenever something regarding notifications is not set up correctly.

known bug is #4484 (won't be fixed for now if no one comes up with an idea..)

🖼️ Screenshots/ Videos

Warning when notifications are not set up correctly

grafik

  • On first login, ask directly with popups for notification permission and to ignore battery optimization.
  • If this is declined or any other setting is wrong, show a permanent warning.
    • Also: When using mobile device management MDM, it may happen that some permissions are not set up while preparing the phones and users won't see any dialogs on first use. With this change, they now see the permanent warning and are guided what changes need to be made.
  • When clicking on the warning, settings are opened and things that need to be changed will blink and have red subtitles.
  • When any permission is revoked it will show a warning again in conversation list.
  • Allow user to disable the warning in conversation list if anything is wrong.
  • If a server has no notification app installed, this will be shown for the regarding account.
notAllowAllPermissionsOnStart.mp4
  • When logging in and allowing all permissions, no warning is necessary.
allowAllPermissionsOnStart.mp4

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

@mahibi mahibi added this to the 20.1.0 milestone Nov 22, 2024
@mahibi mahibi self-assigned this Nov 22, 2024
@mahibi mahibi added the 3. to review Waiting for reviews label Nov 22, 2024
@mahibi
Copy link
Collaborator Author

mahibi commented Nov 22, 2024

/backport to stable-20.0

nickvergessen

This comment was marked as outdated.

@mahibi

This comment was marked as outdated.

@mahibi mahibi added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 25, 2024
@mahibi mahibi marked this pull request as draft November 25, 2024 09:37
@mahibi mahibi force-pushed the featue/noid/addIgnoreBatteryOptimizationHint branch from f392cac to 096b361 Compare November 25, 2024 23:05
@nickvergessen

This comment was marked as outdated.

@mahibi mahibi changed the title Add permanent hint when battery optimization is not ignored Add permanent warning when notifications are not set up correctly Nov 26, 2024
@mahibi mahibi added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 26, 2024
@mahibi mahibi marked this pull request as ready for review November 26, 2024 12:38
Also add an option to not show the hint, when user doesn't want to ignore battery optimization.

Only show these features when gplay services are available. Otherwise don't show them as it would pretend notifications would be possible without gplay.

Signed-off-by: Marcel Hibbe <[email protected]>
…on is enabled

Open notification settings when click on warning

Also, still offer to enable notifications and disable battery optimization on first install

Signed-off-by: Marcel Hibbe <[email protected]>
…log for asking direct permission was not shown by android

Signed-off-by: Marcel Hibbe <[email protected]>
… is is not permitted.

Otherwise user can't enable them anyway

Signed-off-by: Marcel Hibbe <[email protected]>
@mahibi mahibi force-pushed the featue/noid/addIgnoreBatteryOptimizationHint branch from 7a9c281 to 6dc52fe Compare November 26, 2024 14:25
@nickvergessen nickvergessen self-requested a review November 26, 2024 14:26
Copy link
Contributor

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/4476-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.

Copy link
Contributor

Codacy

Lint

TypemasterPR
Warnings94158
Errors13276

SpotBugs

CategoryBaseNew
Bad practice66
Correctness1616
Dodgy code7171
Internationalization33
Malicious code vulnerability33
Performance44
Security11
Total104104

Lint increased!

@mahibi mahibi merged commit f048dc9 into master Nov 26, 2024
13 of 16 checks passed
@mahibi mahibi deleted the featue/noid/addIgnoreBatteryOptimizationHint branch November 26, 2024 14:37
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.

5 participants