fix(#80): broker enable/disable feedback + don't validate a disable

On-hardware test feedback:
- Long-press Enable/Disable was silent on success AND failure. It now confirms
  on success and, on failure, surfaces the device's reason (lastError) instead
  of nothing.
- The client now refuses to quick-Enable a broker that can't work (incomplete
  required fields, e.g. JWT without audience/owner) and says why -- so it never
  reports "enabled" on a broker that won't run. (Firmware should reject too.)
- The editor blocked a DISABLE on blank/invalid fields. Validation now gates
  ENABLING only -- disabling a slot with partial values is fine; it won't be
  active.

Validation rules shared via BrokerConfig.enableError (editor pre-save + list
quick-Enable). Full suite green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pull/99/head
Strycher 4 weeks ago
parent 67db108520
commit 0c737f2052

@ -144,6 +144,19 @@ class BrokerConfig {
/// A slot is occupied iff it has a URL (firmware `cfg.url[0] != '\0'`). /// A slot is occupied iff it has a URL (firmware `cfg.url[0] != '\0'`).
bool get isPopulated => url.isNotEmpty; bool get isPopulated => url.isNotEmpty;
/// Why this broker can't be safely enabled, or null if complete. Shared by
/// the editor's pre-save check and the list's quick Enable so the client
/// never activates a broker that can't work (#80).
String? get enableError {
if (url.isEmpty) return 'URL is required';
if (port < 1 || port > 65535) return 'Port must be between 1 and 65535';
if (authType == BrokerAuthType.jwt &&
(jwtAudience.isEmpty || jwtOwner.isEmpty)) {
return 'JWT auth needs an audience and an owner';
}
return null;
}
/// Build from one slot's decoded `key=value` lines (the OCFG_BROKER_KV bodies). /// Build from one slot's decoded `key=value` lines (the OCFG_BROKER_KV bodies).
/// Unknown keys are ignored; missing keys keep the defaults. /// Unknown keys are ignored; missing keys keep the defaults.
factory BrokerConfig.fromWireFields(int slot, Map<String, String> kv) { factory BrokerConfig.fromWireFields(int slot, Map<String, String> kv) {

@ -112,20 +112,16 @@ class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
bool get _dirty => bool get _dirty =>
_changedFields().isNotEmpty || _enabled != _baseline.enabled; _changedFields().isNotEmpty || _enabled != _baseline.enabled;
/// Reject predictably-bad values before they reach the wire (#80). Returns an /// Pre-enable validation, shared with the list's quick Enable via
/// error message, or null when the form is valid. /// [BrokerConfig.enableError]. Builds a tentative config from the form.
String? _validate() { String? _validate() => BrokerConfig(
if (_url.text.trim().isEmpty) return 'URL is required'; slot: _baseline.slot,
final port = int.tryParse(_port.text.trim()); url: _url.text.trim(),
if (port == null || port < 1 || port > 65535) { port: int.tryParse(_port.text.trim()) ?? -1,
return 'Port must be between 1 and 65535'; authType: _authType,
} jwtAudience: _jwtAudience.text.trim(),
if (_authType == BrokerAuthType.jwt && jwtOwner: _jwtOwner.text.trim(),
(_jwtAudience.text.trim().isEmpty || _jwtOwner.text.trim().isEmpty)) { ).enableError;
return 'JWT auth requires an audience and an owner';
}
return null;
}
void _snack(String msg, {bool isError = false}) { void _snack(String msg, {bool isError = false}) {
ScaffoldMessenger.of(context).showSnackBar( ScaffoldMessenger.of(context).showSnackBar(
@ -137,11 +133,15 @@ class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
} }
Future<void> _save() async { Future<void> _save() async {
// Validation only gates ENABLING disabling a slot with blank/partial
// fields is fine; it won't be active.
if (_enabled) {
final err = _validate(); final err = _validate();
if (err != null) { if (err != null) {
_snack(err, isError: true); _snack(err, isError: true);
return; return;
} }
}
final svc = context.read<ObserverConfigService>(); final svc = context.read<ObserverConfigService>();
final messenger = ScaffoldMessenger.of(context); final messenger = ScaffoldMessenger.of(context);
final navigator = Navigator.of(context); final navigator = Navigator.of(context);

@ -57,9 +57,43 @@ class _MqttBrokersScreenState extends State<MqttBrokersScreen> {
} }
Future<void> _quickToggle(BrokerConfig b, bool enable) async { Future<void> _quickToggle(BrokerConfig b, bool enable) async {
final messenger = ScaffoldMessenger.of(context);
final errorColor = Theme.of(context).colorScheme.error;
// The client refuses to enable a broker that can't work, and says why
// (firmware should reject it too, but don't claim success on the wire).
if (enable) {
final err = b.enableError;
if (err != null) {
messenger.showSnackBar(
SnackBar(
content: Text("Can't enable broker ${b.slot}: $err"),
backgroundColor: errorColor,
),
);
return;
}
}
final svc = context.read<ObserverConfigService>(); final svc = context.read<ObserverConfigService>();
await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0'); final ok = await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0');
if (mounted) await _reload(); if (!mounted) return;
if (ok) {
messenger.showSnackBar(
SnackBar(
content: Text('Broker ${b.slot} ${enable ? 'enabled' : 'disabled'}'),
),
);
await _reload();
} else {
messenger.showSnackBar(
SnackBar(
content: Text(
svc.lastError ??
'Failed to ${enable ? 'enable' : 'disable'} broker ${b.slot}',
),
backgroundColor: errorColor,
),
);
}
} }
Future<void> _clear(BrokerConfig b) async { Future<void> _clear(BrokerConfig b) async {

@ -115,7 +115,7 @@ void main() {
await _open( await _open(
tester, tester,
fake, fake,
const BrokerConfig(slot: 1, url: 'h', port: 1883), const BrokerConfig(slot: 1, url: 'h', port: 1883, enabled: true),
); );
await tester.enterText(find.byKey(const Key('broker_port')), '99999'); await tester.enterText(find.byKey(const Key('broker_port')), '99999');
@ -172,4 +172,57 @@ void main() {
expect(find.text('fromdevice'), findsOneWidget); expect(find.text('fromdevice'), findsOneWidget);
expect(find.text('8883'), findsOneWidget); expect(find.text('8883'), findsOneWidget);
}); });
testWidgets('disabling a slot saves even with incomplete fields', (
tester,
) async {
final fake = _FakeSvc();
await _open(
tester,
fake,
const BrokerConfig(
slot: 2,
url: 'h',
port: 1883,
enabled: true,
authType: BrokerAuthType.jwt, // blank audience/owner
),
);
await tester.tap(find.byType(SwitchListTile)); // toggle enabled OFF
await tester.pumpAndSettle();
await tester.tap(find.byTooltip('Save'));
await tester.pumpAndSettle();
expect(
fake.saveCalls,
hasLength(1),
reason: 'disabling must not be blocked by field validation',
);
expect(fake.saveCalls.single.enable, isFalse);
await _drainSnack(tester);
});
testWidgets('enabling with incomplete JWT is blocked', (tester) async {
final fake = _FakeSvc();
await _open(
tester,
fake,
const BrokerConfig(
slot: 2,
url: 'h',
port: 1883,
authType: BrokerAuthType.jwt, // disabled, blank audience/owner
),
);
await tester.tap(find.byType(SwitchListTile)); // toggle enabled ON
await tester.pumpAndSettle();
await tester.tap(find.byTooltip('Save'));
await tester.pumpAndSettle();
expect(fake.saveCalls, isEmpty);
expect(find.textContaining('needs an audience'), findsOneWidget);
await _drainSnack(tester);
});
} }

@ -18,7 +18,11 @@ class _FakeSvc extends ObserverConfigService {
final List<BrokerConfig> _brokers; final List<BrokerConfig> _brokers;
final List<String> setCalls = []; final List<String> setCalls = [];
final List<int> clearCalls = []; final List<int> clearCalls = [];
bool toggleOk = true;
String? errorText;
@override
String? get lastError => errorText;
@override @override
ObserverConfig? get config => ObserverConfig(brokers: _brokers); ObserverConfig? get config => ObserverConfig(brokers: _brokers);
@override @override
@ -26,7 +30,7 @@ class _FakeSvc extends ObserverConfigService {
@override @override
Future<bool> setBrokerField(int slot, String field, String value) async { Future<bool> setBrokerField(int slot, String field, String value) async {
setCalls.add('$slot.$field=$value'); setCalls.add('$slot.$field=$value');
return true; return toggleOk;
} }
@override @override
@ -97,4 +101,46 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(fake.clearCalls, contains(2)); expect(fake.clearCalls, contains(2));
}); });
testWidgets('quick Enable refuses an incomplete broker with a reason', (
tester,
) async {
final fake = _FakeSvc(const [
BrokerConfig(slot: 2, url: 'a', port: 1883, authType: BrokerAuthType.jwt),
]);
await _pump(tester, fake);
await tester.longPress(find.text('[2] a'));
await tester.pumpAndSettle();
await tester.tap(find.text('Enable'));
await tester.pumpAndSettle();
expect(
fake.setCalls,
isEmpty,
reason: 'an incomplete broker must not be enabled',
);
expect(find.textContaining("Can't enable"), findsOneWidget);
await tester.pump(const Duration(seconds: 5));
});
testWidgets('a failed quick toggle surfaces the device reason', (
tester,
) async {
final fake =
_FakeSvc(const [
BrokerConfig(slot: 2, url: 'a', port: 1883, enabled: true),
])
..toggleOk = false
..errorText = 'ERROR broker busy';
await _pump(tester, fake);
await tester.longPress(find.text('[2] a'));
await tester.pumpAndSettle();
await tester.tap(find.text('Disable'));
await tester.pumpAndSettle();
expect(find.textContaining('ERROR broker busy'), findsOneWidget);
await tester.pump(const Duration(seconds: 5));
});
} }

Loading…
Cancel
Save

Powered by TurnKey Linux.