From feabdfe7be4cb6683bd1f70fff169e3a112df366 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 14:34:37 -0700 Subject: [PATCH 1/3] test: Have eg.user accept avatarUrl --- test/example_data.dart | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index 0e0e87171e..e237b2abc3 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -10,7 +10,12 @@ const String recentZulipVersion = '8.0'; const int recentZulipFeatureLevel = 185; const int futureZulipFeatureLevel = 9999; -User user({int? userId, String? email, String? fullName}) { +User user({ + int? userId, + String? email, + String? fullName, + String? avatarUrl, +}) { return User( userId: userId ?? 123, // TODO generate example IDs deliveryEmailStaleDoNotUse: 'name@example.com', @@ -25,7 +30,7 @@ User user({int? userId, String? email, String? fullName}) { isBot: false, role: 400, timezone: 'UTC', - avatarUrl: null, + avatarUrl: avatarUrl, avatarVersion: 0, profileData: null, ); From 55cf074347b1d932d65bd9581f85a6514caa32bc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 14:37:00 -0700 Subject: [PATCH 2/3] test [nfc]: Move fake-image-request code to its own file --- test/test_images.dart | 68 ++++++++++++++++++++++++++++++++++ test/widgets/content_test.dart | 68 +--------------------------------- 2 files changed, 70 insertions(+), 66 deletions(-) create mode 100644 test/test_images.dart diff --git a/test/test_images.dart b/test/test_images.dart new file mode 100644 index 0000000000..9ec4b32e16 --- /dev/null +++ b/test/test_images.dart @@ -0,0 +1,68 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +class FakeImageHttpClient extends Fake implements HttpClient { + final FakeImageHttpClientRequest request = FakeImageHttpClientRequest(); + + @override + Future getUrl(Uri url) async => request; +} + +class FakeImageHttpClientRequest extends Fake implements HttpClientRequest { + final FakeImageHttpClientResponse response = FakeImageHttpClientResponse(); + + @override + final FakeImageHttpHeaders headers = FakeImageHttpHeaders(); + + @override + Future close() async => response; +} + +class FakeImageHttpHeaders extends Fake implements HttpHeaders { + final Map> values = {}; + + @override + void add(String name, Object value, {bool preserveHeaderCase = false}) { + (values[name] ??= []).add(value.toString()); + } +} + +class FakeImageHttpClientResponse extends Fake implements HttpClientResponse { + @override + int statusCode = HttpStatus.ok; + + late List content; + + @override + int get contentLength => content.length; + + @override + HttpClientResponseCompressionState get compressionState => HttpClientResponseCompressionState.notCompressed; + + @override + StreamSubscription> listen(void Function(List event)? onData, {Function? onError, void Function()? onDone, bool? cancelOnError}) { + return Stream.value(content).listen( + onData, onDone: onDone, onError: onError, cancelOnError: cancelOnError); + } +} + +/// A 100x100 PNG image of solid Zulip blue, [kZulipBrandColor]. +// Made from the following SVG: +// +// +// +// with `inkscape tmp.svg -w 100 --export-png=tmp1.png`, +// `zopflipng tmp1.png tmp.png`, +// and `xxd -i tmp.png`. +const List kSolidBlueAvatar = [ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, + 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x64, 0x00, 0x00, 0x00, 0x64, + 0x01, 0x03, 0x00, 0x00, 0x00, 0x4a, 0x2c, 0x07, 0x17, 0x00, 0x00, 0x00, + 0x03, 0x50, 0x4c, 0x54, 0x45, 0x64, 0x92, 0xfe, 0xf1, 0xd6, 0x69, 0xa5, + 0x00, 0x00, 0x00, 0x13, 0x49, 0x44, 0x41, 0x54, 0x78, 0x01, 0x63, 0xa0, + 0x2b, 0x18, 0x05, 0xa3, 0x60, 0x14, 0x8c, 0x82, 0x51, 0x00, 0x00, 0x05, + 0x78, 0x00, 0x01, 0x1e, 0xcd, 0x28, 0xcd, 0x00, 0x00, 0x00, 0x00, 0x49, + 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82, +]; diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 6485f4d31e..3bf6021807 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -1,4 +1,3 @@ -import 'dart:async'; import 'dart:io'; import 'package:checks/checks.dart'; @@ -13,6 +12,7 @@ import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; +import '../test_images.dart'; import 'dialog_checks.dart'; void main() { @@ -126,7 +126,7 @@ void main() { addTearDown(TestZulipBinding.instance.reset); await globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final httpClient = _FakeHttpClient(); + final httpClient = FakeImageHttpClient(); debugNetworkImageHttpClientProvider = () => httpClient; httpClient.request.response ..statusCode = HttpStatus.ok @@ -162,67 +162,3 @@ void main() { }); }); } - -class _FakeHttpClient extends Fake implements HttpClient { - final _FakeHttpClientRequest request = _FakeHttpClientRequest(); - - @override - Future getUrl(Uri url) async => request; -} - -class _FakeHttpClientRequest extends Fake implements HttpClientRequest { - final _FakeHttpClientResponse response = _FakeHttpClientResponse(); - - @override - final _FakeHttpHeaders headers = _FakeHttpHeaders(); - - @override - Future close() async => response; -} - -class _FakeHttpHeaders extends Fake implements HttpHeaders { - final Map> values = {}; - - @override - void add(String name, Object value, {bool preserveHeaderCase = false}) { - (values[name] ??= []).add(value.toString()); - } -} - -class _FakeHttpClientResponse extends Fake implements HttpClientResponse { - @override - int statusCode = HttpStatus.ok; - - late List content; - - @override - int get contentLength => content.length; - - @override - HttpClientResponseCompressionState get compressionState => HttpClientResponseCompressionState.notCompressed; - - @override - StreamSubscription> listen(void Function(List event)? onData, {Function? onError, void Function()? onDone, bool? cancelOnError}) { - return Stream.value(content).listen( - onData, onDone: onDone, onError: onError, cancelOnError: cancelOnError); - } -} - -/// A 100x100 PNG image of solid Zulip blue, [kZulipBrandColor]. -// Made from the following SVG: -// -// -// -// with `inkscape tmp.svg -w 100 --export-png=tmp1.png`, -// `zopflipng tmp1.png tmp.png`, -// and `xxd -i tmp.png`. -const List kSolidBlueAvatar = [ - 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, - 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x64, 0x00, 0x00, 0x00, 0x64, - 0x01, 0x03, 0x00, 0x00, 0x00, 0x4a, 0x2c, 0x07, 0x17, 0x00, 0x00, 0x00, - 0x03, 0x50, 0x4c, 0x54, 0x45, 0x64, 0x92, 0xfe, 0xf1, 0xd6, 0x69, 0xa5, - 0x00, 0x00, 0x00, 0x13, 0x49, 0x44, 0x41, 0x54, 0x78, 0x01, 0x63, 0xa0, - 0x2b, 0x18, 0x05, 0xa3, 0x60, 0x14, 0x8c, 0x82, 0x51, 0x00, 0x00, 0x05, - 0x78, 0x00, 0x01, 0x1e, 0xcd, 0x28, 0xcd, 0x00, 0x00, 0x00, 0x00, 0x49, - 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82, -]; From aade7838c53b616d706e756e016415c4098f404c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 12:23:27 -0700 Subject: [PATCH 3/3] 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; + }); + }); }