fix: sync notification permission changes on app resume - #4
Merged
Conversation
The resume-time reconciliation (checkAndHandlePermissionStatusChange) saved the new OS permission status before registerDevice compared against it, so registerDevice saw no change and skipped the server update. On devices that do not kill the app on a permission toggle (e.g. Samsung), this left the device stuck at its previous pushNotificationEnabled value in PushFire. Stop pre-writing the last-known status; registerDevice now detects the change, PATCHes, and persists the status only after a successful sync (so a failed sync retries on the next resume). Developer opt-out via setNotificationEnabled(false) still survives an OS re-grant. Adds regression coverage for the resume vs cold-start paths and the reported Samsung scenarios.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On devices that do not kill the app when the notification permission is toggled in system settings (e.g. Samsung), a permission change did not update the device's status in PushFire. Most visibly, re-granting the permission kept the device showing as denied.
Root cause
The app reconciles permission changes on resume via
checkAndHandlePermissionStatusChange(). It saved the new OS permission status topushfire_last_permission_statusbefore callingregisterDevice().registerDevice()compares the device's value against that saved status to decide whether to PATCH the server — so it saw "no change" and skipped the update. The server kept its stale value. (On a Pixel/emulator the OS kills the app on the toggle, so it comes back through the cold-start path, which registers directly and works — hence the intermittency.)Fix
Stop pre-writing the last-known status in
checkAndHandlePermissionStatusChange().registerDevice()now detects the change and PATCHes, and persists the status only after a successful sync (so a failed sync retries on the next resume instead of being lost). The developer opt-out branch (setNotificationEnabled(false)) still records the status itself, so an OS re-grant does not override the opt-out.Tests
device_service_resume_permission_repro_test.dart— resume path vs cold-start path for the same permission change (the resume assertion failed before this fix).device_service_permission_resume_scenarios_test.dart— the reported Samsung scenarios: revoke-on-resume, re-grant-on-resume (restore), opt-out survives re-grant, no-change no-op, no duplicate PATCH on repeat resume.Full suite: 389 tests passing.
flutter analyzeclean (no new issues).Release
Patch bump 0.3.0 -> 0.3.1; CHANGELOG updated.