diff --git a/backend/src/events.rs b/backend/src/events.rs index 9cd7f7d..eb654b0 100644 --- a/backend/src/events.rs +++ b/backend/src/events.rs @@ -448,25 +448,28 @@ pub fn classify_new_device(device: Device, scanner: DeviceEventScanner) -> Optio Some(DeviceChange::New(device)) } -/// Record the device-seen event for a known device and return any notification-worthy changes (it -/// may return both a "back online" and a "changed" entry, or none). Sending is deferred to `notify`. +/// Record the device events for a known device and return any notification-worthy changes (it may +/// return both a "back online" and a "changed" entry, or none). Every sighting records a baseline +/// `DeviceSeen` event (the history heartbeat, no notification); a return after the configured +/// absence and an IP/vendor change each additionally record their own event type and produce a +/// notification. Each event type is deduplicated independently within the configured window, so a +/// recent routine sighting never suppresses a genuine change or return. Sending is deferred to +/// `notify`. pub fn classify_existing_device( existing_device: Device, new_device: Device, scanner: DeviceEventScanner, ) -> Vec { - if !record_event(DeviceEvent::new( + // Baseline presence heartbeat for the event history. Recorded (subject to its own dedup) for + // every sighting and never raises a notification. + record_event(DeviceEvent::new( new_device.mac_address.clone(), Utc::now(), DeviceEventType::DeviceSeen, new_device.ipv4_address.clone(), new_device.vendor.clone(), - scanner, - )) { - // The sighting was deduplicated within the window; suppress its notifications too so the - // window governs the notifications table and channel delivery, not just device_events. - return Vec::new(); - } + scanner.clone(), + )); let mut changes = Vec::new(); @@ -476,6 +479,14 @@ pub fn classify_existing_device( .unwrap_or(Duration::from_secs(0)); if elapsed_since_last_seen >= Duration::from(get_settings().notifications.notify_when_not_seen_for) + && record_event(DeviceEvent::new( + new_device.mac_address.clone(), + Utc::now(), + DeviceEventType::DeviceBackOnline, + new_device.ipv4_address.clone(), + new_device.vendor.clone(), + scanner.clone(), + )) { changes.push(DeviceChange::BackOnline { device: new_device.clone(), @@ -486,7 +497,16 @@ pub fn classify_existing_device( // The device's vendor and/or IP changed. let ip_changed_flag = ip_changed(&existing_device.ipv4_address, &new_device.ipv4_address); let vendor_changed_flag = vendor_changed(&existing_device.vendor, &new_device.vendor); - if ip_changed_flag || vendor_changed_flag { + if (ip_changed_flag || vendor_changed_flag) + && record_event(DeviceEvent::new( + new_device.mac_address.clone(), + Utc::now(), + DeviceEventType::DeviceChanged, + new_device.ipv4_address.clone(), + new_device.vendor.clone(), + scanner, + )) + { changes.push(DeviceChange::Changed { existing: existing_device, new: new_device, @@ -937,6 +957,143 @@ mod tests { ); } + // Count recorded events of a given type for a device. Scoped per-MAC because the test DB is + // shared across tests. + fn event_count(mac: &str, event_type: DeviceEventType) -> usize { + db::device_events::list(Some(mac.to_string()), None, None, None) + .unwrap() + .into_iter() + .filter(|event| event.event_type == event_type) + .count() + } + + #[tokio::test] + async fn changed_sighting_records_a_changed_event_and_notification() { + crate::tests_common::setup().await; + + // A known device, recently seen (so not "back online"), now reports a different IP. + let mut existing = sample_device(Some("changed-device")); + existing.mac_address = "fa:ce:fa:ce:05:01".to_string(); + existing.last_seen = Utc::now(); + let mut new = existing.clone(); + new.ipv4_address = "192.168.1.123".to_string(); + let mac = existing.mac_address.clone(); + + notify(classify_existing_device(existing, new, DeviceEventScanner::Arp)); + + assert_eq!( + event_count(&mac, DeviceEventType::DeviceSeen), + 1, + "Every sighting records a baseline DeviceSeen event" + ); + assert_eq!( + event_count(&mac, DeviceEventType::DeviceChanged), + 1, + "A changed sighting additionally records a DeviceChanged event" + ); + assert_eq!( + event_count(&mac, DeviceEventType::DeviceBackOnline), + 0, + "A recently-seen device is not back online" + ); + + let changed_notifications = db::notifications::list(None, None, None) + .unwrap() + .into_iter() + .filter(|n| { + n.notification_type == NotificationType::DeviceChanged + && n.mac_address.as_deref() == Some(mac.as_str()) + }) + .count(); + assert_eq!( + changed_notifications, 1, + "A changed sighting raises a DeviceChanged notification" + ); + } + + #[tokio::test] + async fn back_online_sighting_records_a_back_online_event_and_notification() { + crate::tests_common::setup().await; + + // A known device, unchanged, absent long enough to be "back online". + let mut existing = sample_device(Some("back-online-device")); + existing.mac_address = "fa:ce:fa:ce:05:02".to_string(); + existing.last_seen = Utc::now() - chrono::Duration::days(30); + let new = existing.clone(); + let mac = existing.mac_address.clone(); + + notify(classify_existing_device(existing, new, DeviceEventScanner::Arp)); + + assert_eq!( + event_count(&mac, DeviceEventType::DeviceSeen), + 1, + "Every sighting records a baseline DeviceSeen event" + ); + assert_eq!( + event_count(&mac, DeviceEventType::DeviceBackOnline), + 1, + "A return after the absence threshold records a DeviceBackOnline event" + ); + assert_eq!( + event_count(&mac, DeviceEventType::DeviceChanged), + 0, + "An unchanged device records no DeviceChanged event" + ); + + let back_online_notifications = db::notifications::list(None, None, None) + .unwrap() + .into_iter() + .filter(|n| { + n.notification_type == NotificationType::DeviceOnlineAfterTime + && n.mac_address.as_deref() == Some(mac.as_str()) + }) + .count(); + assert_eq!( + back_online_notifications, 1, + "A back-online sighting raises a DeviceOnlineAfterTime notification" + ); + } + + #[tokio::test] + async fn changed_and_back_online_sighting_records_three_events_and_two_notifications() { + crate::tests_common::setup().await; + + // A known device that is both absent long enough and reports a different IP on its return. + let mut existing = sample_device(Some("changed-and-back-device")); + existing.mac_address = "fa:ce:fa:ce:05:03".to_string(); + existing.last_seen = Utc::now() - chrono::Duration::days(30); + let mut new = existing.clone(); + new.ipv4_address = "192.168.1.200".to_string(); + let mac = existing.mac_address.clone(); + + notify(classify_existing_device(existing, new, DeviceEventScanner::Arp)); + + assert_eq!(event_count(&mac, DeviceEventType::DeviceSeen), 1); + assert_eq!(event_count(&mac, DeviceEventType::DeviceChanged), 1); + assert_eq!(event_count(&mac, DeviceEventType::DeviceBackOnline), 1); + + let notifications: Vec<_> = db::notifications::list(None, None, None) + .unwrap() + .into_iter() + .filter(|n| n.mac_address.as_deref() == Some(mac.as_str())) + .collect(); + assert_eq!( + notifications.len(), + 2, + "A changed-and-back-online sighting raises exactly two notifications" + ); + assert!( + notifications + .iter() + .any(|n| n.notification_type == NotificationType::DeviceChanged) + ); + assert!( + notifications + .iter() + .any(|n| n.notification_type == NotificationType::DeviceOnlineAfterTime) + ); + } + fn new_device_change(mac: &str, name: &str) -> DeviceChange { let mut device = sample_device(Some(name)); device.mac_address = mac.to_string(); diff --git a/backend/src/model/device_events.rs b/backend/src/model/device_events.rs index b43e74c..26c425a 100644 --- a/backend/src/model/device_events.rs +++ b/backend/src/model/device_events.rs @@ -65,6 +65,8 @@ impl PartialEq for DeviceEvent { pub enum DeviceEventType { NewDevice, DeviceSeen, + DeviceChanged, + DeviceBackOnline, } impl fmt::Display for DeviceEventType { @@ -72,6 +74,8 @@ impl fmt::Display for DeviceEventType { match self { Self::NewDevice => write!(f, "NewDevice"), Self::DeviceSeen => write!(f, "DeviceSeen"), + Self::DeviceChanged => write!(f, "DeviceChanged"), + Self::DeviceBackOnline => write!(f, "DeviceBackOnline"), } } } @@ -94,6 +98,8 @@ impl FromStr for DeviceEventType { match s { "NewDevice" => Ok(DeviceEventType::NewDevice), "DeviceSeen" => Ok(DeviceEventType::DeviceSeen), + "DeviceChanged" => Ok(DeviceEventType::DeviceChanged), + "DeviceBackOnline" => Ok(DeviceEventType::DeviceBackOnline), _ => Err(DeviceEventTypeParseError), } } @@ -180,6 +186,24 @@ impl FromSql for DeviceEventScanner { mod tests { use super::*; + #[test] + fn event_type_display_and_from_str_round_trip() { + for event_type in [ + DeviceEventType::NewDevice, + DeviceEventType::DeviceSeen, + DeviceEventType::DeviceChanged, + DeviceEventType::DeviceBackOnline, + ] { + let text = event_type.to_string(); + assert_eq!(text.parse::().unwrap(), event_type); + } + } + + #[test] + fn unknown_event_type_fails_to_parse() { + assert!("BOGUS".parse::().is_err()); + } + #[test] fn scanner_display_and_from_str_round_trip() { for scanner in [ diff --git a/frontend/lib/devices/device_event_history.dart b/frontend/lib/devices/device_event_history.dart index eedbeb2..cf10bef 100644 --- a/frontend/lib/devices/device_event_history.dart +++ b/frontend/lib/devices/device_event_history.dart @@ -9,7 +9,6 @@ import '../model/device_event_type.dart'; import '../utils/oott_api.dart'; import '../widgets/filter_selector.dart'; import '../theme/dimens.dart'; -import '../utils/placeholders.dart'; enum _TimeRange { today('Today'), @@ -150,11 +149,7 @@ class _DeviceEventHistoryState extends State { else SizedBox( height: 160, - child: _EventChart( - events: events, - device: widget.device, - range: _selectedRange, - ), + child: _EventChart(events: events, range: _selectedRange), ), ], ); @@ -163,14 +158,29 @@ class _DeviceEventHistoryState extends State { class _EventChart extends StatelessWidget { final List events; - final Device device; final _TimeRange range; - const _EventChart({ - required this.events, - required this.device, - required this.range, - }); + const _EventChart({required this.events, required this.range}); + + // Marker radius and colour per event type. The type itself conveys what happened, so the chart + // trusts it rather than comparing each event's snapshot against the device's current state. + static ({double radius, Color color}) _markerStyle( + DeviceEventType type, + ColorScheme scheme, + ) => switch (type) { + DeviceEventType.newDevice => (radius: 9.0, color: scheme.tertiary), + DeviceEventType.deviceSeen => (radius: 6.0, color: scheme.tertiary), + DeviceEventType.deviceChanged => (radius: 7.0, color: scheme.error), + DeviceEventType.deviceBackOnline => (radius: 7.0, color: scheme.primary), + }; + + // Human-readable label for an event type, shown in the tooltip. + static String _eventLabel(DeviceEventType type) => switch (type) { + DeviceEventType.newDevice => 'First seen', + DeviceEventType.deviceSeen => 'Device seen', + DeviceEventType.deviceChanged => 'Device changed', + DeviceEventType.deviceBackOnline => 'Device back online', + }; @override Widget build(BuildContext context) { @@ -187,13 +197,13 @@ class _EventChart extends StatelessWidget { final maxX = (nowMs / intervalMs).ceil() * intervalMs; final spots = events.map((e) { - final isNew = e.eventType == DeviceEventType.newDevice; + final style = _markerStyle(e.eventType, theme.colorScheme); return ScatterSpot( e.createdOn.millisecondsSinceEpoch.toDouble(), 1.0, dotPainter: FlDotCirclePainter( - radius: isNew ? 9.0 : 6.0, - color: theme.colorScheme.tertiary, + radius: style.radius, + color: style.color, ), ); }).toList(); @@ -257,42 +267,8 @@ class _EventChart extends StatelessWidget { final event = events[idx]; final dt = event.createdOn.toLocal(); final dateStr = DateFormat('MMM d, yyyy HH:mm').format(dt); - final baseLabel = event.eventType == DeviceEventType.newDevice - ? 'First seen' - : 'Device seen'; - final typeLabel = '$baseLabel (${event.scannerLabel})'; - - final diffs = []; - if (event.ipv4Address != device.ipv4Address) { - diffs.add( - TextSpan( - text: '\nIP: ${event.ipv4Address} → ${device.ipv4Address}', - style: TextStyle( - color: theme.colorScheme.error, - fontSize: 11, - fontWeight: FontWeight.w600, - ), - ), - ); - } - if (event.vendor != device.vendor) { - final eventVendor = event.vendor.isEmpty - ? Placeholders.emptyValue - : event.vendor; - final currentVendor = device.vendor.isEmpty - ? Placeholders.emptyValue - : device.vendor; - diffs.add( - TextSpan( - text: '\nVendor: $eventVendor → $currentVendor', - style: TextStyle( - color: theme.colorScheme.error, - fontSize: 11, - fontWeight: FontWeight.w600, - ), - ), - ); - } + final typeLabel = + '${_eventLabel(event.eventType)} (${event.scannerLabel})'; return ScatterTooltipItem( '$dateStr\n$typeLabel', @@ -300,7 +276,6 @@ class _EventChart extends StatelessWidget { color: theme.colorScheme.onSurface, fontSize: 12, ), - children: diffs.isEmpty ? null : diffs, ); }, ), diff --git a/frontend/lib/model/device_event_type.dart b/frontend/lib/model/device_event_type.dart index fbaef90..3a4a318 100644 --- a/frontend/lib/model/device_event_type.dart +++ b/frontend/lib/model/device_event_type.dart @@ -1,7 +1,9 @@ /// Type of a recorded device event, mirroring the backend's `DeviceEventType`. enum DeviceEventType { newDevice('NewDevice'), - deviceSeen('DeviceSeen'); + deviceSeen('DeviceSeen'), + deviceChanged('DeviceChanged'), + deviceBackOnline('DeviceBackOnline'); const DeviceEventType(this.wireValue); diff --git a/frontend/test/unit/device_event_test.dart b/frontend/test/unit/device_event_test.dart index 013ab95..32b2921 100644 --- a/frontend/test/unit/device_event_test.dart +++ b/frontend/test/unit/device_event_test.dart @@ -33,6 +33,8 @@ void main() { expect(parsed('NewDevice'), DeviceEventType.newDevice); expect(parsed('DeviceSeen'), DeviceEventType.deviceSeen); + expect(parsed('DeviceChanged'), DeviceEventType.deviceChanged); + expect(parsed('DeviceBackOnline'), DeviceEventType.deviceBackOnline); expect(parsed('something_unexpected'), DeviceEventType.deviceSeen); }); diff --git a/frontend/test/widget/device_event_history_test.dart b/frontend/test/widget/device_event_history_test.dart new file mode 100644 index 0000000..b461e22 --- /dev/null +++ b/frontend/test/widget/device_event_history_test.dart @@ -0,0 +1,59 @@ +import 'package:fl_chart/fl_chart.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:frontend/devices/device_event_history.dart'; +import 'package:frontend/model/device.dart'; +import 'package:http_mock_adapter/http_mock_adapter.dart'; + +import '../helpers/backend_test_harness.dart'; +import '../helpers/fixtures.dart'; +import '../helpers/pump_app.dart'; + +void main() { + late DioAdapter adapter; + const mac = 'aa:bb:cc:dd:ee:ff'; + + setUp(() async { + adapter = await setUpBackendForTest(); + }); + + Device device() => Device.fromJson(deviceJson(macAddress: mac)); + + testWidgets('renders the chart for events of every type', (tester) async { + adapter.onGet( + '/devices/$mac/events', + (server) => server.reply(200, [ + deviceEventJson(id: 1, eventType: 'NewDevice'), + deviceEventJson(id: 2, eventType: 'DeviceSeen'), + deviceEventJson(id: 3, eventType: 'DeviceChanged'), + deviceEventJson(id: 4, eventType: 'DeviceBackOnline'), + ]), + // The widget always sends a (dynamic, now-relative) created_from filter. + queryParameters: {'created_from': Matchers.any}, + ); + + await pumpScreen(tester, DeviceEventHistory(device: device())); + await pumpUntilFound(tester, find.byType(ScatterChart)); + + expect(find.byType(ScatterChart), findsOneWidget); + expect(find.text('No events in this time range'), findsNothing); + + await tearDownTree(tester); + }); + + testWidgets('shows an empty-state message when there are no events', ( + tester, + ) async { + adapter.onGet( + '/devices/$mac/events', + (server) => server.reply(200, []), + queryParameters: {'created_from': Matchers.any}, + ); + + await pumpScreen(tester, DeviceEventHistory(device: device())); + await pumpUntilFound(tester, find.text('No events in this time range')); + + expect(find.byType(ScatterChart), findsNothing); + + await tearDownTree(tester); + }); +}