From aade7838c53b616d706e756e016415c4098f404c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 12:23:27 -0700 Subject: [PATCH] msglist: Use [User.avatarUrl] instead of [Message.avatarUrl] Now, as demonstrated in the new test, if the author of a message changes their avatar, we'll see the update in the message list as soon as we get the event. While we're at it, comment out the property on [Message] to guide future consumers to [User]. Related: #135 --- lib/api/model/model.dart | 5 +-- lib/api/model/model.g.dart | 4 -- lib/widgets/message_list.dart | 5 ++- test/widgets/content_checks.dart | 7 ++++ test/widgets/message_list_test.dart | 57 ++++++++++++++++++++++++++++- 5 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 test/widgets/content_checks.dart diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index e7e8149501..f962bbcfa9 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -248,7 +248,7 @@ class Subscription { /// /// https://zulip.com/api/get-messages#response sealed class Message { - final String? avatarUrl; + // final String? avatarUrl; // Use [User.avatarUrl] instead; will live-update final String client; String content; final String contentType; @@ -276,7 +276,6 @@ sealed class Message { final String? matchSubject; Message({ - this.avatarUrl, required this.client, required this.content, required this.contentType, @@ -315,7 +314,6 @@ class StreamMessage extends Message { final int streamId; StreamMessage({ - super.avatarUrl, required super.client, required super.content, required super.contentType, @@ -417,7 +415,6 @@ class DmMessage extends Message { Iterable get allRecipientIds => displayRecipient.map((e) => e.id); DmMessage({ - super.avatarUrl, required super.client, required super.content, required super.contentType, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 2364111565..78c9ebd704 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -180,7 +180,6 @@ Map _$SubscriptionToJson(Subscription instance) => StreamMessage _$StreamMessageFromJson(Map json) => StreamMessage( - avatarUrl: json['avatar_url'] as String?, client: json['client'] as String, content: json['content'] as String, contentType: json['content_type'] as String, @@ -203,7 +202,6 @@ StreamMessage _$StreamMessageFromJson(Map json) => Map _$StreamMessageToJson(StreamMessage instance) => { - 'avatar_url': instance.avatarUrl, 'client': instance.client, 'content': instance.content, 'content_type': instance.contentType, @@ -239,7 +237,6 @@ Map _$DmRecipientToJson(DmRecipient instance) => }; DmMessage _$DmMessageFromJson(Map json) => DmMessage( - avatarUrl: json['avatar_url'] as String?, client: json['client'] as String, content: json['content'] as String, contentType: json['content_type'] as String, @@ -261,7 +258,6 @@ DmMessage _$DmMessageFromJson(Map json) => DmMessage( ); Map _$DmMessageToJson(DmMessage instance) => { - 'avatar_url': instance.avatarUrl, 'client': instance.client, 'content': instance.content, 'content_type': instance.contentType, diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 0d724a7bd7..28331bbae7 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -475,10 +475,11 @@ class MessageWithSender extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); + final author = store.users[message.senderId]!; - final avatarUrl = message.avatarUrl == null // TODO get from user data + final avatarUrl = author.avatarUrl == null ? null // TODO handle computing gravatars - : resolveUrl(message.avatarUrl!, store.account); + : resolveUrl(author.avatarUrl!, store.account); final avatar = (avatarUrl == null) ? const SizedBox.shrink() : RealmContentNetworkImage( diff --git a/test/widgets/content_checks.dart b/test/widgets/content_checks.dart new file mode 100644 index 0000000000..cf69253810 --- /dev/null +++ b/test/widgets/content_checks.dart @@ -0,0 +1,7 @@ +import 'package:checks/checks.dart'; +import 'package:zulip/widgets/content.dart'; + +extension RealmContentNetworkImageChecks on Subject { + Subject get src => has((i) => i.src, 'src'); + // TODO others +} diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index e0a77369a6..7341c01ca5 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -1,16 +1,23 @@ +import 'dart:io'; + import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/narrow.dart'; +import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/sticky_header.dart'; import 'package:zulip/widgets/store.dart'; import '../api/fake_api.dart'; +import '../test_images.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; +import '../model/test_store.dart'; +import 'content_checks.dart'; Future setupMessageListPage(WidgetTester tester, { required Narrow narrow, @@ -25,8 +32,9 @@ Future setupMessageListPage(WidgetTester tester, { final connection = store.connection as FakeApiConnection; // prepare message list data + store.addUser(eg.selfUser); final List messages = List.generate(10, (index) { - return eg.streamMessage(id: index); + return eg.streamMessage(id: index, sender: eg.selfUser); }); connection.prepare(json: GetMessagesResult( anchor: messages[0].id, @@ -122,4 +130,51 @@ void main() { check(scrollController.position.pixels).equals(0); }); }); + + group('MessageWithSender', () { + testWidgets('Updates avatar on RealmUserUpdateEvent', (tester) async { + addTearDown(TestZulipBinding.instance.reset); + + // TODO recognize avatar more reliably: + // https://github.com/zulip/zulip-flutter/pull/246#discussion_r1282516308 + RealmContentNetworkImage? findAvatarImageWidget(WidgetTester tester) { + return tester.widgetList( + find.descendant( + of: find.byType(MessageWithSender), + matching: find.byType(RealmContentNetworkImage))).firstOrNull; + } + + void checkResultForSender(String? avatarUrl) { + if (avatarUrl == null) { + check(findAvatarImageWidget(tester)).isNull(); + } else { + check(findAvatarImageWidget(tester)).isNotNull() + .src.equals(resolveUrl(avatarUrl, eg.selfAccount)); + } + } + + Future handleNewAvatarEventAndPump(WidgetTester tester, String avatarUrl) async { + final store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id); + store.handleEvent(RealmUserUpdateEvent(id: 1, userId: eg.selfUser.userId, avatarUrl: avatarUrl)); + await tester.pump(); + } + + final httpClient = FakeImageHttpClient(); + debugNetworkImageHttpClientProvider = () => httpClient; + httpClient.request.response + ..statusCode = HttpStatus.ok + ..content = kSolidBlueAvatar; + + await setupMessageListPage(tester, narrow: const AllMessagesNarrow()); + checkResultForSender(eg.selfUser.avatarUrl); + + await handleNewAvatarEventAndPump(tester, '/foo.png'); + checkResultForSender('/foo.png'); + + await handleNewAvatarEventAndPump(tester, '/bar.jpg'); + checkResultForSender('/bar.jpg'); + + debugNetworkImageHttpClientProvider = null; + }); + }); }