Fix redundant device PATCH, preference/server divergence, and non-UTC scheduled workflows

lib/pushfire_sdk.dart

@@ -236,6 +236,8 @@ class PushFireSDK {
 
   /// Create a scheduled workflow execution for subscribers
   ///
+  /// [scheduledFor] is an absolute instant; a local DateTime is converted to UTC before being sent.
+  ///
   /// Returns empty map on web.
   Future<Map<String, dynamic>> createScheduledWorkflowForSubscribers({
     required String workflowId,
@@ -252,6 +254,8 @@ class PushFireSDK {
 
   /// Create a scheduled workflow execution for segments
   ///
+  /// [scheduledFor] is an absolute instant; a local DateTime is converted to UTC before being sent.
+  ///
   /// Returns empty map on web.
   Future<Map<String, dynamic>> createScheduledWorkflowForSegments({
     required String workflowId,

lib/src/models/workflow_execution.dart

@@ -139,7 +139,11 @@ class WorkflowExecutionRequest {
     };
 
     if (scheduledFor != null) {
-      data['scheduledFor'] = scheduledFor!.toIso8601String();
+      // Normalize to UTC so the wire value always carries a `Z` designator.
+      // toIso8601String() omits the designator for local DateTimes (isUtc
+      // == false), which the backend then reads as UTC. toUtc() is a no-op
+      // on an already-UTC value, so this is correct for both inputs.
+      data['scheduledFor'] = scheduledFor!.toUtc().toIso8601String();
     }
 
     return {

lib/src/services/device_service.dart

@@ -86,8 +86,11 @@ class DeviceService {
       if (existingDeviceId != null && lastFcmToken == fcmToken) {
         // Device already registered with same FCM token
         // Check if permission status changed - if so, update device
+        // Compare like for like: lastPermissionStatus is the raw OS
+        // permission (see _savePermissionStatus below), so it must be
+        // compared against the raw osPermission, not the effective value.
         if (lastPermissionStatus != null &&
-            lastPermissionStatus != device.pushNotificationEnabled) {
+            lastPermissionStatus != osPermission) {
           PushFireLogger.info(
               'Device permission status changed - updating device with ID: $existingDeviceId');
           registeredDevice =
@@ -527,9 +530,6 @@ class DeviceService {
             'Cannot set notification preference - no device registered');
       }
 
-      // Save preference locally first
-      await _saveNotificationPreference(enabled);
-
       // PATCH server
       final prefs = await SharedPreferences.getInstance();
       final fcmToken = prefs.getString(_fcmTokenKey);
@@ -553,6 +553,12 @@ class DeviceService {
 
       await _updateDevice(device);
 
+      // Persist locally only after the server confirms. If the PATCH
+      // throws, local state must not diverge from the server, otherwise a
+      // retry would short-circuit above and report success without ever
+      // contacting the server again.
+      await _saveNotificationPreference(enabled);
+
       PushFireLogger.info('Notification preference updated to $enabled');
       return SetNotificationResult.success;
     } on PushFireException {

lib/src/services/workflow_service.dart

@@ -78,6 +78,8 @@ class WorkflowService {
   }
 
   /// Create a scheduled workflow execution for subscribers
+  ///
+  /// [scheduledFor] is an absolute instant; a local DateTime is converted to UTC before being sent.
   Future<Map<String, dynamic>> createScheduledWorkflowForSubscribers({
     required String workflowId,
     required List<String> subscriberIds,
@@ -97,6 +99,8 @@ class WorkflowService {
   }
 
   /// Create a scheduled workflow execution for segments
+  ///
+  /// [scheduledFor] is an absolute instant; a local DateTime is converted to UTC before being sent.
   Future<Map<String, dynamic>> createScheduledWorkflowForSegments({
     required String workflowId,
     required List<String> segmentIds,

test/models/workflow_execution_test.dart

@@ -572,6 +572,37 @@ void main() {
         expect(data['scheduledFor'], '2026-06-15T10:30:00.000Z');
       });
 
+      test('serializes local DateTime scheduledFor as UTC with a Z designator',
+          () {
+        // Regression test for issue #7: a local (non-UTC) DateTime must be
+        // converted to UTC before serialization, otherwise the emitted
+        // string has no timezone designator and the backend misreads it as
+        // UTC, firing the workflow off by the caller's offset.
+        //
+        // Constructing via DateTime(...) (not DateTime.utc(...)) always
+        // produces a local DateTime (isUtc == false) regardless of the
+        // machine's timezone, so this assertion is timezone-independent:
+        // on the buggy implementation (toIso8601String() without toUtc())
+        // the string never ends in 'Z' for a local DateTime, at any offset.
+        final localScheduledDate = DateTime(2026, 6, 15, 10, 30);
+        final request = WorkflowExecutionRequest(
+          workflowId: validUuid1,
+          type: WorkflowExecutionType.scheduled,
+          scheduledFor: localScheduledDate,
+          target: makeTarget(),
+        );
+
+        final json = request.toJson();
+        final data = json['data'] as Map<String, dynamic>;
+
+        expect(data['scheduledFor'], isA<String>());
+        expect(data['scheduledFor'], endsWith('Z'));
+        expect(
+          data['scheduledFor'],
+          localScheduledDate.toUtc().toIso8601String(),
+        );
+      });
+
       test('serializes target with correct structure', () {
         final request = WorkflowExecutionRequest(
           workflowId: validUuid1,

test/services/device_service_notification_preference_test.dart

@@ -182,24 +182,31 @@ void main() {
       );
     });
 
-    test('saves preference locally even if server call fails', () async {
+    test(
+        'does not save preference locally when the server call fails '
+        '(regression for #6)', () async {
+      // Regression test for issue #6: persisting the preference before the
+      // PATCH meant a failed request left local state saying "changed"
+      // while the server still had the old value — and because
+      // setNotificationEnabled short-circuits when the local preference
+      // already matches the requested value, a retry would report success
+      // without ever contacting the server again. The preference must only
+      // be persisted after the PATCH succeeds.
       final apiClient = FakeApiClient();
       final service = createTestService(apiClient: apiClient);
 
-      await service.registerDevice();
+      await service.registerDevice(); // saves default preference true
       apiClient.shouldThrowOnPatch = true;
 
-      // Should throw but preference should be saved locally
-      try {
-        await service.setNotificationEnabled(false);
-        fail('Expected exception');
-      } on PushFireException {
-        // Expected
-      }
+      // Should throw and leave the local preference unchanged
+      await expectLater(
+        () => service.setNotificationEnabled(false),
+        throwsA(isA<PushFireException>()),
+      );
 
-      // Verify preference was saved locally
+      // Verify preference was NOT saved locally
       final prefs = await SharedPreferences.getInstance();
-      expect(prefs.getBool('pushfire_notification_preference'), false);
+      expect(prefs.getBool('pushfire_notification_preference'), true);
     });
   });
 
@@ -456,6 +463,32 @@ void main() {
       expect(prefs.getBool('pushfire_notification_preference'), true);
     });
 
+    test(
+        'does not PATCH again when OS permission is unchanged and preference '
+        'is off (regression for #5)', () async {
+      // Regression test for issue #5: registerDevice() must compare the
+      // raw OS permission against the raw last-saved OS permission
+      // (lastPermissionStatus), not against the effective value
+      // (osPermission && preference). Comparing raw against effective meant
+      // a device with OS permission granted but the developer preference
+      // set to false would PATCH on every single launch, forever.
+      final apiClient = FakeApiClient();
+      final service = createTestService(apiClient: apiClient);
+
+      // First launch: registers with OS permission granted (default
+      // preference true), then the developer turns notifications off.
+      await service.registerDevice();
+      await service.setNotificationEnabled(false);
+      apiClient.patchCalls.clear();
+
+      // Second launch: same OS permission (granted), same FCM token, same
+      // stored device id, preference still off. Nothing has changed, so no
+      // PATCH should be sent.
+      await service.registerDevice();
+
+      expect(apiClient.patchCalls, isEmpty);
+    });
+
     test('does not overwrite existing preference on re-registration', () async {
       // Pre-set preference to false (developer opted out)
       SharedPreferences.setMockInitialValues({

test/services/device_service_permission_resume_scenarios_test.dart

@@ -60,15 +60,17 @@ DeviceService buildService(RecordingApiClient api, PlatformState state) {
 /// no PATCH was sent.
 bool? lastPatchedEnabled(RecordingApiClient api) {
   if (api.patchCalls.isEmpty) return null;
-  return api.patchCalls.last['data']['data']['pushNotificationEnabled'] as bool?;
+  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',
+  test(
+      'Case 1 — granted, then REVOKED in settings, app resumed -> server denied',
       () async {
     final api = RecordingApiClient();
     final state = PlatformState(true);
@@ -123,8 +125,8 @@ void main() {
     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);
+    final anyEnabledPatch = api.patchCalls
+        .any((c) => c['data']['data']['pushNotificationEnabled'] == true);
     expect(anyEnabledPatch, isFalse,
         reason: 'developer opt-out must survive an OS re-grant');
   });