fix: sync notification permission changes on app resume

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## [0.3.1]
+
+### Fixed
+- **Notification permission changes now sync to PushFire when the app is resumed, not just on cold start.** When the OS notification permission changed while the app stayed alive (common on some OEMs, e.g. Samsung, which do not kill the app on a settings toggle), the resume-time reconciliation saved the new permission status *before* `registerDevice` compared against it, so `registerDevice` saw "no change" and skipped the server update — leaving the device stuck at its previous `pushNotificationEnabled` value in PushFire (a re-grant kept showing as denied). The saved status is no longer written up front; `registerDevice` now detects the change and PATCHes, and persists the status only after a successful sync (so a failed sync is retried on the next resume instead of being lost). Developer opt-out via `setNotificationEnabled(false)` still survives an OS re-grant.
+
 ## [0.3.0]
 
 ### Added

lib/src/services/device_service.dart

@@ -648,8 +648,12 @@ class DeviceService {
         return null;
       }
 
-      // OS permission changed — save the new OS status
-      await _savePermissionStatus(currentOsPermission);
+      // OS permission changed. Do NOT persist the new status here:
+      // registerDevice() compares the incoming value against the saved
+      // last-known status to decide whether to PATCH, and saves it itself only
+      // after a successful sync. Writing it up front made registerDevice() see
+      // "no change" and silently skip the server update (and also suppressed
+      // retries, since a failed sync would still have advanced the status).
 
       if (!currentOsPermission) {
         // OS permission was revoked — always PATCH server false
@@ -666,6 +670,9 @@ class DeviceService {
           final device = await registerDevice();
           return device;
         } else {
+          // Developer opted out — don't touch the server, but acknowledge the
+          // OS change so it isn't re-detected on every resume.
+          await _savePermissionStatus(currentOsPermission);
           PushFireLogger.info(
               'OS notification permission re-granted but preference is disabled - not restoring');
           return null;

pubspec.yaml

@@ -3,7 +3,7 @@ description: A lightweight push notification tracking SDK for Firebase.
 homepage: https://github.com/FlywheelStudio/pushfire_sdk/blob/main/README.md
 repository: https://github.com/FlywheelStudio/pushfire_sdk
 
-version: 0.3.0
+version: 0.3.1
 
 environment:
   sdk: '>=3.3.1 <4.0.0'

test/services/device_service_permission_resume_scenarios_test.dart

@@ -0,0 +1,165 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:shared_preferences/shared_preferences.dart';
+import 'package:pushfire_sdk/src/api/pushfire_api_client.dart';
+import 'package:pushfire_sdk/src/config/pushfire_config.dart';
+import 'package:pushfire_sdk/src/services/device_service.dart';
+
+/// End-to-end-ish coverage of the exact scenarios reported on a Samsung device
+/// (SM-A546E) that does NOT kill the app when the notification permission is
+/// toggled in system settings, so the SDK reconciles via the resume path
+/// (checkAndHandlePermissionStatusChange), not a cold start.
+///
+/// Each test asserts what the server (PushFire) should end up being told.
+class RecordingApiClient extends PushFireApiClient {
+  final List<Map<String, dynamic>> postCalls = [];
+  final List<Map<String, dynamic>> patchCalls = [];
+
+  RecordingApiClient()
+      : super(const PushFireConfig(apiKey: 'test', baseUrl: 'http://test/'));
+
+  @override
+  Future<Map<String, dynamic>> post(
+      String endpoint, Map<String, dynamic> data) async {
+    postCalls.add({'endpoint': endpoint, 'data': data});
+    return {'id': 'device-1'};
+  }
+
+  @override
+  Future<Map<String, dynamic>> patch(
+      String endpoint, Map<String, dynamic> data) async {
+    patchCalls.add({'endpoint': endpoint, 'data': data});
+    return {'success': true};
+  }
+}
+
+class PlatformState {
+  bool osPermission;
+  PlatformState(this.osPermission);
+}
+
+const _deviceInfo = {
+  'os': 'android',
+  'osVersion': '16',
+  'language': 'en',
+  'manufacturer': 'samsung',
+  'model': 'SM-A546E',
+  'appVersion': '1.0.0',
+};
+
+DeviceService buildService(RecordingApiClient api, PlatformState state) {
+  return DeviceService(
+    api,
+    const PushFireConfig(apiKey: 'test', baseUrl: 'http://test/'),
+    isPushNotificationEnabledOverride: () async => state.osPermission,
+    getDeviceInfoOverride: () async => _deviceInfo,
+    getFcmTokenOverride: () async => 'fcm-token-1',
+  );
+}
+
+/// The value of pushNotificationEnabled in the most recent PATCH, or null if
+/// no PATCH was sent.
+bool? lastPatchedEnabled(RecordingApiClient api) {
+  if (api.patchCalls.isEmpty) return null;
+  return api.patchCalls.last['data']['data']['pushNotificationEnabled'] as bool?;
+}
+
+void main() {
+  TestWidgetsFlutterBinding.ensureInitialized();
+
+  setUp(() => SharedPreferences.setMockInitialValues({}));
+
+  test('Case 1 — granted, then REVOKED in settings, app resumed -> server denied',
+      () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true);
+    final service = buildService(api, state);
+
+    await service.registerDevice(); // first launch: enabled
+    api.patchCalls.clear();
+
+    // User turns notifications OFF in settings; app is resumed (not killed).
+    state.osPermission = false;
+    await service.checkAndHandlePermissionStatusChange();
+
+    expect(api.patchCalls, hasLength(1));
+    expect(lastPatchedEnabled(api), false);
+  });
+
+  test('Case 2 — denied, then RE-GRANTED in settings, app resumed -> restored',
+      () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true);
+    final service = buildService(api, state);
+
+    await service.registerDevice(); // enabled
+    state.osPermission = false; // revoke
+    await service.checkAndHandlePermissionStatusChange();
+    api.patchCalls.clear();
+
+    // User turns notifications back ON; app resumed.
+    state.osPermission = true;
+    await service.checkAndHandlePermissionStatusChange();
+
+    // This is the case that used to stay "denied" in PushFire.
+    expect(api.patchCalls, hasLength(1));
+    expect(lastPatchedEnabled(api), true);
+  });
+
+  test(
+      'Case 3 — developer disabled via setNotificationEnabled(false): OS '
+      're-grant does NOT restore', () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true);
+    final service = buildService(api, state);
+
+    await service.registerDevice(); // enabled
+    await service.setNotificationEnabled(false); // developer opt-out
+    api.patchCalls.clear();
+
+    // OS toggled off then back on while the app is alive.
+    state.osPermission = false;
+    await service.checkAndHandlePermissionStatusChange();
+    state.osPermission = true;
+    await service.checkAndHandlePermissionStatusChange();
+
+    // Server must never be flipped back to enabled against the opt-out.
+    final anyEnabledPatch = api.patchCalls.any((c) =>
+        c['data']['data']['pushNotificationEnabled'] == true);
+    expect(anyEnabledPatch, isFalse,
+        reason: 'developer opt-out must survive an OS re-grant');
+  });
+
+  test('Case 4 — app resumed with NO permission change -> no server call',
+      () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true);
+    final service = buildService(api, state);
+
+    await service.registerDevice(); // enabled
+    api.patchCalls.clear();
+    api.postCalls.clear();
+
+    // Resume, permission unchanged.
+    await service.checkAndHandlePermissionStatusChange();
+
+    expect(api.patchCalls, isEmpty);
+    expect(api.postCalls, isEmpty);
+  });
+
+  test('Case 5 — resuming again after a synced revoke -> no duplicate PATCH',
+      () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true);
+    final service = buildService(api, state);
+
+    await service.registerDevice(); // enabled
+    state.osPermission = false;
+    await service.checkAndHandlePermissionStatusChange(); // syncs -> denied
+    api.patchCalls.clear();
+
+    // App resumed a second time, still denied, nothing changed.
+    await service.checkAndHandlePermissionStatusChange();
+
+    expect(api.patchCalls, isEmpty);
+  });
+}

test/services/device_service_resume_permission_repro_test.dart

@@ -0,0 +1,125 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:shared_preferences/shared_preferences.dart';
+import 'package:pushfire_sdk/src/api/pushfire_api_client.dart';
+import 'package:pushfire_sdk/src/config/pushfire_config.dart';
+import 'package:pushfire_sdk/src/services/device_service.dart';
+
+/// Reproduction for the permission-change sync bug.
+///
+/// Symptom (reported on a Samsung device that does NOT kill the app when the
+/// user toggles the notification permission in system settings): after the
+/// permission changes and the app is brought back to the foreground, PushFire
+/// still shows the old value. On a Pixel/emulator the OS kills the app on the
+/// permission change, so the working cold-start path runs instead and it looks
+/// fine — hence the intermittency.
+///
+/// These two tests isolate the difference: the SAME permission change is pushed
+/// through the resume path vs the cold-start path.
+class RecordingApiClient extends PushFireApiClient {
+  final List<Map<String, dynamic>> postCalls = [];
+  final List<Map<String, dynamic>> patchCalls = [];
+
+  RecordingApiClient()
+      : super(const PushFireConfig(apiKey: 'test', baseUrl: 'http://test/'));
+
+  @override
+  Future<Map<String, dynamic>> post(
+      String endpoint, Map<String, dynamic> data) async {
+    postCalls.add({'endpoint': endpoint, 'data': data});
+    return {'id': 'device-1'};
+  }
+
+  @override
+  Future<Map<String, dynamic>> patch(
+      String endpoint, Map<String, dynamic> data) async {
+    patchCalls.add({'endpoint': endpoint, 'data': data});
+    return {'success': true};
+  }
+}
+
+class PlatformState {
+  bool osPermission;
+  PlatformState(this.osPermission);
+}
+
+const _deviceInfo = {
+  'os': 'android',
+  'osVersion': '16',
+  'language': 'en',
+  'manufacturer': 'samsung',
+  'model': 'SM-A546E',
+  'appVersion': '1.0.0',
+};
+
+DeviceService buildService(RecordingApiClient api, PlatformState state) {
+  return DeviceService(
+    api,
+    const PushFireConfig(apiKey: 'test', baseUrl: 'http://test/'),
+    isPushNotificationEnabledOverride: () async => state.osPermission,
+    getDeviceInfoOverride: () async => _deviceInfo,
+    getFcmTokenOverride: () async => 'fcm-token-1',
+  );
+}
+
+void main() {
+  TestWidgetsFlutterBinding.ensureInitialized();
+
+  setUp(() => SharedPreferences.setMockInitialValues({}));
+
+  test('CONTROL (cold-start path): OS revoke while app dead -> PATCH false',
+      () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true); // permission granted at first launch
+    final service = buildService(api, state);
+
+    // First launch: registers with enabled = true.
+    await service.registerDevice();
+    api.postCalls.clear();
+    api.patchCalls.clear();
+
+    // Permission revoked while the app was terminated; app relaunches and
+    // auto-register runs registerDevice() directly (the init/cold-start path).
+    state.osPermission = false;
+    await service.registerDevice();
+
+    // Cold-start path correctly syncs the server to disabled.
+    expect(api.patchCalls, hasLength(1));
+    expect(api.patchCalls.first['endpoint'], 'update-device');
+    expect(
+      api.patchCalls.first['data']['data']['pushNotificationEnabled'],
+      false,
+    );
+  });
+
+  test(
+      'BUG (resume path): OS revoke while app alive -> server should be PATCHed '
+      'false but is NOT', () async {
+    final api = RecordingApiClient();
+    final state = PlatformState(true); // permission granted at first launch
+    final service = buildService(api, state);
+
+    // First launch: registers with enabled = true.
+    await service.registerDevice();
+    api.postCalls.clear();
+    api.patchCalls.clear();
+
+    // User toggles the permission OFF in system settings. On a device that does
+    // not kill the app, the app is merely resumed, so the SDK reconciles via
+    // checkAndHandlePermissionStatusChange() (the same entry point used by the
+    // lifecycle observer and syncNotificationPermission()).
+    state.osPermission = false;
+    await service.checkAndHandlePermissionStatusChange();
+
+    // EXPECTED: the server is told the device is now disabled.
+    // ACTUAL (bug): no PATCH is sent, so PushFire keeps showing enabled.
+    expect(
+      api.patchCalls,
+      hasLength(1),
+      reason: 'resume path should PATCH update-device with the new value',
+    );
+    expect(
+      api.patchCalls.first['data']['data']['pushNotificationEnabled'],
+      false,
+    );
+  });
+}