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: zulip#135
chrisbobbe · Aug 3, 2023 · aade783 · aade783
1 parent 55cf074
commit aade783
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 11 deletions.
diff --git 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<int> 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
diff --git 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
@@ -0,0 +1,7 @@
+import 'package:checks/checks.dart';
+import 'package:zulip/widgets/content.dart';
+
+extension RealmContentNetworkImageChecks on Subject<RealmContentNetworkImage> {
+  Subject<String> 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
@@ -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<void> setupMessageListPage(WidgetTester tester, {
   required Narrow narrow,
@@ -25,8 +32,9 @@ Future<void> setupMessageListPage(WidgetTester tester, {
   final connection = store.connection as FakeApiConnection;
 
   // prepare message list data
+  store.addUser(eg.selfUser);
   final List<StreamMessage> 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<RealmContentNetworkImage>(
+          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<void> 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;
+    });
+  });
 }