diff --git a/lib/pushfire_sdk.dart b/lib/pushfire_sdk.dart index 29974ab..1545166 100644 --- a/lib/pushfire_sdk.dart +++ b/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> 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> createScheduledWorkflowForSegments({ required String workflowId, diff --git a/lib/src/models/workflow_execution.dart b/lib/src/models/workflow_execution.dart index 836b37d..3164eb8 100644 --- a/lib/src/models/workflow_execution.dart +++ b/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 { diff --git a/lib/src/services/device_service.dart b/lib/src/services/device_service.dart index 87e2788..c8718c0 100644 --- a/lib/src/services/device_service.dart +++ b/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 { diff --git a/lib/src/services/workflow_service.dart b/lib/src/services/workflow_service.dart index 456f125..5b86169 100644 --- a/lib/src/services/workflow_service.dart +++ b/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> createScheduledWorkflowForSubscribers({ required String workflowId, required List 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> createScheduledWorkflowForSegments({ required String workflowId, required List segmentIds, diff --git a/test/models/workflow_execution_test.dart b/test/models/workflow_execution_test.dart index a4a6b95..72c2fa0 100644 --- a/test/models/workflow_execution_test.dart +++ b/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; + + expect(data['scheduledFor'], isA()); + expect(data['scheduledFor'], endsWith('Z')); + expect( + data['scheduledFor'], + localScheduledDate.toUtc().toIso8601String(), + ); + }); + test('serializes target with correct structure', () { final request = WorkflowExecutionRequest( workflowId: validUuid1, diff --git a/test/services/device_service_notification_preference_test.dart b/test/services/device_service_notification_preference_test.dart index 051f932..d264201 100644 --- a/test/services/device_service_notification_preference_test.dart +++ b/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()), + ); - // 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({ diff --git a/test/services/device_service_permission_resume_scenarios_test.dart b/test/services/device_service_permission_resume_scenarios_test.dart index ebf15be..06e181f 100644 --- a/test/services/device_service_permission_resume_scenarios_test.dart +++ b/test/services/device_service_permission_resume_scenarios_test.dart @@ -60,7 +60,8 @@ 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() { @@ -68,7 +69,8 @@ void main() { 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'); });