api: Add user_settings/update events, with a test for exhaustiveness

chrisbobbe · chrisbobbe · commit f662a017637c · 2023-08-09T13:48:32.000-07:00
It's been tricky to find a way to verify that the event-handling
code keeps up with the settings we add in [UserSettings], the data
class we use in the initial snapshot. It's possible we'll find a
better solution.

In particular, the subclasses of [UserSettingsUpdateEvent] seem
verbose, in both their names and their implementations. That's:

- UserSettingsUpdateEventDisplayEmojiReactionUsers
- UserSettingsUpdateEventUnknown

and presumably later:

- UserSettingsUpdateEventEnableStreamPushNotifications
- UserSettingsUpdateEventEnableFollowedTopicPushNotifications
- (and so on.)

But at least this solution works, with type-checking of the event at
the edge, and a mechanism to ensure that all user settings we store
in our initial snapshot get updated by the user_settings/update
event.
diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart
@@ -1,5 +1,6 @@
 import 'package:json_annotation/json_annotation.dart';
 
+import 'initial_snapshot.dart';
 import 'model.dart';
 
 part 'events.g.dart';
@@ -16,6 +17,12 @@ sealed class Event {
   factory Event.fromJson(Map<String, dynamic> json) {
     switch (json['type'] as String) {
       case 'alert_words': return AlertWordsEvent.fromJson(json);
+      case 'user_settings':
+        switch (json['op'] as String) {
+          case 'update':
+            return UserSettingsUpdateEvent.fromJson(json);
+          default: return UnexpectedEvent.fromJson(json);
+        }
       case 'realm_user':
         switch (json['op'] as String) {
           case 'add': return RealmUserAddEvent.fromJson(json);
@@ -74,6 +81,57 @@ class AlertWordsEvent extends Event {
   Map<String, dynamic> toJson() => _$AlertWordsEventToJson(this);
 }
 
+/// A Zulip event of type `user_settings` with op `update`.
+sealed class UserSettingsUpdateEvent extends Event {
+  @override
+  @JsonKey(includeToJson: true)
+  String get type => 'user_settings';
+
+  String get op => 'update';
+
+  UserSettingsUpdateEvent({
+    required super.id,
+  });
+
+  factory UserSettingsUpdateEvent.fromJson(Map<String, dynamic> json) {
+    switch (UserSettingName.fromRawString(json['property'] as String)) {
+      case UserSettingName.displayEmojiReactionUsers:
+        return UserSettingsUpdateEventDisplayEmojiReactionUsers.fromJson(json);
+      case null:
+        return UserSettingsUpdateEventUnknown.fromJson(json);
+    }
+  }
+}
+
+@JsonSerializable(fieldRename: FieldRename.snake)
+class UserSettingsUpdateEventDisplayEmojiReactionUsers extends UserSettingsUpdateEvent {
+  final bool value;
+
+  UserSettingsUpdateEventDisplayEmojiReactionUsers({
+    required super.id,
+    required this.value,
+  });
+
+  factory UserSettingsUpdateEventDisplayEmojiReactionUsers.fromJson(Map<String, dynamic> json) =>
+    _$UserSettingsUpdateEventDisplayEmojiReactionUsersFromJson(json);
+
+  @override
+  Map<String, dynamic> toJson() => _$UserSettingsUpdateEventDisplayEmojiReactionUsersToJson(this);
+}
+
+@JsonSerializable(fieldRename: FieldRename.snake)
+class UserSettingsUpdateEventUnknown extends UserSettingsUpdateEvent {
+  UserSettingsUpdateEventUnknown({
+    required super.id,
+  });
+
+  factory UserSettingsUpdateEventUnknown.fromJson(Map<String, dynamic> json) =>
+    _$UserSettingsUpdateEventUnknownFromJson(json);
+
+  @override
+  Map<String, dynamic> toJson() => _$UserSettingsUpdateEventUnknownToJson(this);
+}
+
 /// A Zulip event of type `realm_user`.
 ///
 /// The corresponding API docs are in several places for
diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart
diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart
@@ -1,3 +1,4 @@
+import 'package:flutter/cupertino.dart';
 import 'package:json_annotation/json_annotation.dart';
 
 import 'model.dart';
@@ -116,11 +117,14 @@ class RecentDmConversation {
 ///
 /// For docs, search for "user_settings:"
 /// in <https://zulip.com/api/register-queue>.
-@JsonSerializable(fieldRename: FieldRename.snake)
+@JsonSerializable(fieldRename: FieldRename.snake, createFieldMap: true)
 class UserSettings {
-  final bool? displayEmojiReactionUsers; // TODO(server-6)
+  bool? displayEmojiReactionUsers; // TODO(server-6)
 
-  // TODO more, as needed
+  // TODO more, as needed. When adding a setting here, please also:
+  // (1) add it to the [UserSettingName] enum below
+  // (2) write a new [UserSettingsUpdateEvent] subclass for it
+  // (3) handle that new subclass in [PerAccountStore]
 
   UserSettings({
     this.displayEmojiReactionUsers,
@@ -131,3 +135,34 @@ class UserSettings {
 
   Map<String, dynamic> toJson() => _$UserSettingsToJson(this);
 }
+
+/// The name of a user setting that has a property in [UserSettings].
+///
+/// In Zulip event-handling code (for [UserSettingsUpdateEvent]),
+/// we switch exhaustively on a value of this type
+/// to ensure that every setting in [UserSettings] responds to the event.
+@JsonEnum(fieldRename: FieldRename.snake, alwaysCreate: true)
+enum UserSettingName {
+  displayEmojiReactionUsers;
+
+  /// Get a [UserSettingName] from a raw, snake-case string we recognize, else null.
+  ///
+  /// Example:
+  ///   'display_emoji_reaction_users' -> UserSettingName.displayEmojiReactionUsers
+  static UserSettingName? fromRawString(String raw) => _byRawString[raw];
+
+  static final _byRawString = Map.fromEntries(
+   _$UserSettingNameEnumMap // thanks to `alwaysCreate: true` and `fieldRename: FieldRename.snake`
+     .entries.map((entry) => MapEntry(entry.value, entry.key)),
+  );
+
+  /// A list of [UserSettings] properties not accounted for in this enum.
+  ///
+  /// For test code to check that all of those properties are accounted for.
+  @visibleForTesting
+  static List<String> missingNames() {
+    final enumNames = values.map((n) => n.name);
+    final dataClassFieldNames = _$UserSettingsFieldMap.keys; // thanks to `createFieldMap: true`
+    return dataClassFieldNames.where((key) => !enumNames.contains(key)).toList();
+  }
+}
diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart
diff --git a/lib/model/store.dart b/lib/model/store.dart
@@ -174,7 +174,7 @@ class PerAccountStore extends ChangeNotifier {
   final int maxFileUploadSizeMib; // No event for this.
 
   // Data attached to the self-account on the realm.
-  final UserSettings? userSettings; // TODO(#135) update with user_settings/update event
+  final UserSettings? userSettings;
 
   // Users and data about them.
   final Map<int, User> users;
@@ -219,6 +219,14 @@ class PerAccountStore extends ChangeNotifier {
     } else if (event is AlertWordsEvent) {
       assert(debugLog("server event: alert_words"));
       // We don't yet store this data, so there's nothing to update.
+    } else if (event is UserSettingsUpdateEvent) {
+      assert(debugLog("server event: user_settings/update"));
+      switch (event) {
+        case UserSettingsUpdateEventDisplayEmojiReactionUsers():
+          userSettings!.displayEmojiReactionUsers = event.value;
+        case UserSettingsUpdateEventUnknown():
+          // do nothing
+      }
     } else if (event is RealmUserAddEvent) {
       assert(debugLog("server event: realm_user/add"));
       users[event.person.userId] = event.person;
diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart
@@ -1,6 +1,7 @@
 import 'package:checks/checks.dart';
 import 'package:test/scaffolding.dart';
 import 'package:zulip/api/model/events.dart';
+import 'package:zulip/api/model/initial_snapshot.dart';
 
 import '../../example_data.dart' as eg;
 import 'events_checks.dart';
@@ -19,4 +20,26 @@ void main() {
     check(mkEvent([])).message.flags.deepEquals([]);
     check(mkEvent(['read'])).message.flags.deepEquals(['read']);
   });
+
+  test('user_settings: all known settings have event handling', () {
+    final missingEnumNames = UserSettingName.missingNames();
+    check(
+      missingEnumNames,
+      because:
+        'You have added these fields to [UserSettings]\n'
+        'without handling the corresponding forms of the\n'
+        'user_settings/update event in [PerAccountStore]:\n'
+        '  $missingEnumNames\n'
+        'To do that, please follow these steps:\n'
+        '  (1) Add corresponding members to the [UserSettingName] enum.\n'
+        '  (2) Then, when you see a Dart analysis error about\n'
+        '      not exhaustively matching on `UserSettingName?`,\n'
+        '      where we represent user_settings/update events,\n'
+        '      please handle it by adding a new case\n'
+        '      on the pattern of the other cases you see there.\n'
+        '  (3) Then, when you see another Dart analysis error about\n'
+        '      not handling the new event type in [PerAccountStore],\n'
+        '      please do so, on the pattern of the other cases there.'
+    ).isEmpty();
+  });
 }
