Skip to content
Open
56 changes: 22 additions & 34 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 @@ -1613,7 +1591,10 @@ class TypingEvent extends Event {
@JsonKey(includeToJson: true)
String get type => 'typing';

@JsonKey(unknownEnumValue: TypingOp.unknown)
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 +1628,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 All @@ -1664,7 +1645,10 @@ class TypingEvent extends Event {
@JsonEnum(fieldRename: FieldRename.snake)
enum TypingOp {
start,
stop;
stop,

/// A new, unrecognized operation.
unknown;

String toJson() => _$TypingOpEnumMap[this]!;
}
Expand Down Expand Up @@ -1743,6 +1727,7 @@ class ReactionEvent extends Event {
@JsonKey(includeToJson: true)
String get type => 'reaction';

@JsonKey(unknownEnumValue: ReactionOp.unknown)
final ReactionOp op;

final String emojiName;
Expand Down Expand Up @@ -1774,6 +1759,9 @@ class ReactionEvent extends Event {
enum ReactionOp {
add,
remove,

/// A new, unrecognized operation.
unknown;
}

/// A Zulip event of type `heartbeat`: https://zulip.com/api/get-events#heartbeat
Expand Down
24 changes: 16 additions & 8 deletions lib/api/model/events.g.dart

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

52 changes: 45 additions & 7 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,10 @@ sealed class Message<T extends Conversation> extends MessageBase<T> {
@JsonKey(name: 'submessages', readValue: _readPoll, fromJson: Poll.fromJson, toJson: Poll.toJson)
Poll? poll;

String get type;
// 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.
MessageType get type;

// final List<TopicLink> topicLinks; // TODO handle
// final string type; // handled by runtime type of object
Expand Down Expand Up @@ -1280,15 +1283,50 @@ sealed class Message<T extends Conversation> extends MessageBase<T> {
// TODO(dart): This has to be a static method, because factories/constructors
// do not support type parameters: https://github.com/dart-lang/language/issues/647
static Message fromJson(Map<String, dynamic> json) {
final type = json['type'] as String;
if (type == 'stream') return StreamMessage.fromJson(json);
if (type == 'private') return DmMessage.fromJson(json);
throw Exception("Message.fromJson: unexpected message type $type");
return switch (MessageType.fromJson(json['type'] as String)) {
.channel => StreamMessage.fromJson(json),
.direct => DmMessage.fromJson(json),
};
}

Map<String, dynamic> toJson();
}

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

// We don't accept an unknown message type because we generally cannot decide
// how to interpret such data. In particular, when a message with an unknown
// type is received from the server, we cannot determine which message model
// to instantiate: either [StreamMessage] or [DmMessage]. See [Message.fromJson].
// unknown,
;

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 Expand Up @@ -1339,7 +1377,7 @@ enum MessageFlag {
class StreamMessage extends Message<StreamConversation> {
@override
@JsonKey(includeToJson: true)
String get type => 'stream';
MessageType get type => .channel;

@JsonKey(includeToJson: true)
int get streamId => conversation.streamId;
Expand Down Expand Up @@ -1393,7 +1431,7 @@ class StreamMessage extends Message<StreamConversation> {
class DmMessage extends Message<DmConversation> {
@override
@JsonKey(includeToJson: true)
String get type => 'private';
MessageType get type => .direct;

/// The user IDs of all users in the thread, sorted numerically, as in
/// `display_recipient` from the server.
Expand Down
9 changes: 7 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.

3 changes: 2 additions & 1 deletion lib/api/route/messages.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: In sendMessage, use "channel" when supported
…

This is nice to have, but let's move it into another PR, because this is unrelated to #1982 and this series of PRs is already pretty large.

Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,13 @@ Future<SendMessageResult> sendMessage(
String? localId,
bool? readBySender,
}) {
final supportsTypeChannel = connection.zulipFeatureLevel! >= 248; // TODO(server-9)
final supportsTypeDirect = connection.zulipFeatureLevel! >= 174; // TODO(server-7)
final supportsReadBySender = connection.zulipFeatureLevel! >= 236; // TODO(server-8)
return connection.post('sendMessage', SendMessageResult.fromJson, 'messages', {
...(switch (destination) {
StreamDestination() => {
'type': RawParameter('stream'),
'type': supportsTypeChannel ? RawParameter('channel') : RawParameter('stream'),
'to': destination.streamId,
'topic': RawParameter(destination.topic.apiName),
},
Expand Down
6 changes: 5 additions & 1 deletion lib/api/route/typing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import 'messages.dart';
Future<void> setTypingStatus(ApiConnection connection, {
required TypingOp op,
required MessageDestination destination,
}) {
}) async {
if (op == .unknown) {
assert(false, 'TypingOp.unknown is not supported when setting typing status.');
return;
}
switch (destination) {
case StreamDestination():
final supportsTypeChannel = connection.zulipFeatureLevel! >= 248; // TODO(server-9)
Expand Down
8 changes: 6 additions & 2 deletions lib/model/message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -832,18 +832,20 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage
}

void handleReactionEvent(ReactionEvent event) {
if (event.op == .unknown) return;

final message = messages[event.messageId];
if (message == null) return;

switch (event.op) {
case ReactionOp.add:
case .add:
(message.reactions ??= Reactions([])).add(Reaction(
emojiName: event.emojiName,
emojiCode: event.emojiCode,
reactionType: event.reactionType,
userId: event.userId,
));
case ReactionOp.remove:
case .remove:
if (message.reactions == null) { // TODO(log)
return;
}
Expand All @@ -852,6 +854,8 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage
emojiCode: event.emojiCode,
userId: event.userId,
);
case .unknown:
// Shouldn't reach here because of the early return.
}
_notifyMessageListViewsForOneMessage(event.messageId);
}
Expand Down
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
Loading
Loading