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 clearing badge count on reinstall #939

Merged
merged 2 commits into from
Jun 4, 2021

Conversation

emawby
Copy link
Contributor

@emawby emawby commented Jun 3, 2021

Badges were not being cleared on app reinstall until the user was prompted for push permissions, because we were checking for badge permissions before clearing badges. We don't need permission to set the badge count to 0 and in fact if they don't have permission then we definitely should be setting it to 0.

This change affected a unit test that was trying to reproduce an iOS 10.2.1 issue where permissions would be set to denied when backgrounding on the push prompt. I am not sure if this change fixed the iOS 10.2.1 issue, but I modified the test so that it passes.


This change is Reviewable

When reinstalling an app that previously had badges they are not cleared until the user has been prompted and accepts push permissions with badges. If the app doesn't have badge permissions we should be clearing the badges.
Since we no longer ask for the notif permission status when trying to clear badges the observer only fires twice, and the permission status is never set to denied.
@emawby emawby requested review from jkasten2 and Jeasmine June 3, 2021 18:10
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Badges were not being cleared on app reinstall until the user was prompted for push permissions, because we were checking for badge permissions before clearing badges.

I was pretty sure badges were cleared when you uninstall the app then reinstall it on iOS. Also I think if you tried to set applicationIconBadgeNumber without permission it shows a warning in the Xcode log, maybe only if it is a non-zero value though.

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @Jeasmine)

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Regardless if this fixes the actual issue in question I approve this PR as this is good clean up if the extra check is jsut unnecessary.

Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Jeasmine)

@emawby emawby merged commit a6c6ff5 into main Jun 4, 2021
@emawby emawby deleted the fix/clear_badge_count_on_reinstall branch June 4, 2021 17:32
@emawby emawby mentioned this pull request Jun 4, 2021
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.

2 participants