diff --git a/lib/models/observer_config.dart b/lib/models/observer_config.dart index bdcbdfb..76814d6 100644 --- a/lib/models/observer_config.dart +++ b/lib/models/observer_config.dart @@ -144,6 +144,19 @@ class BrokerConfig { /// A slot is occupied iff it has a URL (firmware `cfg.url[0] != '\0'`). 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). /// Unknown keys are ignored; missing keys keep the defaults. factory BrokerConfig.fromWireFields(int slot, Map kv) { diff --git a/lib/screens/settings/broker_editor_screen.dart b/lib/screens/settings/broker_editor_screen.dart index 00eeda2..8952168 100644 --- a/lib/screens/settings/broker_editor_screen.dart +++ b/lib/screens/settings/broker_editor_screen.dart @@ -112,20 +112,16 @@ class _BrokerEditorScreenState extends State { 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; - } + /// Pre-enable validation, shared with the list's quick Enable via + /// [BrokerConfig.enableError]. Builds a tentative config from the form. + String? _validate() => BrokerConfig( + slot: _baseline.slot, + url: _url.text.trim(), + port: int.tryParse(_port.text.trim()) ?? -1, + authType: _authType, + jwtAudience: _jwtAudience.text.trim(), + jwtOwner: _jwtOwner.text.trim(), + ).enableError; void _snack(String msg, {bool isError = false}) { ScaffoldMessenger.of(context).showSnackBar( @@ -137,10 +133,14 @@ class _BrokerEditorScreenState extends State { } Future _save() async { - final err = _validate(); - if (err != null) { - _snack(err, isError: true); - return; + // Validation only gates ENABLING — disabling a slot with blank/partial + // fields is fine; it won't be active. + if (_enabled) { + final err = _validate(); + if (err != null) { + _snack(err, isError: true); + return; + } } final svc = context.read(); final messenger = ScaffoldMessenger.of(context); diff --git a/lib/screens/settings/mqtt_brokers_screen.dart b/lib/screens/settings/mqtt_brokers_screen.dart index 8be2803..1f9c0ba 100644 --- a/lib/screens/settings/mqtt_brokers_screen.dart +++ b/lib/screens/settings/mqtt_brokers_screen.dart @@ -57,9 +57,43 @@ class _MqttBrokersScreenState extends State { } Future _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(); - await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0'); - if (mounted) await _reload(); + final ok = await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0'); + 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 _clear(BrokerConfig b) async { diff --git a/test/screens/broker_editor_screen_test.dart b/test/screens/broker_editor_screen_test.dart index 662be25..361a15e 100644 --- a/test/screens/broker_editor_screen_test.dart +++ b/test/screens/broker_editor_screen_test.dart @@ -115,7 +115,7 @@ void main() { await _open( tester, 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'); @@ -172,4 +172,57 @@ void main() { expect(find.text('fromdevice'), 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); + }); } diff --git a/test/screens/mqtt_brokers_screen_test.dart b/test/screens/mqtt_brokers_screen_test.dart index 8a637bf..7e645f5 100644 --- a/test/screens/mqtt_brokers_screen_test.dart +++ b/test/screens/mqtt_brokers_screen_test.dart @@ -18,7 +18,11 @@ class _FakeSvc extends ObserverConfigService { final List _brokers; final List setCalls = []; final List clearCalls = []; + bool toggleOk = true; + String? errorText; + @override + String? get lastError => errorText; @override ObserverConfig? get config => ObserverConfig(brokers: _brokers); @override @@ -26,7 +30,7 @@ class _FakeSvc extends ObserverConfigService { @override Future setBrokerField(int slot, String field, String value) async { setCalls.add('$slot.$field=$value'); - return true; + return toggleOk; } @override @@ -97,4 +101,46 @@ void main() { await tester.pumpAndSettle(); 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)); + }); }