Skip to content
Open
46 changes: 13 additions & 33 deletions lib/api/model/events.dart

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

api: Replace `stream` with `channel` for MessageType
…

This too is technically out of scope for #1982, but it's fine to have.

Original file line number Diff line number Diff line change
Expand Up @@ -1405,8 +1405,9 @@ class DeleteMessageEvent extends Event {

final List<int> messageIds;
// final int messageId; // Not present; we support the bulk_message_deletion capability
// The server never actually sends "direct" here yet (it's "private" instead),
// but we accept both forms for forward-compatibility.
// The server never actually sends "channel" or "direct" here yet
// (it's "stream" or "private" instead), but we accept both the old
// and new forms for forward-compatibility.
@MessageTypeConverter()
final MessageType messageType;
final int? streamId;
Expand All @@ -1423,7 +1424,7 @@ class DeleteMessageEvent extends Event {
factory DeleteMessageEvent.fromJson(Map<String, dynamic> json) {
final result = _$DeleteMessageEventFromJson(json);
// Crunchy-shell validation
if (result.messageType == MessageType.stream) {
if (result.messageType == .channel) {
result.streamId as int;
result.topic as String;
}
Expand All @@ -1434,30 +1435,6 @@ class DeleteMessageEvent extends Event {
Map<String, dynamic> toJson() => _$DeleteMessageEventToJson(this);
}

/// As in [DeleteMessageEvent.messageType],
/// [UpdateMessageFlagsMessageDetail.type],
/// or [TypingEvent.messageType].
@JsonEnum(alwaysCreate: true)
enum MessageType {
stream,
direct;
}

class MessageTypeConverter extends JsonConverter<MessageType, String> {
const MessageTypeConverter();

@override
MessageType fromJson(String json) {
if (json == 'private') json = 'direct'; // TODO(server-future)
return $enumDecode(_$MessageTypeEnumMap, json);
}

@override
String toJson(MessageType object) {
return _$MessageTypeEnumMap[object]!;
}
}

/// A Zulip event of type `update_message_flags`.
///
/// For the corresponding API docs, see subclasses.
Expand Down Expand Up @@ -1536,8 +1513,9 @@ class UpdateMessageFlagsRemoveEvent extends UpdateMessageFlagsEvent {
/// As in [UpdateMessageFlagsRemoveEvent.messageDetails].
@JsonSerializable(fieldRename: FieldRename.snake)
class UpdateMessageFlagsMessageDetail {
// The server never actually sends "direct" here yet (it's "private" instead),
// but we accept both forms for forward-compatibility.
// The server never actually sends "channel" or "direct" here yet
// (it's "stream" or "private" instead), but we accept both the old
// and new forms for forward-compatibility.
@MessageTypeConverter()
final MessageType type;
final bool? mentioned;
Expand All @@ -1557,10 +1535,10 @@ class UpdateMessageFlagsMessageDetail {
final result = _$UpdateMessageFlagsMessageDetailFromJson(json);
// Crunchy-shell validation
switch (result.type) {
case MessageType.stream:
case .channel:
result.streamId as int;
result.topic as String;
case MessageType.direct:
case .direct:
result.userIds as List<int>;
}
return result;
Expand Down Expand Up @@ -1614,6 +1592,8 @@ class TypingEvent extends Event {
String get type => 'typing';

final TypingOp op;
// The server never actually sends "channel" here yet (it's "stream" instead),
// but we accept both forms for forward-compatibility.
Comment on lines +1595 to +1596

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's have a comment/dartdoc on MessageType explaining this. (For both channel and direct messages). Then we don't have to put this comment on every place MessageType is used.

@MessageTypeConverter()
final MessageType messageType;
@JsonKey(readValue: _readSenderId)
Expand Down Expand Up @@ -1647,10 +1627,10 @@ class TypingEvent extends Event {
final result = _$TypingEventFromJson(json);
// Crunchy-shell validation
switch (result.messageType) {
case MessageType.stream:
case .channel:
result.streamId as int;
result.topic as String;
case MessageType.direct:
case .direct:
result.recipientIds as List<int>;
}
return result;
Expand Down
5 changes: 0 additions & 5 deletions lib/api/model/events.g.dart

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

29 changes: 29 additions & 0 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,35 @@ sealed class Message<T extends Conversation> extends MessageBase<T> {
Map<String, dynamic> toJson();
}

/// As in [DeleteMessageEvent.messageType],
/// [UpdateMessageFlagsMessageDetail.type],
/// or [TypingEvent.messageType].
@JsonEnum(alwaysCreate: true)
enum MessageType {
channel,
direct;

factory fromJson(String json) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Don't think we've used nameless constructors yet.

Suggested change
factory fromJson(String json) {
factory MessageType.fromJson(String json) {

Also without the name, it's easy to confuse it as a method.

if (json == 'stream') json = 'channel'; // TODO(server-future)
if (json == 'private') json = 'direct'; // TODO(server-future)
return $enumDecode(_$MessageTypeEnumMap, json);
Comment on lines +1301 to 1303

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Now that there are more than one cases, let's use switch pattern matching.

}
}

class MessageTypeConverter extends JsonConverter<MessageType, String> {
const MessageTypeConverter();

@override
MessageType fromJson(String json) {
return MessageType.fromJson(json);
}

@override
String toJson(MessageType object) {
return _$MessageTypeEnumMap[object]!;
}
}

/// https://zulip.com/api/update-message-flags#available-flags
@JsonEnum(fieldRename: FieldRename.snake, alwaysCreate: true)
enum MessageFlag {
Expand Down
5 changes: 5 additions & 0 deletions lib/api/model/model.g.dart

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

2 changes: 1 addition & 1 deletion lib/model/narrow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class DmNarrow extends Narrow implements SendableNarrow {
UpdateMessageFlagsMessageDetail detail, {
required int selfUserId,
}) {
assert(detail.type == MessageType.direct);
assert(detail.type == .direct);
return DmNarrow.withOtherUsers(detail.userIds!, selfUserId: selfUserId);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/model/recent_senders.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class RecentSenders {
}

void handleDeleteMessageEvent(DeleteMessageEvent event, Map<int, Message> cachedMessages) {
if (event.messageType != MessageType.stream) return;
if (event.messageType != .channel) return;

final messagesByUser = <int, List<int>>{};
for (final id in event.messageIds) {
Expand Down
4 changes: 2 additions & 2 deletions lib/model/typing_status.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ class TypingStatus extends HasRealmStore with ChangeNotifier {

void handleTypingEvent(TypingEvent event) {
SendableNarrow narrow = switch (event.messageType) {
MessageType.direct => DmNarrow(
.direct => DmNarrow(
allRecipientIds: event.recipientIds!, selfUserId: selfUserId),
MessageType.stream => TopicNarrow(event.streamId!, event.topic!),
.channel => TopicNarrow(event.streamId!, event.topic!),
};

bool hasUpdate = false;
Expand Down
8 changes: 4 additions & 4 deletions lib/model/unreads.dart
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,13 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier {
void handleDeleteMessageEvent(DeleteMessageEvent event) {
mentions.removeAll(event.messageIds);
switch (event.messageType) {
case MessageType.stream:
case .channel:
// All the messages are in [event.streamId] and [event.topic],
// so we can be more efficient than _removeAllInStreamsAndDms.
final streamId = event.streamId!;
final topic = event.topic!;
_removeAllInStreamTopic(Set.of(event.messageIds), streamId, topic);
case MessageType.direct:
case .direct:
_removeAllInStreamsAndDms(event.messageIds, expectOnlyDms: true);
}
for (final messageId in event.messageIds) {
Expand Down Expand Up @@ -490,13 +490,13 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier {
mentions.add(messageId);
}
switch (detail.type) {
case MessageType.stream:
case .channel:
final UpdateMessageFlagsMessageDetail(:streamId, :topic) = detail;
locatorMap[messageId] = TopicNarrow(streamId!, topic!);
final topics = (newlyUnreadInStreams[streamId] ??= makeTopicKeyedMap());
final messageIds = (topics[topic] ??= QueueList());
messageIds.add(messageId);
case MessageType.direct:
case .direct:
final narrow = DmNarrow.ofUpdateMessageFlagsMessageDetail(selfUserId: selfUserId,
detail);
locatorMap[messageId] = narrow;
Expand Down
98 changes: 73 additions & 25 deletions test/api/model/events_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -245,28 +245,39 @@ void main() {
'message_type': 'private',
})).returnsNormally();

// TODO(server-future): remove
final baseJsonStream = {
'id': 1,
'type': 'delete_message',
'message_ids': [1, 2, 3],
'message_type': 'stream',
};

check(() => DeleteMessageEvent.fromJson({
...baseJsonStream
})).throws<void>();
// Future server.
final baseJsonChannel = {
'id': 1,
'type': 'delete_message',
'message_ids': [1, 2, 3],
'message_type': 'channel',
};

check(() => DeleteMessageEvent.fromJson({
...baseJsonStream, 'stream_id': 1, 'topic': 'some topic',
})).returnsNormally();
for (final baseJson in [baseJsonStream, baseJsonChannel]) {
check(() => DeleteMessageEvent.fromJson({
...baseJson
})).throws<void>();

check(() => DeleteMessageEvent.fromJson({
...baseJsonStream, 'stream_id': 1,
})).throws<void>();
check(() => DeleteMessageEvent.fromJson({
...baseJson, 'stream_id': 1, 'topic': 'some topic',
})).returnsNormally();

check(() => DeleteMessageEvent.fromJson({
...baseJsonStream, 'topic': 'some topic',
})).throws<void>();
check(() => DeleteMessageEvent.fromJson({
...baseJson, 'stream_id': 1,
})).throws<void>();

check(() => DeleteMessageEvent.fromJson({
...baseJson, 'topic': 'some topic',
})).throws<void>();
}
});

test('delete_message: private -> direct', () {
Expand All @@ -275,7 +286,18 @@ void main() {
'type': 'delete_message',
'message_ids': [1, 2, 3],
'message_type': 'private',
})).messageType.equals(MessageType.direct);
})).messageType.equals(.direct);
});

test('delete_message: stream -> channel', () {
check(DeleteMessageEvent.fromJson({
'id': 1,
'type': 'delete_message',
'message_ids': [1, 2, 3],
'message_type': 'stream',
'stream_id': 1,
'topic': 'some topic',
})).messageType.equals(.channel);
});

group('update_message_flags/remove', () {
Expand Down Expand Up @@ -310,7 +332,17 @@ void main() {
...messageDetail,
'type': 'private',
}}})).messageDetails.isNotNull()
.values.single.type.equals(MessageType.direct);
.values.single.type.equals(.direct);
});

test('stream -> channel', () {
check(UpdateMessageFlagsRemoveEvent.fromJson({
...baseJson,
'flag': 'read',
'message_details': {
'123': {'type': 'stream', 'mentioned': false, 'stream_id': 1, 'topic': 'some topic'}
}})).messageDetails.isNotNull()
.values.single.type.equals(.channel);
});
});

Expand Down Expand Up @@ -343,19 +375,35 @@ void main() {
check(TypingEvent.fromJson({
...directMessageJson,
'message_type': 'private',
})).messageType.equals(MessageType.direct);
})).messageType.equals(.direct);
});

test('stream type missing streamId/topic', () {
check(() => TypingEvent.fromJson({
...baseJson, 'message_type': 'stream', 'stream_id': 123, 'topic': 'foo'}))
.returnsNormally();
check(() => TypingEvent.fromJson({
...baseJson, 'message_type': 'stream'})).throws<void>();
check(() => TypingEvent.fromJson({
...baseJson, 'message_type': 'stream', 'topic': 'foo'})).throws<void>();
check(() => TypingEvent.fromJson({
...baseJson, 'message_type': 'stream', 'stream_id': 123})).throws<void>();
test('stream/channel type missing streamId/topic', () {
// TODO(server-future): remove
final baseJsonStream = {...baseJson, 'message_type': 'stream'};
// Future server.
final baseJsonChannel = {...baseJson, 'message_type': 'channel'};

for (final baseJson in [baseJsonStream, baseJsonChannel]) {
check(() => TypingEvent.fromJson({
...baseJson, 'stream_id': 123, 'topic': 'foo'}))
.returnsNormally();
check(() => TypingEvent.fromJson({
...baseJson})).throws<void>();
check(() => TypingEvent.fromJson({
...baseJson, 'topic': 'foo'})).throws<void>();
check(() => TypingEvent.fromJson({
...baseJson, 'stream_id': 123})).throws<void>();
}
});

test('stream -> channel', () {
check(TypingEvent.fromJson({
...baseJson,
'stream_id': 123,
'topic': 'foo',
'message_type': 'stream',
})).messageType.equals(.channel);
});

test('direct type sort recipient ids', () {
Expand Down
10 changes: 5 additions & 5 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ DeleteMessageEvent deleteMessageEvent(List<StreamMessage> messages) {
return DeleteMessageEvent(
id: 0,
messageIds: messages.map((message) => message.id).toList(),
messageType: MessageType.stream,
messageType: .channel,
streamId: messages[0].streamId,
topic: messages[0].topic,
);
Expand Down Expand Up @@ -1255,14 +1255,14 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent(
message.id,
switch (message) {
StreamMessage() => UpdateMessageFlagsMessageDetail(
type: MessageType.stream,
type: .channel,
mentioned: mentioned,
streamId: message.streamId,
topic: message.topic,
userIds: null,
),
DmMessage() => UpdateMessageFlagsMessageDetail(
type: MessageType.direct,
type: .direct,
mentioned: mentioned,
streamId: null,
topic: null,
Expand Down Expand Up @@ -1293,13 +1293,13 @@ TypingEvent typingEvent(SendableNarrow narrow, TypingOp op, int senderId) {
switch (narrow) {
case TopicNarrow():
return TypingEvent(id: 0, op: op, senderId: senderId,
messageType: MessageType.stream,
messageType: .channel,
streamId: narrow.channelId,
topic: narrow.topic,
recipientIds: null);
case DmNarrow():
return TypingEvent(id: 0, op: op, senderId: senderId,
messageType: MessageType.direct,
messageType: .direct,
recipientIds: narrow.allRecipientIds,
streamId: null,
topic: null);
Expand Down
Loading