mirror of
https://github.com/rzuasti/oott.git
synced 2026-07-08 19:21:54 +02:00
Add DeviceChanged and DeviceBackOnline event types
Record what happened on each known-device sighting instead of only "seen": a baseline DeviceSeen (history heartbeat, no notification) plus DeviceChanged and DeviceBackOnline events, each deduplicated independently so a recent routine sighting no longer suppresses a genuine change or return notification. The frontend chart now trusts the event type for its marker and tooltip rather than comparing each event's snapshot against the device's current state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
27cb2e1d62
commit
9efe84c099
+167
-10
@@ -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<DeviceChange> {
|
||||
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();
|
||||
|
||||
@@ -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::<DeviceEventType>().unwrap(), event_type);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_event_type_fails_to_parse() {
|
||||
assert!("BOGUS".parse::<DeviceEventType>().is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn scanner_display_and_from_str_round_trip() {
|
||||
for scanner in [
|
||||
|
||||
@@ -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<DeviceEventHistory> {
|
||||
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<DeviceEventHistory> {
|
||||
|
||||
class _EventChart extends StatelessWidget {
|
||||
final List<DeviceEvent> 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 = <TextSpan>[];
|
||||
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,
|
||||
);
|
||||
},
|
||||
),
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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, <dynamic>[]),
|
||||
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);
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user