api: Start accepting null for user_settings.twenty_four_hour_time

Servers can't yet start sending null without breaking clients.
Releasing this will start lowering the number of client installs
that would break, and eventually there will be few enough that the
breakage is acceptable; see discussion (same link as in comment):
  https://chat.zulip.org/#narrow/channel/378-api-design/topic/.60user_settings.2Etwenty_four_hour_time.60/near/2220696

Author: chrisbobbe

lib/api/model/events.dart

@@ -175,6 +175,7 @@ class UserSettingsUpdateEvent extends Event {
     final value = json['value'];
     switch (UserSettingName.fromRawString(json['property'] as String)) {
       case UserSettingName.twentyFourHourTime:
+        return TwentyFourHourTimeMode.fromApiValue(value as bool?);
       case UserSettingName.displayEmojiReactionUsers:
         return value as bool;
       case UserSettingName.emojiset:

lib/api/model/initial_snapshot.dart

@@ -259,7 +259,12 @@ class RecentDmConversation {
 /// in <https://zulip.com/api/register-queue>.
 @JsonSerializable(fieldRename: FieldRename.snake, createFieldMap: true)
 class UserSettings {
-  bool twentyFourHourTime;
+  @JsonKey(
+    fromJson: TwentyFourHourTimeMode.fromApiValue,
+    toJson: TwentyFourHourTimeMode.staticToJson,
+  )
+  TwentyFourHourTimeMode twentyFourHourTime;
+
   bool? displayEmojiReactionUsers; // TODO(server-6)
   Emojiset emojiset;
   bool presenceEnabled;

lib/api/model/initial_snapshot.g.dart

@@ -298,6 +298,35 @@ enum UserSettingName {
   String toJson() => _$UserSettingNameEnumMap[this]!;
 }
 
+/// A value from [UserSettings.twentyFourHourTime].
+enum TwentyFourHourTimeMode {
+  twelveHour(apiValue: false),
+  twentyFourHour(apiValue: true),
+
+  /// The locale's default format (12-hour for en_US, 24-hour for fr_FR, etc.).
+  // TODO(#1727) actually follow this
+  // Not sent by current servers, but planned when most client installs accept it:
+  //   https://chat.zulip.org/#narrow/channel/378-api-design/topic/.60user_settings.2Etwenty_four_hour_time.60/near/2220696
+  // TODO(server-future) Write down what server N starts sending null;
+  //   adjust the comment; leave a TODO(server-N) to delete the comment
+  localeDefault(apiValue: null),
+  ;
+
+  const TwentyFourHourTimeMode({required this.apiValue});
+
+  final bool? apiValue;
+
+  static bool? staticToJson(TwentyFourHourTimeMode instance) => instance.apiValue;
+
+  bool? toJson() => TwentyFourHourTimeMode.staticToJson(this);
+
+  static TwentyFourHourTimeMode fromApiValue(bool? value) => switch (value) {
+    false => twelveHour,
+    true => twentyFourHour,
+    null => localeDefault,
+  };
+}
+
 /// As in [UserSettings.emojiset].
 @JsonEnum(fieldRename: FieldRename.kebab, alwaysCreate: true)
 enum Emojiset {

lib/api/model/model.dart

@@ -9,12 +9,20 @@ Future<void> updateSettings(ApiConnection connection, {
   for (final entry in newSettings.entries) {
     final name = entry.key;
     final valueRaw = entry.value;
-    final value = switch (name) {
-      UserSettingName.twentyFourHourTime => valueRaw as bool,
-      UserSettingName.displayEmojiReactionUsers => valueRaw as bool,
-      UserSettingName.emojiset => RawParameter((valueRaw as Emojiset).toJson()),
-      UserSettingName.presenceEnabled => valueRaw as bool,
-    };
+    final Object? value;
+    switch (name) {
+      case UserSettingName.twentyFourHourTime:
+        final mode = (valueRaw as TwentyFourHourTimeMode);
+        // TODO(server-future) allow localeDefault for servers that support it
+        assert(mode != TwentyFourHourTimeMode.localeDefault);
+        value = mode.toJson();
+      case UserSettingName.displayEmojiReactionUsers:
+        value = valueRaw as bool;
+      case UserSettingName.emojiset:
+        value = RawParameter((valueRaw as Emojiset).toJson());
+      case UserSettingName.presenceEnabled:
+        value = valueRaw as bool;
+    }
     params[name.toJson()] = value;
   }
 

lib/api/route/settings.dart

@@ -707,7 +707,7 @@ class PerAccountStore extends PerAccountStoreBase with
         }
         switch (event.property!) {
           case UserSettingName.twentyFourHourTime:
-            userSettings.twentyFourHourTime        = event.value as bool;
+            userSettings.twentyFourHourTime        = event.value as TwentyFourHourTimeMode;
           case UserSettingName.displayEmojiReactionUsers:
             userSettings.displayEmojiReactionUsers = event.value as bool;
           case UserSettingName.emojiset:

lib/model/store.dart

@@ -17,8 +17,8 @@ void main() {
       for (final name in UserSettingName.values) {
         switch (name) {
           case UserSettingName.twentyFourHourTime:
-            newSettings[name] = true;
-            expectedBodyFields['twenty_four_hour_time'] = 'true';
+            newSettings[name] = TwentyFourHourTimeMode.twelveHour;
+            expectedBodyFields['twenty_four_hour_time'] = 'false';
           case UserSettingName.displayEmojiReactionUsers:
             newSettings[name] = false;
             expectedBodyFields['display_emoji_reaction_users'] = 'false';
@@ -38,4 +38,16 @@ void main() {
         ..bodyFields.deepEquals(expectedBodyFields);
     });
   });
+
+  test('TwentyFourHourTime.localeDefault', () async {
+    return FakeApiConnection.with_((connection) async {
+      connection.prepare(json: {});
+
+      // TODO(server-future) instead, check for twenty_four_hour_time: null
+      //   (could be an error-prone part of the JSONification)
+      check(() => updateSettings(connection,
+        newSettings: {UserSettingName.twentyFourHourTime: TwentyFourHourTimeMode.localeDefault})
+      ).throws<AssertionError>();
+    });
+  });
 }

test/api/route/settings_test.dart

@@ -1262,7 +1262,7 @@ InitialSnapshot initialSnapshot({
     streams: streams ?? [], // TODO add streams to default
     userStatuses: userStatuses ?? {},
     userSettings: userSettings ?? UserSettings(
-      twentyFourHourTime: false,
+      twentyFourHourTime: TwentyFourHourTimeMode.twelveHour,
       displayEmojiReactionUsers: true,
       emojiset: Emojiset.google,
       presenceEnabled: true,

test/example_data.dart

@@ -698,14 +698,16 @@ void main() {
       await preparePoll();
 
       // Pick some arbitrary event and check it gets processed on the store.
-      check(store.userSettings.twentyFourHourTime).isFalse();
+      check(store.userSettings.twentyFourHourTime)
+        .equals(TwentyFourHourTimeMode.twelveHour);
       connection.prepare(json: GetEventsResult(events: [
         UserSettingsUpdateEvent(id: 2,
           property: UserSettingName.twentyFourHourTime, value: true),
       ], queueId: null).toJson());
       updateMachine.debugAdvanceLoop();
       async.elapse(Duration.zero);
-      check(store.userSettings.twentyFourHourTime).isTrue();
+      check(store.userSettings.twentyFourHourTime)
+        .equals(TwentyFourHourTimeMode.twentyFourHour);
     }));
 
     void checkReload(FutureOr<void> Function() prepareError, {
@@ -735,14 +737,16 @@ void main() {
         // The new UpdateMachine updates the new store.
         updateMachine.debugPauseLoop();
         updateMachine.poll();
-        check(store.userSettings.twentyFourHourTime).isFalse();
+        check(store.userSettings.twentyFourHourTime)
+          .equals(TwentyFourHourTimeMode.twelveHour);
         connection.prepare(json: GetEventsResult(events: [
           UserSettingsUpdateEvent(id: 2,
             property: UserSettingName.twentyFourHourTime, value: true),
         ], queueId: null).toJson());
         updateMachine.debugAdvanceLoop();
         async.elapse(Duration.zero);
-        check(store.userSettings.twentyFourHourTime).isTrue();
+        check(store.userSettings.twentyFourHourTime)
+          .equals(TwentyFourHourTimeMode.twentyFourHour);
       });
     }