From 1248c8969a41594df0abe5942aa9f2d088686e16 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Sat, 20 Jul 2024 00:45:09 -0400 Subject: [PATCH 1/6] api [nfc]: Avoid the use of dynamic on Message. Signed-off-by: Zixuan James Li --- lib/api/model/model.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 614a784511..3a70dcc86d 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -493,14 +493,14 @@ sealed class Message { @JsonKey(name: 'match_subject') final String? matchTopic; - static MessageEditState _messageEditStateFromJson(dynamic json) { + static MessageEditState _messageEditStateFromJson(Object? json) { // This is a no-op so that [MessageEditState._readFromMessage] // can return the enum value directly. return json as MessageEditState; } - static Reactions? _reactionsFromJson(dynamic json) { - final list = (json as List); + static Reactions? _reactionsFromJson(Object? json) { + final list = (json as List); return list.isNotEmpty ? Reactions.fromJson(list) : null; } @@ -508,8 +508,8 @@ sealed class Message { return value ?? []; } - static List _flagsFromJson(dynamic json) { - final list = json as List; + static List _flagsFromJson(Object? json) { + final list = json as List; return list.map((raw) => MessageFlag.fromRawString(raw as String)).toList(); } From 0b51072bc2e3288c3b9b4fd1f97ba4a1c00e931b Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 18 Jul 2024 02:44:18 -0400 Subject: [PATCH 2/6] api: Add Submessage. We are not storing the list of `Submessage`s directly on the Message objects as they are pretty obscure until we further parse them into meaningful data structures for things like polls and todos. Signed-off-by: Zixuan James Li --- lib/api/model/model.dart | 1 - lib/api/model/submessage.dart | 36 +++++++++++++++++++++++++++ lib/api/model/submessage.g.dart | 28 +++++++++++++++++++++ test/api/model/submessage_checks.dart | 9 +++++++ test/api/model/submessage_test.dart | 29 +++++++++++++++++++++ 5 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 lib/api/model/submessage.dart create mode 100644 lib/api/model/submessage.g.dart create mode 100644 test/api/model/submessage_checks.dart create mode 100644 test/api/model/submessage_test.dart diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 3a70dcc86d..c98fea005c 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -481,7 +481,6 @@ sealed class Message { final String senderRealmStr; @JsonKey(name: 'subject') String topic; - // final List submessages; // TODO handle final int timestamp; String get type; diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart new file mode 100644 index 0000000000..d2a07ecc63 --- /dev/null +++ b/lib/api/model/submessage.dart @@ -0,0 +1,36 @@ +import 'package:json_annotation/json_annotation.dart'; + +part 'submessage.g.dart'; + +/// Data used for certain experimental Zulip widgets including polls and todo +/// lists. +/// +/// See: +/// https://zulip.com/api/get-messages#response +/// https://zulip.readthedocs.io/en/latest/subsystems/widgets.html +@JsonSerializable(fieldRename: FieldRename.snake) +class Submessage { + const Submessage({ + required this.msgType, + required this.content, + required this.senderId, + }); + + @JsonKey(unknownEnumValue: SubmessageType.unknown) + final SubmessageType msgType; + final String content; + // final int messageId; // ignored; redundant with [Message.id] + final int senderId; + // final int id; // ignored because it is unused + + factory Submessage.fromJson(Map json) => + _$SubmessageFromJson(json); + + Map toJson() => _$SubmessageToJson(this); +} + +/// As in [Submessage.msgType]. +enum SubmessageType { + widget, + unknown, +} diff --git a/lib/api/model/submessage.g.dart b/lib/api/model/submessage.g.dart new file mode 100644 index 0000000000..319064170d --- /dev/null +++ b/lib/api/model/submessage.g.dart @@ -0,0 +1,28 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: constant_identifier_names, unnecessary_cast + +part of 'submessage.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +Submessage _$SubmessageFromJson(Map json) => Submessage( + msgType: $enumDecode(_$SubmessageTypeEnumMap, json['msg_type'], + unknownValue: SubmessageType.unknown), + content: json['content'] as String, + senderId: (json['sender_id'] as num).toInt(), + ); + +Map _$SubmessageToJson(Submessage instance) => + { + 'msg_type': _$SubmessageTypeEnumMap[instance.msgType]!, + 'content': instance.content, + 'sender_id': instance.senderId, + }; + +const _$SubmessageTypeEnumMap = { + SubmessageType.widget: 'widget', + SubmessageType.unknown: 'unknown', +}; diff --git a/test/api/model/submessage_checks.dart b/test/api/model/submessage_checks.dart new file mode 100644 index 0000000000..69890f7153 --- /dev/null +++ b/test/api/model/submessage_checks.dart @@ -0,0 +1,9 @@ + +import 'package:checks/checks.dart'; +import 'package:zulip/api/model/submessage.dart'; + +extension SubmessageChecks on Subject { + Subject get msgType => has((e) => e.msgType, 'msgType'); + Subject get content => has((e) => e.content, 'content'); + Subject get senderId => has((e) => e.senderId, 'senderId'); +} diff --git a/test/api/model/submessage_test.dart b/test/api/model/submessage_test.dart new file mode 100644 index 0000000000..1e073f0f51 --- /dev/null +++ b/test/api/model/submessage_test.dart @@ -0,0 +1,29 @@ +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/api/model/submessage.dart'; + +import '../../example_data.dart' as eg; +import 'submessage_checks.dart'; + +void main() { + group('Message.submessages', () { + test('no crash on unrecognized submessage type', () { + final baseJson = { + 'content': '[]', + 'message_id': 123, + 'sender_id': eg.selfUser.userId, + 'id': 1, + }; + + check(Submessage.fromJson({ + ...baseJson, + 'msg_type': 'widget', + })).msgType.equals(SubmessageType.widget); + + check(Submessage.fromJson({ + ...baseJson, + 'msg_type': 'unknown_widget', + })).msgType.equals(SubmessageType.unknown); + }); + }); +} From 4a1e81e12f2376a58b1996cc45cfee9a00914372 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 19 Jul 2024 14:19:27 -0400 Subject: [PATCH 3/6] submessage: Add SubmessageData classes for polls. The sealed class `SubmessageData` is not actually in use, we could potentially implement a discriminator utilizing the sealed class to deserialize individual submessage content, but it is far easier to do so when we have access to the full list of submessages. `SubmessageData` is there for self-documentation. It is also worth noting that much of these class definitions are based on previous reverse engineering effort and the web implementation. See: - https://github.com/zulip/zulip/blob/304d948416465c1a085122af5d752f03d6797003/web/shared/src/poll_data.ts - https://github.com/zulip/zulip-mobile/blob/2217c858e207f9f092651dd853051843c3f04422/src/api/modelTypes.js#L800-L861 Due to the flexibility of the submessage API, these classes tend to be intentionally defensive against unknown or invalid values. Signed-off-by: Zixuan James Li --- lib/api/model/submessage.dart | 233 ++++++++++++++++++++++++++ lib/api/model/submessage.g.dart | 88 ++++++++++ test/api/model/submessage_checks.dart | 30 ++++ test/api/model/submessage_test.dart | 85 ++++++++++ test/example_data.dart | 8 + 5 files changed, 444 insertions(+) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index d2a07ecc63..d7023ee0e4 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -18,6 +18,11 @@ class Submessage { @JsonKey(unknownEnumValue: SubmessageType.unknown) final SubmessageType msgType; + /// [SubmessageData] encoded in JSON. + // We cannot parse the String into one of the [SubmessageData] classes because + // information from other submessages are required. Specifically, we need: + // * the index of this submessage in [Message.submessages]; + // * the [WidgetType] of the first [Message.submessages]. final String content; // final int messageId; // ignored; redundant with [Message.id] final int senderId; @@ -34,3 +39,231 @@ enum SubmessageType { widget, unknown, } + +sealed class SubmessageData {} + +/// The data encoded in a submessage to make the message a Zulip widget. +/// +/// Expected from the first [Submessage.content] in the "submessages" field on +/// the message when there is an widget. +/// +/// See https://zulip.readthedocs.io/en/latest/subsystems/widgets.html +sealed class WidgetData extends SubmessageData { + WidgetType get widgetType; + + WidgetData(); + + factory WidgetData.fromJson(Object? json) { + final map = json as Map; + final rawWidgetType = map['widget_type'] as String; + return switch (WidgetType.fromRawString(rawWidgetType)) { + WidgetType.poll => PollWidgetData.fromJson(map), + WidgetType.unknown => UnsupportedWidgetData.fromJson(map), + }; + } + + Object? toJson(); +} + +/// As in [WidgetData.widgetType]. +@JsonEnum(alwaysCreate: true) +enum WidgetType { + poll, + unknown; + + static WidgetType fromRawString(String raw) => _byRawString[raw] ?? unknown; + + static final _byRawString = _$WidgetTypeEnumMap + .map((key, value) => MapEntry(value, key)); +} + +/// The data encoded in a submessage to make the message a poll widget. +@JsonSerializable(fieldRename: FieldRename.snake) +class PollWidgetData extends WidgetData { + @override + @JsonKey(includeToJson: true) + WidgetType get widgetType => WidgetType.poll; + + /// The initial question and options on the poll. + final PollWidgetExtraData extraData; + + PollWidgetData({required this.extraData}); + + factory PollWidgetData.fromJson(Map json) => + _$PollWidgetDataFromJson(json); + + @override + Map toJson() => _$PollWidgetDataToJson(this); +} + +/// As in [PollWidgetData.extraData]. +@JsonSerializable(fieldRename: FieldRename.snake) +class PollWidgetExtraData { + final String question; + final List options; + + const PollWidgetExtraData({required this.question, required this.options}); + + factory PollWidgetExtraData.fromJson(Map json) => + _$PollWidgetExtraDataFromJson(json); + + Map toJson() => _$PollWidgetExtraDataToJson(this); +} + +class UnsupportedWidgetData extends WidgetData { + @override + @JsonKey(includeToJson: true) + WidgetType get widgetType => WidgetType.unknown; + + final Object? json; + + UnsupportedWidgetData.fromJson(this.json); + + @override + Object? toJson() => json; +} + +/// The data encoded in a submessage that acts on a poll. +sealed class PollEventSubmessage extends SubmessageData { + PollEventSubmessageType get type; + + PollEventSubmessage(); + + /// The key for identifying the [idx]'th option added by user + /// [senderId] to a poll. + /// + /// For options that are a part of the initial [PollWidgetData], the + /// [senderId] should be `null`. + static String optionKey({required int? senderId, required int idx}) => + // "canned" is a canonical constant coined by the web client. + '${senderId ?? 'canned'},$idx'; + + factory PollEventSubmessage.fromJson(Map json) { + final rawPollEventType = json['type'] as String; + switch (PollEventSubmessageType.fromRawString(rawPollEventType)) { + case PollEventSubmessageType.newOption: return PollNewOptionEventSubmessage.fromJson(json); + case PollEventSubmessageType.question: return PollQuestionEventSubmessage.fromJson(json); + case PollEventSubmessageType.vote: return PollVoteEventSubmessage.fromJson(json); + case PollEventSubmessageType.unknown: return UnknownPollEventSubmessage.fromJson(json); + } + } + + Map toJson(); +} + +/// As in [PollEventSubmessage.type]. +@JsonEnum(fieldRename: FieldRename.snake) +enum PollEventSubmessageType { + newOption, + question, + vote, + unknown; + + static PollEventSubmessageType fromRawString(String raw) => _byRawString[raw]!; + + static final _byRawString = _$PollEventSubmessageTypeEnumMap + .map((key, value) => MapEntry(value, key)); +} + +/// A poll event when an option is added. +@JsonSerializable(fieldRename: FieldRename.snake) +class PollNewOptionEventSubmessage extends PollEventSubmessage { + @override + @JsonKey(includeToJson: true) + PollEventSubmessageType get type => PollEventSubmessageType.newOption; + + final String option; + /// A sequence number for this option, among options added to this poll + /// by this [Submessage.senderId]. + /// + /// See [PollEventSubmessage.optionKey]. + final int idx; + + PollNewOptionEventSubmessage({required this.option, required this.idx}); + + @override + factory PollNewOptionEventSubmessage.fromJson(Map json) => + _$PollNewOptionEventSubmessageFromJson(json); + + @override + Map toJson() => _$PollNewOptionEventSubmessageToJson(this); +} + +/// A poll event when the question has been edited. +@JsonSerializable(fieldRename: FieldRename.snake) +class PollQuestionEventSubmessage extends PollEventSubmessage { + @override + @JsonKey(includeToJson: true) + PollEventSubmessageType get type => PollEventSubmessageType.question; + + final String question; + + PollQuestionEventSubmessage({required this.question}); + + @override + factory PollQuestionEventSubmessage.fromJson(Map json) => + _$PollQuestionEventSubmessageFromJson(json); + + @override + Map toJson() => _$PollQuestionEventSubmessageToJson(this); +} + +/// A poll event when a vote has been cast or removed. +@JsonSerializable(fieldRename: FieldRename.snake) +class PollVoteEventSubmessage extends PollEventSubmessage { + @override + @JsonKey(includeToJson: true) + PollEventSubmessageType get type => PollEventSubmessageType.vote; + + /// The key of the affected option. + /// + /// See [PollEventSubmessage.optionKey]. + final String key; + @JsonKey(name: 'vote', unknownEnumValue: PollVoteOp.unknown) + final PollVoteOp op; + + PollVoteEventSubmessage({required this.key, required this.op}); + + @override + factory PollVoteEventSubmessage.fromJson(Map json) { + final result = _$PollVoteEventSubmessageFromJson(json); + // Crunchy-shell validation + final segments = result.key.split(','); + final [senderId, idx] = segments; + if (senderId != 'canned') { + int.parse(senderId, radix: 10); + } + int.parse(idx, radix: 10); + return result; + } + + @override + Map toJson() => _$PollVoteEventSubmessageToJson(this); +} + +/// As in [PollVoteEventSubmessage.op]. +@JsonEnum(valueField: 'apiValue') +enum PollVoteOp { + add(apiValue: 1), + remove(apiValue: -1), + unknown(apiValue: null); + + const PollVoteOp({required this.apiValue}); + + final int? apiValue; + + int? toJson() => apiValue; +} + +class UnknownPollEventSubmessage extends PollEventSubmessage { + @override + @JsonKey(includeToJson: true) + PollEventSubmessageType get type => PollEventSubmessageType.unknown; + + final Map json; + + UnknownPollEventSubmessage.fromJson(this.json); + + @override + Map toJson() => json; +} diff --git a/lib/api/model/submessage.g.dart b/lib/api/model/submessage.g.dart index 319064170d..d378d76ffc 100644 --- a/lib/api/model/submessage.g.dart +++ b/lib/api/model/submessage.g.dart @@ -26,3 +26,91 @@ const _$SubmessageTypeEnumMap = { SubmessageType.widget: 'widget', SubmessageType.unknown: 'unknown', }; + +PollWidgetData _$PollWidgetDataFromJson(Map json) => + PollWidgetData( + extraData: PollWidgetExtraData.fromJson( + json['extra_data'] as Map), + ); + +Map _$PollWidgetDataToJson(PollWidgetData instance) => + { + 'widget_type': _$WidgetTypeEnumMap[instance.widgetType]!, + 'extra_data': instance.extraData, + }; + +const _$WidgetTypeEnumMap = { + WidgetType.poll: 'poll', + WidgetType.unknown: 'unknown', +}; + +PollWidgetExtraData _$PollWidgetExtraDataFromJson(Map json) => + PollWidgetExtraData( + question: json['question'] as String, + options: + (json['options'] as List).map((e) => e as String).toList(), + ); + +Map _$PollWidgetExtraDataToJson( + PollWidgetExtraData instance) => + { + 'question': instance.question, + 'options': instance.options, + }; + +PollNewOptionEventSubmessage _$PollNewOptionEventSubmessageFromJson( + Map json) => + PollNewOptionEventSubmessage( + option: json['option'] as String, + idx: (json['idx'] as num).toInt(), + ); + +Map _$PollNewOptionEventSubmessageToJson( + PollNewOptionEventSubmessage instance) => + { + 'type': _$PollEventSubmessageTypeEnumMap[instance.type]!, + 'option': instance.option, + 'idx': instance.idx, + }; + +const _$PollEventSubmessageTypeEnumMap = { + PollEventSubmessageType.newOption: 'new_option', + PollEventSubmessageType.question: 'question', + PollEventSubmessageType.vote: 'vote', + PollEventSubmessageType.unknown: 'unknown', +}; + +PollQuestionEventSubmessage _$PollQuestionEventSubmessageFromJson( + Map json) => + PollQuestionEventSubmessage( + question: json['question'] as String, + ); + +Map _$PollQuestionEventSubmessageToJson( + PollQuestionEventSubmessage instance) => + { + 'type': _$PollEventSubmessageTypeEnumMap[instance.type]!, + 'question': instance.question, + }; + +PollVoteEventSubmessage _$PollVoteEventSubmessageFromJson( + Map json) => + PollVoteEventSubmessage( + key: json['key'] as String, + op: $enumDecode(_$PollVoteOpEnumMap, json['vote'], + unknownValue: PollVoteOp.unknown), + ); + +Map _$PollVoteEventSubmessageToJson( + PollVoteEventSubmessage instance) => + { + 'type': _$PollEventSubmessageTypeEnumMap[instance.type]!, + 'key': instance.key, + 'vote': instance.op, + }; + +const _$PollVoteOpEnumMap = { + PollVoteOp.add: 1, + PollVoteOp.remove: -1, + PollVoteOp.unknown: null, +}; diff --git a/test/api/model/submessage_checks.dart b/test/api/model/submessage_checks.dart index 69890f7153..c840d5d9ef 100644 --- a/test/api/model/submessage_checks.dart +++ b/test/api/model/submessage_checks.dart @@ -7,3 +7,33 @@ extension SubmessageChecks on Subject { Subject get content => has((e) => e.content, 'content'); Subject get senderId => has((e) => e.senderId, 'senderId'); } + +extension WidgetDataChecks on Subject { + Subject get widgetType => has((e) => e.widgetType, 'widgetType'); +} + +extension PollWidgetDataChecks on Subject { + Subject get extraData => has((e) => e.extraData, 'extraData'); +} + +extension PollWidgetExtraDataChecks on Subject { + Subject get question => has((e) => e.question, 'question'); + Subject> get options => has((e) => e.options, 'options'); +} + +extension PollEventChecks on Subject { + Subject get type => has((e) => e.type, 'type'); +} + +extension PollOptionEventChecks on Subject { + Subject get option => has((e) => e.option, 'option'); +} + +extension PollQuestionEventChecks on Subject { + Subject get question => has((e) => e.question, 'question'); +} + +extension PollVoteEventChecks on Subject { + Subject get key => has((e) => e.key, 'key'); + Subject get op => has((e) => e.op, 'op'); +} diff --git a/test/api/model/submessage_test.dart b/test/api/model/submessage_test.dart index 1e073f0f51..8aa588d9a5 100644 --- a/test/api/model/submessage_test.dart +++ b/test/api/model/submessage_test.dart @@ -26,4 +26,89 @@ void main() { })).msgType.equals(SubmessageType.unknown); }); }); + + test('smoke WidgetData', () { + check(WidgetData.fromJson(eg.pollWidgetDataFavoriteLetter)).isA() + ..widgetType.equals(WidgetType.poll) + ..extraData.which((x) => x + ..question.equals('favorite letter') + ..options.deepEquals(['A', 'B', 'C']) + ); + }); + + test('invalid widget_type -> UnsupportedWidgetData/throw', () { + check(WidgetData.fromJson({ + ...eg.pollWidgetDataFavoriteLetter, + 'widget_type': 'unknown_foo', + })).isA(); + + check(() => WidgetData.fromJson({ + ...eg.pollWidgetDataFavoriteLetter, + 'widget_type': 123, + })).throws(); + }); + + test('smoke PollEventSubmessage', () { + check(PollEventSubmessage.fromJson({ + 'type': 'new_option', + 'option': 'new option', + 'idx': 0, + })).isA() + ..type.equals(PollEventSubmessageType.newOption) + ..option.equals('new option'); + + check(PollEventSubmessage.fromJson({ + 'type': 'question', + 'question': 'new question', + })).isA() + ..type.equals(PollEventSubmessageType.question) + ..question.equals('new question'); + + check(PollEventSubmessage.fromJson({ + 'type': 'vote', + 'vote': 1, + 'key': PollEventSubmessage.optionKey(senderId: null, idx: 0), + })).isA() + ..type.equals(PollEventSubmessageType.vote) + ..op.equals(PollVoteOp.add) + ..key.equals('canned,0'); + }); + + test('handle unknown poll event', () { + check(() => PollEventSubmessage.fromJson({ + 'type': 'foo', + })).throws(); + }); + + test('crash on poll vote key', () { + final voteData = {'type': 'vote', 'vote': 1}; + + check(() => PollEventSubmessage.fromJson({...voteData, + 'key': PollEventSubmessage.optionKey(senderId: null, idx: 0) + })).returnsNormally(); + check(() => PollEventSubmessage.fromJson({ ...voteData, + 'key': PollEventSubmessage.optionKey(senderId: 5, idx: 0) + })).returnsNormally(); + + check(() => PollEventSubmessage.fromJson({ ...voteData, + 'key': 'foo,0', + })).throws(); + check(() => PollEventSubmessage.fromJson({ ...voteData, + 'key': 'canned,bar', + })).throws(); + check(() => PollEventSubmessage.fromJson({ ...voteData, + 'key': 'canned,0xdeadbeef', + })).throws(); + check(() => PollEventSubmessage.fromJson({ ...voteData, + 'key': '0xdeadbeef,0', + })).throws(); + }); + + test('handle unknown poll vote op', () { + check(PollEventSubmessage.fromJson({ + 'type': 'vote', + 'vote': 'invalid', + 'key': PollEventSubmessage.optionKey(senderId: null, idx: 0) + })).isA().op.equals(PollVoteOp.unknown); + }); } diff --git a/test/example_data.dart b/test/example_data.dart index 5ff4bc80ef..c7820326ce 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -408,6 +408,14 @@ DmMessage dmMessage({ }) as Map); } +const pollWidgetDataFavoriteLetter = { + 'widget_type': 'poll', + 'extra_data': { + 'question': 'favorite letter', + 'options': ['A', 'B', 'C'], + } +}; + //////////////////////////////////////////////////////////////// // Aggregate data structures. // From 819a44a59c5214cd807e4227c3b3961dadeb61e1 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 11 Aug 2024 16:06:20 -0700 Subject: [PATCH 4/6] api [nfc]: Expand comments on Submessage; put fields in logical order --- lib/api/model/submessage.dart | 35 ++++++++++++++++++++------- lib/api/model/submessage.g.dart | 4 +-- test/api/model/submessage_checks.dart | 2 +- test/api/model/submessage_test.dart | 6 ++--- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index d7023ee0e4..def5a3232c 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -2,31 +2,44 @@ import 'package:json_annotation/json_annotation.dart'; part 'submessage.g.dart'; -/// Data used for certain experimental Zulip widgets including polls and todo -/// lists. +/// Data used for Zulip "widgets" within messages, like polls and todo lists. /// -/// See: -/// https://zulip.com/api/get-messages#response +/// For docs, see: +/// https://zulip.com/api/get-messages#response (search for "submessage") /// https://zulip.readthedocs.io/en/latest/subsystems/widgets.html +/// +/// This is an underdocumented part of the Zulip Server API. +/// So in addition to docs, see other clients: +/// https://github.com/zulip/zulip-mobile/blob/2217c858e/src/api/modelTypes.js#L800-L861 +/// https://github.com/zulip/zulip-mobile/blob/2217c858e/src/webview/html/message.js#L118-L192 +/// https://github.com/zulip/zulip/blob/40f59a05c/web/src/submessage.ts +/// https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts @JsonSerializable(fieldRename: FieldRename.snake) class Submessage { const Submessage({ + required this.senderId, required this.msgType, required this.content, - required this.senderId, }); + // TODO(server): should we be sorting a message's submessages by ID? Web seems to: + // https://github.com/zulip/zulip/blob/40f59a05c55e0e4f26ca87d2bca646770e94bff0/web/src/submessage.ts#L88 + // final int id; // ignored because we don't use it + + /// The sender of this submessage (not necessarily of the [Message] it's on). + final int senderId; + + // final int messageId; // ignored; redundant with [Message.id] + @JsonKey(unknownEnumValue: SubmessageType.unknown) final SubmessageType msgType; - /// [SubmessageData] encoded in JSON. + + /// A JSON encoding of a [SubmessageData]. // We cannot parse the String into one of the [SubmessageData] classes because // information from other submessages are required. Specifically, we need: // * the index of this submessage in [Message.submessages]; // * the [WidgetType] of the first [Message.submessages]. final String content; - // final int messageId; // ignored; redundant with [Message.id] - final int senderId; - // final int id; // ignored because it is unused factory Submessage.fromJson(Map json) => _$SubmessageFromJson(json); @@ -35,6 +48,10 @@ class Submessage { } /// As in [Submessage.msgType]. +/// +/// The only type of submessage that actually exists in Zulip (as of 2024, +/// and since this "submessages" subsystem was created in 2017–2018) +/// is [SubmessageType.widget]. enum SubmessageType { widget, unknown, diff --git a/lib/api/model/submessage.g.dart b/lib/api/model/submessage.g.dart index d378d76ffc..6c3aec866a 100644 --- a/lib/api/model/submessage.g.dart +++ b/lib/api/model/submessage.g.dart @@ -9,17 +9,17 @@ part of 'submessage.dart'; // ************************************************************************** Submessage _$SubmessageFromJson(Map json) => Submessage( + senderId: (json['sender_id'] as num).toInt(), msgType: $enumDecode(_$SubmessageTypeEnumMap, json['msg_type'], unknownValue: SubmessageType.unknown), content: json['content'] as String, - senderId: (json['sender_id'] as num).toInt(), ); Map _$SubmessageToJson(Submessage instance) => { + 'sender_id': instance.senderId, 'msg_type': _$SubmessageTypeEnumMap[instance.msgType]!, 'content': instance.content, - 'sender_id': instance.senderId, }; const _$SubmessageTypeEnumMap = { diff --git a/test/api/model/submessage_checks.dart b/test/api/model/submessage_checks.dart index c840d5d9ef..a234f8cc56 100644 --- a/test/api/model/submessage_checks.dart +++ b/test/api/model/submessage_checks.dart @@ -3,9 +3,9 @@ import 'package:checks/checks.dart'; import 'package:zulip/api/model/submessage.dart'; extension SubmessageChecks on Subject { + Subject get senderId => has((e) => e.senderId, 'senderId'); Subject get msgType => has((e) => e.msgType, 'msgType'); Subject get content => has((e) => e.content, 'content'); - Subject get senderId => has((e) => e.senderId, 'senderId'); } extension WidgetDataChecks on Subject { diff --git a/test/api/model/submessage_test.dart b/test/api/model/submessage_test.dart index 8aa588d9a5..a42e4cf1cb 100644 --- a/test/api/model/submessage_test.dart +++ b/test/api/model/submessage_test.dart @@ -9,10 +9,10 @@ void main() { group('Message.submessages', () { test('no crash on unrecognized submessage type', () { final baseJson = { - 'content': '[]', - 'message_id': 123, - 'sender_id': eg.selfUser.userId, 'id': 1, + 'sender_id': eg.selfUser.userId, + 'message_id': 123, + 'content': '[]', }; check(Submessage.fromJson({ From 9938b27c1c2d2d013f6bffae39b85d2c92f2cb6c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 11 Aug 2024 17:12:34 -0700 Subject: [PATCH 5/6] api: Give defaults to question/options on PollWidgetExtraData --- lib/api/model/submessage.dart | 8 ++++++++ lib/api/model/submessage.g.dart | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index def5a3232c..c334b81f43 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -116,7 +116,15 @@ class PollWidgetData extends WidgetData { /// As in [PollWidgetData.extraData]. @JsonSerializable(fieldRename: FieldRename.snake) class PollWidgetExtraData { + // The [question] and [options] fields seem to be always present. + // But both web and zulip-mobile accept them as optional, with default values: + // https://github.com/zulip/zulip-flutter/pull/823#discussion_r1697656896 + // https://github.com/zulip/zulip/blob/40f59a05c55e0e4f26ca87d2bca646770e94bff0/web/src/poll_widget.ts#L29 + // And the server doesn't really enforce any structure on submessage data. + // So match the web and zulip-mobile behavior. + @JsonKey(defaultValue: "") final String question; + @JsonKey(defaultValue: []) final List options; const PollWidgetExtraData({required this.question, required this.options}); diff --git a/lib/api/model/submessage.g.dart b/lib/api/model/submessage.g.dart index 6c3aec866a..a7ffaebc05 100644 --- a/lib/api/model/submessage.g.dart +++ b/lib/api/model/submessage.g.dart @@ -46,9 +46,11 @@ const _$WidgetTypeEnumMap = { PollWidgetExtraData _$PollWidgetExtraDataFromJson(Map json) => PollWidgetExtraData( - question: json['question'] as String, - options: - (json['options'] as List).map((e) => e as String).toList(), + question: json['question'] as String? ?? '', + options: (json['options'] as List?) + ?.map((e) => e as String) + .toList() ?? + [], ); Map _$PollWidgetExtraDataToJson( From 06ce603308742f3376cf25f79f9e7e937e7e618f Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 11 Aug 2024 16:08:37 -0700 Subject: [PATCH 6/6] api [nfc]: Expand comments on SubmessageData and subclasses --- lib/api/model/submessage.dart | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index c334b81f43..900f8dbbde 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -57,6 +57,14 @@ enum SubmessageType { unknown, } +/// The data encoded in a submessage at [Submessage.content]. +/// +/// For widgets (the only existing use of submessages), the submessages +/// on a [Message] consist of: +/// * One submessage with content [WidgetData]; then +/// * Zero or more submessages with content [PollEventSubmessage] if the +/// message is a poll (i.e. if the first submessage was a [PollWidgetData]), +/// and similarly for other types of widgets. sealed class SubmessageData {} /// The data encoded in a submessage to make the message a Zulip widget. @@ -86,6 +94,8 @@ sealed class WidgetData extends SubmessageData { @JsonEnum(alwaysCreate: true) enum WidgetType { poll, + // todo, // TODO(#882) + // zform, // This exists in web but is more a demo than a real feature. unknown; static WidgetType fromRawString(String raw) => _byRawString[raw] ?? unknown; @@ -94,7 +104,9 @@ enum WidgetType { .map((key, value) => MapEntry(value, key)); } -/// The data encoded in a submessage to make the message a poll widget. +/// The data in the first submessage on a poll widget message. +/// +/// Subsequent submessages on the same message will be [PollEventSubmessage]. @JsonSerializable(fieldRename: FieldRename.snake) class PollWidgetData extends WidgetData { @override @@ -148,7 +160,9 @@ class UnsupportedWidgetData extends WidgetData { Object? toJson() => json; } -/// The data encoded in a submessage that acts on a poll. +/// The data in a submessage that acts on a poll. +/// +/// The first submessage on the message should be a [PollWidgetData]. sealed class PollEventSubmessage extends SubmessageData { PollEventSubmessageType get type; @@ -160,7 +174,8 @@ sealed class PollEventSubmessage extends SubmessageData { /// For options that are a part of the initial [PollWidgetData], the /// [senderId] should be `null`. static String optionKey({required int? senderId, required int idx}) => - // "canned" is a canonical constant coined by the web client. + // "canned" is a canonical constant coined by the web client: + // https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts#L238 '${senderId ?? 'canned'},$idx'; factory PollEventSubmessage.fromJson(Map json) { @@ -191,6 +206,8 @@ enum PollEventSubmessageType { } /// A poll event when an option is added. +/// +/// See: https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts#L112-L159 @JsonSerializable(fieldRename: FieldRename.snake) class PollNewOptionEventSubmessage extends PollEventSubmessage { @override @@ -215,6 +232,8 @@ class PollNewOptionEventSubmessage extends PollEventSubmessage { } /// A poll event when the question has been edited. +/// +/// See: https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts#L161-186 @JsonSerializable(fieldRename: FieldRename.snake) class PollQuestionEventSubmessage extends PollEventSubmessage { @override @@ -234,6 +253,8 @@ class PollQuestionEventSubmessage extends PollEventSubmessage { } /// A poll event when a vote has been cast or removed. +/// +/// See: https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts#L188-234 @JsonSerializable(fieldRename: FieldRename.snake) class PollVoteEventSubmessage extends PollEventSubmessage { @override