From 475edcb2cfa35a98e627b8464559f44b0974f656 Mon Sep 17 00:00:00 2001 From: rzuasti Date: Tue, 9 Jun 2026 14:06:56 -0400 Subject: [PATCH] Report delivered device count from the test-notification endpoint POST /api/notifications/test previously returned a blanket 200 even when there were no registered devices, so a test that reached nobody looked like a success. push::send now returns the number of devices the relay confirmed delivery to, and the endpoint returns it as {"delivered": N}. Settings shows "sent to N device(s)" on success and an explicit "No devices are registered..." warning when N is 0, which is the case that previously masqueraded as success. Backend, API and widget tests updated; clippy and dart analyze clean. Co-Authored-By: Claude Opus 4.8 --- backend/src/notifications.rs | 8 +++--- backend/src/notifications/push.rs | 19 ++++++++----- backend/src/web_server.rs | 2 ++ backend/src/web_server/notifications.rs | 25 ++++++++++------- frontend/lib/settings/settings.dart | 20 ++++++++++---- .../lib/utils/api/oott_api_notifications.dart | 8 +++--- frontend/test/api/notifications_api_test.dart | 6 ++--- frontend/test/widget/settings_push_test.dart | 27 ++++++++++++++++--- 8 files changed, 82 insertions(+), 33 deletions(-) diff --git a/backend/src/notifications.rs b/backend/src/notifications.rs index 65e3094..96a9d28 100644 --- a/backend/src/notifications.rs +++ b/backend/src/notifications.rs @@ -18,10 +18,10 @@ pub use delivery::run_delivery; pub use error::DeliveryError; /// Send a one-off test push to every registered device through the push relay. Used by the app's -/// "Send test notification" button to verify end-to-end push delivery. Unlike scan-driven -/// notifications it is not persisted to the notifications list, and it is a no-op (still `Ok`) when -/// no devices are registered. -pub async fn send_test_push() -> Result<(), DeliveryError> { +/// "Send test notification" button to verify end-to-end push delivery. Returns the number of devices +/// the relay confirmed delivery to (0 when none are registered). Unlike scan-driven notifications it +/// is not persisted to the notifications list. +pub async fn send_test_push() -> Result { let config = crate::settings::get_settings() .notifications .push diff --git a/backend/src/notifications/push.rs b/backend/src/notifications/push.rs index d3b292b..6c5871c 100644 --- a/backend/src/notifications/push.rs +++ b/backend/src/notifications/push.rs @@ -48,13 +48,14 @@ fn dead_tokens(results: &[RelayTokenResult]) -> Vec { /// Deliver a notification to all registered devices through the project-operated push relay. Loads /// the stored tokens, forwards only the already-sanitized title/body, and prunes any tokens the -/// relay reports as dead. Best-effort: a relay/network failure returns an error (logged by the -/// caller) but never loses the event, which is already persisted in the notifications table. -pub async fn send(config: &Push, title: String, body: String) -> Result<(), DeliveryError> { +/// relay reports as dead. Returns the number of devices the relay confirmed delivery to. Best-effort: +/// a relay/network failure returns an error (logged by the caller) but never loses the event, which +/// is already persisted in the notifications table. +pub async fn send(config: &Push, title: String, body: String) -> Result { let stored = db::run_blocking(db::push_tokens::list).await?; if stored.is_empty() { debug!("No push tokens registered; nothing to deliver via the push relay"); - return Ok(()); + return Ok(0); } let request = RelayRequest { @@ -80,7 +81,12 @@ pub async fn send(config: &Push, title: String, body: String) -> Result<(), Deli db::run_blocking(move || db::push_tokens::delete_many(&dead)).await?; } - Ok(()) + let delivered = parsed + .results + .iter() + .filter(|result| result.status == "ok") + .count(); + Ok(delivered) } #[cfg(test)] @@ -143,7 +149,8 @@ mod tests { let config = Push { relay_url: format!("http://{addr}/v1/push"), }; - send(&config, "title".into(), "body".into()).await.unwrap(); + let delivered = send(&config, "title".into(), "body".into()).await.unwrap(); + assert_eq!(delivered, 1, "Only the live token should count as delivered"); let all = db::push_tokens::list().unwrap(); assert!( diff --git a/backend/src/web_server.rs b/backend/src/web_server.rs index e2a515c..6ac54e2 100644 --- a/backend/src/web_server.rs +++ b/backend/src/web_server.rs @@ -8,6 +8,7 @@ use crate::model::notifications::{Notification, NotificationListResponse, Notifi use crate::model::push_tokens::{PushPlatform, PushToken}; use crate::settings::get_settings; use crate::web_server::devices::{RegisterDevicePayload, UpdateDevicePayload}; +use crate::web_server::notifications::TestNotificationResponse; use crate::web_server::push_tokens::RegisterPushTokenPayload; use crate::web_server::scanner_status::{ ActiveScannerStatusResponse, PassiveScannerStatusResponse, @@ -83,6 +84,7 @@ pub mod utils; Notification, NotificationListResponse, NotificationType, + TestNotificationResponse, PushToken, PushPlatform, RegisterPushTokenPayload, diff --git a/backend/src/web_server/notifications.rs b/backend/src/web_server/notifications.rs index ec2169a..d6ebd6d 100644 --- a/backend/src/web_server/notifications.rs +++ b/backend/src/web_server/notifications.rs @@ -7,6 +7,8 @@ use axum::{ response::IntoResponse, }; use log::error; +use serde::Serialize; +use utoipa::ToSchema; use crate::{ db, notifications, @@ -14,6 +16,12 @@ use crate::{ web_server::utils, }; +/// Result of a test-notification request: how many devices the relay confirmed delivery to. +#[derive(Serialize, ToSchema)] +pub struct TestNotificationResponse { + pub delivered: usize, +} + #[utoipa::path( get, path = "/api/notifications/{id}", @@ -123,20 +131,17 @@ pub async fn mark_all_as_old() -> impl IntoResponse { path = "/api/notifications/test", tag = "notifications", responses( - (status = 200, description = "Test notification dispatched to the push relay"), + (status = 200, description = "Test notification dispatched", body = TestNotificationResponse), (status = 500, description = "Internal server error"), ), security(("bearer_auth" = [])) )] -pub async fn send_test() -> impl IntoResponse { +pub async fn send_test() -> Result, StatusCode> { match notifications::send_test_push().await { - Ok(()) => (StatusCode::OK, "Test notification sent"), + Ok(delivered) => Ok(Json(TestNotificationResponse { delivered })), Err(err) => { error!("Error sending test push notification: {err}"); - ( - StatusCode::INTERNAL_SERVER_ERROR, - "Error sending test notification, check your logs", - ) + Err(StatusCode::INTERNAL_SERVER_ERROR) } } } @@ -206,7 +211,9 @@ mod tests { db::push_tokens::delete_many(&tokens).unwrap(); } - let response = send_test().await.into_response(); - assert_eq!(response.status(), StatusCode::OK); + let result = send_test() + .await + .expect("test send should succeed when no devices are registered"); + assert_eq!(result.0.delivered, 0, "No devices means nothing delivered"); } } diff --git a/frontend/lib/settings/settings.dart b/frontend/lib/settings/settings.dart index 991bc5e..2878032 100644 --- a/frontend/lib/settings/settings.dart +++ b/frontend/lib/settings/settings.dart @@ -143,12 +143,22 @@ class _SettingsState extends State { Future _sendTestNotification() async { setState(() => _testBusy = true); try { - await BackendAPI.instance.sendTestNotification(); + final delivered = await BackendAPI.instance.sendTestNotification(); if (!mounted) return; - UISnackbars.showSuccess( - context, - 'Test notification sent. It should arrive shortly.', - ); + if (delivered == 0) { + // The request succeeded but no device received it — usually the backend + // lost this device's token (e.g. after a restart); re-toggle to re-register. + UISnackbars.showError( + context, + 'No devices are registered to receive push notifications.', + ); + } else { + final devices = delivered == 1 ? 'device' : 'devices'; + UISnackbars.showSuccess( + context, + 'Test notification sent to $delivered $devices.', + ); + } } catch (e) { debugPrint('Failed to send test notification: $e'); if (!mounted) return; diff --git a/frontend/lib/utils/api/oott_api_notifications.dart b/frontend/lib/utils/api/oott_api_notifications.dart index 856e490..cd02f5e 100644 --- a/frontend/lib/utils/api/oott_api_notifications.dart +++ b/frontend/lib/utils/api/oott_api_notifications.dart @@ -15,9 +15,11 @@ extension NotificationApi on BackendAPI { } /// Asks the backend to deliver a one-off test push to every registered device, - /// used from Settings to verify end-to-end push delivery. - Future sendTestNotification() async { - await _dio.post('/notifications/test'); + /// used from Settings to verify end-to-end push delivery. Returns the number of + /// devices the relay confirmed delivery to (0 when none are registered). + Future sendTestNotification() async { + final response = await _dio.post('/notifications/test'); + return (response.data as Map)['delivered'] as int; } Future<({List items, int totalCount})> listNotifications( diff --git a/frontend/test/api/notifications_api_test.dart b/frontend/test/api/notifications_api_test.dart index 3e25603..e7ccfcb 100644 --- a/frontend/test/api/notifications_api_test.dart +++ b/frontend/test/api/notifications_api_test.dart @@ -36,13 +36,13 @@ void main() { await BackendAPI.instance.markAllNotificationsAsRead(); }); - test('sendTestNotification POSTs to /notifications/test', () async { + test('sendTestNotification POSTs and returns the delivered count', () async { adapter.onPost( '/notifications/test', - (server) => server.reply(200, null), + (server) => server.reply(200, {'delivered': 2}), ); - await BackendAPI.instance.sendTestNotification(); + expect(await BackendAPI.instance.sendTestNotification(), 2); }); test( diff --git a/frontend/test/widget/settings_push_test.dart b/frontend/test/widget/settings_push_test.dart index c214d92..4447d8c 100644 --- a/frontend/test/widget/settings_push_test.dart +++ b/frontend/test/widget/settings_push_test.dart @@ -173,11 +173,13 @@ void main() { expect(find.text('Send test notification'), findsNothing); }); - testWidgets('sends a test notification when push is on', (tester) async { + testWidgets('reports the device count when a test notification is sent', ( + tester, + ) async { stubNotificationMethod('push'); adapter.onPost( '/notifications/test', - (server) => server.reply(200, null), + (server) => server.reply(200, {'delivered': 2}), ); await PrefUtil.setValue('push_enabled', true); await pumpScreen(tester, Settings(pushService: _FakePushService())); @@ -186,7 +188,26 @@ void main() { await tester.tap(find.text('Send test notification')); await pumpUntilFound( tester, - find.text('Test notification sent. It should arrive shortly.'), + find.text('Test notification sent to 2 devices.'), + ); + }); + + testWidgets('warns when a test notification reaches no devices', ( + tester, + ) async { + stubNotificationMethod('push'); + adapter.onPost( + '/notifications/test', + (server) => server.reply(200, {'delivered': 0}), + ); + await PrefUtil.setValue('push_enabled', true); + await pumpScreen(tester, Settings(pushService: _FakePushService())); + await pumpUntilFound(tester, find.text('Send test notification')); + + await tester.tap(find.text('Send test notification')); + await pumpUntilFound( + tester, + find.text('No devices are registered to receive push notifications.'), ); }); }