feat(#80): broker editor screen (staged save, validation, dirty guard)
The edit UI for one broker slot. Staged-save: controls edit local state; Save sends only the CHANGED fields through saveBroker (enabled written last), so a partial save can't leave a live-corrupt broker. - Fields: url, port, transport, auth_type, username, write-only password, topic_prefix, iata_override, jwt_* (when auth=jwt), ca_cert (when tls/wss), plus the enabled toggle. - Validation before any SET: port 1-65535, jwt audience+owner when auth=jwt. - Write-only password: sent only when typed; blank keeps the stored secret. - Refresh: single-slot re-GET (getBroker) + reseed, with a dirty discard guard. - Back button guards unsaved edits (PopScope -> Discard? dialog) -- Gemini's state-loss finding. 4/4 widget tests green. Brokers list screen + Observer-pane wiring next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>feat/64-observer-config
parent
a58e9ccab8
commit
183e9eb7dd
@ -0,0 +1,376 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:provider/provider.dart';
|
||||
|
||||
import '../../models/observer_config.dart';
|
||||
import '../../services/observer_config_service.dart';
|
||||
|
||||
/// Edit one MQTT broker slot (#80). Staged-save: controls edit LOCAL state and
|
||||
/// nothing reaches the device until Save, which writes only the CHANGED fields
|
||||
/// field-at-a-time with `enabled` written LAST (the activation guard) via
|
||||
/// [ObserverConfigService.saveBroker]. A partial save leaves the slot disabled,
|
||||
/// never live-corrupt. Refresh re-reads the single slot; the back button guards
|
||||
/// unsaved edits.
|
||||
class BrokerEditorScreen extends StatefulWidget {
|
||||
const BrokerEditorScreen({super.key, required this.broker});
|
||||
|
||||
final BrokerConfig broker;
|
||||
|
||||
@override
|
||||
State<BrokerEditorScreen> createState() => _BrokerEditorScreenState();
|
||||
}
|
||||
|
||||
class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
|
||||
late final TextEditingController _url;
|
||||
late final TextEditingController _port;
|
||||
late final TextEditingController _username;
|
||||
final _password = TextEditingController();
|
||||
late final TextEditingController _topicPrefix;
|
||||
late final TextEditingController _iataOverride;
|
||||
late final TextEditingController _jwtAudience;
|
||||
late final TextEditingController _jwtRefresh;
|
||||
late final TextEditingController _jwtOwner;
|
||||
late final TextEditingController _jwtEmail;
|
||||
late final TextEditingController _caCert;
|
||||
late BrokerTransport _transport;
|
||||
late BrokerAuthType _authType;
|
||||
late bool _enabled;
|
||||
|
||||
/// The device snapshot the form is diffed against. Updated by Refresh so a
|
||||
/// re-read becomes the new "unchanged" baseline.
|
||||
late BrokerConfig _baseline;
|
||||
bool _busy = false;
|
||||
|
||||
String get _portText => _baseline.isPopulated ? '${_baseline.port}' : '';
|
||||
String get _jwtRefreshText =>
|
||||
_baseline.jwtRefresh == 0 ? '' : '${_baseline.jwtRefresh}';
|
||||
|
||||
@override
|
||||
void initState() {
|
||||
super.initState();
|
||||
_baseline = widget.broker;
|
||||
_url = TextEditingController(text: _baseline.url);
|
||||
_port = TextEditingController(text: _portText);
|
||||
_username = TextEditingController(text: _baseline.username);
|
||||
_topicPrefix = TextEditingController(text: _baseline.topicPrefix);
|
||||
_iataOverride = TextEditingController(text: _baseline.iataOverride);
|
||||
_jwtAudience = TextEditingController(text: _baseline.jwtAudience);
|
||||
_jwtRefresh = TextEditingController(text: _jwtRefreshText);
|
||||
_jwtOwner = TextEditingController(text: _baseline.jwtOwner);
|
||||
_jwtEmail = TextEditingController(text: _baseline.jwtEmail);
|
||||
_caCert = TextEditingController(text: _baseline.caCert);
|
||||
_transport = _baseline.transport;
|
||||
_authType = _baseline.authType;
|
||||
_enabled = _baseline.enabled;
|
||||
}
|
||||
|
||||
@override
|
||||
void dispose() {
|
||||
for (final c in [
|
||||
_url,
|
||||
_port,
|
||||
_username,
|
||||
_password,
|
||||
_topicPrefix,
|
||||
_iataOverride,
|
||||
_jwtAudience,
|
||||
_jwtRefresh,
|
||||
_jwtOwner,
|
||||
_jwtEmail,
|
||||
_caCert,
|
||||
]) {
|
||||
c.dispose();
|
||||
}
|
||||
super.dispose();
|
||||
}
|
||||
|
||||
/// Fields whose local value differs from [_baseline]. Password is write-only:
|
||||
/// it is sent ONLY when the user typed a new one (the stored value is never
|
||||
/// read back, so a blank field keeps it).
|
||||
Map<String, String> _changedFields() {
|
||||
final f = <String, String>{};
|
||||
void diff(String key, String now, String was) {
|
||||
if (now != was) f[key] = now;
|
||||
}
|
||||
|
||||
diff('url', _url.text, _baseline.url);
|
||||
diff('port', _port.text, _portText);
|
||||
if (_transport != _baseline.transport) f['transport'] = _transport.wire;
|
||||
if (_authType != _baseline.authType) f['auth_type'] = _authType.wire;
|
||||
diff('username', _username.text, _baseline.username);
|
||||
diff('topic_prefix', _topicPrefix.text, _baseline.topicPrefix);
|
||||
diff('iata_override', _iataOverride.text, _baseline.iataOverride);
|
||||
diff('jwt_audience', _jwtAudience.text, _baseline.jwtAudience);
|
||||
diff('jwt_refresh', _jwtRefresh.text, _jwtRefreshText);
|
||||
diff('jwt_owner', _jwtOwner.text, _baseline.jwtOwner);
|
||||
diff('jwt_email', _jwtEmail.text, _baseline.jwtEmail);
|
||||
diff('ca_cert', _caCert.text, _baseline.caCert);
|
||||
if (_password.text.isNotEmpty) f['password'] = _password.text;
|
||||
return f;
|
||||
}
|
||||
|
||||
bool get _dirty =>
|
||||
_changedFields().isNotEmpty || _enabled != _baseline.enabled;
|
||||
|
||||
/// Reject predictably-bad values before they reach the wire (#80). Returns an
|
||||
/// error message, or null when the form is valid.
|
||||
String? _validate() {
|
||||
if (_url.text.trim().isEmpty) return 'URL is required';
|
||||
final port = int.tryParse(_port.text.trim());
|
||||
if (port == null || port < 1 || port > 65535) {
|
||||
return 'Port must be between 1 and 65535';
|
||||
}
|
||||
if (_authType == BrokerAuthType.jwt &&
|
||||
(_jwtAudience.text.trim().isEmpty || _jwtOwner.text.trim().isEmpty)) {
|
||||
return 'JWT auth requires an audience and an owner';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
void _snack(String msg, {bool isError = false}) {
|
||||
ScaffoldMessenger.of(context).showSnackBar(
|
||||
SnackBar(
|
||||
content: Text(msg),
|
||||
backgroundColor: isError ? Theme.of(context).colorScheme.error : null,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> _save() async {
|
||||
final err = _validate();
|
||||
if (err != null) {
|
||||
_snack(err, isError: true);
|
||||
return;
|
||||
}
|
||||
final svc = context.read<ObserverConfigService>();
|
||||
final messenger = ScaffoldMessenger.of(context);
|
||||
final navigator = Navigator.of(context);
|
||||
final errorColor = Theme.of(context).colorScheme.error;
|
||||
setState(() => _busy = true);
|
||||
final result = await svc.saveBroker(
|
||||
_baseline.slot,
|
||||
fields: _changedFields(),
|
||||
enable: _enabled,
|
||||
wasLive: _baseline.enabled,
|
||||
);
|
||||
if (!mounted) return;
|
||||
setState(() => _busy = false);
|
||||
if (result.ok) {
|
||||
messenger.showSnackBar(
|
||||
SnackBar(content: Text('Broker ${_baseline.slot} saved')),
|
||||
);
|
||||
navigator.pop(true);
|
||||
} else {
|
||||
messenger.showSnackBar(
|
||||
SnackBar(
|
||||
content: Text(
|
||||
'Save failed at "${result.failedField ?? 'a field'}" — the slot '
|
||||
'was left disabled. Re-read and retry.',
|
||||
),
|
||||
backgroundColor: errorColor,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _refresh() async {
|
||||
final svc = context.read<ObserverConfigService>();
|
||||
final messenger = ScaffoldMessenger.of(context);
|
||||
final errorColor = Theme.of(context).colorScheme.error;
|
||||
if (_dirty && !await _confirmDiscard()) return;
|
||||
if (!mounted) return;
|
||||
setState(() => _busy = true);
|
||||
final fresh = await svc.getBroker(_baseline.slot);
|
||||
if (!mounted) return;
|
||||
setState(() => _busy = false);
|
||||
if (fresh == null) {
|
||||
messenger.showSnackBar(
|
||||
SnackBar(
|
||||
content: Text('Could not read broker ${_baseline.slot}'),
|
||||
backgroundColor: errorColor,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
_seedFrom(fresh);
|
||||
}
|
||||
|
||||
void _seedFrom(BrokerConfig b) {
|
||||
setState(() {
|
||||
_baseline = b;
|
||||
_url.text = b.url;
|
||||
_port.text = _portText;
|
||||
_username.text = b.username;
|
||||
_topicPrefix.text = b.topicPrefix;
|
||||
_iataOverride.text = b.iataOverride;
|
||||
_jwtAudience.text = b.jwtAudience;
|
||||
_jwtRefresh.text = _jwtRefreshText;
|
||||
_jwtOwner.text = b.jwtOwner;
|
||||
_jwtEmail.text = b.jwtEmail;
|
||||
_caCert.text = b.caCert;
|
||||
_transport = b.transport;
|
||||
_authType = b.authType;
|
||||
_enabled = b.enabled;
|
||||
_password.clear();
|
||||
});
|
||||
}
|
||||
|
||||
Future<bool> _confirmDiscard() async {
|
||||
final discard = await showDialog<bool>(
|
||||
context: context,
|
||||
builder: (c) => AlertDialog(
|
||||
title: const Text('Discard changes?'),
|
||||
content: const Text('Your edits to this broker have not been saved.'),
|
||||
actions: [
|
||||
TextButton(
|
||||
onPressed: () => Navigator.pop(c, false),
|
||||
child: const Text('Keep editing'),
|
||||
),
|
||||
TextButton(
|
||||
onPressed: () => Navigator.pop(c, true),
|
||||
child: const Text('Discard'),
|
||||
),
|
||||
],
|
||||
),
|
||||
);
|
||||
return discard ?? false;
|
||||
}
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
return PopScope(
|
||||
canPop: !_dirty,
|
||||
onPopInvokedWithResult: (didPop, _) async {
|
||||
if (didPop) return;
|
||||
final navigator = Navigator.of(context);
|
||||
if (await _confirmDiscard()) navigator.pop();
|
||||
},
|
||||
child: Scaffold(
|
||||
appBar: AppBar(
|
||||
centerTitle: true,
|
||||
title: Text('Broker ${_baseline.slot}'),
|
||||
actions: [
|
||||
IconButton(
|
||||
icon: const Icon(Icons.refresh),
|
||||
tooltip: 'Refresh',
|
||||
onPressed: _busy ? null : _refresh,
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.save),
|
||||
tooltip: 'Save',
|
||||
onPressed: _busy ? null : _save,
|
||||
),
|
||||
],
|
||||
),
|
||||
body: ListView(
|
||||
padding: const EdgeInsets.all(16),
|
||||
children: [
|
||||
SwitchListTile(
|
||||
contentPadding: EdgeInsets.zero,
|
||||
title: const Text('Enabled'),
|
||||
subtitle: const Text('Written last on save (activation guard)'),
|
||||
value: _enabled,
|
||||
onChanged: (v) => setState(() => _enabled = v),
|
||||
),
|
||||
const Divider(height: 24),
|
||||
_field('broker_url', _url, 'URL'),
|
||||
_field('broker_port', _port, 'Port', number: true),
|
||||
const SizedBox(height: 12),
|
||||
_sectionLabel('Transport'),
|
||||
SegmentedButton<BrokerTransport>(
|
||||
segments: const [
|
||||
ButtonSegment(value: BrokerTransport.tcp, label: Text('tcp')),
|
||||
ButtonSegment(value: BrokerTransport.tls, label: Text('tls')),
|
||||
ButtonSegment(value: BrokerTransport.wss, label: Text('wss')),
|
||||
],
|
||||
selected: {_transport},
|
||||
onSelectionChanged: (s) => setState(() => _transport = s.first),
|
||||
),
|
||||
const SizedBox(height: 16),
|
||||
_sectionLabel('Auth'),
|
||||
SegmentedButton<BrokerAuthType>(
|
||||
segments: const [
|
||||
ButtonSegment(value: BrokerAuthType.none, label: Text('none')),
|
||||
ButtonSegment(
|
||||
value: BrokerAuthType.basic,
|
||||
label: Text('basic'),
|
||||
),
|
||||
ButtonSegment(value: BrokerAuthType.jwt, label: Text('jwt')),
|
||||
],
|
||||
selected: {_authType},
|
||||
onSelectionChanged: (s) => setState(() => _authType = s.first),
|
||||
),
|
||||
if (_authType == BrokerAuthType.basic) ...[
|
||||
const SizedBox(height: 12),
|
||||
_field('broker_username', _username, 'Username'),
|
||||
_secretField(),
|
||||
],
|
||||
if (_authType == BrokerAuthType.jwt) ...[
|
||||
const SizedBox(height: 12),
|
||||
_field('broker_jwt_audience', _jwtAudience, 'JWT audience'),
|
||||
_field('broker_jwt_owner', _jwtOwner, 'JWT owner'),
|
||||
_field('broker_jwt_email', _jwtEmail, 'JWT email'),
|
||||
_field(
|
||||
'broker_jwt_refresh',
|
||||
_jwtRefresh,
|
||||
'JWT refresh (sec)',
|
||||
number: true,
|
||||
),
|
||||
],
|
||||
const Divider(height: 24),
|
||||
_field('broker_topic_prefix', _topicPrefix, 'Topic prefix'),
|
||||
_field('broker_iata_override', _iataOverride, 'IATA override'),
|
||||
if (_transport != BrokerTransport.tcp)
|
||||
_field(
|
||||
'broker_ca_cert',
|
||||
_caCert,
|
||||
'CA certificate (PEM)',
|
||||
lines: 3,
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
Widget _sectionLabel(String t) => Padding(
|
||||
padding: const EdgeInsets.only(bottom: 6),
|
||||
child: Text(t, style: Theme.of(context).textTheme.labelLarge),
|
||||
);
|
||||
|
||||
Widget _field(
|
||||
String key,
|
||||
TextEditingController c,
|
||||
String label, {
|
||||
bool number = false,
|
||||
int lines = 1,
|
||||
}) => Padding(
|
||||
padding: const EdgeInsets.only(bottom: 12),
|
||||
child: TextField(
|
||||
key: Key(key),
|
||||
controller: c,
|
||||
maxLines: lines,
|
||||
keyboardType: number ? TextInputType.number : null,
|
||||
inputFormatters: number ? [FilteringTextInputFormatter.digitsOnly] : null,
|
||||
decoration: InputDecoration(
|
||||
labelText: label,
|
||||
border: const OutlineInputBorder(),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
Widget _secretField() => Padding(
|
||||
padding: const EdgeInsets.only(bottom: 12),
|
||||
child: TextField(
|
||||
key: const Key('broker_password'),
|
||||
controller: _password,
|
||||
obscureText: true,
|
||||
decoration: InputDecoration(
|
||||
labelText: _baseline.passwordSet
|
||||
? 'Password (set — leave blank to keep)'
|
||||
: 'Password',
|
||||
border: const OutlineInputBorder(),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
@ -0,0 +1,175 @@
|
||||
// Widget tests for the broker editor (#80).
|
||||
//
|
||||
// Pins the save contract the firmware depends on: only CHANGED fields go on the
|
||||
// wire, a blank password keeps the stored one (write-only), `enable`/`wasLive`
|
||||
// are derived from the slot, and a predictably-invalid value never reaches the
|
||||
// device. Refresh re-reads the single slot and reseeds the form.
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:provider/provider.dart';
|
||||
import 'package:meshcore_open/connector/meshcore_connector.dart';
|
||||
import 'package:meshcore_open/models/observer_config.dart';
|
||||
import 'package:meshcore_open/screens/settings/broker_editor_screen.dart';
|
||||
import 'package:meshcore_open/services/observer_config_service.dart';
|
||||
|
||||
class _DummyConn extends MeshCoreConnector {}
|
||||
|
||||
class _SaveCall {
|
||||
_SaveCall(this.slot, this.fields, this.enable, this.wasLive);
|
||||
final int slot;
|
||||
final Map<String, String> fields;
|
||||
final bool enable;
|
||||
final bool wasLive;
|
||||
}
|
||||
|
||||
class _FakeSvc extends ObserverConfigService {
|
||||
_FakeSvc() : super(_DummyConn());
|
||||
|
||||
final List<_SaveCall> saveCalls = [];
|
||||
BrokerSaveResult result = const BrokerSaveResult.ok();
|
||||
BrokerConfig? fresh;
|
||||
|
||||
@override
|
||||
Future<BrokerSaveResult> saveBroker(
|
||||
int slot, {
|
||||
required Map<String, String> fields,
|
||||
required bool enable,
|
||||
required bool wasLive,
|
||||
}) async {
|
||||
saveCalls.add(_SaveCall(slot, Map.of(fields), enable, wasLive));
|
||||
return result;
|
||||
}
|
||||
|
||||
@override
|
||||
Future<BrokerConfig?> getBroker(int slot) async => fresh;
|
||||
}
|
||||
|
||||
// Push the editor as a second route so its pop-on-success has somewhere to go.
|
||||
Future<void> _open(
|
||||
WidgetTester tester,
|
||||
_FakeSvc fake,
|
||||
BrokerConfig broker,
|
||||
) async {
|
||||
await tester.pumpWidget(
|
||||
ChangeNotifierProvider<ObserverConfigService>.value(
|
||||
value: fake,
|
||||
child: MaterialApp(
|
||||
home: Scaffold(
|
||||
body: Builder(
|
||||
builder: (context) => ElevatedButton(
|
||||
onPressed: () => Navigator.of(context).push(
|
||||
MaterialPageRoute(
|
||||
builder: (_) => BrokerEditorScreen(broker: broker),
|
||||
),
|
||||
),
|
||||
child: const Text('open'),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
await tester.tap(find.text('open'));
|
||||
await tester.pumpAndSettle();
|
||||
}
|
||||
|
||||
Future<void> _drainSnack(WidgetTester tester) async {
|
||||
await tester.pump(const Duration(seconds: 5));
|
||||
await tester.pumpAndSettle();
|
||||
}
|
||||
|
||||
void main() {
|
||||
testWidgets('saves only changed fields; enable/wasLive come from the slot', (
|
||||
tester,
|
||||
) async {
|
||||
final fake = _FakeSvc();
|
||||
await _open(
|
||||
tester,
|
||||
fake,
|
||||
const BrokerConfig(slot: 2, url: 'old', port: 1883, enabled: true),
|
||||
);
|
||||
|
||||
await tester.enterText(find.byKey(const Key('broker_url')), 'newhost');
|
||||
await tester.tap(find.byTooltip('Save'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(fake.saveCalls, hasLength(1));
|
||||
final c = fake.saveCalls.single;
|
||||
expect(c.slot, 2);
|
||||
expect(c.fields['url'], 'newhost');
|
||||
expect(
|
||||
c.fields.containsKey('port'),
|
||||
isFalse,
|
||||
reason: 'an unchanged field must not be re-sent',
|
||||
);
|
||||
expect(c.wasLive, isTrue, reason: 'the slot was enabled on the device');
|
||||
expect(c.enable, isTrue);
|
||||
await _drainSnack(tester);
|
||||
});
|
||||
|
||||
testWidgets('an out-of-range port blocks the save (never reaches the wire)', (
|
||||
tester,
|
||||
) async {
|
||||
final fake = _FakeSvc();
|
||||
await _open(
|
||||
tester,
|
||||
fake,
|
||||
const BrokerConfig(slot: 1, url: 'h', port: 1883),
|
||||
);
|
||||
|
||||
await tester.enterText(find.byKey(const Key('broker_port')), '99999');
|
||||
await tester.tap(find.byTooltip('Save'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(fake.saveCalls, isEmpty);
|
||||
expect(find.textContaining('Port must be'), findsOneWidget);
|
||||
await _drainSnack(tester);
|
||||
});
|
||||
|
||||
testWidgets('a blank password is never sent (write-only keep)', (
|
||||
tester,
|
||||
) async {
|
||||
final fake = _FakeSvc();
|
||||
await _open(
|
||||
tester,
|
||||
fake,
|
||||
const BrokerConfig(
|
||||
slot: 0,
|
||||
url: 'h',
|
||||
port: 1883,
|
||||
authType: BrokerAuthType.basic,
|
||||
passwordSet: true,
|
||||
),
|
||||
);
|
||||
|
||||
await tester.enterText(find.byKey(const Key('broker_username')), 'newuser');
|
||||
await tester.tap(find.byTooltip('Save'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
final c = fake.saveCalls.single;
|
||||
expect(c.fields['username'], 'newuser');
|
||||
expect(
|
||||
c.fields.containsKey('password'),
|
||||
isFalse,
|
||||
reason: 'a blank password field keeps the stored secret',
|
||||
);
|
||||
await _drainSnack(tester);
|
||||
});
|
||||
|
||||
testWidgets('Refresh re-reads the slot and reseeds the form', (tester) async {
|
||||
final fake = _FakeSvc()
|
||||
..fresh = const BrokerConfig(slot: 3, url: 'fromdevice', port: 8883);
|
||||
await _open(
|
||||
tester,
|
||||
fake,
|
||||
const BrokerConfig(slot: 3, url: 'stale', port: 1883),
|
||||
);
|
||||
|
||||
await tester.tap(find.byTooltip('Refresh'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text('fromdevice'), findsOneWidget);
|
||||
expect(find.text('8883'), findsOneWidget);
|
||||
});
|
||||
}
|
||||
Loading…
Reference in new issue