Merge pull request #1079 from nextcloud/feature/neon/settings-reset-i…

…nvalid-values
nextcloud · Dec 16, 2023 · a4b685c · a4b685c
2 parents 50ae5b7 + 172bc9d
commit a4b685c
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 16 deletions.
diff --git a/packages/neon_framework/lib/src/settings/models/option.dart b/packages/neon_framework/lib/src/settings/models/option.dart
@@ -71,6 +71,7 @@ sealed class Option<T> extends ChangeNotifier implements ValueListenable<T>, Dis
   /// listeners.
   @override
   T get value => _value;
+
   @mustCallSuper
   set value(final T newValue) {
     if (_value == newValue) {
@@ -88,6 +89,7 @@ sealed class Option<T> extends ChangeNotifier implements ValueListenable<T>, Dis
   /// value as evaluated by the equality operator ==, this class notifies its
   /// listeners.
   bool get enabled => _enabled;
+
   @mustCallSuper
   set enabled(final bool newValue) {
     if (_enabled == newValue) {
@@ -108,7 +110,9 @@ sealed class Option<T> extends ChangeNotifier implements ValueListenable<T>, Dis
     final value = deserialize(data);
 
     if (value != null) {
-      this.value = value;
+      // Do not trigger the validation to avoid resetting when the values haven't been loaded yet.
+      _value = value;
+      notifyListeners();
     }
   }
 
@@ -203,10 +207,14 @@ class SelectOption<T> extends Option<T> {
 
   @override
   set value(final T value) {
-    super.value = value;
-
-    if (value != null) {
-      unawaited(storage.setString(key.value, serialize()!));
+    if (_values.keys.contains(value)) {
+      super.value = value;
+
+      if (value != null) {
+        unawaited(storage.setString(key.value, serialize()!));
+      }
+    } else {
+      debugPrint('"$value" is not in "${_values.keys.join('", "')}", ignoring');
     }
   }
 
@@ -218,12 +226,16 @@ class SelectOption<T> extends Option<T> {
 
   /// Updates the collection of possible values.
   ///
-  /// It is up to the caller to also change the [value] if it is no longer supported.
+  /// If the current [value] is no longer supported the option will reset to the [defaultValue].
   set values(final Map<T, LabelBuilder> newValues) {
     if (_values == newValues) {
       return;
     }
     _values = newValues;
+    if (!_values.keys.contains(_value)) {
+      debugPrint('"$value" is not in "${_values.keys.join('", "')}", resetting "${key.value}"');
+      reset();
+    }
     notifyListeners();
   }
 

diff --git a/packages/neon_framework/lib/src/utils/global_options.dart b/packages/neon_framework/lib/src/utils/global_options.dart
@@ -101,10 +101,6 @@ class GlobalOptions extends OptionsCollection {
         (final account) => MapEntry(account.id, (final context) => account.humanReadableID),
       ),
     );
-
-    if (!initialAccount.values.containsKey(initialAccount.value)) {
-      initialAccount.reset();
-    }
   }
 
   /// Updates the values of [pushNotificationsDistributor].
@@ -118,12 +114,7 @@ class GlobalOptions extends OptionsCollection {
       ),
     );
 
-    final enabled = pushNotificationsDistributor.values.isNotEmpty;
-    pushNotificationsEnabled.enabled = enabled;
-    if (!enabled) {
-      pushNotificationsDistributor.reset();
-      pushNotificationsEnabled.reset();
-    }
+    pushNotificationsEnabled.enabled = pushNotificationsDistributor.values.isNotEmpty;
   }
 
   /// The theme mode of the app implementing the Neon framework.

diff --git a/packages/neon_framework/test/option_test.dart b/packages/neon_framework/test/option_test.dart
@@ -147,6 +147,21 @@ void main() {
       expect(option.values, newValues, reason: 'Should keep the values.');
     });
 
+    test('Invalid values', () {
+      expect(option.values, equals(valuesLabel));
+
+      option
+        ..value = SelectValues.second
+        ..values = {
+          SelectValues.first: (final _) => 'first',
+          SelectValues.third: (final _) => 'third',
+        };
+      expect(option.value, SelectValues.first, reason: 'Invalid value.');
+
+      option.value = SelectValues.second;
+      expect(option.value, SelectValues.first, reason: 'Invalid value.');
+    });
+
     test('Reset', () {
       final callback = MockCallbackFunction();
       option