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 2, 2023 · f5cde2b · f5cde2b
1 parent a41b1db
commit f5cde2b
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 13 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;
   final 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(avatarUrl, filterQuality: FilterQuality.medium);

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 '../api/fake_api.dart' as fake_api;
+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,
@@ -22,11 +29,12 @@ Future<void> setupMessageListPage(WidgetTester tester, {
 
   await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot());
   final store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id);
-  final connection = store.connection as FakeApiConnection;
+  final connection = store.connection as fake_api.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,
@@ -51,6 +59,58 @@ Future<void> setupMessageListPage(WidgetTester tester, {
 void main() {
   TestZulipBinding.ensureInitialized();
 
+  group('MessageWithSender', () {
+    testWidgets('Updates avatar on RealmUserUpdateEvent', (tester) async {
+      addTearDown(TestZulipBinding.instance.reset);
+
+      RealmContentNetworkImage? findAvatarImageWidget(WidgetTester tester) {
+        final firstMessageWithSender = tester.widgetList(find.byType(MessageWithSender)).first;
+        return tester.widgetList<RealmContentNetworkImage>(
+          find.descendant(
+            of: find.byWidget(firstMessageWithSender),
+            matching: find.byType(RealmContentNetworkImage)),
+        ).singleOrNull;
+      }
+
+      void checkResultForSender(User sender) {
+        final avatarUrl = sender.avatarUrl;
+        switch (avatarUrl) {
+          case String(): {
+            check(findAvatarImageWidget(tester)).isNotNull().src.equals(resolveUrl(avatarUrl, eg.selfAccount));
+          }
+          case null: {
+            check(findAvatarImageWidget(tester)).isNull();
+          }
+        }
+      }
+
+      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 = FakeHttpClient();
+      debugNetworkImageHttpClientProvider = () => httpClient;
+      httpClient.request.response
+        ..statusCode = HttpStatus.ok
+        ..content = kSolidBlueAvatar;
+
+      await setupMessageListPage(tester, narrow: const AllMessagesNarrow());
+      checkResultForSender(eg.selfUser);
+
+      await handleNewAvatarEventAndPump(tester, '/foo.png');
+      // TODO only vary [avatarUrl], not other fields
+      checkResultForSender(eg.user(userId: eg.selfUser.userId, avatarUrl: '/foo.png'));
+
+      await handleNewAvatarEventAndPump(tester, '/bar.jpg');
+      // TODO only vary [avatarUrl], not other fields
+      checkResultForSender(eg.user(userId: eg.selfUser.userId, avatarUrl: '/bar.jpg'));
+
+      debugNetworkImageHttpClientProvider = null;
+    });
+  });
+
   group('ScrollToBottomButton interactions', () {
     ScrollController? findMessageListScrollController(WidgetTester tester) {
       final stickyHeaderListView = tester.widget<StickyHeaderListView>(find.byType(StickyHeaderListView));