Skip to content

model: Use enum for Message.flags[] #298

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 3 commits into from
Sep 7, 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
2 changes: 1 addition & 1 deletion lib/api/model/events.dart
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ class UpdateMessageEvent extends Event {
final bool? renderingOnly; // TODO(server-5)
final int messageId;
final List<int> messageIds;
final List<String> flags; // TODO enum
final List<MessageFlag> flags;
final int? editTimestamp; // TODO(server-5)
final String? streamName;
final int? streamId;
Expand Down
15 changes: 14 additions & 1 deletion lib/api/model/events.g.dart

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

34 changes: 33 additions & 1 deletion lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,16 @@ sealed class Message {

// final List<TopicLink> topicLinks; // TODO handle
// final string type; // handled by runtime type of object
List<String> flags; // TODO enum
@JsonKey(fromJson: _flagsFromJson)
List<MessageFlag> flags; // Unrecognized flags won't roundtrip through {to,from}Json.
final String? matchContent;
final String? matchSubject;

static List<MessageFlag> _flagsFromJson(dynamic json) {
final list = json as List<dynamic>;
return list.map((raw) => MessageFlag.fromRawString(raw as String)).toList();
}

Message({
required this.client,
required this.content,
Expand Down Expand Up @@ -305,6 +311,32 @@ sealed class Message {
Map<String, dynamic> toJson();
}

/// As in [Message.flags].
@JsonEnum(fieldRename: FieldRename.snake, alwaysCreate: true)
enum MessageFlag {
read,
starred,
collapsed,
mentioned,
wildcardMentioned,
hasAlertWord,
historical,
unknown;

/// Get a [MessageFlag] from a raw, snake-case string.
///
/// Will be [MessageFlag.unknown] if we don't recognize the string.
///
/// Example:
/// 'wildcard_mentioned' -> Flag.wildcardMentioned
static MessageFlag fromRawString(String raw) => _byRawString[raw] ?? unknown;

// _$…EnumMap is thanks to `alwaysCreate: true` and `fieldRename: FieldRename.snake`
static final _byRawString = _$MessageFlagEnumMap.map((key, value) => MapEntry(value, key));

String toJson() => _$MessageFlagEnumMap[this]!;
}

@JsonSerializable(fieldRename: FieldRename.snake)
class StreamMessage extends Message {
@override
Expand Down
15 changes: 13 additions & 2 deletions lib/api/model/model.g.dart

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

7 changes: 7 additions & 0 deletions lib/model/narrow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,13 @@ class DmNarrow extends Narrow implements SendableNarrow {
);
}

factory DmNarrow.withUser(int userId, {required int selfUserId}) {
return DmNarrow(
allRecipientIds: {userId, selfUserId}.toList()..sort(),
selfUserId: selfUserId,
);
}

/// The user IDs of everyone in the conversation, sorted.
///
/// Each message in the conversation is sent by one of these users
Expand Down
7 changes: 4 additions & 3 deletions test/api/model/events_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,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 'package:zulip/api/model/model.dart';

import '../../example_data.dart' as eg;
import '../../stdlib_checks.dart';
Expand All @@ -11,15 +12,15 @@ import 'model_checks.dart';
void main() {
test('message: move flags into message object', () {
final message = eg.streamMessage();
MessageEvent mkEvent(List<String> flags) => Event.fromJson({
MessageEvent mkEvent(List<MessageFlag> flags) => Event.fromJson({
'type': 'message',
'id': 1,
'message': message.toJson()..remove('flags'),
'flags': flags,
'flags': flags.map((f) => f.toJson()).toList(),
}) as MessageEvent;
check(mkEvent(message.flags)).message.jsonEquals(message);
check(mkEvent([])).message.flags.deepEquals([]);
check(mkEvent(['read'])).message.flags.deepEquals(['read']);
check(mkEvent([MessageFlag.read])).message.flags.deepEquals([MessageFlag.read]);
});

test('user_settings: all known settings have event handling', () {
Expand Down
2 changes: 1 addition & 1 deletion test/api/model/model_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ extension MessageChecks on Subject<Message> {
Subject<bool> get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage');
Subject<int?> get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp');
Subject<List<Reaction>> get reactions => has((e) => e.reactions, 'reactions');
Subject<List<String>> get flags => has((e) => e.flags, 'flags');
Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');

// TODO accessors for other fields
}
Expand Down
18 changes: 18 additions & 0 deletions test/api/model/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import 'package:test/scaffolding.dart';
import 'package:zulip/api/model/model.dart';

import '../../example_data.dart' as eg;
import '../../stdlib_checks.dart';
import 'model_checks.dart';

void main() {
group('User', () {
Expand Down Expand Up @@ -47,6 +49,22 @@ void main() {
});
});

group('Message', () {
test('no crash on unrecognized flag', () {
final m1 = Message.fromJson(
(deepToJson(eg.streamMessage()) as Map<String, dynamic>)
..['flags'] = ['read', 'something_unknown'],
);
check(m1).flags.deepEquals([MessageFlag.read, MessageFlag.unknown]);

final m2 = Message.fromJson(
(deepToJson(eg.dmMessage(from: eg.selfUser, to: [eg.otherUser])) as Map<String, dynamic>)
..['flags'] = ['read', 'something_unknown'],
);
check(m2).flags.deepEquals([MessageFlag.read, MessageFlag.unknown]);
});
});

group('DmMessage', () {
final Map<String, dynamic> baseJson = Map.unmodifiable(
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]).toJson());
Expand Down
2 changes: 1 addition & 1 deletion test/model/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ void main() async {
id: 1,
messageId: originalMessage.id,
messageIds: [originalMessage.id],
flags: ["starred"],
flags: [MessageFlag.starred],
renderedContent: "<p>Hello, edited</p>",
editTimestamp: 99999,
isMeMessage: true,
Expand Down
21 changes: 21 additions & 0 deletions test/model/narrow_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,27 @@ void main() {
selfUserId: eg.selfUser.userId));
});

test('withUser: same user', () {
final actual = DmNarrow.withUser(5, selfUserId: 5);
check(actual).equals(DmNarrow(
allRecipientIds: [5],
selfUserId: 5));
});

test('withUser: user ID less than selfUserId', () {
final actual = DmNarrow.withUser(3, selfUserId: 5);
check(actual).equals(DmNarrow(
allRecipientIds: [3, 5],
selfUserId: 5));
});

test('withUser: user ID greater than selfUserId', () {
final actual = DmNarrow.withUser(7, selfUserId: 5);
check(actual).equals(DmNarrow(
allRecipientIds: [5, 7],
selfUserId: 5));
});

test('otherRecipientIds', () {
check(DmNarrow(allRecipientIds: [1, 2, 3], selfUserId: 2))
.otherRecipientIds.deepEquals([1, 3]);
Expand Down
2 changes: 1 addition & 1 deletion test/stdlib_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Object? deepToJson(Object? object) {
case List():
result = object.map((x) => deepToJson(x)).toList();
case Map() when object.keys.every((k) => k is String):
result = object.map((k, v) => MapEntry(k, deepToJson(v)));
result = object.map((k, v) => MapEntry<String, dynamic>(k, deepToJson(v)));
default:
return (null, false);
}
Expand Down