diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 67770a6b73..175a7b850d 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -358,6 +358,32 @@ class DmRecipient { _$DmRecipientFromJson(json); Map toJson() => _$DmRecipientToJson(this); + + @override + String toString() => 'DmRecipient(id: $id, email: $email, fullName: $fullName)'; + + @override + bool operator ==(Object other) { + if (other is! DmRecipient) return false; + return other.id == id && other.email == email && other.fullName == fullName; + } + + @override + int get hashCode => Object.hash('DmRecipient', id, email, fullName); +} + +class DmRecipientListConverter extends JsonConverter, List> { + const DmRecipientListConverter(); + + @override + List fromJson(List json) { + return json.map((e) => DmRecipient.fromJson(e as Map)) + .toList(growable: false) + ..sort((a, b) => a.id.compareTo(b.id)); + } + + @override + List toJson(List object) => object; } @JsonSerializable(fieldRename: FieldRename.snake) @@ -366,8 +392,30 @@ class DmMessage extends Message { @JsonKey(includeToJson: true) String get type => 'private'; + /// The `display_recipient` from the server, sorted by user ID numerically. + /// + /// This lists the sender as well as all (other) recipients, and it + /// lists each user just once. In particular the self-user is always + /// included. + /// + /// Note the data here is not updated on changes to the users, so everything + /// other than the user IDs may be stale. + /// Consider using [allRecipientIds] instead, and getting user details + /// from the store. + // TODO(server): Document that it's all users. That statement is based on + // reverse-engineering notes in zulip-mobile:src/api/modelTypes.js at PmMessage. + @DmRecipientListConverter() final List displayRecipient; + /// The user IDs of all users in the thread, sorted numerically. + /// + /// This lists the sender as well as all (other) recipients, and it + /// lists each user just once. In particular the self-user is always + /// included. + /// + /// This is a result of [List.map], so it has an efficient `length`. + Iterable get allRecipientIds => displayRecipient.map((e) => e.id); + DmMessage({ super.avatarUrl, required super.client, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 61aee44cf2..2364111565 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -256,9 +256,8 @@ DmMessage _$DmMessageFromJson(Map json) => DmMessage( flags: (json['flags'] as List).map((e) => e as String).toList(), matchContent: json['match_content'] as String?, matchSubject: json['match_subject'] as String?, - displayRecipient: (json['display_recipient'] as List) - .map((e) => DmRecipient.fromJson(e as Map)) - .toList(), + displayRecipient: const DmRecipientListConverter() + .fromJson(json['display_recipient'] as List), ); Map _$DmMessageToJson(DmMessage instance) => { @@ -280,5 +279,6 @@ Map _$DmMessageToJson(DmMessage instance) => { 'match_content': instance.matchContent, 'match_subject': instance.matchSubject, 'type': instance.type, - 'display_recipient': instance.displayRecipient, + 'display_recipient': + const DmRecipientListConverter().toJson(instance.displayRecipient), }; diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index 0a5fc36846..52f82adb4f 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -90,4 +90,100 @@ class TopicNarrow extends Narrow { int get hashCode => Object.hash('TopicNarrow', streamId, topic); } -// TODO other narrow types: DMs; starred, mentioned; searches; arbitrary +bool _isSortedWithoutDuplicates(List items) { + final length = items.length; + if (length == 0) { + return true; + } + int lastItem = items[0]; + for (int i = 1; i < length; i++) { + final item = items[i]; + if (item <= lastItem) { + return false; + } + lastItem = item; + } + return true; +} + +/// The narrow for a direct-message conversation. +// Zulip has many ways of representing a DM conversation; for example code +// handling many of them, see zulip-mobile:src/utils/recipient.js . +// Please add more constructors and getters here to handle any of those +// as we turn out to need them. +class DmNarrow extends Narrow { + DmNarrow({required this.allRecipientIds, required int selfUserId}) + : assert(_isSortedWithoutDuplicates(allRecipientIds)), + assert(allRecipientIds.contains(selfUserId)), + _selfUserId = selfUserId; + + /// The user IDs of everyone in the conversation, sorted. + /// + /// Each message in the conversation is sent by one of these users + /// and received by all the other users. + /// + /// The self-user is always a member of this list. + /// It has one element for the self-1:1 thread, + /// two elements for other 1:1 threads, + /// and three or more elements for a group DM thread. + /// + /// See also: + /// * [otherRecipientIds], an alternate way of identifying the conversation. + /// * [DmMessage.allRecipientIds], which provides this same format. + final List allRecipientIds; + + /// The user ID of the self-user. + /// + /// The [DmNarrow] implementation needs this information + /// for converting between different forms of referring to the narrow, + /// such as [allRecipientIds] vs. [otherRecipientIds]. + final int _selfUserId; + + /// The user IDs of everyone in the conversation except self, sorted. + /// + /// This is empty for the self-1:1 thread, + /// has one element for other 1:1 threads, + /// and has two or more elements for a group DM thread. + /// + /// See also: + /// * [allRecipientIds], an alternate way of identifying the conversation. + late final List otherRecipientIds = allRecipientIds + .where((userId) => userId != _selfUserId) + .toList(growable: false); + + /// A string that uniquely identifies the DM conversation (within the account). + late final String _key = otherRecipientIds.join(','); + + @override + bool containsMessage(Message message) { + if (message is! DmMessage) return false; + if (message.allRecipientIds.length != allRecipientIds.length) return false; + int i = 0; + for (final userId in message.allRecipientIds) { + if (userId != allRecipientIds[i]) return false; + i++; + } + return true; + } + + // Not [otherRecipientIds], because for the self-1:1 thread that triggers + // a server bug as of Zulip Server 7 (2023-05): an empty list here + // causes a 5xx response from the server. + @override + ApiNarrow apiEncode() => [ApiNarrowDm(allRecipientIds)]; + + @override + bool operator ==(Object other) { + if (other is! DmNarrow) return false; + assert(other._selfUserId == _selfUserId, + 'Two [Narrow]s belonging to different accounts were compared with `==`. ' + 'This is a bug, because a [Narrow] does not contain information to ' + 'reliably detect such a comparison, so it may produce false positives.'); + return other._key == _key; + } + + @override + int get hashCode => Object.hash('DmNarrow', _key); +} + +// TODO other narrow types: starred, mentioned; searches; arbitrary diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 637cc777d8..90424efb58 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -695,6 +695,8 @@ class ComposeBox extends StatelessWidget { return StreamComposeBox(narrow: narrow, streamId: narrow.streamId); } else if (narrow is TopicNarrow) { return const SizedBox.shrink(); // TODO(#144): add a single-topic compose box + } else if (narrow is DmNarrow) { + return const SizedBox.shrink(); // TODO(#144): add a DM compose box } else if (narrow is AllMessagesNarrow) { return const SizedBox.shrink(); } else { diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 11c6401e43..36b5bb31ba 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -68,6 +68,15 @@ class MessageListAppBarTitle extends StatelessWidget { final store = PerAccountStoreWidget.of(context); final streamName = store.streams[streamId]?.name ?? '(unknown stream)'; return Text("#$streamName > $topic"); // TODO show stream privacy icon; format on two lines + + case DmNarrow(:var otherRecipientIds): + final store = PerAccountStoreWidget.of(context); + if (otherRecipientIds.isEmpty) { + return const Text("DMs with yourself"); + } else { + final names = otherRecipientIds.map((id) => store.users[id]?.fullName ?? '(unknown user)'); + return Text("DMs with ${names.join(", ")}"); // TODO show avatars + } } } } @@ -306,12 +315,22 @@ class DmRecipientHeader extends StatelessWidget { @override Widget build(BuildContext context) { - return Align( - alignment: Alignment.centerLeft, - child: RecipientHeaderChevronContainer( - color: _kDmRecipientHeaderColor, - child: const Text("Direct message", // TODO DM recipient headers - style: TextStyle(color: Colors.white)))); + return GestureDetector( + behavior: HitTestBehavior.translucent, + onTap: () { + final store = PerAccountStoreWidget.of(context); + final narrow = DmNarrow( + allRecipientIds: message.allRecipientIds.toList(growable: false), + selfUserId: store.account.userId); + Navigator.push(context, + MessageListPage.buildRoute(context: context, narrow: narrow)); + }, + child: Align( + alignment: Alignment.centerLeft, + child: RecipientHeaderChevronContainer( + color: _kDmRecipientHeaderColor, + child: const Text("Direct message", // TODO DM recipient headers + style: TextStyle(color: Colors.white))))); } } diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index 1ac0b511b0..6ccf59c545 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -2,6 +2,8 @@ import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/model.dart'; +import '../../example_data.dart' as eg; + void main() { group('User', () { final Map baseJson = Map.unmodifiable({ @@ -22,7 +24,7 @@ void main() { 'profile_data': {}, }); - User mkUser (Map specialJson) { + User mkUser(Map specialJson) { return User.fromJson({ ...baseJson, ...specialJson }); } @@ -44,4 +46,66 @@ void main() { check(mkUser({'is_system_bot': true}).isSystemBot).equals(true); }); }); + + group('DmMessage', () { + final Map baseJson = Map.unmodifiable( + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]).toJson()); + + DmMessage parse(Map specialJson) { + return DmMessage.fromJson({ ...baseJson, ...specialJson }); + } + + Iterable asRecipients(Iterable users) { + return users.map((u) => + DmRecipient(id: u.userId, email: u.email, fullName: u.fullName)); + } + + Map withRecipients(Iterable recipients) { + final from = recipients.first; + return { + 'sender_id': from.userId, + 'sender_email': from.email, + 'sender_full_name': from.fullName, + 'display_recipient': asRecipients(recipients).map((r) => r.toJson()).toList(), + }; + } + + User user2 = eg.user(userId: 2); + User user3 = eg.user(userId: 3); + User user11 = eg.user(userId: 11); + + test('displayRecipient', () { + check(parse(withRecipients([user2])).displayRecipient) + .deepEquals(asRecipients([user2])); + + check(parse(withRecipients([user2, user3])).displayRecipient) + .deepEquals(asRecipients([user2, user3])); + check(parse(withRecipients([user3, user2])).displayRecipient) + .deepEquals(asRecipients([user2, user3])); + + check(parse(withRecipients([user2, user3, user11])).displayRecipient) + .deepEquals(asRecipients([user2, user3, user11])); + check(parse(withRecipients([user3, user11, user2])).displayRecipient) + .deepEquals(asRecipients([user2, user3, user11])); + check(parse(withRecipients([user11, user2, user3])).displayRecipient) + .deepEquals(asRecipients([user2, user3, user11])); + }); + + test('allRecipientIds', () { + check(parse(withRecipients([user2])).allRecipientIds) + .deepEquals([2]); + + check(parse(withRecipients([user2, user3])).allRecipientIds) + .deepEquals([2, 3]); + check(parse(withRecipients([user3, user2])).allRecipientIds) + .deepEquals([2, 3]); + + check(parse(withRecipients([user2, user3, user11])).allRecipientIds) + .deepEquals([2, 3, 11]); + check(parse(withRecipients([user3, user11, user2])).allRecipientIds) + .deepEquals([2, 3, 11]); + check(parse(withRecipients([user11, user2, user3])).allRecipientIds) + .deepEquals([2, 3, 11]); + }); + }); } diff --git a/test/example_data.dart b/test/example_data.dart index 75834edd7d..4a9d79d485 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -63,13 +63,12 @@ final _messagePropertiesBase = { 'recipient_id': 32, // obsolescent in API, and ignored }; -// When we have a User object, this can take that as an argument. -Map _messagePropertiesFromSender() { +Map _messagePropertiesFromSender(User? sender) { return { 'client': 'ExampleClient', - 'sender_email': 'a-person@example', - 'sender_full_name': 'A Person', - 'sender_id': 12345, // TODO generate example IDs + 'sender_email': sender?.email ?? 'a-person@example', + 'sender_full_name': sender?.fullName ?? 'A Person', + 'sender_id': sender?.userId ?? 12345, // TODO generate example IDs 'sender_realm_str': 'zulip', }; } @@ -77,7 +76,7 @@ Map _messagePropertiesFromSender() { // When we have a Stream object, this can take that as an argument. // Also it can default explicitly to an example stream. StreamMessage streamMessage( - {String? streamName, int? streamId}) { + {User? sender, String? streamName, int? streamId}) { // The use of JSON here is convenient in order to delegate parts of the data // to helper functions. The main downside is that it loses static typing // of the properties as we're constructing the data. That's probably OK @@ -85,7 +84,7 @@ StreamMessage streamMessage( // dynamically in the constructor, so any ill-typing won't propagate further. return StreamMessage.fromJson({ ..._messagePropertiesBase, - ..._messagePropertiesFromSender(), + ..._messagePropertiesFromSender(sender), 'display_recipient': streamName ?? 'a stream', 'stream_id': streamId ?? 123, // TODO generate example IDs @@ -99,6 +98,29 @@ StreamMessage streamMessage( }); } +/// Construct an example direct message. +/// +/// See also: +/// * [streamMessage], to construct an example stream message. +DmMessage dmMessage({required User from, required List to}) { + assert(!to.any((user) => user.userId == from.userId)); + return DmMessage.fromJson({ + ..._messagePropertiesBase, + ..._messagePropertiesFromSender(from), + 'display_recipient': [from, ...to] + .map((u) => {'id': u.userId, 'email': u.email, 'full_name': u.fullName}) + .toList(growable: false), + + 'content': '

This is an example DM.

', + 'content_type': 'text/html', + 'flags': [], + 'id': 1234567, // TODO generate example IDs + 'subject': '', + 'timestamp': 1678139636, + 'type': 'private', + }); +} + // TODO example data for many more types final InitialSnapshot initialSnapshot = InitialSnapshot( diff --git a/test/model/narrow_checks.dart b/test/model/narrow_checks.dart new file mode 100644 index 0000000000..9d299a8a27 --- /dev/null +++ b/test/model/narrow_checks.dart @@ -0,0 +1,13 @@ + +import 'package:checks/checks.dart'; +import 'package:zulip/api/model/narrow.dart'; +import 'package:zulip/model/narrow.dart'; + +extension NarrowChecks on Subject { + Subject get apiEncode => has((x) => x.apiEncode(), 'apiEncode()'); +} + +extension DmNarrowChecks on Subject { + Subject> get allRecipientIds => has((x) => x.allRecipientIds, 'allRecipientIds'); + Subject> get otherRecipientIds => has((x) => x.otherRecipientIds, 'otherRecipientIds'); +} diff --git a/test/model/narrow_test.dart b/test/model/narrow_test.dart new file mode 100644 index 0000000000..552b1e6901 --- /dev/null +++ b/test/model/narrow_test.dart @@ -0,0 +1,67 @@ + +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/narrow.dart'; + +import '../example_data.dart' as eg; +import 'narrow_checks.dart'; + +void main() { + group('DmNarrow', () { + test('constructor assertions', () { + check(() => DmNarrow(allRecipientIds: [2, 12], selfUserId: 2)).returnsNormally(); + check(() => DmNarrow(allRecipientIds: [2], selfUserId: 2)).returnsNormally(); + + check(() => DmNarrow(allRecipientIds: [12, 2], selfUserId: 2)).throws(); + check(() => DmNarrow(allRecipientIds: [2, 2], selfUserId: 2)).throws(); + check(() => DmNarrow(allRecipientIds: [2, 12], selfUserId: 1)).throws(); + check(() => DmNarrow(allRecipientIds: [], selfUserId: 2)).throws(); + }); + + test('otherRecipientIds', () { + check(DmNarrow(allRecipientIds: [1, 2, 3], selfUserId: 2)) + .otherRecipientIds.deepEquals([1, 3]); + check(DmNarrow(allRecipientIds: [1, 2], selfUserId: 2)) + .otherRecipientIds.deepEquals([1]); + check(DmNarrow(allRecipientIds: [2], selfUserId: 2)) + .otherRecipientIds.deepEquals([]); + }); + + test('containsMessage', () { + final user1 = eg.user(userId: 1); + final user2 = eg.user(userId: 2); + final user3 = eg.user(userId: 3); + final narrow2 = DmNarrow(allRecipientIds: [2], selfUserId: 2); + final narrow12 = DmNarrow(allRecipientIds: [1, 2], selfUserId: 2); + final narrow123 = DmNarrow(allRecipientIds: [1, 2, 3], selfUserId: 2); + + Message dm(User from, List to) => eg.dmMessage(from: from, to: to); + final streamMessage = eg.streamMessage(sender: user2); + + check(narrow2.containsMessage(streamMessage)).isFalse(); + check(narrow2.containsMessage(dm(user2, []))).isTrue(); + check(narrow2.containsMessage(dm(user1, [user2]))).isFalse(); + check(narrow2.containsMessage(dm(user2, [user1]))).isFalse(); + check(narrow2.containsMessage(dm(user1, [user2, user3]))).isFalse(); + check(narrow2.containsMessage(dm(user2, [user1, user3]))).isFalse(); + check(narrow2.containsMessage(dm(user3, [user1, user2]))).isFalse(); + + check(narrow12.containsMessage(streamMessage)).isFalse(); + check(narrow12.containsMessage(dm(user2, []))).isFalse(); + check(narrow12.containsMessage(dm(user1, [user2]))).isTrue(); + check(narrow12.containsMessage(dm(user2, [user1]))).isTrue(); + check(narrow12.containsMessage(dm(user1, [user2, user3]))).isFalse(); + check(narrow12.containsMessage(dm(user2, [user1, user3]))).isFalse(); + check(narrow12.containsMessage(dm(user3, [user1, user2]))).isFalse(); + + check(narrow123.containsMessage(streamMessage)).isFalse(); + check(narrow123.containsMessage(dm(user2, []))).isFalse(); + check(narrow123.containsMessage(dm(user1, [user2]))).isFalse(); + check(narrow123.containsMessage(dm(user2, [user1]))).isFalse(); + check(narrow123.containsMessage(dm(user1, [user2, user3]))).isTrue(); + check(narrow123.containsMessage(dm(user2, [user1, user3]))).isTrue(); + check(narrow123.containsMessage(dm(user3, [user1, user2]))).isTrue(); + }); + }); +}