Skip to content

model: Start tracking a few user_settings, including display_emoji_reaction_users #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions lib/api/model/events.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:json_annotation/json_annotation.dart';

import 'initial_snapshot.dart';
import 'model.dart';

part 'events.g.dart';
Expand All @@ -16,6 +17,11 @@ 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);
Expand Down Expand Up @@ -74,6 +80,54 @@ class AlertWordsEvent extends Event {
Map<String, dynamic> toJson() => _$AlertWordsEventToJson(this);
}

/// A Zulip event of type `user_settings` with op `update`.
@JsonSerializable(fieldRename: FieldRename.snake)
class UserSettingsUpdateEvent extends Event {
@override
@JsonKey(includeToJson: true)
String get type => 'user_settings';

@JsonKey(includeToJson: true)
String get op => 'update';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly this should get marked includeToJson so that the toJson round-trips.


/// The name of the setting, or null if we don't recognize it.
@JsonKey(unknownEnumValue: JsonKey.nullForUndefinedEnumValue)
final UserSettingName? property;

/// The new value, or null if we don't recognize the setting.
///
/// This will have the type appropriate for [property]; for example,
/// if the setting is boolean, then `value is bool` will always be true.
/// This invariant is enforced by [UserSettingsUpdateEvent.fromJson].
@JsonKey(readValue: _readValue)
final Object? value;

/// [value], with a check that its type corresponds to [property]
/// (e.g., `value as bool`).
static Object? _readValue(Map json, String key) {
final value = json['value'];
switch (UserSettingName.fromRawString(json['property'] as String)) {
case UserSettingName.twentyFourHourTime:
case UserSettingName.displayEmojiReactionUsers:
return value as bool;
case null:
return null;
}
}

UserSettingsUpdateEvent({
required super.id,
required this.property,
required this.value,
});

factory UserSettingsUpdateEvent.fromJson(Map<String, dynamic> json) =>
_$UserSettingsUpdateEventFromJson(json);

@override
Map<String, dynamic> toJson() => _$UserSettingsUpdateEventToJson(this);
}

/// A Zulip event of type `realm_user`.
///
/// The corresponding API docs are in several places for
Expand Down
24 changes: 24 additions & 0 deletions lib/api/model/events.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 61 additions & 0 deletions lib/api/model/initial_snapshot.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:flutter/foundation.dart';
import 'package:json_annotation/json_annotation.dart';

import 'model.dart';
Expand Down Expand Up @@ -30,6 +31,14 @@ class InitialSnapshot {

final List<ZulipStream> streams;

// Servers pre-5.0 don't have `user_settings`, and instead provide whatever
// user settings they support at toplevel in the initial snapshot. Since we're
// likely to desupport pre-5.0 servers before wide release, we prefer to
// ignore the toplevel fields and use `user_settings` where present instead,
// even at the expense of functionality with pre-5.0 servers.
// TODO(server-5) remove pre-5.0 comment
Comment on lines +34 to +39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, sounds good. This does seem like a spot where properly supporting both old and new versions would be kind of a pain. (Would have been totally doable, if the timing were such that we had to — we'd probably use a readValue that would just be one more spot that needs another line or two of code per setting — but enough of a pain to not be worth it given that we don't have to.)

final UserSettings? userSettings; // TODO(server-5)

final int maxFileUploadSizeMib;

@JsonKey(readValue: _readUsersIsActiveFallbackTrue)
Expand Down Expand Up @@ -71,6 +80,7 @@ class InitialSnapshot {
required this.recentPrivateConversations,
required this.subscriptions,
required this.streams,
required this.userSettings,
required this.maxFileUploadSizeMib,
required this.realmUsers,
required this.realmNonActiveUsers,
Expand Down Expand Up @@ -102,3 +112,54 @@ class RecentDmConversation {

Map<String, dynamic> toJson() => _$RecentDmConversationToJson(this);
}

/// The `user_settings` dictionary.
///
/// For docs, search for "user_settings:"
/// in <https://zulip.com/api/register-queue>.
@JsonSerializable(fieldRename: FieldRename.snake, createFieldMap: true)
class UserSettings {
bool twentyFourHourTime;
bool? displayEmojiReactionUsers; // TODO(server-6)

// TODO more, as needed. When adding a setting here, please also:
// (1) add it to the [UserSettingName] enum below
// (2) then re-run the command to refresh the .g.dart files
// (3) handle the event that signals an update to the setting

UserSettings({
required this.twentyFourHourTime,
required this.displayEmojiReactionUsers,
});

factory UserSettings.fromJson(Map<String, dynamic> json) =>
_$UserSettingsFromJson(json);

Map<String, dynamic> toJson() => _$UserSettingsToJson(this);

/// A list of [UserSettings]'s properties, as strings.
// _$…FieldMap is thanks to `createFieldMap: true`
@visibleForTesting
static final Iterable<String> debugKnownNames = _$UserSettingsFieldMap.keys;
}

/// 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 {
twentyFourHourTime,
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];

// _$…EnumMap is thanks to `alwaysCreate: true` and `fieldRename: FieldRename.snake`
static final _byRawString = _$UserSettingNameEnumMap
.map((key, value) => MapEntry(value, key));
}
26 changes: 26 additions & 0 deletions lib/api/model/initial_snapshot.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ class PerAccountStore extends ChangeNotifier {
required InitialSnapshot initialSnapshot,
}) : zulipVersion = initialSnapshot.zulipVersion,
maxFileUploadSizeMib = initialSnapshot.maxFileUploadSizeMib,
userSettings = initialSnapshot.userSettings,
users = Map.fromEntries(
initialSnapshot.realmUsers
.followedBy(initialSnapshot.realmNonActiveUsers)
Expand All @@ -172,6 +173,9 @@ class PerAccountStore extends ChangeNotifier {
final String zulipVersion; // TODO get from account; update there on initial snapshot
final int maxFileUploadSizeMib; // No event for this.

// Data attached to the self-account on the realm.
final UserSettings? userSettings; // TODO(server-5)

// Users and data about them.
final Map<int, User> users;

Expand Down Expand Up @@ -215,6 +219,19 @@ 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 ${event.property?.name ?? '[unrecognized]'}"));
if (event.property == null) {
// unrecognized setting; do nothing
return;
}
switch (event.property!) {
case UserSettingName.twentyFourHourTime:
userSettings?.twentyFourHourTime = event.value as bool;
case UserSettingName.displayEmojiReactionUsers:
userSettings?.displayEmojiReactionUsers = event.value as bool;
}
notifyListeners();
} else if (event is RealmUserAddEvent) {
assert(debugLog("server event: realm_user/add"));
users[event.person.userId] = event.person;
Expand Down
21 changes: 21 additions & 0 deletions test/api/model/events_test.dart
Original file line number Diff line number Diff line change
@@ -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 '../../stdlib_checks.dart';
Expand All @@ -20,4 +21,24 @@ void main() {
check(mkEvent([])).message.flags.deepEquals([]);
check(mkEvent(['read'])).message.flags.deepEquals(['read']);
});

test('user_settings: all known settings have event handling', () {
final dataClassFieldNames = UserSettings.debugKnownNames;
final enumNames = UserSettingName.values.map((n) => n.name);
final missingEnumNames = dataClassFieldNames.where((key) => !enumNames.contains(key)).toList();
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, re-run the command to refresh the .g.dart files.\n'
' (3) Resolve the Dart analysis errors about not exhaustively\n'
' matching on that enum, by adding new `switch` cases\n'
' on the pattern of the existing cases.'
).isEmpty();
});
}
2 changes: 2 additions & 0 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ InitialSnapshot initialSnapshot({
List<RecentDmConversation>? recentPrivateConversations,
List<Subscription>? subscriptions,
List<ZulipStream>? streams,
UserSettings? userSettings,
int? maxFileUploadSizeMib,
List<User>? realmUsers,
List<User>? realmNonActiveUsers,
Expand All @@ -224,6 +225,7 @@ InitialSnapshot initialSnapshot({
recentPrivateConversations: recentPrivateConversations ?? [],
subscriptions: subscriptions ?? [], // TODO add subscriptions to default
streams: streams ?? [], // TODO add streams to default
userSettings: userSettings, // TODO add userSettings to default
maxFileUploadSizeMib: maxFileUploadSizeMib ?? 25,
realmUsers: realmUsers ?? [],
realmNonActiveUsers: realmNonActiveUsers ?? [],
Expand Down